[Mesa-dev] [PATCH v3 2/6] i965: Enable resource streamer for the batchbuffer

[Chris Wilson]

On Tue, Jun 16, 2015 at 03:08:56PM +0300, Abdiel Janulgue wrote:
> Check first if the hardware and kernel supports resource streamer. If this
> is allowed, tell the kernel to enable the resource streamer enable bit on
> MI_BATCHBUFFER_START by specifying I915_EXEC_RESOURCE_STREAMER
> execbuffer flags.
> 
> v2: - Use new I915_PARAM_HAS_RESOURCE_STREAMER ioctl to check if kernel
>       supports RS (Ken).
>     - Add brw_device_info::has_resource_streamer and toggle it for
>       Haswell, Broadwell, Cherryview, Skylake, and Broxton (Ken).
> v3: - Update I915_PARAM_HAS_RESOURCE_STREAMER to match updated kernel.
> 
> Cc: kenneth at whitecape.org
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_context.c       |  5 +++++
>  src/mesa/drivers/dri/i965/brw_context.h       |  1 +
>  src/mesa/drivers/dri/i965/brw_device_info.c   |  5 ++++-
>  src/mesa/drivers/dri/i965/brw_device_info.h   |  1 +
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c |  8 +++++++-
>  src/mesa/drivers/dri/i965/intel_screen.c      | 12 ++++++++++++
>  src/mesa/drivers/dri/i965/intel_screen.h      |  5 +++++
>  7 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index f39b350..27abf0e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -896,6 +896,11 @@ brwCreateContext(gl_api api,
>  
>     brw->predicate.state = BRW_PREDICATE_STATE_RENDER;
>  
> +   brw->use_resource_streamer = devinfo->has_resource_streamer &&
> +      screen->has_resource_streamer &&
> +      (brw_env_var_as_boolean("INTEL_USE_HW_BT", false) ||
> +       brw_env_var_as_boolean("INTEL_USE_GATHER", false));

Do you expect the envionment to change between contexts? Looks like all
of these can be used to compute screen->has_resource_streamer.
dev_info->has_resource_streamer is superfluous as well since it just
requires the kernel support.

>  /**
>   * This is the driver specific part of the createNewScreen entry point.
>   * Called when using DRI2.
> @@ -1420,6 +1425,13 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>     intelScreen->compiler = brw_compiler_create(intelScreen,
>                                                 intelScreen->devinfo);
>  
> +   if (intelScreen->devinfo->is_haswell || intelScreen->devinfo->gen >= 8) {
> +      getparam.param = I915_PARAM_HAS_RESOURCE_STREAMER;

So devinfo->has_resource_streamer was just a mirage after all.

> +      const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GETPARAM, &getparam);
> +      intelScreen->has_resource_streamer = (ret != -1 || errno != EINVAL);

NO!!! You must always inspect the getparam.value. The trick is.

getparam.param = HAS_RESOURCE_STREAMER;
getparam.value = &val;

val = -1;
drmIoctl(psp->fd, DRM_IOCTL_I915_GETPARAM, &getparam);
intelScreen->has_resource_streamer = val > 0;

Hmm, now I need to double check the kernel patch for you GETPARAM.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre