[Mesa-dev] [PATCH v2] nv50: fix PIPE_QUERY_TIMESTAMP_DISJOINT, based on nvc0

[Ilia Mirkin]

On Sat, May 23, 2015 at 12:59 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
> On 05/23/2015 06:53 PM, Ilia Mirkin wrote:
>>
>> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>
>> Seems in line with nvc0, but I assume you tested this? e.g. the
>> occlquery mesa demo, or the relevant piglit tests?
>
>
> Yes. And I added a temporary entry to the Gallium HUD for testing
> timestamp-disjoint.

I meant just general testing of queries, since you're redoing the
whole mechanism. Fixing timestamp disjoint is just icing on the
cake...

>
>
>>
>> On Sat, May 23, 2015 at 12:52 PM, Samuel Pitoiset
>> <samuel.pitoiset at gmail.com> wrote:
>>>
>>> PIPE_QUERY_TIMESTAMP_DISJOINT could not work because q->ready was always
>>> set to FALSE. To fix this issue, add more different states for queries
>>> according to nvc0.
>>>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>> ---
>>>   src/gallium/drivers/nouveau/nv50/nv50_query.c | 39
>>> +++++++++++++++------------
>>>   1 file changed, 22 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
>>> b/src/gallium/drivers/nouveau/nv50/nv50_query.c
>>> index da41209..55fcac8 100644
>>> --- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
>>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
>>> @@ -27,6 +27,11 @@
>>>   #include "nv50/nv50_context.h"
>>>   #include "nv_object.xml.h"
>>>
>>> +#define NV50_QUERY_STATE_READY   0
>>> +#define NV50_QUERY_STATE_ACTIVE  1
>>> +#define NV50_QUERY_STATE_ENDED   2
>>> +#define NV50_QUERY_STATE_FLUSHED 3
>>> +
>>>   /* XXX: Nested queries, and simultaneous queries on multiple gallium
>>> contexts
>>>    * (since we use only a single GPU channel per screen) will not work
>>> properly.
>>>    *
>>> @@ -42,8 +47,7 @@ struct nv50_query {
>>>      struct nouveau_bo *bo;
>>>      uint32_t base;
>>>      uint32_t offset; /* base + i * 32 */
>>> -   boolean ready;
>>> -   boolean flushed;
>>> +   uint8_t state;
>>>      boolean is64bit;
>>>      struct nouveau_mm_allocation *mm;
>>>      struct nouveau_fence *fence;
>>> @@ -66,7 +70,7 @@ nv50_query_allocate(struct nv50_context *nv50, struct
>>> nv50_query *q, int size)
>>>      if (q->bo) {
>>>         nouveau_bo_ref(NULL, &q->bo);
>>>         if (q->mm) {
>>> -         if (q->ready)
>>> +         if (q->state == NV50_QUERY_STATE_READY)
>>>               nouveau_mm_free(q->mm);
>>>            else
>>>               nouveau_fence_work(screen->base.fence.current,
>>> nouveau_mm_free_work,
>>> @@ -203,7 +207,7 @@ nv50_query_begin(struct pipe_context *pipe, struct
>>> pipe_query *pq)
>>>      default:
>>>         break;
>>>      }
>>> -   q->ready = FALSE;
>>> +   q->state = NV50_QUERY_STATE_ACTIVE;
>>>      return true;
>>>   }
>>>
>>> @@ -214,6 +218,8 @@ nv50_query_end(struct pipe_context *pipe, struct
>>> pipe_query *pq)
>>>      struct nouveau_pushbuf *push = nv50->base.pushbuf;
>>>      struct nv50_query *q = nv50_query(pq);
>>>
>>> +   q->state = NV50_QUERY_STATE_ENDED;
>>> +
>>>      switch (q->type) {
>>>      case PIPE_QUERY_OCCLUSION_COUNTER:
>>>         nv50_query_get(push, q, 0, 0x0100f002);
>>> @@ -256,29 +262,27 @@ nv50_query_end(struct pipe_context *pipe, struct
>>> pipe_query *pq)
>>>         break;
>>>      case PIPE_QUERY_TIMESTAMP_DISJOINT:
>>>         /* This query is not issued on GPU because disjoint is forced to
>>> FALSE */
>>> -      q->ready = TRUE;
>>> +      q->state = NV50_QUERY_STATE_READY;
>>>         break;
>>>      default:
>>>         assert(0);
>>>         break;
>>>      }
>>> -   q->ready = q->flushed = FALSE;
>>>
>>>      if (q->is64bit)
>>>         nouveau_fence_ref(nv50->screen->base.fence.current, &q->fence);
>>>   }
>>>
>>> -static INLINE boolean
>>> -nv50_query_ready(struct nv50_query *q)
>>> +static INLINE void
>>> +nv50_query_update(struct nv50_query *q)
>>>   {
>>>      if (q->is64bit) {
>>>         if (nouveau_fence_signalled(q->fence))
>>> -         return TRUE;
>>> +         q->state = NV50_QUERY_STATE_READY;
>>>      } else {
>>>         if (q->data[0] == q->sequence)
>>> -         return TRUE;
>>> +         q->state = NV50_QUERY_STATE_READY;
>>>      }
>>> -   return FALSE;
>>>   }
>>>
>>>   static boolean
>>> @@ -293,13 +297,14 @@ nv50_query_result(struct pipe_context *pipe, struct
>>> pipe_query *pq,
>>>      uint64_t *data64 = (uint64_t *)q->data;
>>>      int i;
>>>
>>> -   if (!q->ready) /* update ? */
>>> -      q->ready = nv50_query_ready(q);
>>> -   if (!q->ready) {
>>> +   if (q->state != NV50_QUERY_STATE_READY)
>>> +      nv50_query_update(q);
>>> +
>>> +   if (q->state != NV50_QUERY_STATE_READY) {
>>>         if (!wait) {
>>>            /* for broken apps that spin on GL_QUERY_RESULT_AVAILABLE */
>>> -         if (!q->flushed) {
>>> -            q->flushed = TRUE;
>>> +         if (q->state != NV50_QUERY_STATE_FLUSHED) {
>>> +            q->state = NV50_QUERY_STATE_FLUSHED;
>>>               PUSH_KICK(nv50->base.pushbuf);
>>>            }
>>>            return FALSE;
>>> @@ -307,7 +312,7 @@ nv50_query_result(struct pipe_context *pipe, struct
>>> pipe_query *pq,
>>>         if (nouveau_bo_wait(q->bo, NOUVEAU_BO_RD,
>>> nv50->screen->base.client))
>>>            return FALSE;
>>>      }
>>> -   q->ready = TRUE;
>>> +   q->state = NV50_QUERY_STATE_READY;
>>>
>>>      switch (q->type) {
>>>      case PIPE_QUERY_GPU_FINISHED:
>>> --
>>> 2.4.1
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>