| Message ID | 20180209095404.76418-3-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Maarten Lankhorst (2018-02-09 09:54:01) > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_sprite.c | 10 ++++++---- > 3 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index b886bd459acc..eda9543a0199 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -845,7 +845,7 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc) > } > > /* I915_READ_FW, only for fast reads of display block, no need for forcewake etc. */ > -static int __intel_get_crtc_scanline(struct intel_crtc *crtc) > +int __intel_get_crtc_scanline(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 468ec1e90e16..fbdbbe741b2f 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1340,6 +1340,7 @@ static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv) > } > > int intel_get_crtc_scanline(struct intel_crtc *crtc); > +int __intel_get_crtc_scanline(struct intel_crtc *crtc); > void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, > u8 pipe_mask); > void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 971a1ea0db45..3a34be4fd956 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -119,6 +119,7 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) > crtc->debug.max_vbl = max; > trace_i915_pipe_update_start(crtc); > > + spin_lock(&dev_priv->uncore.lock); > for (;;) { > /* > * prepare_to_wait() has a memory barrier, which guarantees > @@ -127,7 +128,7 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) > */ > prepare_to_wait(wq, &wait, TASK_UNINTERRUPTIBLE); > > - scanline = intel_get_crtc_scanline(crtc); > + scanline = __intel_get_crtc_scanline(crtc); > if (scanline < min || scanline > max) > break; > > @@ -137,11 +138,11 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) > break; > } > > - local_irq_enable(); > + spin_unlock_irq(&dev_priv->uncore.lock); > > timeout = schedule_timeout(timeout); > > - local_irq_disable(); > + spin_lock_irq(&dev_priv->uncore.lock); > } > > finish_wait(wq, &wait); There's no danger that drm_crtc_vblank_put() does something crazy here. (Feels like a layering violation to call into DRM with the low level uncore.lock held at least.) It looks like the driver can be tricked into called ->disable_vblank()? Overall though, I think it is just this need_vlv_dsi_wa chunk that has any benefit here (although trading lock_irq for lock_irqsave is enough to justify a change if frequently hit). -Chris
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b886bd459acc..eda9543a0199 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -845,7 +845,7 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc) } /* I915_READ_FW, only for fast reads of display block, no need for forcewake etc. */ -static int __intel_get_crtc_scanline(struct intel_crtc *crtc) +int __intel_get_crtc_scanline(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 468ec1e90e16..fbdbbe741b2f 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1340,6 +1340,7 @@ static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv) } int intel_get_crtc_scanline(struct intel_crtc *crtc); +int __intel_get_crtc_scanline(struct intel_crtc *crtc); void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, u8 pipe_mask); void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 971a1ea0db45..3a34be4fd956 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -119,6 +119,7 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) crtc->debug.max_vbl = max; trace_i915_pipe_update_start(crtc); + spin_lock(&dev_priv->uncore.lock); for (;;) { /* * prepare_to_wait() has a memory barrier, which guarantees @@ -127,7 +128,7 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) */ prepare_to_wait(wq, &wait, TASK_UNINTERRUPTIBLE); - scanline = intel_get_crtc_scanline(crtc); + scanline = __intel_get_crtc_scanline(crtc); if (scanline < min || scanline > max) break; @@ -137,11 +138,11 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) break; } - local_irq_enable(); + spin_unlock_irq(&dev_priv->uncore.lock); timeout = schedule_timeout(timeout); - local_irq_disable(); + spin_lock_irq(&dev_priv->uncore.lock); } finish_wait(wq, &wait); @@ -162,8 +163,9 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) * FIXME figure out if BXT+ DSI suffers from this as well */ while (need_vlv_dsi_wa && scanline == vblank_start) - scanline = intel_get_crtc_scanline(crtc); + scanline = __intel_get_crtc_scanline(crtc); + spin_unlock(&dev_priv->uncore.lock); crtc->debug.scanline_start = scanline; crtc->debug.start_vbl_time = ktime_get(); crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_sprite.c | 10 ++++++---- 3 files changed, 8 insertions(+), 5 deletions(-)