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

["Belgaumkar, Vinay" <vinay.belgaumkar@intel.com>]

On 9/10/2026 3:17 PM, Rodrigo Vivi wrote:
> 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.

It was Sashiko's suggestion :). If we fail runtime_idle check due to 
someone holding a forcewake, there is no mechanism to re-trigger the 
autosuspend when they release the forcewake. I believe it is only 
retriggered on a pm_ref put(). So, system will be stuck in D0 state even 
after the forcewake has been released (until some execution happens and 
a pm_ref count goes to zero again). We only need the autosuspend when 
the check fails, so should be part of the same patch? I added it to both 
paths since it is a noop in the case where we succeed in the idle check 
(as it returns 0 to the PM subsystem).

Thanks,

Vinay.

>
>> +
>> +	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
>>