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

["Souza, Jose" <jose.souza@intel.com>]

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.

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