| Message ID | 20240405113533.338553-6-vinod.govindapillai@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | QGV/SAGV related fixes | expand |
On Fri, Apr 05, 2024 at 02:35:32PM +0300, Vinod Govindapillai wrote: > From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > There could be multiple qgv and psf gv points with similar values. > Apparently pcode's handling og psf and qgv points are different. For > qgv case, pcode sets whatever is asked by the driver. But in case > of psf gv points, it compares the bw from points before setting the > mask. This can cause problems in scenarios where we have to disable > sagv by setting the highest bw point and there could be multiple > points with highest bw. So to set the maximum psf gv point, find > out all the points with the highest bw and set all together. > > v1: - use the same treatment to qgv points as well (Vinod) > > v2: - pcode confirms that for qgv points, it sets whatever the > driver sets (Vinod) > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > --- > drivers/gpu/drm/i915/display/intel_bw.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c > index 5f4f93524bef..6fb228a1a28f 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.c > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > @@ -874,6 +874,8 @@ static unsigned int icl_max_bw_psf_gv_point_mask(struct drm_i915_private *i915) > if (max_data_rate > max_bw) { > max_bw_point_mask = BIT(i); > max_bw = max_data_rate; > + } else if (max_data_rate == max_bw) { > + max_bw_point_mask |= BIT(i); So we just came back to where we started. Wondering still, why it even bothers to expose two equal PSF GV points. Not only having duplicate points doesn't make much sense for the driver(since the BW they provide is the same), but also requires some additional logic on top to handle those. Stan > } > } > > -- > 2.34.1 >
On Mon, 2024-04-08 at 11:29 +0300, Lisovskiy, Stanislav wrote: > On Fri, Apr 05, 2024 at 02:35:32PM +0300, Vinod Govindapillai wrote: > > From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > > > There could be multiple qgv and psf gv points with similar values. > > Apparently pcode's handling og psf and qgv points are different. For > > qgv case, pcode sets whatever is asked by the driver. But in case > > of psf gv points, it compares the bw from points before setting the > > mask. This can cause problems in scenarios where we have to disable > > sagv by setting the highest bw point and there could be multiple > > points with highest bw. So to set the maximum psf gv point, find > > out all the points with the highest bw and set all together. > > > > v1: - use the same treatment to qgv points as well (Vinod) > > > > v2: - pcode confirms that for qgv points, it sets whatever the > > driver sets (Vinod) > > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_bw.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c > > index 5f4f93524bef..6fb228a1a28f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_bw.c > > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > > @@ -874,6 +874,8 @@ static unsigned int icl_max_bw_psf_gv_point_mask(struct drm_i915_private > > *i915) > > if (max_data_rate > max_bw) { > > max_bw_point_mask = BIT(i); > > max_bw = max_data_rate; > > + } else if (max_data_rate == max_bw) { > > + max_bw_point_mask |= BIT(i); > > So we just came back to where we started. Wondering still, why it even bothers to expose > two equal PSF GV points. Not only having duplicate points doesn't make much sense for > the driver(since the BW they provide is the same), but also requires some additional > logic on top to handle those. Yes. Unfortunately this is what pcode expects for psf gv points. (Please have a look at the email thread with the pcode team/Art etc.) For qgv, it sets whatever the driver sets! BR Vinod > > Stan > > > } > > } > > > > -- > > 2.34.1 > >
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index 5f4f93524bef..6fb228a1a28f 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -874,6 +874,8 @@ static unsigned int icl_max_bw_psf_gv_point_mask(struct drm_i915_private *i915) if (max_data_rate > max_bw) { max_bw_point_mask = BIT(i); max_bw = max_data_rate; + } else if (max_data_rate == max_bw) { + max_bw_point_mask |= BIT(i); } }