On Thu, Mar 09, 2017 at 05:44:34PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Use I915_{READ,WRITE}_FW() for updating the DSPARB registers on > VLV/CHV. This is less expesive as we can grab the uncore.lock across > the entire sequence of reads and writes instead of each register > access grabbing it. > > This also allows us to eliminate the dsparb lock entirely as the > uncore.lock now effectively protects the contents of the DSPARB > registers. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.c | 1 - > drivers/gpu/drm/i915/i915_drv.h | 3 --- > drivers/gpu/drm/i915/intel_pm.c | 36 +++++++++++++++++++++--------------- > 3 files changed, 21 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index b1e9027a4f80..debb74a2b2a9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -827,7 +827,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, > > spin_lock_init(&dev_priv->mm.object_stat_lock); > spin_lock_init(&dev_priv->mmio_flip_lock); > - spin_lock_init(&dev_priv->wm.dsparb_lock); > mutex_init(&dev_priv->sb_lock); > mutex_init(&dev_priv->modeset_restore_lock); > mutex_init(&dev_priv->av_mutex); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 3002996ddbed..6af0b1d33cab 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2375,9 +2375,6 @@ struct drm_i915_private { > } sagv_status; > > struct { > - /* protects DSPARB registers on pre-g4x/vlv/chv */ > - spinlock_t dsparb_lock; > - > /* > * Raw watermark latency values: > * in 0.1us units for WM0, > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 99e09f63d4b3..24cdc13a416a 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -1358,13 +1358,19 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state, > > trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size); > > - spin_lock(&dev_priv->wm.dsparb_lock); > + /* > + * uncore.lock serves a double purpose here. It allows us to > + * use the less expensive I915_{READ,WRITE}_FW() functions, and > + * it protects the DSPARB registers from getting clobbered by > + * parallel updates from multiple pipes. > + */ > + spin_lock(&dev_priv->uncore.lock); Might be nice to document that the irq is disabled by intel_pipe_update_start() (hence why spin_lock is safe). -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx