* [PATCH] drm/i915: Specify bsd rings through exec flag [not found] <CABVU7+uxdZEiH77CtdtveJtKi5x0mTAxuPiXT8389Y2EKRsRfQ@mail.gmail.com> @ 2014-11-17 11:06 ` Rodrigo Vivi 2014-11-17 18:58 ` Daniel Vetter 2014-11-18 7:39 ` shuang.he 0 siblings, 2 replies; 10+ messages in thread From: Rodrigo Vivi @ 2014-11-17 11:06 UTC (permalink / raw) To: intel-gfx; +Cc: Rodrigo Vivi From: Zhipeng Gong <zhipeng.gong@intel.com> 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. v2: fix whitespace (Rodrigo) v3: remove incorrect chunk that came on -collector rebase. (Rodrigo) Signed-off-by: Zhipeng Gong <zhipeng.gong@intel.com> Reviewed-by-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 e1ed85a..d9081ec 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1273,8 +1273,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 2502622..fcb16bf 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -737,7 +737,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.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Specify bsd rings through exec flag 2014-11-17 11:06 ` [PATCH] drm/i915: Specify bsd rings through exec flag Rodrigo Vivi @ 2014-11-17 18:58 ` Daniel Vetter 2014-11-18 7:39 ` shuang.he 1 sibling, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2014-11-17 18:58 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx On Mon, Nov 17, 2014 at 03:06:01AM -0800, Rodrigo Vivi wrote: > From: Zhipeng Gong <zhipeng.gong@intel.com> > > 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. > > v2: fix whitespace (Rodrigo) > v3: remove incorrect chunk that came on -collector rebase. (Rodrigo) > > Signed-off-by: Zhipeng Gong <zhipeng.gong@intel.com> Somehow Zhipeng isn't on the CC list - check your git send-email settings? Iirc the author should get added by default ... > Reviewed-by-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Already explained internally: If we don't have public&reviewed (by the libva maintainer) code using this then Dave Airlie will rip my head off if I merge this ;-) Also this needs adjustments to gem_exec_params (prts should catch the regression, if not gem_exec_params/invalid-flags is broken). And some tiny nits below. Cheers, 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 e1ed85a..d9081ec 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1273,8 +1273,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]; Do we have any use-case for selecting ring1 specifically? I've thought it's only ring2 that is special? > + break; > + case I915_EXEC_BSD_RING2: This needs a if (!IS_SKL(dev) return -EINVAL; check since the flag isnt valid anywhere else. Also you must add code to reject these flags if userspace selects a ring other than bsd. And all these new -EINVAL cases need new subtests to validate them in gem_exec_params.c. And I might have missed some case, ioctl validation is hard ;-) So please double-check that really no insane combination that userspace might try to abuse is caught and has a testcase in gem_exec_params. > + 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 2502622..fcb16bf 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -737,7 +737,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.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Specify bsd rings through exec flag 2014-11-17 11:06 ` [PATCH] drm/i915: Specify bsd rings through exec flag Rodrigo Vivi 2014-11-17 18:58 ` Daniel Vetter @ 2014-11-18 7:39 ` shuang.he 1 sibling, 0 replies; 10+ messages in thread From: shuang.he @ 2014-11-18 7:39 UTC (permalink / raw) To: shuang.he, intel-gfx, rodrigo.vivi Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate BYT: pass/total=290/291->290/291 PNV: pass/total=356/356->354/356 ILK: pass/total=371/372->371/372 IVB: pass/total=544/546->545/546 SNB: pass/total=424/425->424/425 HSW: pass/total=579/579->578/579 BDW: pass/total=432/435->432/435 -------------------------------------Detailed------------------------------------- test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)... BYT: Intel_gpu_tools, igt_gem_exec_params_invalid-flag, FAIL(3, M36)PASS(1, M36) -> FAIL(1, M36)PASS(3, M36) BYT: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M36)PASS(9, M31M36) -> TIMEOUT(3, M36)PASS(1, M36) PNV: Intel_gpu_tools, igt_gem_exec_params_invalid-flag, FAIL(3, M25)PASS(1, M25) -> FAIL(1, M25)PASS(3, M25) PNV: Intel_gpu_tools, igt_kms_sysfs_edid_timing, PASS(4, M25) -> FAIL(1, M25)PASS(3, M25) ILK: Intel_gpu_tools, igt_gem_exec_params_invalid-flag, FAIL(3, M37)PASS(1, M37) -> FAIL(1, M37)PASS(3, M37) ILK: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M37)PASS(9, M26M37) -> TIMEOUT(3, M37)PASS(1, M37) IVB: Intel_gpu_tools, igt_gem_exec_params_invalid-flag, FAIL(3, M34)PASS(1, M21) -> FAIL(1, M34)PASS(3, M34) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-random, DMESG_WARN(1, M21)PASS(6, M21M34) -> TIMEOUT(1, M34)PASS(3, M34) IVB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M21)PASS(6, M21M34) -> TIMEOUT(3, M34)PASS(1, M34) SNB: Intel_gpu_tools, igt_gem_exec_params_invalid-flag, FAIL(3, M35)PASS(1, M35) -> FAIL(1, M35)PASS(3, M35) SNB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M35)PASS(9, M35M22) -> TIMEOUT(3, M35)PASS(1, M35) HSW: Intel_gpu_tools, igt_gem_exec_params_invalid-flag, FAIL(3, M20)PASS(1, M19) -> FAIL(1, M20)PASS(3, M20) BDW: Intel_gpu_tools, igt_gem_exec_params_invalid-flag, FAIL(3, M28)PASS(1, M28) -> FAIL(1, M28)PASS(3, M28) BDW: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M28)PASS(6, M30M28) -> TIMEOUT(3, M28)PASS(1, M28) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] drm/i915: Specify bsd rings through exec flag @ 2014-12-10 2:04 Zhipeng Gong 0 siblings, 0 replies; 10+ messages in thread From: Zhipeng Gong @ 2014-12-10 2:04 UTC (permalink / raw) To: intel-gfx; +Cc: Rodrigo Vivi 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. v2: fix whitespace (Rodrigo) v3: remove incorrect chunk that came on -collector rebase. (Rodrigo) v4: change the comment (Zhipeng) Signed-off-by: Zhipeng Gong <zhipeng.gong@intel.com> Reviewed-by-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 0c25f62..de652a5 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1317,8 +1317,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 2502622..fcb16bf 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -737,7 +737,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.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] drm/i915: Specify bsd rings through exec flag @ 2014-08-05 7:54 Zhipeng Gong 2014-08-05 8:28 ` Chris Wilson 2014-08-05 8:44 ` Daniel Vetter 0 siblings, 2 replies; 10+ messages in thread From: Zhipeng Gong @ 2014-08-05 7:54 UTC (permalink / raw) To: intel-gfx 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) + +#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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Specify bsd rings through exec flag 2014-08-05 7:54 Zhipeng Gong @ 2014-08-05 8:28 ` Chris Wilson [not found] ` <A0A308111D30BF46A5A1E3C3D8A187A801139816@SHSMSX103.ccr.corp.intel.com> 2014-08-05 8:44 ` Daniel Vetter 1 sibling, 1 reply; 10+ messages in thread From: Chris Wilson @ 2014-08-05 8:28 UTC (permalink / raw) To: Zhipeng Gong; +Cc: 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 -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <A0A308111D30BF46A5A1E3C3D8A187A801139816@SHSMSX103.ccr.corp.intel.com>]
* Re: [PATCH] drm/i915: Specify bsd rings through exec flag [not found] ` <A0A308111D30BF46A5A1E3C3D8A187A801139816@SHSMSX103.ccr.corp.intel.com> @ 2014-08-06 3:19 ` Gong, Zhipeng 0 siblings, 0 replies; 10+ messages in thread From: Gong, Zhipeng @ 2014-08-06 3:19 UTC (permalink / raw) To: Chris Wilson; +Cc: 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Specify bsd rings through exec flag 2014-08-05 7:54 Zhipeng Gong 2014-08-05 8:28 ` Chris Wilson @ 2014-08-05 8:44 ` Daniel Vetter 2014-08-06 1:32 ` Zhao, Yakui 1 sibling, 1 reply; 10+ messages in thread From: Daniel Vetter @ 2014-08-05 8:44 UTC (permalink / raw) To: Zhipeng Gong; +Cc: 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> 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Specify bsd rings through exec flag 2014-08-05 8:44 ` Daniel Vetter @ 2014-08-06 1:32 ` Zhao, Yakui 2014-08-06 7:45 ` Daniel Vetter 0 siblings, 1 reply; 10+ messages in thread From: Zhao, Yakui @ 2014-08-06 1:32 UTC (permalink / raw) To: Daniel Vetter; +Cc: 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Specify bsd rings through exec flag 2014-08-06 1:32 ` Zhao, Yakui @ 2014-08-06 7:45 ` Daniel Vetter 0 siblings, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2014-08-06 7:45 UTC (permalink / raw) To: Zhao, Yakui; +Cc: intel-gfx 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-12-10 2:23 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CABVU7+uxdZEiH77CtdtveJtKi5x0mTAxuPiXT8389Y2EKRsRfQ@mail.gmail.com>
2014-11-17 11:06 ` [PATCH] drm/i915: Specify bsd rings through exec flag Rodrigo Vivi
2014-11-17 18:58 ` Daniel Vetter
2014-11-18 7:39 ` shuang.he
2014-12-10 2:04 Zhipeng Gong
-- strict thread matches above, loose matches on Subject: below --
2014-08-05 7:54 Zhipeng Gong
2014-08-05 8:28 ` Chris Wilson
[not found] ` <A0A308111D30BF46A5A1E3C3D8A187A801139816@SHSMSX103.ccr.corp.intel.com>
2014-08-06 3:19 ` Gong, Zhipeng
2014-08-05 8:44 ` Daniel Vetter
2014-08-06 1:32 ` Zhao, Yakui
2014-08-06 7:45 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.