Re: [PATCH v4 5/8] drm/xe: Only toggle scheduling in TDR if GuC is running

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Fri, Nov 21, 2025 at 02:06:41PM -0800, Matthew Brost wrote:
>On Thu, Nov 20, 2025 at 11:48:13AM -0800, Niranjana Vishwanathapura wrote:
>> On Wed, Nov 19, 2025 at 02:41:03PM -0800, Matthew Brost wrote:
>> > If the firmware is not running during TDR (e.g., when the driver is
>> > unloading), there's no need to toggle scheduling in the GuC. In such
>> > cases, skip this step.
>> >
>> > v4:
>> > - Bail on wait UC not running (Niranjana)
>> >
>> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>> > ---
>> > drivers/gpu/drm/xe/xe_guc_submit.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> > index 3ee35d4873bc..648c9ea06749 100644
>> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
>> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
>> > @@ -1277,7 +1277,7 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>> > 		if (exec_queue_reset(q))
>> > 			err = -EIO;
>> >
>> > -		if (!exec_queue_destroyed(q)) {
>> > +		if (!exec_queue_destroyed(q) && xe_uc_fw_is_running(&guc->fw)) {
>> > 			/*
>> > 			 * Wait for any pending G2H to flush out before
>> > 			 * modifying state
>> > @@ -1312,6 +1312,7 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>> > 		 */
>> > 		smp_rmb();
>> > 		ret = wait_event_timeout(guc->ct.wq,
>> > +					 !xe_uc_fw_is_running(&guc->fw) ||
>> > 					 !exec_queue_pending_disable(q) ||
>> > 					 xe_guc_read_stopped(guc) ||
>> > 					 vf_recovery(guc), HZ * 5);
>>
>> What if the wait exits because of '!xe_uc_fw_is_running(&guc->fw)'?
>> It is not clear where the control goes in that case based on all the
>> 'if' checks that follows this wait. Should there be a specific check
>> for '!!xe_uc_fw_is_running(&guc->fw)' following the wait here?
>>
>
>Return will be zero and we should tear down the queue. Also I believe
>this is covering case where the driver is unbinding and schedule disable
>CT blew up a warning. I think the logic works as is or at least I don't
>see a problem.
>

Ok, sounds good.
Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

>Matt
>
>> Niranjana
>>
>> > --
>> > 2.34.1
>> >