| Message ID | 1407225244-6457-1-git-send-email-zhipeng.gong@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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> > --- > 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) There is room in the ring selection flags for expansion. -Chris
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. Also you'd need igt tests and all that too. -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
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 >
> > 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> > > --- > > 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) > > There is room in the ring selection flags for expansion. > -Chris Hi Chris There is only three rooms in the I915_EXEC_RING_MASK, this feature will occupy two rooms. If use these rooms, it will left few rooms for more possible rings in the future and it also introduces a little bit complexity to get the ring in the i915_do_gem_execbuffer. -Zhipeng
On Wed, Aug 6, 2014 at 3:32 AM, Zhao, Yakui <yakui.zhao@intel.com> wrote: > >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? Sure, but I'm not sure whether this is the correct way to do this - if some other libva client does an execbuf in-between couldn't these register get overwritten? It sounds like the kernel should capture these registers for userspace in a race-free way and then provide them with maybe an extension to the reset-stats ioctl? I definitely want to see how this is used in libva before signing up for a new kernel interface. -Daniel
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) \
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> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 19 +++++++++++++++++-- include/uapi/drm/i915_drm.h | 8 +++++++- 2 files changed, 24 insertions(+), 3 deletions(-)