[PATCH v3 1/4] drm/xe/bmg: Update Wa_14022085890

[Lucas De Marchi]

On Wed, Jun 04, 2025 at 12:06:52AM +0000, Stuart Summers wrote:
>On Tue, 2025-06-03 at 13:31 -0700, Belgaumkar, Vinay wrote:
>>
>> On 6/3/2025 12:31 PM, Summers, Stuart wrote:
>> > On Tue, 2025-06-03 at 11:41 -0700, Belgaumkar, Vinay wrote:
>> > > On 6/3/2025 10:36 AM, Summers, Stuart wrote:
>> > > > On Mon, 2025-06-02 at 16:44 -0700, Vinay Belgaumkar wrote:
>> > > > > Set GT min frequency to 1200Mhz once driver load is
>> > > > > complete.
>> > > > >
>> > > > > v2: Review comments (Rodrigo)
>> > > > > v3: Apply Wa earlier so user_req_min is not clobbered.
>> > > > >
>> > > > > Cc: Matt Roper <matthew.d.roper at intel.com>
>> > > > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> > > > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>> > > > > ---
>> > > > >    drivers/gpu/drm/xe/xe_guc_pc.c     | 5 +++++
>> > > > >    drivers/gpu/drm/xe/xe_wa_oob.rules | 1 +
>> > > > >    2 files changed, 6 insertions(+)
>> > > > >
>> > > > > diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c
>> > > > > b/drivers/gpu/drm/xe/xe_guc_pc.c
>> > > > > index 18c623992035..cb0563494fcc 100644
>> > > > > --- a/drivers/gpu/drm/xe/xe_guc_pc.c
>> > > > > +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
>> > > > > @@ -51,6 +51,7 @@
>> > > > >   
>> > > > >    #define LNL_MERT_FREQ_CAP      800
>> > > > >    #define BMG_MERT_FREQ_CAP      2133
>> > > > > +#define BMG_MIN_FREQ           1200
>> > > > >   
>> > > > >    #define SLPC_RESET_TIMEOUT_MS 5 /* roughly 5ms, but no
>> > > > > need for
>> > > > > precision */
>> > > > >    #define SLPC_RESET_EXTENDED_TIMEOUT_MS 1000 /* To be used
>> > > > > only
>> > > > > at
>> > > > > pc_start */
>> > > > > @@ -843,6 +844,9 @@ static int pc_adjust_freq_bounds(struct
>> > > > > xe_guc_pc
>> > > > > *pc)
>> > > > >           if (pc_get_min_freq(pc) > pc->rp0_freq)
>> > > > >                   ret = pc_set_min_freq(pc, pc->rp0_freq);
>> > > > >   
>> > > > > +       if (XE_WA(pc_to_gt(pc), 14022085890))
>> > > > > +               ret = pc_set_min_freq(pc, max(BMG_MIN_FREQ,
>> > > > > pc_get_min_freq(pc)));
>> > > > Do we want this above the get_min_freq() call above? Seems like
>> > > > that
>> > > > would further limit which is what we'd want in that case right?
>> > > No, this is during SLPC init, we don't need to read the min freq
>> > > at
>> > > this
>> > > point. We will adjust it later in the flow if there was a user
>> > > request.
>>
>> This ensures min frequency is set to 1200. That is required by the
>> WA.
>> If there was no previous user request (like before a GT reset), the
>> min
>> will be set to 1200 instead of the usual RPe (which is around 500).
>
>Ok makes sense to me. I did confirm the workaround frequency called out
>in bspec matches here:
>Reviewed-by: Stuart Summers <stuart.summers at intel.com>

but why do we do 2 rmw instead of 1? Something like this

	u32 min_freq;
	...

	/*
	 * Same thing happens for Server platforms where min is listed as
	 * RPMax
	 */
	min_freq = min(pc_get_min_freq(pc), pc->rp0_freq);

	/* But shouldn't go to less than BMG_MIN_FREQ on affected platforms */
	if (XE_WA(pc_to_gt(pc), 14022085890))
		min_freq = max(BMG_MIN_FREQ, min_freq);

	ret = pc_set_min_freq(pc, min_freq);

>
>>
>> Thanks,
>>
>> Vinay.
>>
>> > So then what is the purpose of the code just above this?
>> >
>> > -Stuart
>> >
>> > > Thanks,
>> > >
>> > > Vinay.
>> > >
>> > > > Thanks,
>> > > > Stuart
>> > > >
>> > > > > +
>> > > > >    out:
>> > > > >           return ret;
>> > > > >    }
>> > > > > @@ -879,6 +883,7 @@ static int pc_set_mert_freq_cap(struct
>> > > > > xe_guc_pc
>> > > > > *pc)
>> > > > >                   ret = xe_guc_pc_get_min_freq(pc, &pc-
>> > > > > > stashed_min_freq);
>> > > > >                   if (!ret)
>> > > > >                           ret = xe_guc_pc_get_max_freq(pc,
>> > > > > &pc-
>> > > > > > stashed_max_freq);
>> > > > > +

nit: spurious newline added in unrelated function?

I have the local changes locally and can create a new rev later. Stuart,
since you reviewed this... do you agree?

Lucas De Marchi

>> > > > >                   if (ret)
>> > > > >                           return ret;
>> > > > >   
>> > > > > diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules
>> > > > > b/drivers/gpu/drm/xe/xe_wa_oob.rules
>> > > > > index 9efc5accd43d..75fbdab68d44 100644
>> > > > > --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
>> > > > > +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
>> > > > > @@ -59,3 +59,4 @@ no_media_l3   MEDIA_VERSION(3000)
>> > > > >                   MEDIA_VERSION_RANGE(1301, 3000)
>> > > > >    16026508708    GRAPHICS_VERSION_RANGE(1200, 3001)
>> > > > >                   MEDIA_VERSION_RANGE(1300, 3000)
>> > > > > +14022085890    GRAPHICS_VERSION(2001)
>