[PATCH v6] drm/xe/pm: Disable RPM for SR-IOV VFs

[Matthew Auld]

On 18/08/2025 03:13, Matthew Brost wrote:
> On Sun, Aug 17, 2025 at 12:40:55PM -0400, Rodrigo Vivi wrote:
>> On Tue, Aug 12, 2025 at 10:06:13PM +0530, Satyanarayana K V P wrote:
>>> VFs without native PCIe Power Management (PM) capabilities inherit their
>>> PF's power state as per PCIe specifications(§5.10.1 PCIe Base Spec 7.0).
>>> Enabling Runtime Power Management (RPM) for these VFs trigger unnecessary
>>> driver suspend/resume operations that ultimately perform no PCI-level power
>>> transition.
>>>
>>> Since VFs without PM capabilities cannot independently enter low-power
>>> states, the existing RPM workflow becomes redundant:
>>> 1. Driver executes full suspend/resume sequence
>>> 2. PCI PM transition step becomes no-op
>>> 3. VF power state remains tied to PF's status
>>>
>>> Disabling RPM for VFs eliminates this redundant processing while
>>> maintaining proper power management through PF dependency. This
>>> optimization ensures VFs follow their PF's power state without superfluous
>>> runtime handling.
>>>
>>> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p at intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>> Cc: Matthew Brost <matthew.brost at intel.com>
>>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>>> Cc: Anshuman Gupta <anshuman.gupta at intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>> Cc: Maarten Lankhorst <dev at lankhorst.se>
>>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>
>> and pushing
>>
> 
> This might need a little more thought(?). What are the full consequences
> of this patch? For example, will xe_pm_suspend()/xe_pm_resume() never be
> called on a VF? I think that’s correct and desirable, but consider code
> paths like:
> 
> if (xe_pm_runtime_get_if_active())
>          /* Take action assuming GPU state is valid */
> else
>          /* Take action assuming GPU state has been lost */
> 
> 
> On a VF, this is now broken, since we don’t lose GPU state—unless I’m
> missing something, or xe_pm_runtime_get_if_active() always returns true
> on a VF which in that case we should be good.

It should behave as though the system is always RPM_ACTIVE with an 
elevated refcount (since driver is no longer dropping the refcount to 
turn on RPM) which will make it always return true for if_active() and 
others.

> 
> Matt
> 
>>>
>>> ---
>>> V5 -> V6:
>>> - Updated comment (Rodrigo).
>>>
>>> V4 -> V5:
>>> - Disable RPM for VFs without checking pm_cap (Rodrigo).
>>> - Updated commit title.
>>>
>>> V3 -> V4:
>>> - Fixed review comments (Rodrigo & Maarten)
>>>
>>> V2 -> V3:
>>> - Fixed review comments (Michal Wajdeczko).
>>>
>>> V1 -> V2:
>>> - Disable RPM only for VF devices when PM cap is not implemented.
>>> ---
>>>   drivers/gpu/drm/xe/xe_pm.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
>>> index 5e8126ca8e27..51934d5dc44f 100644
>>> --- a/drivers/gpu/drm/xe/xe_pm.c
>>> +++ b/drivers/gpu/drm/xe/xe_pm.c
>>> @@ -247,6 +247,10 @@ static void xe_pm_runtime_init(struct xe_device *xe)
>>>   {
>>>   	struct device *dev = xe->drm.dev;
>>>   
>>> +	/* Our current VFs do not support RPM. so, disable it */
>>> +	if (IS_SRIOV_VF(xe))
>>> +		return;
>>> +
>>>   	/*
>>>   	 * Disable the system suspend direct complete optimization.
>>>   	 * We need to ensure that the regular device suspend/resume functions
>>> @@ -367,6 +371,10 @@ static void xe_pm_runtime_fini(struct xe_device *xe)
>>>   {
>>>   	struct device *dev = xe->drm.dev;
>>>   
>>> +	/* Our current VFs do not support RPM. so, disable it */
>>> +	if (IS_SRIOV_VF(xe))
>>> +		return;
>>> +
>>>   	pm_runtime_get_sync(dev);
>>>   	pm_runtime_forbid(dev);
>>>   }
>>> -- 
>>> 2.43.0
>>>