Re: [PATCH v2 1/2] drm/xe: Inspect all forcewake bits for runtime idleness

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

On Thu, Sep 10, 2026 at 02:28:22PM -0700, Vinay Belgaumkar wrote:
> Inspect all GT force wake bits in the idle check for runtime suspend.
> This includes the ones not typically used by Xe KMD as well. Skip suspend
> when any of these bits are set, since it could mean someone is trying to
> keep the GT awake.
> 
> Also add a call to pm_runtime_autosuspend() inside the idle check. This
> ensures a PM subsystem retry at a later time.
> 
> v2: Only check the GT domain FW register. Other domains require a GT force
> wake applied before we can read their registers. Retry if idleness check
> fails (Sashiko)
> 
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Assisted-by: Claude:Claude-Sonnet-5
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_force_wake.c | 25 +++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_force_wake.h |  1 +
>  drivers/gpu/drm/xe/xe_pci.c        |  6 +++++-
>  drivers/gpu/drm/xe/xe_pm.c         | 19 +++++++++++++++++++
>  drivers/gpu/drm/xe/xe_pm.h         |  1 +
>  5 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c
> index 197e2197bd0a..0acb796c756c 100644
> --- a/drivers/gpu/drm/xe/xe_force_wake.c
> +++ b/drivers/gpu/drm/xe/xe_force_wake.c
> @@ -148,6 +148,31 @@ static int domain_sleep_wait(struct xe_gt *gt,
>  	return __domain_wait(gt, domain, false);
>  }
>  
> +/**
> + * xe_force_wake_any_bit_set - Return true if any force wake bit is set
> + * @fw: struct xe_force_wake
> + * @id: Force wake domain id
> + *
> + * Check if any multithread force wake bit is set for a given domain ID.
> + *
> + * Return: true if any bit in the force wake reg is set
> + */
> +bool xe_force_wake_any_bit_set(struct xe_force_wake *fw, enum xe_force_wake_domain_id id)
> +{
> +	struct xe_gt *gt = fw->gt;
> +	struct xe_force_wake_domain *domain = &fw->domains[id];
> +	u32 val;
> +
> +	val = xe_mmio_read32(&gt->mmio, domain->reg_ctl);
> +	if (val & 0xFFFF) {
> +		xe_gt_dbg(gt, "FW bit set in domain: %d, reg: 0x%x, val: 0x%x",
> +			  domain->id, domain->reg_ctl.addr, val);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  /**
>   * xe_force_wake_get() : Increase the domain refcount
>   * @fw: struct xe_force_wake
> diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h
> index e2721f205d6c..53941e5a0fbe 100644
> --- a/drivers/gpu/drm/xe/xe_force_wake.h
> +++ b/drivers/gpu/drm/xe/xe_force_wake.h
> @@ -18,6 +18,7 @@ void xe_force_wake_init_engines(struct xe_gt *gt,
>  unsigned int __must_check xe_force_wake_get(struct xe_force_wake *fw,
>  					    enum xe_force_wake_domains domains);
>  void xe_force_wake_put(struct xe_force_wake *fw, unsigned int fw_ref);
> +bool xe_force_wake_any_bit_set(struct xe_force_wake *fw, enum xe_force_wake_domain_id id);
>  
>  const char *xe_force_wake_domain_to_str(enum xe_force_wake_domain_id id);
>  
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index ab4da1d9a9f1..a79d928ad75a 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -1438,10 +1438,14 @@ static int xe_pci_runtime_idle(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct xe_device *xe = pdev_to_xe_device(pdev);
> +	int ret;
>  
>  	xe_pm_d3cold_allowed_toggle(xe);
>  
> -	return 0;
> +	ret = xe_pm_check_runtime_idle(xe);
> +	pm_runtime_autosuspend(dev);

do we really need this autosuspend here?
I believe this is what may have confused Sashiko.
And if needed it probably worth a separate patch with explanation.

> +
> +	return ret;
>  }
>  
>  static const struct dev_pm_ops xe_pm_ops = {
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index f517bf453b54..e8d4cdbaef88 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -17,6 +17,8 @@
>  #include "xe_bo.h"
>  #include "xe_bo_evict.h"
>  #include "xe_device.h"
> +#include "xe_force_wake.h"
> +#include "xe_force_wake_types.h"
>  #include "xe_ggtt.h"
>  #include "xe_gt.h"
>  #include "xe_gt_idle.h"
> @@ -1030,6 +1032,23 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
>  	mutex_unlock(&xe->d3cold.lock);
>  }
>  
> +/**
> + * xe_pm_check_runtime_idle() - Perform checks before signaling runtime idle.
> + *
> + * Return: 0 on success, non-zero on finding device not idle
> + */
> +int xe_pm_check_runtime_idle(struct xe_device *xe)
> +{
> +	struct xe_gt *gt;
> +	u8 id;
> +
> +	for_each_gt(gt, xe, id)
> +		if (xe_force_wake_any_bit_set(gt_to_fw(gt), XE_FW_DOMAIN_ID_GT))
> +			return -EBUSY;
> +
> +	return 0;
> +}
> +
>  /**
>   * xe_pm_module_init() - Perform xe_pm specific module initialization.
>   *
> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
> index 6d5ab09cb769..d1f6a694b7c4 100644
> --- a/drivers/gpu/drm/xe/xe_pm.h
> +++ b/drivers/gpu/drm/xe/xe_pm.h
> @@ -38,6 +38,7 @@ struct task_struct *xe_pm_read_callback_task(struct xe_device *xe);
>  int xe_pm_block_on_suspend(struct xe_device *xe);
>  void xe_pm_might_block_on_suspend(void);
>  int xe_pm_module_init(void);
> +int xe_pm_check_runtime_idle(struct xe_device *xe);
>  
>  static inline void __xe_pm_runtime_noop(struct xe_device *xe) {}
>  
> -- 
> 2.38.1
>