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

[Jani Nikula <jani.nikula@linux.intel.com>]

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.

> +{
> +	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)

-- 
Jani Nikula, Intel