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

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

On 9/8/2026 6:56 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Returning -EAGAIN from `xe_pci_runtime_idle` aborts the runtime PM idle transition but fails to schedule a retry, permanently preventing runtime suspend.
> - [Medium] Using `xe_gt_info` in a runtime PM path will cause severe log spam when forcewake is held.
> --
>
> commit 987ddc78bc72644b422f8d130f4407129128a641
> Author: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
> drm/xe: Inspect all forcewake bits for runtime idleness
>
> This commit updates the runtime idleness check to inspect all forcewake
> bits across all domains. If any bit is set, it returns -EAGAIN to skip
> the suspend operation.
>
>> diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c
> [ ... ]
>> +bool xe_force_wake_any_bit_set(struct xe_force_wake *fw)
>> +{
>> +	struct xe_gt *gt = fw->gt;
>> +	struct xe_force_wake_domain *domain;
>> +	unsigned int tmp;
>> +	u32 val;
>> +
>> +	for_each_fw_domain(domain, fw, tmp) {
>> +		val = xe_mmio_read32(&gt->mmio, domain->reg_ctl);
>> +		if (val & 0xFFFF) {
>> +			xe_gt_info(gt, "FW bit set in domain: %d, val: 0x%x",
>> +				   domain->id, val);
> [Severity: Medium]
> Will this cause severe log spam in high-frequency runtime PM paths?
>
> Forcewake can be legitimately held for long periods (for example, by
> userspace profiling tools, IGT tests, or debugfs readers). Every time the
> driver drops its PM reference and triggers a runtime idle check, this
> unconditional info message will print if the external agent is still holding
> forcewake.
>
> Should this be a debug-level log (xe_gt_dbg) or rate-limited instead?
yes, will change to xe_gt_dbg instead.
>
>> +			return true;
>> +		}
>> +	}
>> +
>> +	return false;
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> [ ... ]
>> @@ -1437,9 +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);
>> +	struct xe_gt *gt;
>> +	u8 id;
>>   
>>   	xe_pm_d3cold_allowed_toggle(xe);
>>   
>> +	for_each_gt(gt, xe, id)
>> +		if (xe_force_wake_any_bit_set(gt_to_fw(gt)))
>> +			return -EAGAIN;
> [Severity: High]
> Will returning -EAGAIN here permanently prevent runtime suspend?
>
> When the PM core's rpm_idle() function receives a non-zero return value, it
> aborts the idle transition and bypasses the subsequent call to rpm_suspend().
> Returning an error code does not automatically instruct the PM core to poll
> or reschedule an autosuspend.
>
> If a forcewake bit happens to be set at the moment the PM usage count drops
> to 0, it seems the device will remain stranded in the active (D0) state
> indefinitely.
>
> Should this explicitly schedule a retry, perhaps by calling
> pm_runtime_mark_last_busy() and pm_runtime_autosuspend() before returning?

Might be possibly required, will evaluate.

Thanks,

Vinay.

>
>>   	return 0;
>>   }
>>