On Tue, Oct 17, 2017 at 10:38:15PM +0200, Christian Gmeiner wrote:
> Passes most occlusion query piglits. The following piglits are broken:
> - spec@arb_occlusion_query@occlusion_query_meta_fragments
> - spec@arb_occlusion_query@occlusion_query_meta_save
> - spec@arb_occlusion_query2@render
> 
> Signed-off-by: Christian Gmeiner <christian.gmei...@gmail.com>

Comments inline

> +static void
> +occlusion_stop(struct etna_hw_query *hq, struct etna_context *ctx)
> +{
> +   etna_set_state(ctx->stream, VIVS_GL_OCCLUSION_QUERY_CONTROL, 0x1DF5E76);

Does the actual value written matter here?
If so, it'd make sense to define a constant (or bit definitions in the rnndb).
If not it's fine like this, or add a comment that it's just a "random" nonce.

> +static const struct etna_hw_sample_provider occlusion_counter = {
> +   .query_type = PIPE_QUERY_OCCLUSION_COUNTER,
> +   .start = occlusion_start,
> +   .stop = occlusion_stop,
> +   .suspend = occlusion_suspend,
> +   .resume = occlusion_resume,
> +   .result = occlusion_counter_result,
> +};
> +
> +static const struct etna_hw_sample_provider occlusion_predicate = {
> +   .query_type = PIPE_QUERY_OCCLUSION_PREDICATE,
> +   .start = occlusion_start,
> +   .stop = occlusion_stop,
> +   .suspend = occlusion_suspend,
> +   .resume = occlusion_resume,
> +   .result = occlusion_predicate_result,
> +};
> +
> +static const struct etna_hw_sample_provider occlusion_predicate_conservative 
> = {
> +   .query_type = PIPE_QUERY_OCCLUSION_PREDICATE_CONSERVATIVE,
> +   .start = occlusion_start,
> +   .stop = occlusion_stop,
> +   .suspend = occlusion_suspend,
> +   .resume = occlusion_resume,
> +   .result = occlusion_predicate_result,
> +};

Is it intentional that this defines the same fields three times?

Why not return the same structure for all three cases? Is this expected to 
change to
specific implementations soon in another patch?

Wladimir
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to