Re: [PATCH 3/5] drm/xe: Helper to get dpa from pfn

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Mar 13, 2024 at 11:35:51PM -0400, Oak Zeng wrote:
> Since we now create struct page backing for each vram page,
> each vram page now also has a pfn, just like system memory.
> This allow us to calcuate device physical address from pfn.
> 
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_types.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index bbea40b57e84..bf349321f037 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -576,4 +576,12 @@ static inline struct xe_tile *mem_region_to_tile(struct xe_mem_region *mr)
>  	return container_of(mr, struct xe_tile, mem.vram);
>  }
>  
> +static inline u64 vram_pfn_to_dpa(struct xe_mem_region *mr, u64 pfn)
> +{
> +	u64 dpa;
> +	u64 offset = (pfn << PAGE_SHIFT) - mr->hpa_base;

Can't this be negative? 

e.g. if pfn == 0, offset == -mr->hpa_base.

Or is the assumption (pfn << PAGE_SHIFT) is always > mr->hpa_base?

If so can we an assert or something to ensure we using this function correctly.

> +	dpa = mr->dpa_base + offset;
> +	return dpa;
> +}

Same as previous patch, should be *.h not a *_types.h file.

Also as this is xe_mem_region not explictly vram. Maybe:

s/vram_pfn_to_dpa/xe_mem_region_pfn_to_dpa/

Matt

> +
>  #endif
> -- 
> 2.26.3
>