Re: [PATCH v2 11/11] drm/i915/bw: insert a sw bw info entry to cater low data rate usecases

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

On Tue, 2026-05-12 at 14:51 +0300, Jani Nikula wrote:
> On Mon, 11 May 2026, Vinod Govindapillai
> <vinod.govindapillai@intel.com> wrote:
> > In xe3+, soc can lower the fabric frequency when the display
> > needs less bandwidth than the minimum GV point. This threshold
> > has been defined as 20GB/s. To enable this,
> > 
> > Add a new low bw info point with this peakbw threshold of 20GB/s
> > based on the following conditions:
> > 1. Only for xe3+ versions
> > 2. There is at least one QGV point
> > 3. Number QGV points is less than 8
> > 4. Lowest peak bw across all the QGV point is less than 20 GB/s
> > 5. And the derated bw is in the lowest peak bw qgv point is also
> >    less than this threshold of 20GB/s
> > 
> > This will make the driver to send this new threshold of 20GB/s
> > as the pmdemand request whenever the bw required for a usecase
> > is less than 20GB/s. The current pcode can handle this lower
> > peakbw value and adjust the fabric frequency accordingly.
> > 
> > Bspec: 68880
> > Assisted-by: Copilot:claude-sonnet-4.6
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bw.c       | 54
> > +++++++++++++++++++
> >  .../drm/i915/display/intel_display_device.h   |  1 +
> >  2 files changed, 55 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> > b/drivers/gpu/drm/i915/display/intel_bw.c
> > index 938c0294c251..747279075e29 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -54,6 +54,8 @@ struct intel_qgv_point {
> >  
> >  #define DEPROGBWPCLIMIT		60
> >  
> > +#define PEAK_BW_THRESHOLD	20000
> > +
> >  struct intel_psf_gv_point {
> >  	u8 clk; /* clock in multiples of 16.6666 MHz */
> >  };
> > @@ -589,6 +591,50 @@ static int icl_get_bw_info(struct
> > intel_display *display,
> >  	return 0;
> >  }
> >  
> > +static bool xe3_check_lower_peakbw(struct intel_display *display,
> > +				   const struct intel_qgv_info
> > *qi,
> > +				   int num_channels)
> 
> My pet peeve is naming functions with "check". Is it an assert? What
> does it do? What does the return value mean?
> 
> BR,
> Jani.
> 

hmm.. Intention is that, it should check the conditions for inserting
the low peak bw.. 

May be xe3_needs_lower_peakbw_point() ?

BR
Vinod

> > +{
> > +	unsigned int lowest_peakbw;
> > +
> > +	if (!HAS_PEAK_BW_THRESHOLD(display))
> > +		return false;
> > +
> > +	if (qi->num_points >= I915_NUM_QGV_POINTS) {
> > +		drm_warn(display->drm, "Cannot insert lowest QGV
> > point, not enough space\n");
> > +		return false;
> > +	}
> > +
> > +	lowest_peakbw = DIV_ROUND_CLOSEST(qi->points[0].dclk *
> > +					  qi->channel_width *
> > num_channels, 8);
> > +	if (lowest_peakbw <= PEAK_BW_THRESHOLD) {
> > +		drm_dbg_kms(display->drm,
> > +			    "Lowest QGV point has peak BW %u MB/s,
> > no need to insert lower point\n",
> > +			    lowest_peakbw);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static void xe3_insert_lowest_qgv_point(struct intel_display
> > *display,
> > +					struct intel_bw_info *bi)
> > +{
> > +	if (bi->num_qgv_points >= ARRAY_SIZE(bi->deratedbw))
> > +		return;
> > +
> > +	memmove(&bi->deratedbw[1], &bi->deratedbw[0],
> > +		bi->num_qgv_points * sizeof(*bi->deratedbw));
> > +
> > +	memmove(&bi->peakbw[1], &bi->peakbw[0],
> > +		bi->num_qgv_points * sizeof(*bi->peakbw));
> > +
> > +	/* Keep the derated bandwidth as the threshold*/
> > +	bi->deratedbw[0] = PEAK_BW_THRESHOLD;
> > +	bi->peakbw[0] = PEAK_BW_THRESHOLD;
> > +	bi->num_qgv_points++;
> > +}
> > +
> >  static int tgl_get_bw_info(struct intel_display *display,
> >  			   const struct dram_info *dram_info,
> >  			   const struct intel_sa_info *sa)
> > @@ -598,6 +644,7 @@ static int tgl_get_bw_info(struct intel_display
> > *display,
> >  	int num_channels = max_t(u8, 1, dram_info->num_channels);
> >  	int ipqdepth, ipqdepthpch = 16;
> >  	int dclk_max;
> > +	bool insert_low_peakbw;
> >  	int maxdebw, peakbw;
> >  	int clperchgroup;
> >  	int num_groups = ARRAY_SIZE(display->bw.max);
> > @@ -636,6 +683,10 @@ static int tgl_get_bw_info(struct
> > intel_display *display,
> >  	 */
> >  	clperchgroup = 4 * DIV_ROUND_UP(8, num_channels) *
> > qi.deinterleave;
> >  
> > +	insert_low_peakbw = xe3_check_lower_peakbw(display, &qi,
> > num_channels);
> > +
> > +	display->bw.max[i].num_planes = 0;
> > +
> >  	for (i = 0; i < num_groups; i++) {
> >  		struct intel_bw_info *bi = &display->bw.max[i];
> >  		struct intel_bw_info *bi_next;
> > @@ -678,6 +729,9 @@ static int tgl_get_bw_info(struct intel_display
> > *display,
> >  							 
> > qi.channel_width, 8);
> >  		}
> >  
> > +		if (insert_low_peakbw)
> > +			xe3_insert_lowest_qgv_point(display, bi);
> > +
> >  		for (j = 0; j < qi.num_psf_points; j++) {
> >  			const struct intel_psf_gv_point *sp =
> > &qi.psf_points[j];
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> > b/drivers/gpu/drm/i915/display/intel_display_device.h
> > index 65283286771a..b31ec42c3248 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> > @@ -192,6 +192,7 @@ struct intel_display_platforms {
> >  #define HAS_MBUS_JOINING(__display)	((__display)-
> > >platform.alderlake_p || DISPLAY_VER(__display) >= 14)
> >  #define HAS_MSO(__display)		(DISPLAY_VER(__display) >=
> > 12)
> >  #define HAS_OVERLAY(__display)		(DISPLAY_INFO(__display)-
> > >has_overlay)
> > +#define
> > HAS_PEAK_BW_THRESHOLD(__display)	(DISPLAY_VER(__display) >= 30)
> >  #define HAS_PIPEDMC(__display)		(DISPLAY_VER(__display) >=
> > 12)
> >  #define
> > HAS_PIXEL_NORMALIZER(__display)	(DISPLAY_VER(__display) >= 35)
> >  #define
> > HAS_PMDEMAND(__display)		(DISPLAY_VER(__display) >= 14)
>