On Tue, 2026-05-12 at 14:51 +0300, Jani Nikula wrote:
> On Mon, 11 May 2026, Vinod Govindapillai
> <[email protected]> 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 <[email protected]>
> > ---
> > 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)
>
