Re: [Intel-gfx] [PATCH v12 6/7] drm/i915/mtl: find the best QGV point for the SAGV configuration
On Mon, 2023-06-05 at 12:49 +0300, Imre Deak wrote:
> On Thu, Jun 01, 2023 at 07:03:49PM +0300, Vinod Govindapillai wrote:
> > From MTL onwards, we need to find the best QGV point based on
> > the required data rate and pass the peak BW of that point to
> > the punit to lock the corresponding QGV point.
> >
> > v1: Fix for warning from kernel test robot
> >
> > Bspec: 64636
> >
> > Reported-by: kernel test robot
> > Closes: https://lore.kernel.org/r/202305280253.ab8brv2w-...@intel.com/
> > Reported-by: Dan Carpenter
> > Closes: https://lore.kernel.org/r/202305280253.ab8brv2w-...@intel.com/
> > Signed-off-by: Vinod Govindapillai
> > Reviewed-by: Stanislav Lisovskiy
> > ---
> > drivers/gpu/drm/i915/display/intel_bw.c | 87 -
> > drivers/gpu/drm/i915/display/intel_bw.h | 6 ++
> > 2 files changed, 91 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> > b/drivers/gpu/drm/i915/display/intel_bw.c
> > index b1cbeda0b2e3..7672963dc49c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -803,6 +803,85 @@ intel_atomic_get_bw_state(struct intel_atomic_state
> > *state)
> > return to_intel_bw_state(bw_state);
> > }
> >
> > +static int mtl_find_qgv_points(struct drm_i915_private *i915,
> > + unsigned int data_rate,
> > + unsigned int num_active_planes,
> > + const struct intel_bw_state *old_bw_state,
> > + struct intel_bw_state *new_bw_state)
> > +{
> > + unsigned int best_rate = UINT_MAX;
> > + unsigned int num_qgv_points =
> > i915->display.bw.max[0].num_qgv_points;
> > + unsigned int qgv_peak_bw = 0;
> > + int i;
> > + int ret;
> > +
> > + ret = intel_atomic_lock_global_state(_bw_state->base);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * If SAGV cannot be enabled, disable the pcode SAGV by passing all
> > 1's
> > + * for qgv peak bw in PM Demand request. So assign UINT_MAX if SAGV
> > is
> > + * not enabled. PM Demand code will clamp the value for the register
> > + */
> > + if (!intel_can_enable_sagv(i915, new_bw_state)) {
> > + new_bw_state->qgv_point_peakbw = UINT_MAX;
> > + drm_dbg_kms(>drm, "No SAGV, use UINT_MAX as peak
> > bw.");
> > + goto out;
> > + }
> > +
> > + /*
> > + * Find the best QGV point by comparing the data_rate with max data
> > rate
> > + * offered per plane group
> > + */
> > + for (i = 0; i < num_qgv_points; i++) {
> > + unsigned int bw_index =
> > + tgl_max_bw_index(i915, num_active_planes, i);
> > + unsigned int max_data_rate;
> > +
> > + if (bw_index >= ARRAY_SIZE(i915->display.bw.max))
> > + continue;
> > +
> > + max_data_rate = i915->display.bw.max[bw_index].deratedbw[i];
> > +
> > + if (max_data_rate < data_rate)
> > + continue;
> > +
> > + if (max_data_rate - data_rate < best_rate) {
> > + best_rate = max_data_rate - data_rate;
> > + qgv_peak_bw =
> > i915->display.bw.max[bw_index].peakbw[i];
> > + }
> > +
> > + drm_dbg_kms(>drm, "QGV point %d: max bw %d required
> > %d qgv_peak_bw: %d\n",
> > + i, max_data_rate, data_rate, qgv_peak_bw);
> > + }
> > +
> > + drm_dbg_kms(>drm, "Matching peaks QGV bw: %d for required
> > data rate: %d\n",
> > + qgv_peak_bw, data_rate);
> > +
> > + /*
> > + * The display configuration cannot be supported if no QGV point
> > + * satisfying the required data rate is found
> > + */
> > + if (qgv_peak_bw == 0) {
> > + drm_dbg_kms(>drm, "No QGV points for bw %d for
> > display configuration(%d
> > active planes).\n",
> > + data_rate, num_active_planes);
> > + return -EINVAL;
> > + }
> > +
> > + /* MTL PM DEMAND expects QGV BW parameter in multiples of 100 mbps
> > */
> > + new_bw_state->qgv_point_peakbw = DIV_ROUND_CLOSEST(qgv_peak_bw,
> > 100);
> > +
> > +out:
> > + if (new_bw_state->qgv_point_peakbw !=
> > old_bw_state->qgv_point_peakbw) {
> > + ret =
> > intel_atomic_serialize_global_state(_bw_state->base);
>
> qgv_point_beakbw is used as a parameter for the pmdemand command, for
> which there is a check later whether programming it is required or not.
> So why is global state serialized here?
Yes. good point. This need not to be serialized here. Will remove it.
Thanks
Vinod
>
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
>
Re: [Intel-gfx] [PATCH v12 6/7] drm/i915/mtl: find the best QGV point for the SAGV configuration
On Thu, Jun 01, 2023 at 07:03:49PM +0300, Vinod Govindapillai wrote:
> From MTL onwards, we need to find the best QGV point based on
> the required data rate and pass the peak BW of that point to
> the punit to lock the corresponding QGV point.
>
> v1: Fix for warning from kernel test robot
>
> Bspec: 64636
>
> Reported-by: kernel test robot
> Closes: https://lore.kernel.org/r/202305280253.ab8brv2w-...@intel.com/
> Reported-by: Dan Carpenter
> Closes: https://lore.kernel.org/r/202305280253.ab8brv2w-...@intel.com/
> Signed-off-by: Vinod Govindapillai
> Reviewed-by: Stanislav Lisovskiy
> ---
> drivers/gpu/drm/i915/display/intel_bw.c | 87 -
> drivers/gpu/drm/i915/display/intel_bw.h | 6 ++
> 2 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> b/drivers/gpu/drm/i915/display/intel_bw.c
> index b1cbeda0b2e3..7672963dc49c 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -803,6 +803,85 @@ intel_atomic_get_bw_state(struct intel_atomic_state
> *state)
> return to_intel_bw_state(bw_state);
> }
>
> +static int mtl_find_qgv_points(struct drm_i915_private *i915,
> +unsigned int data_rate,
> +unsigned int num_active_planes,
> +const struct intel_bw_state *old_bw_state,
> +struct intel_bw_state *new_bw_state)
> +{
> + unsigned int best_rate = UINT_MAX;
> + unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
> + unsigned int qgv_peak_bw = 0;
> + int i;
> + int ret;
> +
> + ret = intel_atomic_lock_global_state(_bw_state->base);
> + if (ret)
> + return ret;
> +
> + /*
> + * If SAGV cannot be enabled, disable the pcode SAGV by passing all 1's
> + * for qgv peak bw in PM Demand request. So assign UINT_MAX if SAGV is
> + * not enabled. PM Demand code will clamp the value for the register
> + */
> + if (!intel_can_enable_sagv(i915, new_bw_state)) {
> + new_bw_state->qgv_point_peakbw = UINT_MAX;
> + drm_dbg_kms(>drm, "No SAGV, use UINT_MAX as peak bw.");
> + goto out;
> + }
> +
> + /*
> + * Find the best QGV point by comparing the data_rate with max data rate
> + * offered per plane group
> + */
> + for (i = 0; i < num_qgv_points; i++) {
> + unsigned int bw_index =
> + tgl_max_bw_index(i915, num_active_planes, i);
> + unsigned int max_data_rate;
> +
> + if (bw_index >= ARRAY_SIZE(i915->display.bw.max))
> + continue;
> +
> + max_data_rate = i915->display.bw.max[bw_index].deratedbw[i];
> +
> + if (max_data_rate < data_rate)
> + continue;
> +
> + if (max_data_rate - data_rate < best_rate) {
> + best_rate = max_data_rate - data_rate;
> + qgv_peak_bw = i915->display.bw.max[bw_index].peakbw[i];
> + }
> +
> + drm_dbg_kms(>drm, "QGV point %d: max bw %d required %d
> qgv_peak_bw: %d\n",
> + i, max_data_rate, data_rate, qgv_peak_bw);
> + }
> +
> + drm_dbg_kms(>drm, "Matching peaks QGV bw: %d for required data
> rate: %d\n",
> + qgv_peak_bw, data_rate);
> +
> + /*
> + * The display configuration cannot be supported if no QGV point
> + * satisfying the required data rate is found
> + */
> + if (qgv_peak_bw == 0) {
> + drm_dbg_kms(>drm, "No QGV points for bw %d for display
> configuration(%d active planes).\n",
> + data_rate, num_active_planes);
> + return -EINVAL;
> + }
> +
> + /* MTL PM DEMAND expects QGV BW parameter in multiples of 100 mbps */
> + new_bw_state->qgv_point_peakbw = DIV_ROUND_CLOSEST(qgv_peak_bw, 100);
> +
> +out:
> + if (new_bw_state->qgv_point_peakbw != old_bw_state->qgv_point_peakbw) {
> + ret = intel_atomic_serialize_global_state(_bw_state->base);
qgv_point_beakbw is used as a parameter for the pmdemand command, for
which there is a check later whether programming it is required or not.
So why is global state serialized here?
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int icl_find_qgv_points(struct drm_i915_private *i915,
> unsigned int data_rate,
> unsigned int num_active_planes,
> @@ -928,8 +1007,12 @@ static int intel_bw_check_qgv_points(struct
> drm_i915_private *i915,
>
> data_rate = DIV_ROUND_UP(data_rate, 1000);
>
> - return icl_find_qgv_points(i915, data_rate, num_active_planes,
> -old_bw_state, new_bw_state);
> + if (DISPLAY_VER(i915) >= 14)
> + return
[Intel-gfx] [PATCH v12 6/7] drm/i915/mtl: find the best QGV point for the SAGV configuration
>From MTL onwards, we need to find the best QGV point based on
the required data rate and pass the peak BW of that point to
the punit to lock the corresponding QGV point.
v1: Fix for warning from kernel test robot
Bspec: 64636
Reported-by: kernel test robot
Closes: https://lore.kernel.org/r/202305280253.ab8brv2w-...@intel.com/
Reported-by: Dan Carpenter
Closes: https://lore.kernel.org/r/202305280253.ab8brv2w-...@intel.com/
Signed-off-by: Vinod Govindapillai
Reviewed-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_bw.c | 87 -
drivers/gpu/drm/i915/display/intel_bw.h | 6 ++
2 files changed, 91 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
b/drivers/gpu/drm/i915/display/intel_bw.c
index b1cbeda0b2e3..7672963dc49c 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -803,6 +803,85 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state)
return to_intel_bw_state(bw_state);
}
+static int mtl_find_qgv_points(struct drm_i915_private *i915,
+ unsigned int data_rate,
+ unsigned int num_active_planes,
+ const struct intel_bw_state *old_bw_state,
+ struct intel_bw_state *new_bw_state)
+{
+ unsigned int best_rate = UINT_MAX;
+ unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points;
+ unsigned int qgv_peak_bw = 0;
+ int i;
+ int ret;
+
+ ret = intel_atomic_lock_global_state(_bw_state->base);
+ if (ret)
+ return ret;
+
+ /*
+* If SAGV cannot be enabled, disable the pcode SAGV by passing all 1's
+* for qgv peak bw in PM Demand request. So assign UINT_MAX if SAGV is
+* not enabled. PM Demand code will clamp the value for the register
+*/
+ if (!intel_can_enable_sagv(i915, new_bw_state)) {
+ new_bw_state->qgv_point_peakbw = UINT_MAX;
+ drm_dbg_kms(>drm, "No SAGV, use UINT_MAX as peak bw.");
+ goto out;
+ }
+
+ /*
+* Find the best QGV point by comparing the data_rate with max data rate
+* offered per plane group
+*/
+ for (i = 0; i < num_qgv_points; i++) {
+ unsigned int bw_index =
+ tgl_max_bw_index(i915, num_active_planes, i);
+ unsigned int max_data_rate;
+
+ if (bw_index >= ARRAY_SIZE(i915->display.bw.max))
+ continue;
+
+ max_data_rate = i915->display.bw.max[bw_index].deratedbw[i];
+
+ if (max_data_rate < data_rate)
+ continue;
+
+ if (max_data_rate - data_rate < best_rate) {
+ best_rate = max_data_rate - data_rate;
+ qgv_peak_bw = i915->display.bw.max[bw_index].peakbw[i];
+ }
+
+ drm_dbg_kms(>drm, "QGV point %d: max bw %d required %d
qgv_peak_bw: %d\n",
+ i, max_data_rate, data_rate, qgv_peak_bw);
+ }
+
+ drm_dbg_kms(>drm, "Matching peaks QGV bw: %d for required data
rate: %d\n",
+ qgv_peak_bw, data_rate);
+
+ /*
+* The display configuration cannot be supported if no QGV point
+* satisfying the required data rate is found
+*/
+ if (qgv_peak_bw == 0) {
+ drm_dbg_kms(>drm, "No QGV points for bw %d for display
configuration(%d active planes).\n",
+ data_rate, num_active_planes);
+ return -EINVAL;
+ }
+
+ /* MTL PM DEMAND expects QGV BW parameter in multiples of 100 mbps */
+ new_bw_state->qgv_point_peakbw = DIV_ROUND_CLOSEST(qgv_peak_bw, 100);
+
+out:
+ if (new_bw_state->qgv_point_peakbw != old_bw_state->qgv_point_peakbw) {
+ ret = intel_atomic_serialize_global_state(_bw_state->base);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int icl_find_qgv_points(struct drm_i915_private *i915,
unsigned int data_rate,
unsigned int num_active_planes,
@@ -928,8 +1007,12 @@ static int intel_bw_check_qgv_points(struct
drm_i915_private *i915,
data_rate = DIV_ROUND_UP(data_rate, 1000);
- return icl_find_qgv_points(i915, data_rate, num_active_planes,
- old_bw_state, new_bw_state);
+ if (DISPLAY_VER(i915) >= 14)
+ return mtl_find_qgv_points(i915, data_rate, num_active_planes,
+ old_bw_state, new_bw_state);
+ else
+ return icl_find_qgv_points(i915, data_rate, num_active_planes,
+ old_bw_state, new_bw_state);
}
static bool intel_bw_state_changed(struct drm_i915_private *i915,
diff --git
