Re: [Mesa-dev] [PATCH 3/5] etnaviv: add support for occlusion queries

2017-10-19 Thread Wladimir J. van der Laan 
> There is one difference - how the sum is interpreted - uint64_t vs. bool
> value. In general the code

Ok in that case it's ok like this, just looked like unnecessary/accidental
duplication.

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

Re: [Mesa-dev] [PATCH 3/5] etnaviv: add support for occlusion queries

2017-10-19 Thread Wladimir J. van der Laan 
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 

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

[Mesa-dev] [PATCH 3/5] etnaviv: add support for occlusion queries

2017-10-17 Thread Christian Gmeiner 
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 
---
 src/gallium/drivers/etnaviv/etnaviv_query_hw.c | 106 +
 1 file changed, 106 insertions(+)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_query_hw.c 
b/src/gallium/drivers/etnaviv/etnaviv_query_hw.c
index a1dead335c..b9223e1361 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_query_hw.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_query_hw.c
@@ -30,9 +30,82 @@
 #include "util/u_memory.h"
 
 #include "etnaviv_context.h"
+#include "etnaviv_debug.h"
+#include "etnaviv_emit.h"
 #include "etnaviv_query_hw.h"
 #include "etnaviv_screen.h"
 
+/*
+ * Occlusion Query:
+ *
+ * OCCLUSION_COUNTER and OCCLUSION_PREDICATE differ only in how they
+ * interpret results
+ */
+
+static void
+occlusion_start(struct etna_hw_query *hq, struct etna_context *ctx)
+{
+   struct etna_resource *rsc = etna_resource(hq->prsc);
+   struct etna_reloc r = {
+  .bo = rsc->bo,
+  .flags = ETNA_RELOC_WRITE
+   };
+
+   if (hq->samples > 63) {
+  hq->samples = 63;
+  BUG("samples overflow");
+   }
+
+   r.offset = hq->samples * 8; /* 64bit value */
+
+   etna_set_state_reloc(ctx->stream, VIVS_GL_OCCLUSION_QUERY_ADDR, );
+}
+
+static void
+occlusion_stop(struct etna_hw_query *hq, struct etna_context *ctx)
+{
+   etna_set_state(ctx->stream, VIVS_GL_OCCLUSION_QUERY_CONTROL, 0x1DF5E76);
+}
+
+static void
+occlusion_suspend(struct etna_hw_query *hq, struct etna_context *ctx)
+{
+   occlusion_stop(hq, ctx);
+}
+
+static void
+occlusion_resume(struct etna_hw_query *hq, struct etna_context *ctx)
+{
+   hq->samples++;
+   occlusion_start(hq, ctx);
+}
+
+static void
+occlusion_counter_result(struct etna_hw_query *hq, void *buf,
+ union pipe_query_result *result)
+{
+   uint64_t sum = 0;
+   uint64_t *ptr = (uint64_t *)buf;
+
+   for (unsigned i = 0; i <= hq->samples; i++)
+  sum += *(ptr + i);
+
+   result->u64 = sum;
+}
+
+static void
+occlusion_predicate_result(struct etna_hw_query *hq, void *buf,
+   union pipe_query_result *result)
+{
+   uint64_t sum = 0;
+   uint64_t *ptr = (uint64_t *)buf;
+
+   for (unsigned i = 0; i <= hq->samples; i++)
+  sum += *(ptr + i);
+
+   result->b = !!sum;
+}
+
 static void
 etna_hw_destroy_query(struct etna_context *ctx, struct etna_query *q)
 {
@@ -44,6 +117,33 @@ etna_hw_destroy_query(struct etna_context *ctx, struct 
etna_query *q)
FREE(hq);
 }
 
+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,
+};
+
 static void
 realloc_query_bo(struct etna_context *ctx, struct etna_hw_query *hq)
 {
@@ -153,6 +253,12 @@ static inline const struct etna_hw_sample_provider *
 query_sample_provider(unsigned query_type)
 {
switch (query_type) {
+   case PIPE_QUERY_OCCLUSION_COUNTER:
+  return _counter;
+   case PIPE_QUERY_OCCLUSION_PREDICATE:
+  return _predicate;
+   case PIPE_QUERY_OCCLUSION_PREDICATE_CONSERVATIVE:
+  return _predicate_conservative;
default:
   return NULL;
}
-- 
2.13.6

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