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

[Julia Filipchuk <julia.filipchuk@intel.com>]

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@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@intel.com>