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

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

On 15-07-2024 23:44, Souza, Jose wrote:
> On Mon, 2024-07-15 at 23:26 +0530, Ghimiray, Himal Prasad wrote:
>>
>> 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 ?
> 
> Yep but xe it self also does xe_execs.
> 
>>
>>> 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 ?
> 
> We are luck that so far none of the xe kmd internal usage has a batch buffer with that address that sets the va_bits - 1, if that happened the address
> would not be in canonical format and HW would not execute the batch buffer.

Ok got it. Thanks for the reply.

> 
>>
>>>
>>>>
>>>> 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);
>>>
>