On Thu, Mar 09, 2017 at 09:26:36PM +0000, Chris Wilson wrote: > 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). Sure. I'll add a note. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx