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

[Abdiel Janulgue]

On 06/16/2015 03:20 PM, Chris Wilson wrote:
> 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.

I think dev_info is supposed to be some "guide" that needs to be
consulted of whether HW supports certain features. This kernel check is
just there to see if the kernel has the support as well. But this is
just my opinion, I'll let Ken and others comment on this.

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

Ok

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