On Fri, Mar 10, 2017 at 12:07:53PM +0200, Ville Syrjälä wrote:
> 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.
+ * intel_pipe_update_start() has already disabled interrupts
+ * for us, so a plain spin_lock() is sufficient here.
*/
And entire series pushed to dinq. Thanks for the reviews.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
