| Message ID | 1472731101-21982-1-git-send-email-joonas.lahtinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 01, 2016 at 02:58:21PM +0300, Joonas Lahtinen wrote: > Use atomic type and operands for dev_priv->mm.bsd_engine_dispatch_index > to avoid one struct_mutex locking scenario. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Zhao Yakui <yakui.zhao@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Looks fine, it's one that I've simply been too lazy to worry about. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Thu, Sep 01, 2016 at 12:20:18PM -0000, Patchwork wrote: > == Series Details == > > Series: drm/i915: Use atomic for dev_priv->mm.bsd_engine_dispatch_index > URL : https://patchwork.freedesktop.org/series/11890/ > State : failure > > == Summary == > > Series 11890v1 drm/i915: Use atomic for dev_priv->mm.bsd_engine_dispatch_index > http://patchwork.freedesktop.org/api/1.0/series/11890/revisions/1/mbox/ > > Test prime_vgem: > Subgroup basic-fence-flip: > pass -> FAIL (fi-bdw-5557u) Hmm, this is probably some condition I forgot to drain before starting the test. What's the actual failure? -Chris
On to, 2016-09-01 at 13:27 +0100, Chris Wilson wrote: > On Thu, Sep 01, 2016 at 12:20:18PM -0000, Patchwork wrote: > > Test prime_vgem: > > Subgroup basic-fence-flip: > > pass -> FAIL (fi-bdw-5557u) > Hmm, this is probably some condition I forgot to drain before starting > the test. What's the actual failure? (prime_vgem:29095) CRITICAL: Test assertion failure function flip_to_vgem, file prime_vgem.c:651: (prime_vgem:29095) CRITICAL: Failed assertion: poll(&pfd, 1, 20000) == 1 (prime_vgem:29095) CRITICAL: error: 0 != 1 Subtest basic-fence-flip failed. **** DEBUG **** (prime_vgem:29089) ioctl-wrappers-DEBUG: Test requirement passed: has_modifiers (prime_vgem:29089) ioctl-wrappers-DEBUG: Test requirement passed: has_modifiers (prime_vgem:29089) DEBUG: Test requirement passed: (crtc_id = set_fb_on_crtc(i915, 0, &bo[0], fb_id[0])) **** END **** Regards, Joonas
On to, 2016-09-01 at 15:32 +0300, Joonas Lahtinen wrote: > On to, 2016-09-01 at 13:27 +0100, Chris Wilson wrote: > > > > On Thu, Sep 01, 2016 at 12:20:18PM -0000, Patchwork wrote: > > > > > > Test prime_vgem: > > > Subgroup basic-fence-flip: > > > pass -> FAIL (fi-bdw-5557u) > > Hmm, this is probably some condition I forgot to drain before starting > > the test. What's the actual failure? > (prime_vgem:29095) CRITICAL: Test assertion failure function flip_to_vgem, file prime_vgem.c:651: > (prime_vgem:29095) CRITICAL: Failed assertion: poll(&pfd, 1, 20000) == 1 > (prime_vgem:29095) CRITICAL: error: 0 != 1 > Subtest basic-fence-flip failed. > **** DEBUG **** > (prime_vgem:29089) ioctl-wrappers-DEBUG: Test requirement passed: has_modifiers > (prime_vgem:29089) ioctl-wrappers-DEBUG: Test requirement passed: has_modifiers > (prime_vgem:29089) DEBUG: Test requirement passed: (crtc_id = set_fb_on_crtc(i915, 0, &bo[0], fb_id[0])) > **** END **** > Anyway, pushed the patch, thanks for review. Regards, Joonas > Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c413587..43d5298 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1338,7 +1338,7 @@ struct i915_gem_mm { bool interruptible; /* the indicator for dispatch video commands on two BSD rings */ - unsigned int bsd_engine_dispatch_index; + atomic_t bsd_engine_dispatch_index; /** Bit 6 swizzling required for X tiling */ uint32_t bit_6_swizzle_x; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index d37b441..2401818 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4622,6 +4622,8 @@ i915_gem_load_init(struct drm_device *dev) dev_priv->mm.interruptible = true; + atomic_set(&dev_priv->mm.bsd_engine_dispatch_index, 0); + spin_lock_init(&dev_priv->fb_tracking.lock); } diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 57d096a..9432d4c 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1535,13 +1535,9 @@ gen8_dispatch_bsd_engine(struct drm_i915_private *dev_priv, struct drm_i915_file_private *file_priv = file->driver_priv; /* Check whether the file_priv has already selected one ring. */ - if ((int)file_priv->bsd_engine < 0) { - /* If not, use the ping-pong mechanism to select one. */ - mutex_lock(&dev_priv->drm.struct_mutex); - file_priv->bsd_engine = dev_priv->mm.bsd_engine_dispatch_index; - dev_priv->mm.bsd_engine_dispatch_index ^= 1; - mutex_unlock(&dev_priv->drm.struct_mutex); - } + if ((int)file_priv->bsd_engine < 0) + file_priv->bsd_engine = atomic_fetch_xor(1, + &dev_priv->mm.bsd_engine_dispatch_index); return file_priv->bsd_engine; }
Use atomic type and operands for dev_priv->mm.bsd_engine_dispatch_index to avoid one struct_mutex locking scenario. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Imre Deak <imre.deak@intel.com> Cc: Zhao Yakui <yakui.zhao@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gem.c | 2 ++ drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 +++------- 3 files changed, 6 insertions(+), 8 deletions(-)