[Mesa-dev] [PATCH] i965: Only enable ARB_query_buffer_object for newer kernels on Haswell.

[Jordan Justen]

On 2016-05-05 03:15:51, Kenneth Graunke wrote:
> On Haswell, we need version 6 of the kernel command parser in order to
> write the math registers.  Our implementation of ARB_query_buffer_object
> heavily relies on MI_MATH, so we should only advertise it when MI_MATH
> is available.
> 

Doh! Command Parser!! :(

> We're going to want to use MI_MATH in more places in the future.
> To make checking for it easier, introduce a screen->has_mi_math flag.
> 
> Cc: Jordan Justen <jljusten at gmail.com>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/intel_extensions.c | 2 +-
>  src/mesa/drivers/dri/i965/intel_screen.c     | 8 ++++++++
>  src/mesa/drivers/dri/i965/intel_screen.h     | 5 +++++
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> Note: kernel command parser version 7 isn't actually a real thing yet.
> You need the patch I just sent to intel-gfx:
> [PATCH] drm/i915: Allow MI_LOAD_REGISTER_REG between whitelisted registers.
> 
> Also, Piglit's qbo test's
> query-GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN-ASYNC subtest fails for me
> on Haswell.  I haven't looked into why yet.
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 588df1a..7794c2f 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -366,7 +366,7 @@ intelInitExtensions(struct gl_context *ctx)
>        }
>     }
>  
> -   if (brw->gen >= 8 || brw->is_haswell) {
> +   if (brw->intelScreen->has_mi_math) {

Should we bump the check to gen >= 8 for now?

With just changing the check to not include hsw:

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

>        ctx->Extensions.ARB_query_buffer_object = true;
>     }
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 878901a..24fe523 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1541,6 +1541,14 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>     if (ret == -1)
>        intelScreen->cmd_parser_version = 0;
>  
> +   /* Haswell requires command parser version 6 in order to write to the
> +    * MI_MATH GPR registers, and version 7 in order to use
> +    * MI_LOAD_REGISTER_REG (which all users of MI_MATH use).
> +    */
> +   intelScreen->has_mi_math = intelScreen->devinfo->gen >= 8 ||
> +                              (intelScreen->devinfo->is_haswell &&
> +                               intelScreen->cmd_parser_version >= 7);

Do you think has_mi_math should imply LRR? Command parser version 6
should allow MI_MATH to be used. I guess the Vulkan driver uses
MI_MATH without LRR.

-Jordan

> +
>     psp->extensions = !intelScreen->has_context_reset_notification
>        ? intelScreenExtensions : intelRobustScreenExtensions;
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index 01d45d0..8000c4c 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -56,6 +56,11 @@ struct intel_screen
>     bool has_resource_streamer;
>  
>     /**
> +    * Does the current hardware and kernel support MI_MATH?
> +    */
> +   bool has_mi_math;
> +
> +   /**
>      * Does the kernel support context reset notifications?
>      */
>     bool has_context_reset_notification;
> -- 
> 2.8.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev