Re: [PATCH v6 10/13] drm/xe/guc_submit: Allow calling guc_exec_queue_resume with pending resume

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

On Tue, Aug 06, 2024 at 09:27:08PM +0200, Francois Dugast wrote:
> Make it safe to have multiple calls to guc_exec_queue_resume() for the
> same exec queue. This allows the use of this function from a context where
> knowledge about prior calls is missing. In this case the function does
> nothing but at least it does not fail on assert.
> 
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 50013e1e7455..782561b21d24 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1623,10 +1623,9 @@ static int guc_exec_queue_suspend_wait(struct xe_exec_queue *q)
>  static void guc_exec_queue_resume(struct xe_exec_queue *q)
>  {
>  	struct xe_sched_msg *msg = q->guc->static_msgs + STATIC_MSG_RESUME;
> -	struct xe_guc *guc = exec_queue_to_guc(q);
> -	struct xe_device *xe = guc_to_xe(guc);
>  
> -	xe_assert(xe, !q->guc->suspend_pending);

I think the existing assert is correct. We certainly don't want to call
resume until suspend_wait returns (sets q->guc->suspend_pending ==
false). 

I think if we want to make this safe to be called multiple times we need
a new function 'guc_exec_queue_try_add_msg'. This function would check the
msg->link link under lock for empty and if empty add the message. It is
harmless for __guc_exec_queue_process_msg_resume to be called multple
times, it is not harmless to use the list twice.

Matt

> +	if (q->guc->suspend_pending)
> +		return;
>  
>  	guc_exec_queue_add_msg(q, msg, RESUME);
>  }
> -- 
> 2.43.0
>