[Mesa-dev] [PATCH 01/18] i965: Query whether we have kernel support for the TIMESTAMP register once

[Martin Peres]

On 06/07/15 13:33, Chris Wilson wrote:
> Move the query for the TIMESTAMP register from context init to the
> screen, so that it is only queried once for all contexts.
>
> On 32bit systems, some old kernels trigger a hw bug resulting in the
> TIMESTAMP register being shifted and the low bits always zero. Detect
> this by repeating the read a few times and check the register is
> incrementing.

You do not do the latter. You only check for the low bits.

I guess the counter is supposed to be monotonically increasing and with 
a resolution of a few microseconds which would make this perfectly 
valid. Could you confirm and make sure to add this information in the 
commit message please?

>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   src/mesa/drivers/dri/i965/intel_extensions.c |  6 +-----
>   src/mesa/drivers/dri/i965/intel_screen.c     | 15 +++++++++++++++
>   src/mesa/drivers/dri/i965/intel_screen.h     |  2 ++
>   3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 3423190..740ac81 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -282,8 +282,6 @@ intelInitExtensions(struct gl_context *ctx)
>      }
>   
>      if (brw->gen >= 6) {
> -      uint64_t dummy;
> -
>         ctx->Extensions.ARB_blend_func_extended =
>            !driQueryOptionb(&brw->optionCache, "disable_blend_func_extended");
>         ctx->Extensions.ARB_conditional_render_inverted = true;
> @@ -307,9 +305,7 @@ intelInitExtensions(struct gl_context *ctx)
>         ctx->Extensions.EXT_transform_feedback = true;
>         ctx->Extensions.OES_depth_texture_cube_map = true;
>   
> -      /* Test if the kernel has the ioctl. */
> -      if (drm_intel_reg_read(brw->bufmgr, TIMESTAMP, &dummy) == 0)
> -         ctx->Extensions.ARB_timer_query = true;
> +      ctx->Extensions.ARB_timer_query = brw->intelScreen->hw_has_timestamp;
>   
>         /* Only enable this in core profile because other parts of Mesa behave
>          * slightly differently when the extension is enabled.
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index f9398d7..839a984 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1123,6 +1123,20 @@ intel_detect_swizzling(struct intel_screen *screen)
>         return true;
>   }
>   
> +static bool
> +intel_detect_timestamp(struct intel_screen *screen)
> +{
> +   uint64_t dummy = 0;
> +   int loop = 10;
> +
> +   do {
> +      if (drm_intel_reg_read(screen->bufmgr, TIMESTAMP, &dummy))
> +	 return false;
> +   } while ((dummy & 0xffffffff) == 0 && --loop);
> +
> +   return loop > 0;
> +}
> +
>   /**
>    * Return array of MSAA modes supported by the hardware. The array is
>    * zero-terminated and sorted in decreasing order.
> @@ -1378,6 +1392,7 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>      intelScreen->hw_must_use_separate_stencil = intelScreen->devinfo->gen >= 7;
>   
>      intelScreen->hw_has_swizzling = intel_detect_swizzling(intelScreen);
> +   intelScreen->hw_has_timestamp = intel_detect_timestamp(intelScreen);
>   
>      const char *force_msaa = getenv("INTEL_FORCE_MSAA");
>      if (force_msaa) {
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index 742b3d3..941e0fc 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -52,6 +52,8 @@ struct intel_screen
>   
>      bool hw_has_swizzling;
>   
> +   bool hw_has_timestamp;
> +
>      /**
>       * Does the kernel support context reset notifications?
>       */