On Thu, Feb 10, 2022 at 08:24:02AM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Drop the locks around most primary plane register writes. > The lock isn't needed since each plane's register are neatly > contained on their own cachelines. > > The one exception we have to make is DSPADDR/DSPSURF which is > (ab)used to also trigger FBC nukes on pre-snb (since the > hardware doesn't seem to have any dedicated mechanism to > trigger nukes). So we need to keep the lock around it to > protect against the rmw performed by the fbc code. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/i9xx_plane.c | 24 +++++++++-------------- > 1 file changed, 9 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c > index a87b65cd41fd..af190bacdd97 100644 > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c > @@ -418,9 +418,6 @@ static void i9xx_plane_update_noarm(struct intel_plane *plane, > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; > - unsigned long irqflags; > - > - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > intel_de_write_fw(dev_priv, DSPSTRIDE(i9xx_plane), > plane_state->view.color_plane[0].mapping_stride); > @@ -441,8 +438,6 @@ static void i9xx_plane_update_noarm(struct intel_plane *plane, > intel_de_write_fw(dev_priv, DSPSIZE(i9xx_plane), > DISP_HEIGHT(crtc_h - 1) | DISP_WIDTH(crtc_w - 1)); > } > - > - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > > static void i9xx_plane_update_arm(struct intel_plane *plane, > @@ -465,8 +460,6 @@ static void i9xx_plane_update_arm(struct intel_plane *plane, > else > dspaddr_offset = linear_offset; > > - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > - > if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) { > int crtc_x = plane_state->uapi.dst.x1; > int crtc_y = plane_state->uapi.dst.y1; > @@ -496,13 +489,15 @@ static void i9xx_plane_update_arm(struct intel_plane *plane, > * the control register just before the surface register. > */ > intel_de_write_fw(dev_priv, DSPCNTR(i9xx_plane), dspcntr); > + > + /* lock to protect against rmw in fbc nuke */ > + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > if (DISPLAY_VER(dev_priv) >= 4) > intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane), > intel_plane_ggtt_offset(plane_state) + dspaddr_offset); > else > intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane), > intel_plane_ggtt_offset(plane_state) + dspaddr_offset); > - > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > > @@ -540,14 +535,14 @@ static void i9xx_plane_disable_arm(struct intel_plane *plane, > */ > dspcntr = i9xx_plane_ctl_crtc(crtc_state); > > - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > - > intel_de_write_fw(dev_priv, DSPCNTR(i9xx_plane), dspcntr); > + > + /* lock to protect against rmw in fbc nuke */ > + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > if (DISPLAY_VER(dev_priv) >= 4) > intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane), 0); > else > intel_de_write_fw(dev_priv, DSPADDR(i9xx_plane), 0); > - > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > > @@ -566,8 +561,10 @@ g4x_primary_async_flip(struct intel_plane *plane, > if (async_flip) > dspcntr |= DISP_ASYNC_FLIP; > > - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > intel_de_write_fw(dev_priv, DSPCNTR(i9xx_plane), dspcntr); > + > + /* lock to protect against rmw in fbc nuke */ > + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > intel_de_write_fw(dev_priv, DSPSURF(i9xx_plane), > intel_plane_ggtt_offset(plane_state) + dspaddr_offset); > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > @@ -582,12 +579,9 @@ vlv_primary_async_flip(struct intel_plane *plane, > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > u32 dspaddr_offset = plane_state->view.color_plane[0].offset; > enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; > - unsigned long irqflags; > > - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > intel_de_write_fw(dev_priv, DSPADDR_VLV(i9xx_plane), > intel_plane_ggtt_offset(plane_state) + dspaddr_offset); > - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > > static void > -- > 2.34.1 >