Re: [PATCH 1/1] drm/xe: Set GT rp min frequency as 1.2GHz default for BMG/CRI

["Kumar G, Naresh" <naresh.kumar.g@intel.com>]

On 27-02-2026 06:00, Matt Roper wrote:
> On Thu, Feb 26, 2026 at 11:37:17PM +0000, Summers, Stuart wrote:
>> On Mon, 2026-02-16 at 19:21 +0530, Nareshkumar Gollakoti wrote:
>>>   While previously applied only to the BMG G21(Battle image) platform
>>>   via workaround Wa_14022085890,this 1.2 GHz minimum is now the
>>
>> nit: space after comma
>>
>>> default
>>>   for BMG and CRI platforms.
>>>
>>>   Setting this frequency floor(1.2GHz) default is critical in multi
>>> GPU
>>>   environment for supporting effective Peer-to-Peer(P2P) transactions.
>>>
>>> Signed-off-by: Nareshkumar Gollakoti <naresh.kumar.g@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_guc_pc.c | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c
>>> b/drivers/gpu/drm/xe/xe_guc_pc.c
>>> index 5e5495a39a3c..881054c9cf92 100644
>>> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
>>> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
>>> @@ -860,6 +860,7 @@ static int pc_adjust_freq_bounds(struct xe_guc_pc
>>> *pc)
>>>   {
>>>          struct xe_tile *tile = gt_to_tile(pc_to_gt(pc));
>>>          int ret;
>>> +       struct xe_device *xe = pc_to_xe(pc);
>>>   
>>>          lockdep_assert_held(&pc->freq_lock);
>>>   
>>> @@ -885,7 +886,13 @@ 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_DEVICE_WA(tile_to_xe(tile), 14022085890))
>>> +       /*
>>> +        * Setting GT RP min frequency to 1.2GHz by default for
>>> +        * all BMG and CRI(For BMG G21 WA do the same).
>>
>> Should we add a comment here that this is the active frequency, so if
>> the device is idle we aren't expecting high power output across the
>> board with this?
> 

Noted

> But I think we are going to see higher power because of this?  We're
> raising the minimum frequency that the GT is able to drop to, so during
> light usage (e.g., casual desktop use) we'd be using more power now.  I
> don't know how much extra power (and heat) the frequency change would
> translate to in real-world usage, but I imagine it's probably going to
> be measurable.
> 
> I'm a bit worried that we may be adding a power regression here for the
> majority of existing BMG users in an attempt to improve performance for
> a very specialized corner case (multi-GPU with cross-device sharing).
> Can't we tighten this logic up and only boost the minimum when we're
> sure there are multiple GPUs and/or there's actually cross-GPU sharing
> going on as suggested earlier in the thread?
> 
>>
>>> +        */
>>> +       if (XE_DEVICE_WA(tile_to_xe(tile), 14022085890) ||
>>> +           (xe->info.platform == XE_BATTLEMAGE ||
>>> +                        xe->info.platform == XE_CRESCENTISLAND))
> 
> Aside from the concerns above, I think we should also avoid putting
> exact platform matches in "random" functions in the driver when
> possible.  If this truly needs to be based solely on platforms then it
> should be a feature flag in the device descriptors for those platforms
> so that it's easy to extend when enabling new platforms in the future
> without needing to track down specific spots in the code like this.  Or
> maybe we can even use an existing feature flag --- is there anything
> truly specific to BMG/CRI here, or should this really be an 'is_dgpu'
> check?
> 
> If we can tighten up the condition based on runtime detection, then the
> condition here could either be a function call to a function that checks
> the necessary conditions, or an xe->info field that we can adjust
> elsewhere as the conditions change.
> 
> 
> Matt

Hi Matt/Vinay,

whats your thoughts after discussion with Ankur?
Let me know your ack on the same.

> 
>>
>> Thomas already mentioned the indentation. Would be nice to get an ack
>> also from Vinay (cc'd) on this one.
>>
>> Otherwise:
>> Reviewed-by: Stuart Summers <stuart.summers@intel.com>
>>
>> Thanks,
>> Stuart
>>

Noted Stuart and Thomas.

Thanks
Naresh

>>>                  ret = pc_set_min_freq(pc, max(BMG_MIN_FREQ,
>>> pc_get_min_freq(pc)));
>>>   
>>>   out:
>>
>