Re: [PATCH 1/2] drm/i915/hsw: Add I915_EXEC_RESOURCE_STREAMER flag

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 08, 2013 at 04:38:31PM -0700, Ben Widawsky wrote:
> On Tue, Oct 08, 2013 at 10:50:50PM +0100, Chris Wilson wrote:
> > On Wed, Oct 09, 2013 at 12:09:51AM +0300, Abdiel Janulgue wrote:
> > > Ensures that the batch buffer is executed by the resource streamer.
> > > 
> > > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 ++
> > >  include/uapi/drm/i915_drm.h                |    5 +++++
> > >  2 files changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > index 0ce0d47..4a56c58 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > @@ -962,6 +962,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> > >  	case I915_EXEC_DEFAULT:
> > >  	case I915_EXEC_RENDER:
> > >  		ring = &dev_priv->ring[RCS];
> > > +		flags |= (args->flags & I915_EXEC_RESOURCE_STREAMER) ?
> > > +		  I915_EXEC_RESOURCE_STREAMER : 0;
> > 
> > Besides this being flags |= args->flags & I915_EXEC_RESOURCE_STREAMER;
> > flags is a completely different bitfield and you should be translating
> > into a dispatch value rather than the execbuffer value.
> > -Chris
> > 
> 
> To decrypt Chris just a bit, though he was rather more verbose than
> usual ;-), see the translation of I915_EXEC_SECURE to
> I915_DISPATCH_SECURE.

Also an i-g-t testcase which checks that we correctly reject this flag
would be good (i.e. reject it on non-render rings and on platforms that
don't support the resource streamer). We don't yet have any execbuf flags
tests at all, so this is a good opportunity to fix this.

I've tried to sign up Abdiel for this on Jira, but he's not there ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch