Re: [PATCH 7/7] drm/xe: Check for GuC responses on disabling scheduling

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Oct 13, 2025 at 10:31:35PM +0000, Stuart Summers wrote:
> Currently if the GuC becomes unresponsive during a schedule
> disable event, after we send the CT request, the driver
> doesn't have a good way to recover. In most other cases,
> we explicitly wait for GuC to respond by checking either
> pending_enable, pending_disable, or some other state change
> that we expect to be set after the response from GuC is
> received for that particular request. Add a similar check
> on the schedule disable side and make sure the queue state
> for the queue being disabled is reset properly in that case.
> 
> v2: Only call the deregistration sequence since in this
>     case the scheduling handler should be reset during
>     the GT reset.
>     By doing that, we don't need a way to track the scheduling
>     disable request handler for that queue, making this sequence
>     simpler. As a result, don't mark the queue as banned.
> 
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index c923f13a13ef..ca37c7a8c5ed 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -924,6 +924,8 @@ int xe_guc_read_stopped(struct xe_guc *guc)
>  		GUC_CONTEXT_##enable_disable,				\
>  	}
>  
> +static void handle_deregister_done(struct xe_guc *guc, struct xe_exec_queue *q);
> +
>  static void disable_scheduling_deregister(struct xe_guc *guc,
>  					  struct xe_exec_queue *q)
>  {
> @@ -961,6 +963,17 @@ static void disable_scheduling_deregister(struct xe_guc *guc,
>  	xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action),
>  		       G2H_LEN_DW_SCHED_CONTEXT_MODE_SET +
>  		       G2H_LEN_DW_DEREGISTER_CONTEXT, 2);
> +
> +	ret = wait_event_timeout(guc->ct.wq,
> +				 !exec_queue_pending_disable(q) ||
> +				 xe_guc_read_stopped(guc),
> +				 HZ * 5);
> +	if (!ret || xe_guc_read_stopped(guc)) {
> +		xe_gt_warn(guc_to_gt(guc), "Schedule disable failed to respond");
> +		handle_deregister_done(guc, q);
> +		xe_gt_reset_async(q->gt);
> +	}
> +

I thought I already stated this in eariler reviews - this path is
designed to be fully async (i.e., no blocking here). If H2G get lost for
whatever reason the driver should be smart enough to cleanup the memory
references assicoiated for the lost H2G.

Matt

>  }
>  
>  static void xe_guc_exec_queue_trigger_cleanup(struct xe_exec_queue *q)
> -- 
> 2.34.1
>