Re: [PATCH 12/15] drm/i915: Zero out HOWM registers before writing new WM/HOWM register values

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Dec 01, 2016 at 03:43:07PM +0100, Maarten Lankhorst wrote:
> Op 28-11-16 om 18:37 schreef ville.syrjala@linux.intel.com:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > On VLV/CHV some of the watermark values are split across two registers:
> > low order bits in one, and high order bits in another. So we may not be
> > able to update a single watermark value atomically, and thus we must be
> > careful that we don't temporarily introduce out of bounds values during
> > the reprogramming. To prevent this we can simply zero out all the high
> > order bits initially, then we update the low order bits, and finally
> > we update the high order bits with the final value.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 9f25f2195a6a..8a3441bbff30 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -877,6 +877,17 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
> >  		   (wm->ddl[pipe].plane[PLANE_SPRITE0] << DDL_SPRITE_SHIFT(0)) |
> >  		   (wm->ddl[pipe].plane[PLANE_PRIMARY] << DDL_PLANE_SHIFT));
> >  
> > +	/*
> > +	 * Zero the (unused) WM1 watermarks, and also clear all the
> > +	 * high order bits so that there are no out of bounds values
> > +	 * present in the registers during the reprogramming.
> > +	 */
> > +	I915_WRITE(DSPHOWM, 0);
> > +	I915_WRITE(DSPHOWM1, 0);
> > +	I915_WRITE(DSPFW4, 0);
> > +	I915_WRITE(DSPFW5, 0);
> > +	I915_WRITE(DSPFW6, 0);
> Watermarks for DSPHOWM are inverted right? And lower values just mean more wakeups?

Yes, it's all inverted.

> Should be harmless then.
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >  	I915_WRITE(DSPFW1,
> >  		   FW_WM(wm->sr.plane, SR) |
> >  		   FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) |
> > @@ -924,12 +935,6 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
> >  			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY] >> 8, PLANEA_HI));
> >  	}
> >  
> > -	/* zero (unused) WM1 watermarks */
> > -	I915_WRITE(DSPFW4, 0);
> > -	I915_WRITE(DSPFW5, 0);
> > -	I915_WRITE(DSPFW6, 0);
> > -	I915_WRITE(DSPHOWM1, 0);
> > -
> >  	POSTING_READ(DSPFW1);
> >  }
> >  
> 

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx