> -----Original Message-----
> From: Govindapillai, Vinod <[email protected]>
> Sent: Wednesday, July 15, 2026 6:01 PM
> To: Shankar, Uma <[email protected]>; [email protected];
> [email protected]
> Cc: Kandpal, Suraj <[email protected]>
> Subject: Re: [v2 2/2] drm/i915/dram: Interpret 0xF populated-channel count as 
> 16
> 
> Hi Uma,
> 
> On Wed, 2026-07-15 at 16:11 +0530, Uma Shankar wrote:
> > 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.
> >
> > The 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 and let the bandwidth
> > math handle the larger channel count.
> >
> > Gate the behaviour through
> > intel_display_wa(INTEL_DISPLAY_WA_16030862157)
> > instead of an open-coded DISPLAY_VER() check.
> >
> > v2: Switched to intel_display_wa.c framework (Suraj)
> >
> > 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   | 20 +++++++++++++++++---
> >  drivers/gpu/drm/i915/display/intel_dram.c | 12 ++++++++++++
> >  2 files changed, 29 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..aaa0350dca78 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -14,6 +14,7 @@
> >  #include "intel_display_regs.h"
> >  #include "intel_display_types.h"
> >  #include "intel_display_utils.h"
> > +#include "intel_display_wa.h"
> >  #include "intel_dram.h"
> >  #include "intel_mchbar.h"
> >  #include "intel_parent.h"
> > @@ -272,7 +273,14 @@ 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 =
> > +                           intel_display_wa(display,
> > INTEL_DISPLAY_WA_16030862157) ? 16 : 8;
> 
> As per the bspec 68859, max channel of 16 v. 8 is based on a specific sku. Do 
> we
> have any further info on that how to variate that?
> 
> But the INTEL_DISPLAY_WA_16030862157 applies to display versions >= 35
> 
> In tgl_get_bw_info(), we have this adjustment to the deinterleave
> 
>       if (num_channels < qi.max_numchannels && DISPLAY_VER(display)
> >= 12)
>               qi.deinterleave = max(qi.deinterleave / 2, 1);
> 
> So with the above change we will always end up in this loop and adjust the
> qi.deinterleave for all versions >= 35 not just the specific "sku"
> which I think is not correct. May be need to check from the IP team or
> clarify/update the bspec?

This is extended for all NVL SKU's, hence the check. However will drop > so that
its restricted only for NVL.

> And another suggestion for your consideration is:
> 
> I didnt find this 1603862157 as part of wa database. Is there any wa bspec? 
> So do
> we need to  implement this as a wa?

This is the WA lineage number which XE driver and bspec follows, hence used the 
same
here.

> If this is considered as a wa:
> 
> If I understand this correctly, wa is mainly about part where bios populate 
> the
> MTL_N_OF_POPULATED_CH_MASK fields as 0xf in cased of channels 16 and
> we interpret that as channels = 16 instead of 15.
> 
> IMO, ideally it is better to split this into two parts,
> 
> 1. wa definitions in the existing driver wa framework + changes in
> xelpdp_get_dram_info() related to this wa as a single patch like other wa
> implementations in the driver so far.
> 
> 2. Update the bw info changes to add support for max_number of channels to 16 
> in
> case of a specific SKU and the update to the c9_clperchperblock calculation 
> part.

Sure, sounds good. I will change accordingly.

Regards,
Uma Shankar

> BR
> Vinod
> 
> 
> >                     qi->channel_width = 16;
> >                     qi->deinterleave = 4;
> >                     break;
> > @@ -624,10 +632,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..7e2fc24e240c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dram.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dram.c
> > @@ -13,6 +13,7 @@
> >  #include "intel_display_core.h"
> >  #include "intel_display_utils.h"
> >  #include "intel_display_regs.h"
> > +#include "intel_display_wa.h"
> >  #include "intel_dram.h"
> >  #include "intel_mchbar.h"
> >  #include "intel_parent.h"
> > @@ -796,6 +797,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 (intel_display_wa(display, INTEL_DISPLAY_WA_16030862157)
> > &&
> > +       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);
> >

Reply via email to