[v2 15/31] drm/xe: Introduce a helper to get dpa from pfn
Matthew Brost
matthew.brost at intel.com
Wed Apr 10 21:35:10 UTC 2024
On Tue, Apr 09, 2024 at 04:17:26PM -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.
>
> 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 at 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"
Hmm, including all these headers is a frowned upon and indicates to me
this likely the wrong location. The new header hopefully is clean with
only forward delc and function defs. I know Xe headers are not great at
this but lets not make this worse than it is.
Maybe should be xe_device.h? Also if we move the entire implementation 1
*.c file it is possible this function be private to that C file too.
> +
> +/**
> + * 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)
I'd change to xe_mem_region_page_to_dpa with a struct page argument
rather than pfn. The pfn then be derived from the page.
I think this better as we will 3 types pfn all with different values /
shifts.
- hmm pfn
- migrate pfn
- linux core pfn
If a migrate pfn or hmm pfn were passed in as argument we'd get the
wrong dpa. I think passing in a page is safer and less bug prone. In my
example if we had a migrate pfn or hmm pfn, we'd use the appropriate
helper to get the struct page.
This also aligns with how a similar AMD helper (svm_migrate_addr, [1]) is
implemented.
Matt
[1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c#L234
> +{
> + u64 dpa;
> + struct xe_tile *tile = xe_mem_region_to_tile(mr);
> + 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);
> --
> 2.26.3
>
More information about the Intel-xe
mailing list