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

[Jose Fonseca]

----- 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;



> -      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
>