> -----Original Message-----
> From: Kandpal, Suraj <[email protected]>
> Sent: Tuesday, July 14, 2026 2:58 PM
> To: Shankar, Uma <[email protected]>; [email protected];
> [email protected]
> Cc: Govindapillai, Vinod <[email protected]>
> Subject: RE: [PATCH] drm/i915/display/xe3p: Interpret 0xF populated-channel
> count as 16
>
> > Subject: [PATCH] drm/i915/display/xe3p: Interpret 0xF
> > populated-channel count as 16
> >
>
> Hi,
> Thanks for the patch mostly looks good I have some comments you can find them
> below
Thanks Suraj for the reviews.
> > The register MEM_SS_INFO_GLOBAL [Number of populated channels] field
> > definition is updated with an encoding for 16 channels.
> >
> > For 16-channel configuration, program 1111b. A programmed value of
> > 1111b must be interpreted as 16 channels for memory bandwidth calculations.
> >
> > WA: 16030862157
> > Bspec: 69131, 68859
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Uma Shankar <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_bw.c | 18 +++++++++++++++---
> > drivers/gpu/drm/i915/display/intel_dram.c | 11 +++++++++++
> > 2 files changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> > b/drivers/gpu/drm/i915/display/intel_bw.c
> > index 41539fdfeac5..f1197ce2d9fa 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -272,7 +272,13 @@ static int icl_get_qgv_points(struct
> > intel_display *display,
> > case INTEL_DRAM_LPDDR4:
> > case INTEL_DRAM_LPDDR5:
> > qi->t_bl = 16;
> > - qi->max_numchannels = 8;
> > + /*
> > + * Wa_16030862157
> > + * Xe3p supports a fully-populated 16-channel LPDDR
> > + * config (4 memory controllers x 4 channels); earlier
> > + * D14+ platforms top out at 8.
> > + */
> > + qi->max_numchannels = DISPLAY_VER(display) >= 35 ?
>
> Should this be == 35 and later be extended as and when WA is extended ?
>
This is addressing a limitation of 4 bits assigned for memory channels, this
will be
Applicable for all future platforms as well. No hardware issue as such, just a
BIOS->driver
interface fix.
> > 16 : 8;
>
> Since this is a WA lets use the intel_display_wa framework Which would mean
> defining intel_display_wa enum for this wa And updating the function
> intel_display_wa() Then we can call this function instead of
> DISPLAY_VER(display) >= 35
Sure, will switch to that.
> > qi->channel_width = 16;
> > qi->deinterleave = 4;
> > break;
> > @@ -624,10 +630,16 @@ static int tgl_get_bw_info(struct intel_display
> > *display,
> >
> > ipqdepth = min(ipqdepthpch, display_bw_params->displayrtids /
> > num_channels);
> > /*
> > + * Wa_16030862157
> > * clperchgroup = 4kpagespermempage * clperchperblock,
> > - * clperchperblock = 8 / num_channels * interleave
> > + * clperchperblock = max(8 / num_channels, 1) * interleave
> > + *
> > + * The 8 / num_channels truncating divide collapses to 0 for
> > + * >8-channel configs (16-channel: 8 / 16 = 0); the max(..., 1) floor
> > + * keeps clperchperblock >= 1 there while preserving the literal
> > + * truncating divide for <=8-channel configs.
> > */
> > - clperchgroup = 4 * (8 / num_channels) * qi.deinterleave;
> > + clperchgroup = 4 * max(8 / num_channels, 1) * qi.deinterleave;
> >
> > display->bw.num_qgv_points = qi.num_qgv_points;
> > display->bw.num_psf_gv_points = qi.num_psf_points; diff --git
> > a/drivers/gpu/drm/i915/display/intel_dram.c
> > b/drivers/gpu/drm/i915/display/intel_dram.c
> > index f103f7cba018..df58effca785 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dram.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dram.c
> > @@ -796,6 +796,17 @@ static int xelpdp_get_dram_info(struct
> > intel_display *display, struct dram_info
> > dram_info->num_qgv_points =
> > REG_FIELD_GET(MTL_N_OF_ENABLED_QGV_POINTS_MASK, val);
> > /* PSF GV points not supported in D14+ */
> >
> > + /*
> > + * Wa_16030862157
> > + * MEM_SS_INFO_GLOBAL populated-channel field is only 4 bits and
> > + * cannot encode 16, so on Xe3p the BIOS programs the saturated field
> > + * value (0xf) to indicate the fully-populated 16-channel config (4
> > + * memory controllers x 4 channels). Interpret it as 16.
> > + */
> > + if (DISPLAY_VER(display) >= 35 &&
>
>
> Same here
>
> I also think this patch series can be broken into two 1st patch which
> introduces
> the WA enum and function updating along with this change in dram The 2nd patch
> updating the bw side of things.
>
> Also the subject for 1st patch can be drm/i915/dram and 2nd patch can be
> drm/i915/bw
I will break in 2 parts, one to add the wa macros, and other to implement
actual change.
Regards,
Uma Shankar
> Regards,
> Suraj Kandpal
>
> > + dram_info->num_channels ==
> > REG_FIELD_MAX(MTL_N_OF_POPULATED_CH_MASK))
> > + dram_info->num_channels = 16;
> > +
> > if (DISPLAY_VER(display) >= 35)
> > dram_info->ecc_impacting_de_bw =
> > REG_FIELD_GET(XE3P_ECC_IMPACTING_DE, val);
> >
> > --
> > 2.50.1
