Re: [PATCH v3 1/6] drm/xe: Add functions to convert regular address to canonical address and back

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

On Mon, 2024-01-29 at 13:13 -0800, Matt Roper wrote:
> On Mon, Jan 29, 2024 at 10:17:37AM -0800, José Roberto de Souza wrote:
> > Some instructions requires canonical address like
> > MI_BATCH_BUFFER_START(UMDs must call xe_exec with a canonical address
> > for Xe2+).
> > 
> > So here adding functions to convert regular address to canonical
> > address and back, the first user of this functions will be added
> > in the next patches.
> > 
> > v3:
> > - inline removed
> > - rename highest_address_bit_get() to ppgtt_msb_get()
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Maarten Lankhorst <dev@lankhorst.se>
> > Cc: Stuart Summers <stuart.summers@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_device.c | 15 +++++++++++++++
> >  drivers/gpu/drm/xe/xe_device.h |  3 +++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index 6faa7865b1aab..6172d4f987937 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -747,3 +747,18 @@ void xe_device_snapshot_print(struct xe_device *xe, struct drm_printer *p)
> >  		drm_printf(p, "\tCS reference clock: %u\n", gt->info.reference_clock);
> >  	}
> >  }
> > +
> > +static int ppgtt_msb_get(struct xe_device *xe)
> > +{
> > +	return xe->info.dma_mask_size > 48 ? 57 : 47;
> 
> Can we just use (xe->info.va_bits - 1)?  As far as I can tell, what we
> really care about for canonical form is the size of the PPGTT virtual
> address space, not the DMA mask, right?  That also matches platforms
> like Xe_LP better where:
> 
>         .dma_mask_size = 39,                                                                                   
>         .va_bits = 48,
> 

ouch, missed va_bits while rebasing this patch.

thanks for pointing out.

>                                                                                          
> 
> 
> Matt
> 
> > +}
> > +
> > +u64 xe_device_canonicalize_addr(struct xe_device *xe, u64 address)
> > +{
> > +	return sign_extend64(address, ppgtt_msb_get(xe));
> > +}
> > +
> > +u64 xe_device_uncanonicalize_addr(struct xe_device *xe, u64 address)
> > +{
> > +	return address & GENMASK_ULL(ppgtt_msb_get(xe), 0);
> > +}
> > diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> > index 270124da1e00e..462f59e902b12 100644
> > --- a/drivers/gpu/drm/xe/xe_device.h
> > +++ b/drivers/gpu/drm/xe/xe_device.h
> > @@ -177,4 +177,7 @@ 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);
> > +
> >  #endif
> > -- 
> > 2.43.0
> > 
>