Re: [PATCH] drm/xe: Initialize GuC ID mgr before registering guc_submit_fini

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 05.04.2024 23:27, Matthew Brost wrote:
> It reasonable to assume that guc_submit_fini may rely on GuC ID mgr
> being available. With that, register guc_submit_fini after initializing
> the GuC ID mgr. This will ensure the GuC ID mgr is available in
> guc_submit_fini due the reverse execution order of
> drmm_add_action_or_reset.

that wont fly right now as GuC ID manager expects submission_state.lock
mutex being still available during it's cleanup, but now it will be
destroyed in __guc_submit_fini() called prior to __fini_idm()

please take a look at [1] which should solve all the problems

[1] https://patchwork.freedesktop.org/series/132118/

> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 9c30bd9ac8c0..148c5dc2c1ac 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -232,15 +232,20 @@ static struct workqueue_struct *get_submit_wq(struct xe_guc *guc)
>  }
>  #endif
>  
> -static void guc_submit_fini(struct drm_device *drm, void *arg)
> +static void __guc_submit_fini(struct xe_guc *guc)
>  {
> -	struct xe_guc *guc = arg;
> -
>  	xa_destroy(&guc->submission_state.exec_queue_lookup);
>  	free_submit_wq(guc);
>  	mutex_destroy(&guc->submission_state.lock);
>  }
>  
> +static void guc_submit_fini(struct drm_device *drm, void *arg)
> +{
> +	struct xe_guc *guc = arg;
> +
> +	__guc_submit_fini(guc);
> +}
> +
>  static const struct xe_exec_queue_ops guc_exec_queue_ops;
>  
>  static void primelockdep(struct xe_guc *guc)
> @@ -277,11 +282,13 @@ int xe_guc_submit_init(struct xe_guc *guc)
>  
>  	primelockdep(guc);
>  
> -	err = drmm_add_action_or_reset(&xe->drm, guc_submit_fini, guc);
> -	if (err)
> +	err = xe_guc_id_mgr_init(&guc->submission_state.idm, ~0);
> +	if (err) {
> +		__guc_submit_fini(guc);
>  		return err;
> +	}
>  
> -	err = xe_guc_id_mgr_init(&guc->submission_state.idm, ~0);
> +	err = drmm_add_action_or_reset(&xe->drm, guc_submit_fini, guc);
>  	if (err)
>  		return err;
>