Re: [Intel-gfx] [PATCH v4 4/8] drm/i915/gen9: WM memory bandwidth related workaround

[Paulo Zanoni]

Em Qui, 2016-11-10 às 11:24 +0530, Mahesh Kumar escreveu:
> Hi,
> 
> 
> 
(removed a bunch of stuff here)

> > 
> > > 
> > 
> > 
> > > 
> > > + bool y_tile_enabled = false;
> > > +
> > if (!platforms_that_require_the_wa) {
> > wa = WATERMARK_WA_NONE;
> > return;
> > }
> this function is not called by any GEN other than GEN9, will it be ok
> to 
> add "if (!GEN9)" check?

Well, there's Gemnilake support floating around the mailing list now...

> > 
> > > 
> > > + if (!memdev_info->valid)
> > > + goto exit;
> > Our default behavior should be to apply the WA then in doubt, not
> > to avoid it, so the return value here and in the other error cases
> > should be WATERAMARK_WA_Y_TILED.
> > 
> > Also, you can avoid the gotos by just setting mem_wa at the
> > beginning
> > of the function, then you can just "return" later instead of goto.
> > 
> > 
> > > 
> > > +
> > > + system_bw = memdev_info->mem_speed_khz * memdev_info-
> > > > 
> > > > num_channels *
> > > + memdev_info-
> > > >channel_width_bytes;
> > > +
> > > + if (!system_bw)
> > > + goto exit;
> > This shouldn't be possible. Otherwise, the previous patch needs to
> > be
> > fixed in a way to not allow system_bw to end up as zero. Please
> > either
> > remove the check or change to "if (WARN_ON(!system_bw))".
> yes, ideally this should not be possible, but what if because of any 
> reason BIOS is not writing the register OR we don't have BIOS
> altogether?

Then the previous patch should be able to deal with it, and leave us
with memdev_info->valid=false.


> 
> If we add WARN_ON here, it'll flood the logs, we can add
> WARN_ON_ONCE, 
> but we are anyway printing the warning in previous patch, if
> frequency 
> or any other variable calculated is ZERO, IMO will add more prints
> in 
> previous patch instead of adding here.

Usually we add WARNs when we expect them to never ever be triggered. So
instead of making it WARN_ON_ONCE, we should make sure the condition is
never met: if the memory info is bad, set memdev_info->valid to false.


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

Re: [Intel-gfx] [PATCH v4 4/8] drm/i915/gen9: WM memory bandwidth related workaround

[Mahesh Kumar]

Hi,


On Friday 04 November 2016 10:39 PM, Paulo Zanoni wrote:

Em Qui, 2016-10-13 às 16:28 +0530, Kumar, Mahesh escreveu:

This patch implemnets Workariunds related to display arbitrated
memory
bandwidth. These WA are applicabe for all gen-9 based platforms.

Changes since v1:
  - Rebase on top of Paulo's patch series
Changes since v2:
  - Rebase/rework after addressing Paulo's comments in previous patch

A lot of this code has changed since then, so this will need a
significant rebase. In the meantime, I added skl_needs_memory_bw_wa()
and we're now applying the WA by default: we just won't apply the WA
when we're pretty sure we don't need to. This helps avoiding underruns
by default.

See more below.

make sense, Will change default WA to WA_T_TILED

Signed-off-by: "Kumar, Mahesh" 
---
  drivers/gpu/drm/i915/i915_drv.h  |   9 +++
  drivers/gpu/drm/i915/intel_drv.h |  11 +++
  drivers/gpu/drm/i915/intel_pm.c  | 146
+++
  3 files changed, 166 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index adbd9aa..c169360 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1092,6 +1092,13 @@ enum intel_sbi_destination {
SBI_MPHY,
  };
  
+/* SKL+ Watermark arbitrated display bandwidth Workarounds */

+enum watermark_memory_wa {
+   WATERMARK_WA_NONE,
+   WATERMARK_WA_X_TILED,
+   WATERMARK_WA_Y_TILED,
+};
+
  #define QUIRK_PIPEA_FORCE (1<<0)
  #define QUIRK_LVDS_SSC_DISABLE (1<<1)
  #define QUIRK_INVERT_BRIGHTNESS (1<<2)
