| Message ID | 1381266592-7558-2-git-send-email-abdiel.janulgue@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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
On 10/08/2013 02:09 PM, 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; > break; > case I915_EXEC_BSD: > ring = &dev_priv->ring[VCS]; > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 3a4e97b..5a4bd16 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -731,6 +731,11 @@ struct drm_i915_gem_execbuffer2 { > */ > #define I915_EXEC_HANDLE_LUT (1<<12) > > +/** Tell the kernel that the batchbuffer is processed by > + * the resource streamer. > + */ > +#define I915_EXEC_RESOURCE_STREAMER (1<<13) > + > #define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_HANDLE_LUT<<1) > > #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) > I think you need to change __I915_EXEC_UNKNOWN_FLAGS to: #define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER<<1) since you've added a new bit.
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; break; case I915_EXEC_BSD: ring = &dev_priv->ring[VCS]; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 3a4e97b..5a4bd16 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -731,6 +731,11 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_HANDLE_LUT (1<<12) +/** Tell the kernel that the batchbuffer is processed by + * the resource streamer. + */ +#define I915_EXEC_RESOURCE_STREAMER (1<<13) + #define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_HANDLE_LUT<<1) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)
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(+)