On 10/19/2015 11:01 AM, Pierre Moreau wrote:
Hi Samuel,

(some comments below)

On 11:36 PM - Oct 18 2015, Samuel Pitoiset wrote:
While we are at it, store the rotate offset for occlusion queries to
nv50_hw_query like on nvc0.

Signed-off-by: Samuel Pitoiset <[email protected]>
---
  src/gallium/drivers/nouveau/nv50/nv50_query_hw.c | 45 +++++++++++++++++-------
  src/gallium/drivers/nouveau/nv50/nv50_query_hw.h |  3 +-
  2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query_hw.c 
b/src/gallium/drivers/nouveau/nv50/nv50_query_hw.c
index fcdd183..6260410 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query_hw.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query_hw.c
@@ -126,9 +126,9 @@ nv50_hw_begin_query(struct nv50_context *nv50, struct 
nv50_query *q)
      * query might set the initial render condition to false even *after* we 
re-
      * initialized it to true.
      */
-   if (q->type == PIPE_QUERY_OCCLUSION_COUNTER) {
-      hq->offset += 32;
-      hq->data += 32 / sizeof(*hq->data);
+   if (hq->rotate) {
+      hq->offset += hq->rotate;
+      hq->data += hq->rotate / sizeof(*hq->data);
        if (hq->offset - hq->base_offset == NV50_HW_QUERY_ALLOC_SPACE)
           nv50_hw_query_allocate(nv50, q, NV50_HW_QUERY_ALLOC_SPACE);
@@ -330,6 +330,7 @@ nv50_hw_create_query(struct nv50_context *nv50, unsigned type, unsigned index)
  {
     struct nv50_hw_query *hq;
     struct nv50_query *q;
+   unsigned space;
hq = CALLOC_STRUCT(nv50_hw_query);
     if (!hq)
@@ -339,22 +340,42 @@ nv50_hw_create_query(struct nv50_context *nv50, unsigned 
type, unsigned index)
     q->funcs = &hw_query_funcs;
     q->type = type;
- if (!nv50_hw_query_allocate(nv50, q, NV50_HW_QUERY_ALLOC_SPACE)) {
+   switch (q->type) {
+   case PIPE_QUERY_OCCLUSION_COUNTER:
+      hq->rotate = 32;
You should have `hq->rotate` default to 0 in other cases, as IIRC, you have no
guaranty about the value of an uninitialised variable.

CALLOC_STRUCT will be initialize all fields to 0.


+      space = NV50_HW_QUERY_ALLOC_SPACE;
+      break;
+   case PIPE_QUERY_PRIMITIVES_GENERATED:
+   case PIPE_QUERY_PRIMITIVES_EMITTED:
+   case PIPE_QUERY_SO_STATISTICS:
+   case PIPE_QUERY_PIPELINE_STATISTICS:
+      hq->is64bit = true;
Same comment as for `hq->rotate`: have `hq->is64bit` default to `false`.

+      space = NV50_HW_QUERY_ALLOC_SPACE;
+      break;
+   case PIPE_QUERY_TIME_ELAPSED:
+   case PIPE_QUERY_TIMESTAMP:
+   case PIPE_QUERY_TIMESTAMP_DISJOINT:
+   case PIPE_QUERY_GPU_FINISHED:
+   case NVA0_HW_QUERY_STREAM_OUTPUT_BUFFER_OFFSET:
+      space = NV50_HW_QUERY_ALLOC_SPACE;
+      break;
+   default:
+      debug_printf("invalid query type: %u\n", type);
+      FREE(q);
+      return NULL;
+   }
+
+   if (!nv50_hw_query_allocate(nv50, q, space)) {
`space` is always `NV50_HW_QUERY_ALLOC_SPACE`. Is there an advantage to
introducing this `space` variable? Do you plan to later add other possible
values to it?

I have a patch locally which reduces the size of that buffer for some queries, but this is not really related to this series. I'll submit it later (with other patches).


Pierre


        FREE(hq);
        return NULL;
     }
- if (q->type == PIPE_QUERY_OCCLUSION_COUNTER) {
+   if (hq->rotate) {
        /* we advance before query_begin ! */
-      hq->offset -= 32;
-      hq->data -= 32 / sizeof(*hq->data);
+      hq->offset -= hq->rotate;
+      hq->data -= hq->rotate / sizeof(*hq->data);
     }
- hq->is64bit = (type == PIPE_QUERY_PRIMITIVES_GENERATED ||
-                 type == PIPE_QUERY_PRIMITIVES_EMITTED ||
-                 type == PIPE_QUERY_SO_STATISTICS ||
-                 type == PIPE_QUERY_PIPELINE_STATISTICS);
-
     return q;
  }
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query_hw.h b/src/gallium/drivers/nouveau/nv50/nv50_query_hw.h
index ea2bf24..3a53e8a 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query_hw.h
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query_hw.h
@@ -24,9 +24,10 @@ struct nv50_hw_query {
     uint32_t sequence;
     struct nouveau_bo *bo;
     uint32_t base_offset;
-   uint32_t offset; /* base + i * 32 */
+   uint32_t offset; /* base + i * rotate */
     uint8_t state;
     bool is64bit;
+   uint8_t rotate;
     int nesting; /* only used for occlusion queries */
     struct nouveau_mm_allocation *mm;
     struct nouveau_fence *fence;
--
2.6.1

_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to