Re: [PATCH] drm/i915: Specify bsd rings through exec flag

["Zhao, Yakui" <yakui.zhao@intel.com>]

On Tue, 2014-08-05 at 02:44 -0600, Daniel Vetter wrote:
> On Tue, Aug 05, 2014 at 03:54:04PM +0800, Zhipeng Gong wrote:
> > On Broadwell GT3 we have 2 Video Command Streamers (VCS), 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.
> > 
> > Signed-off-by: Zhipeng Gong <zhipeng.gong@intel.com>
> 
> This needs an open-source user and proper justification why we need this.
> On bdw. Iirc the only users is content protection which isn't open-source
> due to the usual concerns, so if that hasn't changed this patch is
> rejected.

Hi, Daniel

    The open-source media driver also needs this feature for BDW. Now we
are planning to add the following function that depends on this flag for
BDW with two BSD rings.
    >After the GPU hang occurs on BSD ring during decoding, it needs to
specify the corresponding BSD ring to read the decoding status registers
related with the BSD ring. 
     
      Can this be regarded as one open-source usage scenario?

> 
> Also you'd need igt tests and all that too.

I agree. The igt test is needed.

Maybe one patch is missing that exposes the flag of dual BSD rings.
Only when the flag exists, we can specify which bsd ring to dispatch the
BSD video command.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 19 +++++++++++++++++--
> >  include/uapi/drm/i915_drm.h                |  8 +++++++-
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 60998fc..f9ed8e0 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -1279,8 +1279,23 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> >  	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 ff57f07..421420a 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -736,7 +736,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) \
> > -- 
> > 2.0.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>