Re: [PATCH 2/2] drm/xe: Remove function xe_device_canonicalize_addr

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 15-07-2024 19:35, Souza, Jose wrote:
> On Mon, 2024-07-15 at 16:25 +0530, Himal Prasad Ghimiray wrote:
>> There is no need for converting regular address to canonical address in
>> driver, therefore remove unused xe_device_canonicalize_addr.
> 
> I can see several places that should use it but are not.
> Like every place that does xe_exec, the address needs to canonical in Xe2+ platforms.

Hi Jose, Thanks for feedback. Isn't it the responsibility of UMDs to 
call xe_exec with a canonical address ?

> We are probably not getting any issues so far by luck, so I would not remove it to add it later.

Can you please elaborate on what do you mean by luck here ?

> 
>>
>> Cc: José Roberto de Souza <jose.souza@intel.com>
>> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_device.c | 5 -----
>>   drivers/gpu/drm/xe/xe_device.h | 1 -
>>   2 files changed, 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>> index 06cebaffb451..eb3bb434b180 100644
>> --- a/drivers/gpu/drm/xe/xe_device.c
>> +++ b/drivers/gpu/drm/xe/xe_device.c
>> @@ -885,11 +885,6 @@ void xe_device_snapshot_print(struct xe_device *xe, struct drm_printer *p)
>>   	}
>>   }
>>   
>> -u64 xe_device_canonicalize_addr(struct xe_device *xe, u64 address)
>> -{
>> -	return sign_extend64(address, xe->info.va_bits - 1);
>> -}
>> -
>>   u64 xe_device_uncanonicalize_addr(struct xe_device *xe, u64 address)
>>   {
>>   	return address & GENMASK_ULL(xe->info.va_bits - 1, 0);
>> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
>> index 0a2a3e7fd402..14d4171c467e 100644
>> --- a/drivers/gpu/drm/xe/xe_device.h
>> +++ b/drivers/gpu/drm/xe/xe_device.h
>> @@ -158,7 +158,6 @@ u32 xe_device_ccs_bytes(struct xe_device *xe, u64 size);
>>   
>>   void xe_device_snapshot_print(struct xe_device *xe, struct drm_printer *p);
>>   
>> -u64 xe_device_canonicalize_addr(struct xe_device *xe, u64 address);
>>   u64 xe_device_uncanonicalize_addr(struct xe_device *xe, u64 address);
>>   
>>   void xe_device_td_flush(struct xe_device *xe);
>