@@ -1644,6 +1651,8 @@ struct skl_ddb_allocation {
  
  struct skl_wm_values {

unsigned dirty_pipes;
+   /* any WaterMark memory workaround Required */

We can remove this comment since it doesn't say anything the variable
name doesn't.


+   enum watermark_memory_wa mem_wa;

Now that we have a proper variable in the state struct, it probably
makes sense to just kill skl_needs_memory_bw_wa() and read this
variable when we need to.



struct skl_ddb_allocation ddb;
uint32_t wm_linetime[I915_MAX_PIPES];
uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index f48e79a..2c1897b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1813,6 +1813,17 @@ intel_atomic_get_crtc_state(struct
drm_atomic_state *state,
return to_intel_crtc_state(crtc_state);
  }
  
+static inline struct intel_crtc_state *

+intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
+ struct intel_crtc *crtc)
+{
+   struct drm_crtc_state *crtc_state;
+
+   crtc_state = drm_atomic_get_existing_crtc_state(state,
>base);
+
+   return to_intel_crtc_state(crtc_state);

I really don't like the idea of calling to_intel_crtc_state() on a
potentially NULL pointer so the caller of this function will also check
for NULL. Even though it works today, I still think it's unsafe
practice. Please check crtc_state for NULL directly and then return
NULL.

Also, I think this function should be extracted to its own commit, and
we'd probably be able to find some callers in the existing i915 code.

Will extract this function & include NULL check for crtc_state itself



+}
+
  static inline struct intel_plane_state *
  intel_atomic_get_existing_plane_state(struct drm_atomic_state
*state,
  struct intel_plane *plane)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 84ec6b1..5b8f715 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3589,6 +3589,8 @@ static int skl_compute_plane_wm(const struct
drm_i915_private *dev_priv,
  {
struct drm_plane_state *pstate = _pstate->base;
struct drm_framebuffer *fb = pstate->fb;
+   struct intel_atomic_state *intel_state =
+   to_intel_atomic_state(cstate->base.state);
uint32_t latency = dev_priv->wm.skl_latency[level];
uint32_t method1, method2;
uint32_t plane_bytes_per_line, plane_blocks_per_line;
@@ -3598,10 +3600,17 @@ static int skl_compute_plane_wm(const struct
drm_i915_private *dev_priv,
uint32_t width = 0, height = 0;
uint32_t plane_pixel_rate;
uint32_t y_tile_minimum, y_min_scanlines;
+   enum watermark_memory_wa mem_wa;
  
  	if (latency == 0 || !cstate->base.active || !intel_pstate-

base.visible)

return 0;
  
+	mem_wa = intel_state ? intel_state->wm_results.mem_wa :

WATERMARK_WA_NONE;
+   if (mem_wa != WATERMARK_WA_NONE) {
+   if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
+   latency += 15;
+   }
+
width = drm_rect_width(_pstate->base.src) >> 16;
height = drm_rect_height(_pstate->base.src) >> 16;
  
@@ -3634,6 +3643,9 @@ static int skl_compute_plane_wm(const 

Re: [Intel-gfx] [PATCH v4 4/8] drm/i915/gen9: WM memory bandwidth related workaround

[Ville Syrjälä]

On Fri, Nov 04, 2016 at 03:09:04PM -0200, Paulo Zanoni wrote:
> Em Qui, 2016-10-13 às 16:28 +0530, Kumar, Mahesh escreveu:
> > This patch implemnets Workariunds related to display arbitrated
> > memory
> > bandwidth. These WA are applicabe for all gen-9 based platforms.
> > 
> > Changes since v1:
> >  - Rebase on top of Paulo's patch series
> > Changes since v2:
> >  - Rebase/rework after addressing Paulo's comments in previous patch
> 
> A lot of this code has changed since then, so this will need a
> significant rebase. In the meantime, I added skl_needs_memory_bw_wa()
> and we're now applying the WA by default: we just won't apply the WA
> when we're pretty sure we don't need to. This helps avoiding underruns
> by default.
> 
> See more below.
> 
> 
> > 
> > Signed-off-by: "Kumar, Mahesh" 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |   9 +++
> >  drivers/gpu/drm/i915/intel_drv.h |  11 +++
> >  drivers/gpu/drm/i915/intel_pm.c  | 146
> > +++
> >  3 files changed, 166 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index adbd9aa..c169360 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1092,6 +1092,13 @@ enum intel_sbi_destination {
> >     SBI_MPHY,
> >  };
> >  
> > +/* SKL+ Watermark arbitrated display bandwidth Workarounds */
> > +enum watermark_memory_wa {
> > +   WATERMARK_WA_NONE,
> > +   WATERMARK_WA_X_TILED,
> > +   WATERMARK_WA_Y_TILED,
> > +};
> > +
> >  #define QUIRK_PIPEA_FORCE (1<<0)
> >  #define QUIRK_LVDS_SSC_DISABLE (1<<1)
> >  #define QUIRK_INVERT_BRIGHTNESS (1<<2)
> > @@ -1644,6 +1651,8 @@ struct skl_ddb_allocation {
> >  
> >  struct skl_wm_values {
> >     unsigned dirty_pipes;
> > +   /* any WaterMark memory workaround Required */
> 
> We can remove this comment since it doesn't say anything the variable
> name doesn't.
> 
> > +   enum watermark_memory_wa mem_wa;
> 
> Now that we have a proper variable in the state struct, it probably
> makes sense to just kill skl_needs_memory_bw_wa() and read this
> variable when we need to.
> 
> 
> >     struct skl_ddb_allocation ddb;
> >     uint32_t wm_linetime[I915_MAX_PIPES];
> >     uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index f48e79a..2c1897b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1813,6 +1813,17 @@ intel_atomic_get_crtc_state(struct
> > drm_atomic_state *state,
> >     return to_intel_crtc_state(crtc_state);
> >  }
> >  
> > +static inline struct intel_crtc_state *
> > +intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
> > +     struct intel_crtc *crtc)
> > +{
> > +   struct drm_crtc_state *crtc_state;
> > +
> > +   crtc_state = drm_atomic_get_existing_crtc_state(state,
> > >base);
> > +
> > +   return to_intel_crtc_state(crtc_state);
> 
> I really don't like the idea of calling to_intel_crtc_state() on a
> potentially NULL pointer so the caller of this function will also check
> for NULL. Even though it works today, I still think it's unsafe
> practice. Please check crtc_state for NULL directly and then return
> NULL.

I want to make this safe by making it a compile error if offsetof(base) != 0.
https://lists.freedesktop.org/archives/intel-gfx/2016-October/108175.html

But I think we want to go further than that patch by adding a bit more
type safety to things. I did play around with this stuff a bit more,
and I have something sitting on a branch, but I didn't quite figure out
what I want to do about const vs. non const yet.

> 
> Also, I think this function should be extracted to its own commit, and
> we'd probably be able to find some callers in the existing i915 code.

I have, on some branch again, _intel_ versions of the for_each_foo_in_state()
macros as well. I think those are going to allow a lot of ugly casting
stuff to disappear. But I think I'll hold off until Maarten's new
iterators go in before I try to send those out.

-- 
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 v4 4/8] drm/i915/gen9: WM memory bandwidth related workaround

[Paulo Zanoni]

Em Qui, 2016-10-13 às 16:28 +0530, Kumar, Mahesh escreveu:
> This patch implemnets Workariunds related to display arbitrated
> memory
> bandwidth. These WA are applicabe for all gen-9 based platforms.
> 
> Changes since v1:
>  - Rebase on top of Paulo's patch series
> Changes since v2:
>  - Rebase/rework after addressing Paulo's comments in previous patch

A lot of this code has changed since then, so this will need a
significant rebase. In the meantime, I added skl_needs_memory_bw_wa()
and we're now applying the WA by default: we just won't apply the WA
when we're pretty sure we don't need to. This helps avoiding underruns
by default.

See more below.


> 
> Signed-off-by: "Kumar, Mahesh" 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |   9 +++
>  drivers/gpu/drm/i915/intel_drv.h |  11 +++
>  drivers/gpu/drm/i915/intel_pm.c  | 146
> +++
>  3 files changed, 166 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index adbd9aa..c169360 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1092,6 +1092,13 @@ enum intel_sbi_destination {
>   SBI_MPHY,
>  };
>  
> +/* SKL+ Watermark arbitrated display bandwidth Workarounds */
> +enum watermark_memory_wa {
> + WATERMARK_WA_NONE,
> + WATERMARK_WA_X_TILED,
> + WATERMARK_WA_Y_TILED,
> +};
> +
>  #define QUIRK_PIPEA_FORCE (1<<0)
>  #define QUIRK_LVDS_SSC_DISABLE (1<<1)
>  #define QUIRK_INVERT_BRIGHTNESS (1<<2)
> @@ -1644,6 +1651,8 @@ struct skl_ddb_allocation {
>  
>  struct skl_wm_values {
>   unsigned dirty_pipes;
> + /* any WaterMark memory workaround Required */

We can remove this comment since it doesn't say anything the variable
name doesn't.

> + enum watermark_memory_wa mem_wa;

Now that we have a proper variable in the state struct, it probably
makes sense to just kill skl_needs_memory_bw_wa() and read this
variable when we need to.


>   struct skl_ddb_allocation ddb;
>   uint32_t wm_linetime[I915_MAX_PIPES];
>   uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f48e79a..2c1897b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1813,6 +1813,17 @@ intel_atomic_get_crtc_state(struct
> drm_atomic_state *state,
>   return to_intel_crtc_state(crtc_state);
>  }
>  
> +static inline struct intel_crtc_state *
> +intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
> +   struct intel_crtc *crtc)
> +{
> + struct drm_crtc_state *crtc_state;
> +
> + crtc_state = drm_atomic_get_existing_crtc_state(state,
> >base);
> +
> + return to_intel_crtc_state(crtc_state);

I really don't like the idea of calling to_intel_crtc_state() on a
potentially NULL pointer so the caller of this function will also check
for NULL. Even though it works today, I still think it's unsafe
practice. Please check crtc_state for NULL directly and then return
NULL.

Also, I think this function should be extracted to its own commit, and
we'd probably be able to find some callers in the existing i915 code.


> +}
> +
>  static inline struct intel_plane_state *
>  intel_atomic_get_existing_plane_state(struct drm_atomic_state
> *state,
>     struct intel_plane *plane)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 84ec6b1..5b8f715 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3589,6 +3589,8 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>  {
>   struct drm_plane_state *pstate = _pstate->base;
>   struct drm_framebuffer *fb = pstate->fb;
> + struct intel_atomic_state *intel_state =
> + to_intel_atomic_state(cstate->base.state);
>   uint32_t latency = dev_priv->wm.skl_latency[level];
>   uint32_t method1, method2;
>   uint32_t plane_bytes_per_line, plane_blocks_per_line;
> @@ -3598,10 +3600,17 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>   uint32_t width = 0, height = 0;
>   uint32_t plane_pixel_rate;
>   uint32_t y_tile_minimum, y_min_scanlines;
> + enum watermark_memory_wa mem_wa;
>  
>   if (latency == 0 || !cstate->base.active || !intel_pstate-
> >base.visible)
>   return 0;
>  
> + mem_wa = intel_state ? intel_state->wm_results.mem_wa :
> WATERMARK_WA_NONE;
> + if (mem_wa != WATERMARK_WA_NONE) {
> + if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
> + latency += 15;
> + }
> +
>   width = drm_rect_width(_pstate->base.src) >> 16;
>   height = drm_rect_height(_pstate->base.src) >> 16;
>  
> @@ -3634,6 +3643,9 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>   

[Intel-gfx] [PATCH v4 4/8] drm/i915/gen9: WM memory bandwidth related workaround

[Kumar, Mahesh]

This patch implemnets Workariunds related to display arbitrated memory
bandwidth. These WA are applicabe for all gen-9 based platforms.

Changes since v1:
 - Rebase on top of Paulo's patch series
Changes since v2:
 - Rebase/rework after addressing Paulo's comments in previous patch

Signed-off-by: "Kumar, Mahesh" 
---
 drivers/gpu/drm/i915/i915_drv.h  |   9 +++
 drivers/gpu/drm/i915/intel_drv.h |  11 +++
 drivers/gpu/drm/i915/intel_pm.c  | 146 +++
 3 files changed, 166 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index adbd9aa..c169360 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1092,6 +1092,13 @@ enum intel_sbi_destination {
SBI_MPHY,
 };
 
+/* SKL+ Watermark arbitrated display bandwidth Workarounds */
+enum watermark_memory_wa {
+   WATERMARK_WA_NONE,
+   WATERMARK_WA_X_TILED,
+   WATERMARK_WA_Y_TILED,
+};
+
 #define QUIRK_PIPEA_FORCE (1<<0)
 #define QUIRK_LVDS_SSC_DISABLE (1<<1)
 #define QUIRK_INVERT_BRIGHTNESS (1<<2)
@@ -1644,6 +1651,8 @@ struct skl_ddb_allocation {
 
 struct skl_wm_values {
unsigned dirty_pipes;
+   /* any WaterMark memory workaround Required */
+   enum watermark_memory_wa mem_wa;
struct skl_ddb_allocation ddb;
uint32_t wm_linetime[I915_MAX_PIPES];
uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f48e79a..2c1897b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1813,6 +1813,17 @@ intel_atomic_get_crtc_state(struct drm_atomic_state 
*state,
return to_intel_crtc_state(crtc_state);
 }
 
+static inline struct intel_crtc_state *
+intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state,
+ struct intel_crtc *crtc)
+{
+   struct drm_crtc_state *crtc_state;
+
+   crtc_state = drm_atomic_get_existing_crtc_state(state, >base);
+
+   return to_intel_crtc_state(crtc_state);
+}
+
 static inline struct intel_plane_state *
 intel_atomic_get_existing_plane_state(struct drm_atomic_state *state,
  struct intel_plane *plane)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 84ec6b1..5b8f715 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3589,6 +3589,8 @@ static int skl_compute_plane_wm(const struct 
drm_i915_private *dev_priv,
 {
struct drm_plane_state *pstate = _pstate->base;
struct drm_framebuffer *fb = pstate->fb;
+   struct intel_atomic_state *intel_state =
+   to_intel_atomic_state(cstate->base.state);
uint32_t latency = dev_priv->wm.skl_latency[level];
uint32_t method1, method2;
uint32_t plane_bytes_per_line, plane_blocks_per_line;
@@ -3598,10 +3600,17 @@ static int skl_compute_plane_wm(const struct 
drm_i915_private *dev_priv,
uint32_t width = 0, height = 0;
uint32_t plane_pixel_rate;
uint32_t y_tile_minimum, y_min_scanlines;
+   enum watermark_memory_wa mem_wa;
 
if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible)
return 0;
 
+   mem_wa = intel_state ? intel_state->wm_results.mem_wa : 
WATERMARK_WA_NONE;
+   if (mem_wa != WATERMARK_WA_NONE) {
+   if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
+   latency += 15;
+   }
+
width = drm_rect_width(_pstate->base.src) >> 16;
height = drm_rect_height(_pstate->base.src) >> 16;
 
@@ -3634,6 +3643,9 @@ static int skl_compute_plane_wm(const struct 
drm_i915_private *dev_priv,
y_min_scanlines = 4;
}
 
+   if (mem_wa == WATERMARK_WA_Y_TILED)
+   y_min_scanlines *= 2;
+
plane_bytes_per_line = width * cpp;
if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
@@ -4075,6 +4087,15 @@ skl_include_affected_pipes(struct drm_atomic_state 
*state)
intel_state->wm_results.dirty_pipes = ~0;
}
 
+   /*
+* If Watermark workaround is changed we need to recalculate
+* watermark values for all active pipes
+*/
+   if (intel_state->wm_results.mem_wa != dev_priv->wm.skl_hw.mem_wa) {
+   realloc_pipes = ~0;
+   intel_state->wm_results.dirty_pipes = ~0;
+   }
+
for_each_intel_crtc_mask(dev, intel_crtc, realloc_pipes) {
struct intel_crtc_state *cstate;
 
@@ -4087,6 +4108,129 @@ skl_include_affected_pipes(struct drm_atomic_state 
*state)
 }
 
 static void
+skl_set_memory_bandwidth_wm_wa(struct drm_atomic_state *state)
+{
+   struct drm_device *dev = state->dev;
+   struct drm_i915_private *dev_priv = to_i915(dev);
+   struct intel_crtc *intel_crtc;