| Message ID | 20230601160350.43888-7-vinod.govindapillai@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtl: add support for pmdemand | expand |
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 <lkp@intel.com> > Closes: https://lore.kernel.org/r/202305280253.Ab8bRV2w-lkp@intel.com/ > Reported-by: Dan Carpenter <error27@gmail.com> > Closes: https://lore.kernel.org/r/202305280253.Ab8bRV2w-lkp@intel.com/ > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > --- > 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(&new_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(&i915->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(&i915->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(&i915->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(&i915->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(&new_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 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 a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h > index f20292143745..67ae66a3fcdd 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.h > +++ b/drivers/gpu/drm/i915/display/intel_bw.h > @@ -34,6 +34,12 @@ struct intel_bw_state { > /* bitmask of active pipes */ > u8 active_pipes; > > + /* > + * From MTL onwards, to lock a QGV point, punit expects the peak BW of > + * the selected QGV point as the parameter in multiples of 100MB/s > + */ > + unsigned int qgv_point_peakbw; > + > /* > * Current QGV points mask, which restricts > * some particular SAGV states, not to confuse > -- > 2.34.1 >
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 <lkp@intel.com> > > Closes: https://lore.kernel.org/r/202305280253.Ab8bRV2w-lkp@intel.com/ > > Reported-by: Dan Carpenter <error27@gmail.com> > > Closes: https://lore.kernel.org/r/202305280253.Ab8bRV2w-lkp@intel.com/ > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > --- > > 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(&new_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(&i915->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(&i915->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(&i915->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(&i915->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(&new_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; > > +} > > + > > 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 a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h > > index f20292143745..67ae66a3fcdd 100644 > > --- a/drivers/gpu/drm/i915/display/intel_bw.h > > +++ b/drivers/gpu/drm/i915/display/intel_bw.h > > @@ -34,6 +34,12 @@ struct intel_bw_state { > > /* bitmask of active pipes */ > > u8 active_pipes; > > > > + /* > > + * From MTL onwards, to lock a QGV point, punit expects the peak BW of > > + * the selected QGV point as the parameter in multiples of 100MB/s > > + */ > > + unsigned int qgv_point_peakbw; > > + > > /* > > * Current QGV points mask, which restricts > > * some particular SAGV states, not to confuse > > -- > > 2.34.1 > >
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(&new_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(&i915->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(&i915->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(&i915->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(&i915->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(&new_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 a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h index f20292143745..67ae66a3fcdd 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.h +++ b/drivers/gpu/drm/i915/display/intel_bw.h @@ -34,6 +34,12 @@ struct intel_bw_state { /* bitmask of active pipes */ u8 active_pipes; + /* + * From MTL onwards, to lock a QGV point, punit expects the peak BW of + * the selected QGV point as the parameter in multiples of 100MB/s + */ + unsigned int qgv_point_peakbw; + /* * Current QGV points mask, which restricts * some particular SAGV states, not to confuse