[PATCH v1] drm/xe/guc: Disable lite restore for MTL

[Dong, Zhanjun]

Thanks @Filipchuk, Julia and @Wajdeczko, Michal for the review.

Yes, I will post another revision as recommended.

Regards,
Zhanjun Dong

> -----Original Message-----
> From: Filipchuk, Julia <julia.filipchuk at intel.com>
> Sent: March 18, 2025 5:35 PM
> To: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>; Dong, Zhanjun
> <zhanjun.dong at intel.com>; intel-xe at lists.freedesktop.org
> Subject: Re: [PATCH v1] drm/xe/guc: Disable lite restore for MTL
> 
> On 3/18/2025 7:35 AM, Michal Wajdeczko wrote:
> >
> >
> > On 18.03.2025 15:18, Zhanjun Dong wrote:
> >> Lite restore for MTL is not POR for Xe, then disable it for MTL.
> >>
> >> Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_guc.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> >> index bc5714a5b36b..0c2629e578d9 100644
> >> --- a/drivers/gpu/drm/xe/xe_guc.c
> >> +++ b/drivers/gpu/drm/xe/xe_guc.c
> >> @@ -80,7 +80,9 @@ static u32 guc_ctl_debug_flags(struct xe_guc *guc)
> >>
> >>  static u32 guc_ctl_feature_flags(struct xe_guc *guc)
> >>  {
> >> -	u32 flags = GUC_CTL_ENABLE_LITE_RESTORE;
> >> +	u32 flags = (guc_to_xe(guc)->info.platform == XE_METEORLAKE) ?
> >> +		    0 : GUC_CTL_ENABLE_LITE_RESTORE;
> >> +
> >
> > please avoid long conditional initialization code,
> > all this could be done in a simpler/unified way:
> >
> > 	struct xe_device *xe = guc_to_xe(guc);
> > 	u32 flags = 0;
> >
> > 	if (xe->info.platform != XE_METEORLAKE)
> > 		flags |= GUC_CTL_ENABLE_LITE_RESTORE;
> >
> >>
> >>  	if (!guc_to_xe(guc)->info.skip_guc_pc)
> >>  		flags |= GUC_CTL_ENABLE_SLPC;
> >
> 
> Agreed with this suggestion for clarity. Since
> GUC_CTL_ENABLE_LITE_RESTORE is
> the default suggest just clearing it for MTL instead of an enable-if-not.
> 
> 	struct xe_device *xe = guc_to_xe(guc);
> 	u32 flags = GUC_CTL_ENABLE_LITE_RESTORE;
> 
> 	if (xe->info.platform == XE_METEORLAKE)
> 		flags &= ~GUC_CTL_ENABLE_LITE_RESTORE;
> 
> 	if (!xe->info.skip_guc_pc)
> 		flags |= GUC_CTL_ENABLE_SLPC;
> 
> Also, if updated the replace the additional guc_to_xe(guc) in the function.
> 
> 
> Reviewed-by: Julia Filipchuk <julia.filipchuk at intel.com>