Re: [PATCH v4] drm/xe/pm: Disable RPM for SR-IOV VFs with no PCIe PM capability

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Aug 05, 2025 at 05:40:58PM +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 PM-incapable 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@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Maarten Lankhorst <dev@lankhorst.se>
> ---
> 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 | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 5e8126ca8e27..9d62d3f99ed9 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -245,8 +245,13 @@ static bool xe_pm_pci_d3cold_capable(struct xe_device *xe)
>  
>  static void xe_pm_runtime_init(struct xe_device *xe)
>  {
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>  	struct device *dev = xe->drm.dev;
>  
> +	/* For VF that doesn't implement pm_caps, the rpm functions are no-op */
> +	if (IS_SRIOV_VF(xe) && !pdev->pm_cap)

I still dislike the fact that the driver is checking !pdev->pm_cap this should be
done at the pci-subsystem/runtime_pm level... not at driver level...

> +		return;
> +
>  	/*
>  	 * Disable the system suspend direct complete optimization.
>  	 * We need to ensure that the regular device suspend/resume functions
> @@ -365,8 +370,13 @@ int xe_pm_init(struct xe_device *xe)
>  
>  static void xe_pm_runtime_fini(struct xe_device *xe)
>  {
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>  	struct device *dev = xe->drm.dev;
>  
> +	/* For VF that doesn't implement pm_caps, the rpm functions are no-op */
> +	if (IS_SRIOV_VF(xe) && !pdev->pm_cap)
> +		return;

and it is even worse that we are checking that twice...

> +
>  	pm_runtime_get_sync(dev);
>  	pm_runtime_forbid(dev);
>  }
> -- 
> 2.43.0
>