[Mesa-dev] [PATCH 1/2] gallium: fix PIPE_QUERY_TIMESTAMP_DISJOINT

[Jose Fonseca]

----- Original Message -----
> 
> 
> ----- Original Message -----
> > From: Roland Scheidegger <sroland at vmware.com>
> > 
> > The semantics didn't really make sense, not really matching neither d3d9
> > (though the docs are all broken there) nor d3d10. So make it match d3d10
> > semantics, which actually gives meaning to the "disjoint" part.
> > Drivers are fixed up in a very primitive way, I have no idea what could
> > actually cause the counter to become unreliable so just always return
> > FALSE for the disjoint part.
> > ---
> >  src/gallium/docs/source/context.rst   |   10 ++++++----
> >  src/gallium/drivers/nv50/nv50_query.c |    5 +++--
> >  src/gallium/drivers/nvc0/nvc0_query.c |    4 +---
> >  3 files changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/gallium/docs/source/context.rst
> > b/src/gallium/docs/source/context.rst
> > index ede89be..bfd58a4 100644
> > --- a/src/gallium/docs/source/context.rst
> > +++ b/src/gallium/docs/source/context.rst
> > @@ -330,11 +330,13 @@ scaled to nanoseconds, recorded after all commands
> > issued prior to
> >  This query does not require a call to ``begin_query``.
> >  The result is an unsigned 64-bit integer.
> >  
> > -``PIPE_QUERY_TIMESTAMP_DISJOINT`` can be used to check whether the
> > -internal timer resolution is good enough to distinguish between the
> > -events at ``begin_query`` and ``end_query``.
> > +``PIPE_QUERY_TIMESTAMP_DISJOINT`` can be used to check the
> > +internal timer resolution and whether the timestamp counter has become
> > +unreliable due to things like throttling etc. - only if this is FALSE
> > +a timestamp query (within the timestamp_disjoint query) should be trusted.
> >  The result is a 64-bit integer specifying the timer resolution in Hz,
> > -followed by a boolean value indicating whether the timer has incremented.
> > +followed by a boolean value indicating whether the timestamp counter
> > +is discontinuous or disjoint.
> >  
> >  ``PIPE_QUERY_PRIMITIVES_GENERATED`` returns a 64-bit integer indicating
> >  the number of primitives processed by the pipeline (regardless of whether
> > diff --git a/src/gallium/drivers/nv50/nv50_query.c
> > b/src/gallium/drivers/nv50/nv50_query.c
> > index 656ff9d..b97eff2 100644
> > --- a/src/gallium/drivers/nv50/nv50_query.c
> > +++ b/src/gallium/drivers/nv50/nv50_query.c
> > @@ -181,7 +181,6 @@ nv50_query_begin(struct pipe_context *pipe, struct
> > pipe_query *pq)
> >        nv50_query_get(push, q, 0x20, 0x05805002);
> >        nv50_query_get(push, q, 0x30, 0x06805002);
> >        break;
> > -   case PIPE_QUERY_TIMESTAMP_DISJOINT:
> >     case PIPE_QUERY_TIME_ELAPSED:
> >        nv50_query_get(push, q, 0x10, 0x00005002);
> >        break;
> > @@ -229,6 +228,8 @@ nv50_query_end(struct pipe_context *pipe, struct
> > pipe_query *pq)
> >     case NVA0_QUERY_STREAM_OUTPUT_BUFFER_OFFSET:
> >        nv50_query_get(push, q, 0, 0x0d005002 | (q->index << 5));
> >        break;
> > +   case PIPE_QUERY_TIMESTAMP_DISJOINT:
> > +      break;
> >     default:
> >        assert(0);
> >        break;
> > @@ -289,7 +290,7 @@ nv50_query_result(struct pipe_context *pipe, struct
> > pipe_query *pq,
> >        break;
> >     case PIPE_QUERY_TIMESTAMP_DISJOINT: /* u32 sequence, u32 0, u64 time */
> >        res64[0] = 1000000000;
> 
> I don't think this statement is needed. Same below. Otherwise looks good.
> 
> Jose
> 
> > -      res8[8] = (data64[1] == data64[3]) ? FALSE : TRUE;
> > +      res8[8] = FALSE;
> >        break;
> >     case PIPE_QUERY_TIME_ELAPSED:
> >        res64[0] = data64[1] - data64[3];
> > diff --git a/src/gallium/drivers/nvc0/nvc0_query.c
> > b/src/gallium/drivers/nvc0/nvc0_query.c
> > index 8e584c9..3f5a9fb 100644
> > --- a/src/gallium/drivers/nvc0/nvc0_query.c
> > +++ b/src/gallium/drivers/nvc0/nvc0_query.c
> > @@ -285,7 +285,6 @@ nvc0_query_begin(struct pipe_context *pipe, struct
> > pipe_query *pq)
> >     case PIPE_QUERY_SO_OVERFLOW_PREDICATE:
> >        nvc0_query_get(push, q, 0x10, 0x03005002 | (q->index << 5));
> >        break;
> > -   case PIPE_QUERY_TIMESTAMP_DISJOINT:
> >     case PIPE_QUERY_TIME_ELAPSED:
> >        nvc0_query_get(push, q, 0x10, 0x00005002);
> >        break;
> > @@ -360,7 +359,6 @@ nvc0_query_end(struct pipe_context *pipe, struct
> > pipe_query *pq)
> >        nvc0_query_get(push, q, 0x20, 0x00005002);
> >        break;
> >     case PIPE_QUERY_TIMESTAMP:
> > -   case PIPE_QUERY_TIMESTAMP_DISJOINT:
> >     case PIPE_QUERY_TIME_ELAPSED:
> >        nvc0_query_get(push, q, 0, 0x00005002);
> >        break;
> > @@ -478,7 +476,7 @@ nvc0_query_result(struct pipe_context *pipe, struct
> > pipe_query *pq,
> >        break;
> >     case PIPE_QUERY_TIMESTAMP_DISJOINT: /* u32 sequence, u32 0, u64 time */
> >        res64[0] = 1000000000;
> 

Actually, jsut noticed it should be using a `struct pipe_query_data_timestamp_disjoint *` instead of res64/res8.

Jose

> 
> 
> > -      res8[8] = (data64[1] == data64[3]) ? FALSE : TRUE;
> > +      res8[8] = FALSE;
> >        break;
> >     case PIPE_QUERY_TIME_ELAPSED:
> >        res64[0] = data64[1] - data64[3];
> > --
> > 1.7.9.5
> >