Re: [PATCH 4/8] drm/xe: Introduce a helper to get dpa from pfn

["Welty, Brian" <brian.welty@intel.com>]

On 3/19/2024 8:44 PM, 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.
> 
> v1: move the function to xe_svm.h (Matt)
>      s/vram_pfn_to_dpa/xe_mem_region_pfn_to_dpa (Matt)
>      add kernel document for the helper (Thomas)
> 
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_svm.h | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
> index e944971cfc6d..8a34429eb674 100644
> --- a/drivers/gpu/drm/xe/xe_svm.h
> +++ b/drivers/gpu/drm/xe/xe_svm.h
> @@ -6,8 +6,31 @@
>   #ifndef __XE_SVM_H
>   #define __XE_SVM_H
>   
> -struct xe_tile;
> -struct xe_mem_region;
> +#include "xe_device_types.h"
> +#include "xe_device.h"
> +#include "xe_assert.h"
> +
> +/**
> + * xe_mem_region_pfn_to_dpa() - Calculate page's dpa from pfn
> + *
> + * @mr: The memory region that page resides in
> + * @pfn: page frame number of the page
> + *
> + * Returns: the device physical address of the page
> + */
> +static inline u64 xe_mem_region_pfn_to_dpa(struct xe_mem_region *mr, u64 pfn)
> +{
> +	u64 dpa;
> +	struct xe_tile *tile = xe_mem_region_to_tile(mr);

I believe you are dropping this patch for now, but wanted to comment.

Is above the only usage of xe_mem_region_to_tile() ?

As mentioned in earlier review, it's better to avoid the tight 
association with xe_mem_region and tile.  Such as for the UMA we talked
before, it's nice to have flexibility for xe_mem_region to be associated 
with multiple tiles.  But I guess this is all future/pending, so can
leave to solve in future if you prefer.

Not sure you have other usage of xe_mem_region_to_tile().  But here,
looks only used for getting tile->xe pointer.
Can caller of xe_mem_region_pfn_to_dpa() just pass in the xe device pointer?

-Brian


> +	struct xe_device *xe = tile_to_xe(tile);
> +	u64 offset;
> +
> +	xe_assert(xe, (pfn << PAGE_SHIFT) >= mr->hpa_base);
> +	offset = (pfn << PAGE_SHIFT) - mr->hpa_base;
> +	dpa = mr->dpa_base + offset;
> +
> +	return dpa;
> +}
>   
>   int xe_devm_add(struct xe_tile *tile, struct xe_mem_region *mr);
>   void xe_devm_remove(struct xe_tile *tile, struct xe_mem_region *mr);