Re: [PATCH] drm/xe/guc: Drop redundant ARAT timer enabling

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

On 04.06.2024 02:43, Matthew Brost wrote:
> On Mon, Jun 03, 2024 at 09:07:47PM +0200, Michal Wajdeczko wrote:
>> This interrupt is enabled by default and there is no need to
>> re-enable it as there is no other code in the Xe driver that
>> could change this setting.
>>
> 
> I agree this is likely the wrong location but also think it is little
> unsafe to rely on default values. I recall having to add this in in i915
> at one point and without this GuC submission doesn't work. How about
> moving it to the IRQ layer plus a comment along the lines of should be
> enabled by default but let's be paranoid and set it anyways?

but on i915 restoring the value of the ARAT_EXPIRED_INTRMSK was likely
more legit as there was another code that could modify this register
(see intel_rps.c) but on Xe driver we don't have any similar code, so
not sure if we need to be paranoid

> 
> Matt 
> 
>> Bspec: 52572
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_guc.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>> index 086a048876ba..f4d728350694 100644
>> --- a/drivers/gpu/drm/xe/xe_guc.c
>> +++ b/drivers/gpu/drm/xe/xe_guc.c
>> @@ -862,9 +862,6 @@ int xe_guc_enable_communication(struct xe_guc *guc)
>>  			return err;
>>  	}
>>  
>> -	xe_mmio_rmw32(guc_to_gt(guc), PMINTRMSK,
>> -		      ARAT_EXPIRED_INTRMSK, 0);
>> -
>>  	err = xe_guc_ct_enable(&guc->ct);
>>  	if (err)
>>  		return err;
>> -- 
>> 2.43.0
>>