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

[Ville Syrjälä]

On Thu, Dec 01, 2016 at 03:43:07PM +0100, Maarten Lankhorst wrote:
> Op 28-11-16 om 18:37 schreef ville.syrj...@linux.intel.com:
> > From: Ville Syrjälä 
> >
> > 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ä 
> > ---
> >  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 
> > 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

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

[Maarten Lankhorst]

Op 28-11-16 om 18:37 schreef ville.syrj...@linux.intel.com:
> From: Ville Syrjälä 
>
> 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ä 
> ---
>  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?
Should be harmless then.

Reviewed-by: Maarten Lankhorst 
>   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);
>  }
>  


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[ville . syrjala]

From: Ville Syrjälä 

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ä 
---
 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);
+
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);
 }
 
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx