Re: [PATCH v2 5/6] drm/xe: Move HuC init before GuC init

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

On Tue, Aug 20, 2024 at 03:42:35PM -0500, Lucas De Marchi wrote:
> On Tue, Aug 20, 2024 at 10:29:57AM GMT, Matthew Brost wrote:
> > The GuC fini, also fini the HuC so move HuC init first.
> 
> AFAIU this is about
> 
> guc_fini_hw()
>   xe_uc_fini_hw()
>     xe_uc_sanitize_reset()
>       xe_uc_sanitize()
>         xe_huc_sanitize()
> 	xe_guc_sanitize()
> 
> 
> but why exactly do we do that layering? Shouldn't xe_uc_init()
> be the one adding an action to xe_uc_fini_hw() or let each of the
> uc to sanitize itself?
> 

Yea we probably should just cleanup fini call stack so each of UC
sanitize itself. Was lazy with this fix, will respin with a proper fix.

Matt

> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_uc.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
> > index 0d073a9987c2..5b7d6f44a7de 100644
> > --- a/drivers/gpu/drm/xe/xe_uc.c
> > +++ b/drivers/gpu/drm/xe/xe_uc.c
> > @@ -36,6 +36,10 @@ int xe_uc_init(struct xe_uc *uc)
> > {
> > 	int ret;
> > 
> > +	ret = xe_huc_init(&uc->huc);
> > +	if (ret)
> > +		goto err;
> > +
> 
> if we are keeping this patch, then the comment below should be on top,
> or maybe even on top of the function.
> 
> Lucas De Marchi
> 
> > 	/*
> > 	 * We call the GuC/HuC/GSC init functions even if GuC submission is off
> > 	 * to correctly move our tracking of the FW state to "disabled".
> > @@ -44,10 +48,6 @@ int xe_uc_init(struct xe_uc *uc)
> > 	if (ret)
> > 		goto err;
> > 
> > -	ret = xe_huc_init(&uc->huc);
> > -	if (ret)
> > -		goto err;
> > -
> > 	ret = xe_gsc_init(&uc->gsc);
> > 	if (ret)
> > 		goto err;
> > -- 
> > 2.34.1
> >