[Intel-gfx] [PATCH 1/2] drm/i915: Specify bsd rings through exec flag

[Rodrigo Vivi]

Sorry for the delay.

Also for v5:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

On Tue, Jan 13, 2015 at 4:36 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Tue, Jan 13, 2015 at 08:48:24AM +0800, Zhipeng Gong wrote:
>> On Skylake GT3 we have 2 Video Command Streamers (VCS), which is asymmetrical.
>> For example, HEVC GPU commands can be only dispatched to VCS1 ring.
>> But userspace has no control when using VCS1 or VCS2. This patch introduces
>> a mechanism to avoid the default ping-pong mode and use one specific ring
>> through execution flag. This mechanism is usable for all the platforms
>> with 2 VCS rings.
>>
>> The open source usage is from these two commits in vaapi/intel:
>> commit 702050f04131a44ef8ac16651708ce8a8d98e4b8
>> Author: Zhao, Yakui <yakui.zhao at intel.com>
>> Date:   Mon Nov 17 12:44:19 2014 +0800
>>
>>    Allow the batchbuffer to be submitted with override flag
>>
>> commit a56efcdf27d11ad9b21664b4a2cda72d7f90f5a8
>> Author: Zhao Yakui <yakui.zhao at intel.com>
>> Date:   Mon Nov 17 12:44:22 2014 +0800
>>
>>    Add the override flag to assure that HEVC video command
>> always uses BSD ring0 for SKL GT3 machine
>>
>> v2: fix whitespace (Rodrigo)
>> v3: remove incorrect chunk that came on -collector rebase. (Rodrigo)
>> v4: change the comment (Zhipeng)
>> v5: address Daniel's comment (Zhipeng)
>
> Can you please be more specific here for the in-patch changelog? The idea
> is that people understand the changes with just this, as-is you need to
> dig out the old review thread (which is pretty much impossible).
>
> I'll add that to the commit message when merging, so no need to
> resend. I'll merge as soon as Rodrigo has doublechecked the revised
> patches.
> -Daniel
>
>>
>> Signed-off-by: Zhipeng Gong <zhipeng.gong at intel.com>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com> (for v4)
>> ---
>>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 26 ++++++++++++++++++++++++--
>>  include/uapi/drm/i915_drm.h                |  8 +++++++-
>>  2 files changed, 31 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> index e3ef177..b773368 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> @@ -1380,13 +1380,35 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>>   return -EINVAL;
>>   }
>>
>> + if (((args->flags & I915_EXEC_RING_MASK) != I915_EXEC_BSD) &&
>> +    ((args->flags & I915_EXEC_BSD_MASK) != 0)) {
>> + DRM_DEBUG("execbuf with non bsd ring but with invalid "
>> + "bsd dispatch flags: %d\n", (int)(args->flags));
>> + return -EINVAL;
>> + }
>> +
>>   if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT)
>>   ring = &dev_priv->ring[RCS];
>>   else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
>>   if (HAS_BSD2(dev)) {
>>   int ring_id;
>> - ring_id = gen8_dispatch_bsd_ring(dev, file);
>> - ring = &dev_priv->ring[ring_id];
>> +
>> + switch (args->flags & I915_EXEC_BSD_MASK) {
>> + case I915_EXEC_BSD_DEFAULT:
>> + ring_id = gen8_dispatch_bsd_ring(dev, file);
>> + ring = &dev_priv->ring[ring_id];
>> + break;
>> + case I915_EXEC_BSD_RING1:
>> + ring = &dev_priv->ring[VCS];
>> + break;
>> + case I915_EXEC_BSD_RING2:
>> + ring = &dev_priv->ring[VCS2];
>> + break;
>> + default:
>> + DRM_DEBUG("execbuf with unknown bsd ring: %d\n",
>> +  (int)(args->flags & I915_EXEC_BSD_MASK));
>> + return -EINVAL;
>> + }
>>   } else
>>   ring = &dev_priv->ring[VCS];
>>   } else
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index 2e559f6e..dc84561 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -750,7 +750,13 @@ struct drm_i915_gem_execbuffer2 {
>>   */
>>  #define I915_EXEC_HANDLE_LUT (1<<12)
>>
>> -#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_HANDLE_LUT<<1)
>> +/** Used for switching BSD rings on the platforms with two BSD rings */
>> +#define I915_EXEC_BSD_MASK (3<<13)
>> +#define I915_EXEC_BSD_DEFAULT (0<<13) /* default ping-pong mode */
>> +#define I915_EXEC_BSD_RING1 (1<<13)
>> +#define I915_EXEC_BSD_RING2 (2<<13)
>> +
>> +#define __I915_EXEC_UNKNOWN_FLAGS -(1<<15)
>>
>>  #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)
>>  #define i915_execbuffer2_set_context_id(eb2, context) \
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br