Re: [Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code
On Tue, Mar 10, 2015 at 11:26:29AM +0100, Daniel Vetter wrote:
> On Thu, Mar 05, 2015 at 09:19:49PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Assuming the PND deadline mechanism works reasonably we should do
> > memory requests as early as possible so that PND has schedule the
> > requests more intelligently. Currently we're still calculating
> > the watermarks as if VLV/CHV are identical to g4x, which isn't
> > the case.
> >
> > The current code also seems to calculate insufficient watermarks
> > and hence we're seeing some underruns, especially on high resolution
> > displays.
> >
> > To fix it just rip out the current code and replace is with something
> > that tries to utilize PND as efficiently as possible.
> >
> > We now calculate the WM watermark to trigger when the FIFO still has
> > 256us worth of data. 256us is the maximum deadline value supoorted by
> > PND, so issuing memory requests earlier would mean we probably couldn't
> > utilize the full FIFO as PND would attempt to return the data at
> > least in at least 256us. We also clamp the watermark to at least 8
> > cachelines as that's the magic watermark that enabling trickle feed
> > would also impose. I'm assuming it matches some burst size.
> >
> > In theory we could just enable trickle feed and ignore the WM values,
> > except trickle feed doesn't work with max fifo mode anyway, so we'd
> > still need to calculate the SR watermarks. It seems cleaner to just
> > disable trickle feed and calculate all watermarks the same way. Also
> > trickle feed wouldn't account for the 256us max deadline value, thoguh
> > that may be a moot point in non-max fifo mode sicne the FIFOs are fairly
> > small.
> >
> > On VLV max fifo mode can be used with either primary or sprite planes.
> > So the code now also checks all the planes (apart from the cursor)
> > when calculating the SR plane watermark.
> >
> > We don't have to worry about the WM1 watermarks since we're using the
> > PND deadline scheme which means the hardware ignores WM1 values.
> >
> > v2: Use plane->state->fb instead of plane->fb
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 11 ++
> > drivers/gpu/drm/i915/i915_reg.h | 4 +-
> > drivers/gpu/drm/i915/intel_pm.c | 330
> > +---
> > 3 files changed, 188 insertions(+), 157 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 5de69a0..b191b12 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1516,6 +1516,17 @@ struct ilk_wm_values {
> >
> > struct vlv_wm_values {
> > struct {
> > + uint16_t primary;
> > + uint16_t sprite[2];
> > + uint8_t cursor;
> > + } pipe[3];
> > +
> > + struct {
> > + uint16_t plane;
> > + uint8_t cursor;
> > + } sr;
> > +
> > + struct {
> > uint8_t cursor;
> > uint8_t sprite[2];
> > uint8_t primary;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 8178610..9f98384 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4127,7 +4127,7 @@ enum skl_disp_power_wells {
> > /* vlv/chv high order bits */
> > #define DSPHOWM(VLV_DISPLAY_BASE + 0x70064)
> > #define DSPFW_SR_HI_SHIFT24
> > -#define DSPFW_SR_HI_MASK (1<<24)
> > +#define DSPFW_SR_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */
> > #define DSPFW_SPRITEF_HI_SHIFT 23
> > #define DSPFW_SPRITEF_HI_MASK(1<<23)
> > #define DSPFW_SPRITEE_HI_SHIFT 22
> > @@ -4148,7 +4148,7 @@ enum skl_disp_power_wells {
> > #define DSPFW_PLANEA_HI_MASK (1<<0)
> > #define DSPHOWM1 (VLV_DISPLAY_BASE + 0x70068)
> > #define DSPFW_SR_WM1_HI_SHIFT24
> > -#define DSPFW_SR_WM1_HI_MASK (1<<24)
> > +#define DSPFW_SR_WM1_HI_MASK (3<<24) /* 2 bits for chv, 1
> > for vlv */
> > #define DSPFW_SPRITEF_WM1_HI_SHIFT 23
> > #define DSPFW_SPRITEF_WM1_HI_MASK(1<<23)
> > #define DSPFW_SPRITEE_WM1_HI_SHIFT 22
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index bdb0f5d..497847c 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -778,6 +778,55 @@ static void vlv_write_wm_values(struct intel_crtc
> > *crtc,
> >(wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) |
> >(wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
> >
> > + I915_WRITE(DSPFW1,
> > + ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> > + ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) &
> > DSPFW_CURSORB_MASK) |
> > + ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) &
> > DSPFW_PLANEB_MA
Re: [Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code
On Thu, Mar 05, 2015 at 09:19:49PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Assuming the PND deadline mechanism works reasonably we should do
> memory requests as early as possible so that PND has schedule the
> requests more intelligently. Currently we're still calculating
> the watermarks as if VLV/CHV are identical to g4x, which isn't
> the case.
>
> The current code also seems to calculate insufficient watermarks
> and hence we're seeing some underruns, especially on high resolution
> displays.
>
> To fix it just rip out the current code and replace is with something
> that tries to utilize PND as efficiently as possible.
>
> We now calculate the WM watermark to trigger when the FIFO still has
> 256us worth of data. 256us is the maximum deadline value supoorted by
> PND, so issuing memory requests earlier would mean we probably couldn't
> utilize the full FIFO as PND would attempt to return the data at
> least in at least 256us. We also clamp the watermark to at least 8
> cachelines as that's the magic watermark that enabling trickle feed
> would also impose. I'm assuming it matches some burst size.
>
> In theory we could just enable trickle feed and ignore the WM values,
> except trickle feed doesn't work with max fifo mode anyway, so we'd
> still need to calculate the SR watermarks. It seems cleaner to just
> disable trickle feed and calculate all watermarks the same way. Also
> trickle feed wouldn't account for the 256us max deadline value, thoguh
> that may be a moot point in non-max fifo mode sicne the FIFOs are fairly
> small.
>
> On VLV max fifo mode can be used with either primary or sprite planes.
> So the code now also checks all the planes (apart from the cursor)
> when calculating the SR plane watermark.
>
> We don't have to worry about the WM1 watermarks since we're using the
> PND deadline scheme which means the hardware ignores WM1 values.
>
> v2: Use plane->state->fb instead of plane->fb
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_drv.h | 11 ++
> drivers/gpu/drm/i915/i915_reg.h | 4 +-
> drivers/gpu/drm/i915/intel_pm.c | 330
> +---
> 3 files changed, 188 insertions(+), 157 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5de69a0..b191b12 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1516,6 +1516,17 @@ struct ilk_wm_values {
>
> struct vlv_wm_values {
> struct {
> + uint16_t primary;
> + uint16_t sprite[2];
> + uint8_t cursor;
> + } pipe[3];
> +
> + struct {
> + uint16_t plane;
> + uint8_t cursor;
> + } sr;
> +
> + struct {
> uint8_t cursor;
> uint8_t sprite[2];
> uint8_t primary;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8178610..9f98384 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4127,7 +4127,7 @@ enum skl_disp_power_wells {
> /* vlv/chv high order bits */
> #define DSPHOWM (VLV_DISPLAY_BASE + 0x70064)
> #define DSPFW_SR_HI_SHIFT 24
> -#define DSPFW_SR_HI_MASK (1<<24)
> +#define DSPFW_SR_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */
> #define DSPFW_SPRITEF_HI_SHIFT 23
> #define DSPFW_SPRITEF_HI_MASK (1<<23)
> #define DSPFW_SPRITEE_HI_SHIFT 22
> @@ -4148,7 +4148,7 @@ enum skl_disp_power_wells {
> #define DSPFW_PLANEA_HI_MASK (1<<0)
> #define DSPHOWM1 (VLV_DISPLAY_BASE + 0x70068)
> #define DSPFW_SR_WM1_HI_SHIFT 24
> -#define DSPFW_SR_WM1_HI_MASK (1<<24)
> +#define DSPFW_SR_WM1_HI_MASK (3<<24) /* 2 bits for chv, 1
> for vlv */
> #define DSPFW_SPRITEF_WM1_HI_SHIFT 23
> #define DSPFW_SPRITEF_WM1_HI_MASK (1<<23)
> #define DSPFW_SPRITEE_WM1_HI_SHIFT 22
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bdb0f5d..497847c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -778,6 +778,55 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
> (wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) |
> (wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
>
> + I915_WRITE(DSPFW1,
> +((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> +((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) &
> DSPFW_CURSORB_MASK) |
> +((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) &
> DSPFW_PLANEB_MASK_VLV) |
> +((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) &
> DSPFW_PLANEA_MASK_VLV));
> + I915_WRITE(DSPFW2,
> +((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) &
> DSPFW_SPRITEB_MASK_VLV) |
> +((wm-
Re: [Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code
On 03/06/2015 10:14 AM, Ville Syrjälä wrote: > On Fri, Mar 06, 2015 at 09:31:20AM -0800, Jesse Barnes wrote: >> On 03/05/2015 11:19 AM, ville.syrj...@linux.intel.com wrote: >>> From: Ville Syrjälä >> I wonder if we should be warning if the wm values we end up with exceed >> the mask size (the fact that you write them with a shift and mask made >> me think of it), but that could be a follow on, or even put into the >> calc code instead. Anyway that's something we can do later after all >> the atomic changes hit. > > IIRC we always have enough bits up to any legal FIFO size, so the clamping > done by vlv_compute_wm() should be enough. I should double check that though > since that isn't the case on a bunch of the other platforms. > > I think in general I'd really like magic register bitfield macros that > scream whenever we overflow something by accident. But that's a much > bigger topic. For one we'd have to parametrize all the macros rather than > using raw shifts. Yeah and that would have the added benefit of more readability. Something for another day if/when we see underruns due to failed wm programming in the future. :) > >> >> Does this fix any of our underrun bugs? Should it have any references: >> lines? > > Those should probably be at the DDR DVFS disable patch since before that > pretty much anything can happen. I was too lazy to trawl bugzilla though. > Pretty much hoping QA can just go retest all display bugs once we get > this landed. Ok, sounds good. Thanks, Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code
On Fri, Mar 06, 2015 at 09:31:20AM -0800, Jesse Barnes wrote:
> On 03/05/2015 11:19 AM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Assuming the PND deadline mechanism works reasonably we should do
> > memory requests as early as possible so that PND has schedule the
> > requests more intelligently. Currently we're still calculating
> > the watermarks as if VLV/CHV are identical to g4x, which isn't
> > the case.
> >
> > The current code also seems to calculate insufficient watermarks
> > and hence we're seeing some underruns, especially on high resolution
> > displays.
> >
> > To fix it just rip out the current code and replace is with something
> > that tries to utilize PND as efficiently as possible.
> >
> > We now calculate the WM watermark to trigger when the FIFO still has
> > 256us worth of data. 256us is the maximum deadline value supoorted by
> > PND, so issuing memory requests earlier would mean we probably couldn't
> > utilize the full FIFO as PND would attempt to return the data at
> > least in at least 256us. We also clamp the watermark to at least 8
> > cachelines as that's the magic watermark that enabling trickle feed
> > would also impose. I'm assuming it matches some burst size.
> >
> > In theory we could just enable trickle feed and ignore the WM values,
> > except trickle feed doesn't work with max fifo mode anyway, so we'd
> > still need to calculate the SR watermarks. It seems cleaner to just
> > disable trickle feed and calculate all watermarks the same way. Also
> > trickle feed wouldn't account for the 256us max deadline value, thoguh
> > that may be a moot point in non-max fifo mode sicne the FIFOs are fairly
> > small.
> >
> > On VLV max fifo mode can be used with either primary or sprite planes.
> > So the code now also checks all the planes (apart from the cursor)
> > when calculating the SR plane watermark.
> >
> > We don't have to worry about the WM1 watermarks since we're using the
> > PND deadline scheme which means the hardware ignores WM1 values.
> >
> > v2: Use plane->state->fb instead of plane->fb
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 11 ++
> > drivers/gpu/drm/i915/i915_reg.h | 4 +-
> > drivers/gpu/drm/i915/intel_pm.c | 330
> > +---
> > 3 files changed, 188 insertions(+), 157 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 5de69a0..b191b12 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1516,6 +1516,17 @@ struct ilk_wm_values {
> >
> > struct vlv_wm_values {
> > struct {
> > + uint16_t primary;
> > + uint16_t sprite[2];
> > + uint8_t cursor;
> > + } pipe[3];
> > +
> > + struct {
> > + uint16_t plane;
> > + uint8_t cursor;
> > + } sr;
> > +
> > + struct {
> > uint8_t cursor;
> > uint8_t sprite[2];
> > uint8_t primary;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 8178610..9f98384 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4127,7 +4127,7 @@ enum skl_disp_power_wells {
> > /* vlv/chv high order bits */
> > #define DSPHOWM(VLV_DISPLAY_BASE + 0x70064)
> > #define DSPFW_SR_HI_SHIFT24
> > -#define DSPFW_SR_HI_MASK (1<<24)
> > +#define DSPFW_SR_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */
> > #define DSPFW_SPRITEF_HI_SHIFT 23
> > #define DSPFW_SPRITEF_HI_MASK(1<<23)
> > #define DSPFW_SPRITEE_HI_SHIFT 22
> > @@ -4148,7 +4148,7 @@ enum skl_disp_power_wells {
> > #define DSPFW_PLANEA_HI_MASK (1<<0)
> > #define DSPHOWM1 (VLV_DISPLAY_BASE + 0x70068)
> > #define DSPFW_SR_WM1_HI_SHIFT24
> > -#define DSPFW_SR_WM1_HI_MASK (1<<24)
> > +#define DSPFW_SR_WM1_HI_MASK (3<<24) /* 2 bits for chv, 1
> > for vlv */
> > #define DSPFW_SPRITEF_WM1_HI_SHIFT 23
> > #define DSPFW_SPRITEF_WM1_HI_MASK(1<<23)
> > #define DSPFW_SPRITEE_WM1_HI_SHIFT 22
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index bdb0f5d..497847c 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -778,6 +778,55 @@ static void vlv_write_wm_values(struct intel_crtc
> > *crtc,
> >(wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) |
> >(wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
> >
> > + I915_WRITE(DSPFW1,
> > + ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> > + ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) &
> > DSPFW_CURSORB_MASK) |
> > + ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) &
> > DSPFW_PLANEB_MASK_VLV) |
> > +
Re: [Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code
On Fri, Mar 06, 2015 at 09:31:20AM -0800, Jesse Barnes wrote: > I wonder if we should be warning if the wm values we end up with exceed > the mask size (the fact that you write them with a shift and mask made > me think of it), but that could be a follow on, or even put into the > calc code instead. Anyway that's something we can do later after all > the atomic changes hit. Hm, I've thought the overall plan for wm computation code is that we calculate the wm lines we need and then the magic atomic consolidation step meshes it all together appropriately. But hey I actually about something approaching 0 clue on this topic, it's really all magic here ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code
On 03/05/2015 11:19 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Assuming the PND deadline mechanism works reasonably we should do
> memory requests as early as possible so that PND has schedule the
> requests more intelligently. Currently we're still calculating
> the watermarks as if VLV/CHV are identical to g4x, which isn't
> the case.
>
> The current code also seems to calculate insufficient watermarks
> and hence we're seeing some underruns, especially on high resolution
> displays.
>
> To fix it just rip out the current code and replace is with something
> that tries to utilize PND as efficiently as possible.
>
> We now calculate the WM watermark to trigger when the FIFO still has
> 256us worth of data. 256us is the maximum deadline value supoorted by
> PND, so issuing memory requests earlier would mean we probably couldn't
> utilize the full FIFO as PND would attempt to return the data at
> least in at least 256us. We also clamp the watermark to at least 8
> cachelines as that's the magic watermark that enabling trickle feed
> would also impose. I'm assuming it matches some burst size.
>
> In theory we could just enable trickle feed and ignore the WM values,
> except trickle feed doesn't work with max fifo mode anyway, so we'd
> still need to calculate the SR watermarks. It seems cleaner to just
> disable trickle feed and calculate all watermarks the same way. Also
> trickle feed wouldn't account for the 256us max deadline value, thoguh
> that may be a moot point in non-max fifo mode sicne the FIFOs are fairly
> small.
>
> On VLV max fifo mode can be used with either primary or sprite planes.
> So the code now also checks all the planes (apart from the cursor)
> when calculating the SR plane watermark.
>
> We don't have to worry about the WM1 watermarks since we're using the
> PND deadline scheme which means the hardware ignores WM1 values.
>
> v2: Use plane->state->fb instead of plane->fb
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_drv.h | 11 ++
> drivers/gpu/drm/i915/i915_reg.h | 4 +-
> drivers/gpu/drm/i915/intel_pm.c | 330
> +---
> 3 files changed, 188 insertions(+), 157 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5de69a0..b191b12 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1516,6 +1516,17 @@ struct ilk_wm_values {
>
> struct vlv_wm_values {
> struct {
> + uint16_t primary;
> + uint16_t sprite[2];
> + uint8_t cursor;
> + } pipe[3];
> +
> + struct {
> + uint16_t plane;
> + uint8_t cursor;
> + } sr;
> +
> + struct {
> uint8_t cursor;
> uint8_t sprite[2];
> uint8_t primary;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8178610..9f98384 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4127,7 +4127,7 @@ enum skl_disp_power_wells {
> /* vlv/chv high order bits */
> #define DSPHOWM (VLV_DISPLAY_BASE + 0x70064)
> #define DSPFW_SR_HI_SHIFT 24
> -#define DSPFW_SR_HI_MASK (1<<24)
> +#define DSPFW_SR_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */
> #define DSPFW_SPRITEF_HI_SHIFT 23
> #define DSPFW_SPRITEF_HI_MASK (1<<23)
> #define DSPFW_SPRITEE_HI_SHIFT 22
> @@ -4148,7 +4148,7 @@ enum skl_disp_power_wells {
> #define DSPFW_PLANEA_HI_MASK (1<<0)
> #define DSPHOWM1 (VLV_DISPLAY_BASE + 0x70068)
> #define DSPFW_SR_WM1_HI_SHIFT 24
> -#define DSPFW_SR_WM1_HI_MASK (1<<24)
> +#define DSPFW_SR_WM1_HI_MASK (3<<24) /* 2 bits for chv, 1
> for vlv */
> #define DSPFW_SPRITEF_WM1_HI_SHIFT 23
> #define DSPFW_SPRITEF_WM1_HI_MASK (1<<23)
> #define DSPFW_SPRITEE_WM1_HI_SHIFT 22
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bdb0f5d..497847c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -778,6 +778,55 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
> (wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) |
> (wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
>
> + I915_WRITE(DSPFW1,
> +((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> +((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) &
> DSPFW_CURSORB_MASK) |
> +((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) &
> DSPFW_PLANEB_MASK_VLV) |
> +((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) &
> DSPFW_PLANEA_MASK_VLV));
> + I915_WRITE(DSPFW2,
> +((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) &
> DSPFW_SPRITEB_MASK_VLV) |
> +((wm->pipe[PIPE_A].curs
[Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code
From: Ville Syrjälä
Assuming the PND deadline mechanism works reasonably we should do
memory requests as early as possible so that PND has schedule the
requests more intelligently. Currently we're still calculating
the watermarks as if VLV/CHV are identical to g4x, which isn't
the case.
The current code also seems to calculate insufficient watermarks
and hence we're seeing some underruns, especially on high resolution
displays.
To fix it just rip out the current code and replace is with something
that tries to utilize PND as efficiently as possible.
We now calculate the WM watermark to trigger when the FIFO still has
256us worth of data. 256us is the maximum deadline value supoorted by
PND, so issuing memory requests earlier would mean we probably couldn't
utilize the full FIFO as PND would attempt to return the data at
least in at least 256us. We also clamp the watermark to at least 8
cachelines as that's the magic watermark that enabling trickle feed
would also impose. I'm assuming it matches some burst size.
In theory we could just enable trickle feed and ignore the WM values,
except trickle feed doesn't work with max fifo mode anyway, so we'd
still need to calculate the SR watermarks. It seems cleaner to just
disable trickle feed and calculate all watermarks the same way. Also
trickle feed wouldn't account for the 256us max deadline value, thoguh
that may be a moot point in non-max fifo mode sicne the FIFOs are fairly
small.
On VLV max fifo mode can be used with either primary or sprite planes.
So the code now also checks all the planes (apart from the cursor)
when calculating the SR plane watermark.
We don't have to worry about the WM1 watermarks since we're using the
PND deadline scheme which means the hardware ignores WM1 values.
v2: Use plane->state->fb instead of plane->fb
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_drv.h | 11 ++
drivers/gpu/drm/i915/i915_reg.h | 4 +-
drivers/gpu/drm/i915/intel_pm.c | 330 +---
3 files changed, 188 insertions(+), 157 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5de69a0..b191b12 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1516,6 +1516,17 @@ struct ilk_wm_values {
struct vlv_wm_values {
struct {
+ uint16_t primary;
+ uint16_t sprite[2];
+ uint8_t cursor;
+ } pipe[3];
+
+ struct {
+ uint16_t plane;
+ uint8_t cursor;
+ } sr;
+
+ struct {
uint8_t cursor;
uint8_t sprite[2];
uint8_t primary;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8178610..9f98384 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4127,7 +4127,7 @@ enum skl_disp_power_wells {
/* vlv/chv high order bits */
#define DSPHOWM(VLV_DISPLAY_BASE + 0x70064)
#define DSPFW_SR_HI_SHIFT24
-#define DSPFW_SR_HI_MASK (1<<24)
+#define DSPFW_SR_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */
#define DSPFW_SPRITEF_HI_SHIFT 23
#define DSPFW_SPRITEF_HI_MASK(1<<23)
#define DSPFW_SPRITEE_HI_SHIFT 22
@@ -4148,7 +4148,7 @@ enum skl_disp_power_wells {
#define DSPFW_PLANEA_HI_MASK (1<<0)
#define DSPHOWM1 (VLV_DISPLAY_BASE + 0x70068)
#define DSPFW_SR_WM1_HI_SHIFT24
-#define DSPFW_SR_WM1_HI_MASK (1<<24)
+#define DSPFW_SR_WM1_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */
#define DSPFW_SPRITEF_WM1_HI_SHIFT 23
#define DSPFW_SPRITEF_WM1_HI_MASK(1<<23)
#define DSPFW_SPRITEE_WM1_HI_SHIFT 22
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bdb0f5d..497847c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -778,6 +778,55 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
(wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) |
(wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
+ I915_WRITE(DSPFW1,
+ ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
+ ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) &
DSPFW_CURSORB_MASK) |
+ ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) &
DSPFW_PLANEB_MASK_VLV) |
+ ((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) &
DSPFW_PLANEA_MASK_VLV));
+ I915_WRITE(DSPFW2,
+ ((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) &
DSPFW_SPRITEB_MASK_VLV) |
+ ((wm->pipe[PIPE_A].cursor << DSPFW_CURSORA_SHIFT) &
DSPFW_CURSORA_MASK) |
+ ((wm->pipe[PIPE_A].sprite[0] << DSPFW_SPRITEA_SHIFT) &
DSPFW_SPRITEA_MASK_VLV));
+ I915_WRITE(DSPFW3,
+ ((wm->sr.cursor << DSPFW_CURSOR_SR_SHI
