[PATCH 1/2] drm/xe: add interface to request physical alignment for buffer objects
Cavitt, Jonathan
jonathan.cavitt at intel.com
Wed Oct 9 17:11:11 UTC 2024
-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Juha-Pekka Heikkila
Sent: Wednesday, October 9, 2024 8:20 AM
To: intel-xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
Subject: [PATCH 1/2] drm/xe: add interface to request physical alignment for buffer objects
>
> Add xe_bo_create_pin_map_at_aligned() which augment
> xe_bo_create_pin_map_at() with alignment parameter allowing to pass
> required alignemnt if it differ from default.
>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
Some nits below, but otherwise
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> ---
> .../compat-i915-headers/gem/i915_gem_stolen.h | 2 +-
> drivers/gpu/drm/xe/xe_bo.c | 29 +++++++++++++++----
> drivers/gpu/drm/xe/xe_bo.h | 8 ++++-
> drivers/gpu/drm/xe/xe_bo_types.h | 5 ++++
> drivers/gpu/drm/xe/xe_ggtt.c | 2 +-
> 5 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_stolen.h b/drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_stolen.h
> index cb6c7598824b..9c4cf050059a 100644
> --- a/drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_stolen.h
> +++ b/drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_stolen.h
> @@ -29,7 +29,7 @@ static inline int i915_gem_stolen_insert_node_in_range(struct xe_device *xe,
>
> bo = xe_bo_create_locked_range(xe, xe_device_get_root_tile(xe),
> NULL, size, start, end,
> - ttm_bo_type_kernel, flags);
> + ttm_bo_type_kernel, flags, 0);
> if (IS_ERR(bo)) {
> err = PTR_ERR(bo);
> bo = NULL;
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 5e8f60a8d431..d5d30a0ff1e7 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1454,7 +1454,8 @@ static struct xe_bo *
> __xe_bo_create_locked(struct xe_device *xe,
> struct xe_tile *tile, struct xe_vm *vm,
> size_t size, u64 start, u64 end,
> - u16 cpu_caching, enum ttm_bo_type type, u32 flags)
> + u16 cpu_caching, enum ttm_bo_type type, u32 flags,
> + u64 alignment)
> {
> struct xe_bo *bo = NULL;
> int err;
> @@ -1483,6 +1484,8 @@ __xe_bo_create_locked(struct xe_device *xe,
> if (IS_ERR(bo))
> return bo;
>
> + bo->min_align = alignment;
> +
> /*
> * Note that instead of taking a reference no the drm_gpuvm_resv_bo(),
> * to ensure the shared resv doesn't disappear under the bo, the bo
> @@ -1523,16 +1526,18 @@ struct xe_bo *
> xe_bo_create_locked_range(struct xe_device *xe,
> struct xe_tile *tile, struct xe_vm *vm,
> size_t size, u64 start, u64 end,
> - enum ttm_bo_type type, u32 flags)
> + enum ttm_bo_type type, u32 flags, u64 alignment)
> {
> - return __xe_bo_create_locked(xe, tile, vm, size, start, end, 0, type, flags);
> + return __xe_bo_create_locked(xe, tile, vm, size, start, end, 0, type,
> + flags, alignment);
> }
>
> struct xe_bo *xe_bo_create_locked(struct xe_device *xe, struct xe_tile *tile,
> struct xe_vm *vm, size_t size,
> enum ttm_bo_type type, u32 flags)
> {
> - return __xe_bo_create_locked(xe, tile, vm, size, 0, ~0ULL, 0, type, flags);
> + return __xe_bo_create_locked(xe, tile, vm, size, 0, ~0ULL, 0, type,
> + flags, 0);
> }
>
> struct xe_bo *xe_bo_create_user(struct xe_device *xe, struct xe_tile *tile,
> @@ -1542,7 +1547,7 @@ struct xe_bo *xe_bo_create_user(struct xe_device *xe, struct xe_tile *tile,
> {
> struct xe_bo *bo = __xe_bo_create_locked(xe, tile, vm, size, 0, ~0ULL,
> cpu_caching, ttm_bo_type_device,
> - flags | XE_BO_FLAG_USER);
> + flags | XE_BO_FLAG_USER, 0);
> if (!IS_ERR(bo))
> xe_bo_unlock_vm_held(bo);
>
> @@ -1565,6 +1570,17 @@ struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_tile *tile
> struct xe_vm *vm,
> size_t size, u64 offset,
> enum ttm_bo_type type, u32 flags)
> +{
> + return xe_bo_create_pin_map_at_aligned(xe, tile, vm, size, offset,
> + type, flags, 0);
> +}
> +
> +struct xe_bo *xe_bo_create_pin_map_at_aligned(struct xe_device *xe,
> + struct xe_tile *tile,
> + struct xe_vm *vm,
> + size_t size, u64 offset,
> + enum ttm_bo_type type, u32 flags,
> + u64 alignment)
>
Nit:
It would be debatably better to define xe_bo_create_pin_map_at_aligned
above xe_bo_create_pin_map_at, as the latter calls the former, but doing
so is not strictly necessary.
{
> struct xe_bo *bo;
> int err;
> @@ -1576,7 +1592,8 @@ struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_tile *tile
> flags |= XE_BO_FLAG_GGTT;
>
> bo = xe_bo_create_locked_range(xe, tile, vm, size, start, end, type,
> - flags | XE_BO_FLAG_NEEDS_CPU_ACCESS);
> + flags | XE_BO_FLAG_NEEDS_CPU_ACCESS,
> + alignment);
> if (IS_ERR(bo))
> return bo;
>
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 31f4ba3bd8c1..7fa44a0138b0 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -77,7 +77,7 @@ struct xe_bo *
> xe_bo_create_locked_range(struct xe_device *xe,
> struct xe_tile *tile, struct xe_vm *vm,
> size_t size, u64 start, u64 end,
> - enum ttm_bo_type type, u32 flags);
> + enum ttm_bo_type type, u32 flags, u64 alignment);
> struct xe_bo *xe_bo_create_locked(struct xe_device *xe, struct xe_tile *tile,
> struct xe_vm *vm, size_t size,
> enum ttm_bo_type type, u32 flags);
> @@ -94,6 +94,12 @@ struct xe_bo *xe_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
> struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_tile *tile,
> struct xe_vm *vm, size_t size, u64 offset,
> enum ttm_bo_type type, u32 flags);
> +struct xe_bo *xe_bo_create_pin_map_at_aligned(struct xe_device *xe,
> + struct xe_tile *tile,
> + struct xe_vm *vm,
> + size_t size, u64 offset,
> + enum ttm_bo_type type, u32 flags,
> + u64 alignment);
> struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> const void *data, size_t size,
> enum ttm_bo_type type, u32 flags);
> diff --git a/drivers/gpu/drm/xe/xe_bo_types.h b/drivers/gpu/drm/xe/xe_bo_types.h
> index 8b9201775081..13c6d8a69e91 100644
> --- a/drivers/gpu/drm/xe/xe_bo_types.h
> +++ b/drivers/gpu/drm/xe/xe_bo_types.h
> @@ -76,6 +76,11 @@ struct xe_bo {
>
> /** @vram_userfault_link: Link into @mem_access.vram_userfault.list */
> struct list_head vram_userfault_link;
> +
> + /** @min_align: minimum alignment needed for this BO if different
> + * from default
> + */
> + u64 min_align;
> };
>
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 47bfd9d2635d..1b3178226987 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -603,7 +603,7 @@ static int __xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
> u64 start, u64 end)
> {
> int err;
> - u64 alignment = XE_PAGE_SIZE;
> + u64 alignment = bo->min_align > 0 ? bo->min_align : XE_PAGE_SIZE;
Nit:
I don't think there'd ever be a case where bo->min_align < 0, so you might
be able to simplify this ternary operation into:
"""
u64 alignment = bo->min_align ?: XE_PAGE_SIZE;
"""
Note that this shortcut returns bo->min_align if it's not 0, and XE_PAGE_SIZE
otherwise.
This is just a matter of preference, though, so either works.
-Jonathan Cavitt
>
> if (xe_bo_is_vram(bo) && ggtt->flags & XE_GGTT_FLAGS_64K)
> alignment = SZ_64K;
> --
> 2.45.2
>
>
More information about the Intel-gfx
mailing list