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 <samuel.pitoi...@gmail.com> > --- > 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. > + 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? 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 > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev