Re: [Intel-gfx] [PATCH v3] drm/i915: Restrict qgv points which don't have enough bandwidth.

2019-10-14 Thread Ville Syrjälä
On Mon, Oct 14, 2019 at 09:33:32AM -0700, James Ausmus wrote:
> On Mon, Oct 14, 2019 at 05:50:18PM +0300, Ville Syrjälä wrote:
> > On Mon, Oct 14, 2019 at 02:13:31PM +0300, Lisovskiy, Stanislav wrote:
> > > On Fri, 2019-10-11 at 16:49 -0700, James Ausmus wrote:
> > > > > + new_qgv_points_mask |= new_mask_bit;
> > > > > + }
> > > > > +
> > > > > + ret = icl_pcode_restrict_qgv_points(dev_priv,
> > > > > new_qgv_points_mask);
> > > > > + if (ret < 0)
> > > > > + DRM_DEBUG_KMS("Could not restrict required gqv
> > > > > points(%d)\n", ret);
> > > > 
> > > > s/gqv/qgv/
> > > > 
> > > > 
> > > > Also, if we fail masking off the qgv points that can't support our BW
> > > > req, shouldn't we handle that failure somehow - maybe just disable
> > > > SAGV
> > > > entirely?  Better we lose power than have flickering screens...
> > 
> > Sounds like dead code to me. My approach is: don't deal with hw/firmware
> > failures until they are proven to exist.
> > 
> > The debug msg should be an error so that we get a bug report if this
> > ever happens.
> 
> That works - however, I think we should return the error rather than
> continuing.

No. We're way past the point of no return.

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Restrict qgv points which don't have enough bandwidth.

2019-10-14 Thread James Ausmus
On Mon, Oct 14, 2019 at 05:50:18PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 14, 2019 at 02:13:31PM +0300, Lisovskiy, Stanislav wrote:
> > On Fri, 2019-10-11 at 16:49 -0700, James Ausmus wrote:
> > > > +   new_qgv_points_mask |= new_mask_bit;
> > > > +   }
> > > > +
> > > > +   ret = icl_pcode_restrict_qgv_points(dev_priv,
> > > > new_qgv_points_mask);
> > > > +   if (ret < 0)
> > > > +   DRM_DEBUG_KMS("Could not restrict required gqv
> > > > points(%d)\n", ret);
> > > 
> > > s/gqv/qgv/
> > > 
> > > 
> > > Also, if we fail masking off the qgv points that can't support our BW
> > > req, shouldn't we handle that failure somehow - maybe just disable
> > > SAGV
> > > entirely?  Better we lose power than have flickering screens...
> 
> Sounds like dead code to me. My approach is: don't deal with hw/firmware
> failures until they are proven to exist.
> 
> The debug msg should be an error so that we get a bug report if this
> ever happens.

That works - however, I think we should return the error rather than
continuing.

-James

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Restrict qgv points which don't have enough bandwidth.

2019-10-14 Thread James Ausmus
On Mon, Oct 14, 2019 at 04:13:31AM -0700, Lisovskiy, Stanislav wrote:
> On Fri, 2019-10-11 at 16:49 -0700, James Ausmus wrote:
> > On Wed, Sep 25, 2019 at 03:17:37PM +0300, Stanislav Lisovskiy wrote:
> > > According to BSpec 53998, we should try to
> > > restrict qgv points, which can't provide
> > > enough bandwidth for desired display configuration.
> > > 
> > > Currently we are just comparing against all of
> > > those and take minimum(worst case).
> > > 
> > > v2: Fixed wrong PCode reply mask, removed hardcoded
> > > values.
> > > 
> > > v3: Forbid simultaneous legacy SAGV PCode requests and
> > > restricting qgv points. Put the actual restriction
> > > to commit function, added serialization(thanks to Ville)
> > > to prevent commit being applied out of order in case of
> > > nonblocking and/or nomodeset commits.
> 
> Hi James,
> 
> Thank you for great review! 
> 
> While many of your comments are definitely
> good findings, still will leave reply to a few,
> just to keep things clear.
> 
> 
> > > 
> > > Signed-off-by: Stanislav Lisovskiy 
> > > Cc: Ville Syrjälä 
> > > Cc: James Ausmus 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_atomic.c   | 16 
> > >  drivers/gpu/drm/i915/display/intel_atomic.h   |  3 +
> > >  drivers/gpu/drm/i915/display/intel_bw.c   | 79 +
> > > --
> > >  drivers/gpu/drm/i915/display/intel_bw.h   |  2 +
> > >  drivers/gpu/drm/i915/display/intel_display.c  | 78
> > > +-
> > >  .../drm/i915/display/intel_display_types.h|  3 +
> > >  drivers/gpu/drm/i915/i915_drv.h   |  2 +
> > >  drivers/gpu/drm/i915/i915_reg.h   |  3 +
> > >  8 files changed, 160 insertions(+), 26 deletions(-)
> 
> > if (max_data_rate >= data_rate)
> > allowed_points |= 1 << i;
> > DRM_DEBUG_KMS...
> > 
> > > + allowed_points |= 1 << i;
> > > + }
> > 
> > According to the BSpec page, we also need to save off the QGV point
> > that has
> > the most available bandwidth:
> > 
> > "At least one GV point must always remain unmasked. The point
> > providing the
> > highest bandwidth for display must always remain unmasked."
> > 
> > We should stash that point separately, and ensure it always remains
> > unmasked.
> > 
> > > + }
> > > +
> > > + if (allowed_points == 0) {
> > > + DRM_DEBUG_KMS("Could not find any suitable QGV
> > > points\n");
> > >   return -EINVAL;
> > >   }
> 
> This actually guarantees that, I think - we will never allow a 
> config which will require us to mask all of the points to work.

Yeah, fair enough - if *any* points allow a high enough bandwidth, then
it's certain that the *highest* BW point will be unmasked. If *no*
points allow enough BW, then it doesn't matter anyway because we're
going to fail out and not allow that config...

Thanks!

-James

> 
> > >  
> > > + state->qgv_points_mask = (~allowed_points) & ((1 <<
> > > qi.num_points) - 1);
> > > +
> > > + /*
> > > +  * If the actual mask had changed we need to make sure that
> > > +  * the commits are serialized(in case this is a nomodeset,
> > > nonblocking)
> > > +  */
> > > + if (state->qgv_points_mask != dev_priv->qgv_points_mask) {
> > > + ret = intel_atomic_serialize_global_state(state);
> > > + if (ret) {
> > > + DRM_DEBUG_KMS("Could not serialize global
> > > state\n");
> > > + return ret;
> > > + }
> > > + }
> > > +
> > >   return 0;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.h
> > > b/drivers/gpu/drm/i915/display/intel_bw.h
> > > index 9db10af012f4..66bf9bc10b73 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_bw.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> > > @@ -28,5 +28,7 @@ int intel_bw_init(struct drm_i915_private
> > > *dev_priv);
> > >  int intel_bw_atomic_check(struct intel_atomic_state *state);
> > >  void intel_bw_crtc_update(struct intel_bw_state *bw_state,
> > > const struct intel_crtc_state *crtc_state);
> > > +int icl_pcode_restrict_qgv_points(struct drm_i915_private
> > > *dev_priv,
> > > +   u32 points_mask);
> > >  
> > >  #endif /* __INTEL_BW_H__ */
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 5ecf54270181..c3196d0e4be3 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -13960,6 +13960,68 @@ static void
> > > intel_atomic_cleanup_work(struct work_struct *work)
> > >   intel_atomic_helper_free_state(i915);
> > >  }
> > >  
> > > +static void intel_qgv_point_pre_update(struct intel_atomic_state
> > > *state)
> > 
> > It would be nice to name this either "mask" or "unmask", so it's
> > easier
> > at first glance to see which function is turning on masked bits vs
> > toggling them off.
> > 
> > > +{
> > > + struct drm_device *dev = state->base.dev;
> > > + struct drm_i9

Re: [Intel-gfx] [PATCH v3] drm/i915: Restrict qgv points which don't have enough bandwidth.

2019-10-14 Thread Ville Syrjälä
On Mon, Oct 14, 2019 at 02:13:31PM +0300, Lisovskiy, Stanislav wrote:
> On Fri, 2019-10-11 at 16:49 -0700, James Ausmus wrote:
> > > + new_qgv_points_mask |= new_mask_bit;
> > > + }
> > > +
> > > + ret = icl_pcode_restrict_qgv_points(dev_priv,
> > > new_qgv_points_mask);
> > > + if (ret < 0)
> > > + DRM_DEBUG_KMS("Could not restrict required gqv
> > > points(%d)\n", ret);
> > 
> > s/gqv/qgv/
> > 
> > 
> > Also, if we fail masking off the qgv points that can't support our BW
> > req, shouldn't we handle that failure somehow - maybe just disable
> > SAGV
> > entirely?  Better we lose power than have flickering screens...

Sounds like dead code to me. My approach is: don't deal with hw/firmware
failures until they are proven to exist.

The debug msg should be an error so that we get a bug report if this
ever happens.

-- 
Ville Syrjälä
Intel
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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

Re: [Intel-gfx] [PATCH v3] drm/i915: Restrict qgv points which don't have enough bandwidth.

2019-10-14 Thread Lisovskiy, Stanislav
On Fri, 2019-10-11 at 16:49 -0700, James Ausmus wrote:
> On Wed, Sep 25, 2019 at 03:17:37PM +0300, Stanislav Lisovskiy wrote:
> > According to BSpec 53998, we should try to
> > restrict qgv points, which can't provide
> > enough bandwidth for desired display configuration.
> > 
> > Currently we are just comparing against all of
> > those and take minimum(worst case).
> > 
> > v2: Fixed wrong PCode reply mask, removed hardcoded
> > values.
> > 
> > v3: Forbid simultaneous legacy SAGV PCode requests and
> > restricting qgv points. Put the actual restriction
> > to commit function, added serialization(thanks to Ville)
> > to prevent commit being applied out of order in case of
> > nonblocking and/or nomodeset commits.

Hi James,

Thank you for great review! 

While many of your comments are definitely
good findings, still will leave reply to a few,
just to keep things clear.


> > 
> > Signed-off-by: Stanislav Lisovskiy 
> > Cc: Ville Syrjälä 
> > Cc: James Ausmus 
> > ---
> >  drivers/gpu/drm/i915/display/intel_atomic.c   | 16 
> >  drivers/gpu/drm/i915/display/intel_atomic.h   |  3 +
> >  drivers/gpu/drm/i915/display/intel_bw.c   | 79 +
> > --
> >  drivers/gpu/drm/i915/display/intel_bw.h   |  2 +
> >  drivers/gpu/drm/i915/display/intel_display.c  | 78
> > +-
> >  .../drm/i915/display/intel_display_types.h|  3 +
> >  drivers/gpu/drm/i915/i915_drv.h   |  2 +
> >  drivers/gpu/drm/i915/i915_reg.h   |  3 +
> >  8 files changed, 160 insertions(+), 26 deletions(-)

> if (max_data_rate >= data_rate)
>   allowed_points |= 1 << i;
> DRM_DEBUG_KMS...
> 
> > +   allowed_points |= 1 << i;
> > +   }
> 
> According to the BSpec page, we also need to save off the QGV point
> that has
> the most available bandwidth:
> 
> "At least one GV point must always remain unmasked. The point
> providing the
> highest bandwidth for display must always remain unmasked."
> 
> We should stash that point separately, and ensure it always remains
> unmasked.
> 
> > +   }
> > +
> > +   if (allowed_points == 0) {
> > +   DRM_DEBUG_KMS("Could not find any suitable QGV
> > points\n");
> > return -EINVAL;
> > }

This actually guarantees that, I think - we will never allow a 
config which will require us to mask all of the points to work.

> >  
> > +   state->qgv_points_mask = (~allowed_points) & ((1 <<
> > qi.num_points) - 1);
> > +
> > +   /*
> > +* If the actual mask had changed we need to make sure that
> > +* the commits are serialized(in case this is a nomodeset,
> > nonblocking)
> > +*/
> > +   if (state->qgv_points_mask != dev_priv->qgv_points_mask) {
> > +   ret = intel_atomic_serialize_global_state(state);
> > +   if (ret) {
> > +   DRM_DEBUG_KMS("Could not serialize global
> > state\n");
> > +   return ret;
> > +   }
> > +   }
> > +
> > return 0;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.h
> > b/drivers/gpu/drm/i915/display/intel_bw.h
> > index 9db10af012f4..66bf9bc10b73 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.h
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> > @@ -28,5 +28,7 @@ int intel_bw_init(struct drm_i915_private
> > *dev_priv);
> >  int intel_bw_atomic_check(struct intel_atomic_state *state);
> >  void intel_bw_crtc_update(struct intel_bw_state *bw_state,
> >   const struct intel_crtc_state *crtc_state);
> > +int icl_pcode_restrict_qgv_points(struct drm_i915_private
> > *dev_priv,
> > + u32 points_mask);
> >  
> >  #endif /* __INTEL_BW_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 5ecf54270181..c3196d0e4be3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13960,6 +13960,68 @@ static void
> > intel_atomic_cleanup_work(struct work_struct *work)
> > intel_atomic_helper_free_state(i915);
> >  }
> >  
> > +static void intel_qgv_point_pre_update(struct intel_atomic_state
> > *state)
> 
> It would be nice to name this either "mask" or "unmask", so it's
> easier
> at first glance to see which function is turning on masked bits vs
> toggling them off.
> 
> > +{
> > +   struct drm_device *dev = state->base.dev;
> > +   struct drm_i915_private *dev_priv = to_i915(dev);
> > +   int i, ret;
> > +
> > +   /*
> > +* Restrict required qgv points before updating the
> > configuration.
> > +* According to BPsec we can't mask and unmask qgv points at
> > the same
> 
> s/BPsec/BSpec/
> 
> > +* time. Also masking should be done before updating the
> > configuration
> > +* and unmasking afterwards.
> > +*/
> > +   u32 new_qgv_points_mask = dev_priv->qgv_points_mask;
> > +   int num_points = dev_priv->max_bw[0].num_qgv_points;
> > +
> > +   for (i = num_points; i > 

Re: [Intel-gfx] [PATCH v3] drm/i915: Restrict qgv points which don't have enough bandwidth.

2019-10-11 Thread James Ausmus
On Wed, Sep 25, 2019 at 03:17:37PM +0300, Stanislav Lisovskiy wrote:
> According to BSpec 53998, we should try to
> restrict qgv points, which can't provide
> enough bandwidth for desired display configuration.
> 
> Currently we are just comparing against all of
> those and take minimum(worst case).
> 
> v2: Fixed wrong PCode reply mask, removed hardcoded
> values.
> 
> v3: Forbid simultaneous legacy SAGV PCode requests and
> restricting qgv points. Put the actual restriction
> to commit function, added serialization(thanks to Ville)
> to prevent commit being applied out of order in case of
> nonblocking and/or nomodeset commits.
> 
> Signed-off-by: Stanislav Lisovskiy 
> Cc: Ville Syrjälä 
> Cc: James Ausmus 
> ---
>  drivers/gpu/drm/i915/display/intel_atomic.c   | 16 
>  drivers/gpu/drm/i915/display/intel_atomic.h   |  3 +
>  drivers/gpu/drm/i915/display/intel_bw.c   | 79 +--
>  drivers/gpu/drm/i915/display/intel_bw.h   |  2 +
>  drivers/gpu/drm/i915/display/intel_display.c  | 78 +-
>  .../drm/i915/display/intel_display_types.h|  3 +
>  drivers/gpu/drm/i915/i915_drv.h   |  2 +
>  drivers/gpu/drm/i915/i915_reg.h   |  3 +
>  8 files changed, 160 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index 698802da07b7..903537c4fb0f 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -208,6 +208,22 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>   return &crtc_state->base;
>  }
>  
> +int intel_atomic_serialize_global_state(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_crtc *crtc;
> +
> + for_each_intel_crtc(&dev_priv->drm, crtc) {
> + struct intel_crtc_state *crtc_state;
> +
> + crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
> + if (IS_ERR(crtc_state))
> + return PTR_ERR(crtc_state);
> + }
> +
> + return 0;
> +}
> +

I'll leave reviewing the atomic serialization bits to Ville and I'll
just review the rest, as I don't have a good head for atomic at this
point...


>  /**
>   * intel_crtc_destroy_state - destroy crtc state
>   * @crtc: drm crtc
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h 
> b/drivers/gpu/drm/i915/display/intel_atomic.h
> index 58065d3161a3..fd17b3ca257f 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.h
> @@ -7,6 +7,7 @@
>  #define __INTEL_ATOMIC_H__
>  
>  #include 
> +#include "intel_display_types.h"
>  
>  struct drm_atomic_state;
>  struct drm_connector;
> @@ -38,6 +39,8 @@ void intel_crtc_destroy_state(struct drm_crtc *crtc,
>  struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
>  void intel_atomic_state_clear(struct drm_atomic_state *state);
>  
> +int intel_atomic_serialize_global_state(struct intel_atomic_state *state);
> +
>  struct intel_crtc_state *
>  intel_atomic_get_crtc_state(struct drm_atomic_state *state,
>   struct intel_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
> b/drivers/gpu/drm/i915/display/intel_bw.c
> index cd58e47ab7b2..81e3468306fa 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -8,6 +8,7 @@
>  #include "intel_bw.h"
>  #include "intel_display_types.h"
>  #include "intel_sideband.h"
> +#include "intel_atomic.h"
>  
>  /* Parameters for Qclk Geyserville (QGV) */
>  struct intel_qgv_point {
> @@ -90,6 +91,27 @@ static int icl_pcode_read_qgv_point_info(struct 
> drm_i915_private *dev_priv,
>   return 0;
>  }
>  
> +int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
> +   u32 points_mask)
> +{
> + int ret;
> +
> + /* bspec says to keep retrying for at least 1 ms */
> + ret = skl_pcode_request(dev_priv, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
> + points_mask,
> + GEN11_PCODE_POINTS_RESTRICTED_MASK,
> + GEN11_PCODE_POINTS_RESTRICTED,
> + 1);
> +
> + if (ret < 0) {
> + DRM_ERROR("Failed to disable qgv points (%d)\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +
>  static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
> struct intel_qgv_info *qi)
>  {
> @@ -247,22 +269,6 @@ void intel_bw_init_hw(struct drm_i915_private *dev_priv)
>   icl_get_bw_info(dev_priv, &icl_sa_info);
>  }
>  
> -static unsigned int intel_max_data_rate(struct drm_i915_private *dev_priv,
> - int num_planes)
> -{
> - if (INTEL_GEN(dev_priv) >= 11)
> - /*
> -

[Intel-gfx] [PATCH v3] drm/i915: Restrict qgv points which don't have enough bandwidth.

2019-09-25 Thread Stanislav Lisovskiy
According to BSpec 53998, we should try to
restrict qgv points, which can't provide
enough bandwidth for desired display configuration.

Currently we are just comparing against all of
those and take minimum(worst case).

v2: Fixed wrong PCode reply mask, removed hardcoded
values.

v3: Forbid simultaneous legacy SAGV PCode requests and
restricting qgv points. Put the actual restriction
to commit function, added serialization(thanks to Ville)
to prevent commit being applied out of order in case of
nonblocking and/or nomodeset commits.

Signed-off-by: Stanislav Lisovskiy 
Cc: Ville Syrjälä 
Cc: James Ausmus 
---
 drivers/gpu/drm/i915/display/intel_atomic.c   | 16 
 drivers/gpu/drm/i915/display/intel_atomic.h   |  3 +
 drivers/gpu/drm/i915/display/intel_bw.c   | 79 +--
 drivers/gpu/drm/i915/display/intel_bw.h   |  2 +
 drivers/gpu/drm/i915/display/intel_display.c  | 78 +-
 .../drm/i915/display/intel_display_types.h|  3 +
 drivers/gpu/drm/i915/i915_drv.h   |  2 +
 drivers/gpu/drm/i915/i915_reg.h   |  3 +
 8 files changed, 160 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index 698802da07b7..903537c4fb0f 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -208,6 +208,22 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
return &crtc_state->base;
 }
 
+int intel_atomic_serialize_global_state(struct intel_atomic_state *state)
+{
+   struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+   struct intel_crtc *crtc;
+
+   for_each_intel_crtc(&dev_priv->drm, crtc) {
+   struct intel_crtc_state *crtc_state;
+
+   crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+   if (IS_ERR(crtc_state))
+   return PTR_ERR(crtc_state);
+   }
+
+   return 0;
+}
+
 /**
  * intel_crtc_destroy_state - destroy crtc state
  * @crtc: drm crtc
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h 
b/drivers/gpu/drm/i915/display/intel_atomic.h
index 58065d3161a3..fd17b3ca257f 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic.h
@@ -7,6 +7,7 @@
 #define __INTEL_ATOMIC_H__
 
 #include 
+#include "intel_display_types.h"
 
 struct drm_atomic_state;
 struct drm_connector;
@@ -38,6 +39,8 @@ void intel_crtc_destroy_state(struct drm_crtc *crtc,
 struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
 void intel_atomic_state_clear(struct drm_atomic_state *state);
 
+int intel_atomic_serialize_global_state(struct intel_atomic_state *state);
+
 struct intel_crtc_state *
 intel_atomic_get_crtc_state(struct drm_atomic_state *state,
struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c 
b/drivers/gpu/drm/i915/display/intel_bw.c
index cd58e47ab7b2..81e3468306fa 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -8,6 +8,7 @@
 #include "intel_bw.h"
 #include "intel_display_types.h"
 #include "intel_sideband.h"
+#include "intel_atomic.h"
 
 /* Parameters for Qclk Geyserville (QGV) */
 struct intel_qgv_point {
@@ -90,6 +91,27 @@ static int icl_pcode_read_qgv_point_info(struct 
drm_i915_private *dev_priv,
return 0;
 }
 
+int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
+ u32 points_mask)
+{
+   int ret;
+
+   /* bspec says to keep retrying for at least 1 ms */
+   ret = skl_pcode_request(dev_priv, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
+   points_mask,
+   GEN11_PCODE_POINTS_RESTRICTED_MASK,
+   GEN11_PCODE_POINTS_RESTRICTED,
+   1);
+
+   if (ret < 0) {
+   DRM_ERROR("Failed to disable qgv points (%d)\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+
 static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
  struct intel_qgv_info *qi)
 {
@@ -247,22 +269,6 @@ void intel_bw_init_hw(struct drm_i915_private *dev_priv)
icl_get_bw_info(dev_priv, &icl_sa_info);
 }
 
-static unsigned int intel_max_data_rate(struct drm_i915_private *dev_priv,
-   int num_planes)
-{
-   if (INTEL_GEN(dev_priv) >= 11)
-   /*
-* FIXME with SAGV disabled maybe we can assume
-* point 1 will always be used? Seems to match
-* the behaviour observed in the wild.
-*/
-   return min3(icl_max_bw(dev_priv, num_planes, 0),
-   icl_max_bw(dev_priv, num_planes, 1),
-   icl_max_bw(dev_priv, num_planes, 2));
-   else
-   return UI