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

[John Harrison <john.c.harrison@intel.com>]

On 3/19/2025 10:28 AM, Zhanjun Dong wrote:
> Lite restore for MTL is not POR for Xe, then disable it for MTL.
The point is that it is causing unexplained problems. And it is Xe as a 
whole that is not POR on MTL, not just lite restore. Hence no priority 
to investigate what is going wrong and how to fix it properly.

You should also reference the upstream bug that this is in response to. 
I think the tag is 'Closes:'?

>
> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
> Reviewed-by: Julia Filipchuk <julia.filipchuk@intel.com>
> ---
> Changes from prior revs:
>    v2:- Changed long conditional initialization to simple if statement.
>
>   drivers/gpu/drm/xe/xe_guc.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index bc5714a5b36b..d9a0daf584b4 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -80,9 +80,13 @@ static u32 guc_ctl_debug_flags(struct xe_guc *guc)
>   
>   static u32 guc_ctl_feature_flags(struct xe_guc *guc)
>   {
> +	struct xe_device *xe = guc_to_xe(guc);
>   	u32 flags = GUC_CTL_ENABLE_LITE_RESTORE;
>   
> -	if (!guc_to_xe(guc)->info.skip_guc_pc)
> +	if (xe->info.platform == XE_METEORLAKE)
> +		flags &= ~GUC_CTL_ENABLE_LITE_RESTORE;
This needs a comment to explain this non-standard behaviour. Otherwise, 
in the future, someone might try to turn it back on thinking this was 
some kind of oversight. E.g.
     /* Lite restore seems to cause issues on MTL (see bug XXX) so 
disable it for now */

Maybe even add a one line synopsis of the symptoms being seen in the bug 
report.

John.

> +
> +	if (!xe->info.skip_guc_pc)
>   		flags |= GUC_CTL_ENABLE_SLPC;
>   
>   	return flags;