| Message ID | 20241009151947.2240099-2-juhapekka.heikkila@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Align framebuffers according to what display minimum alignment states | expand |
-----Original Message----- From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Juha-Pekka Heikkila Sent: Wednesday, October 9, 2024 8:20 AM To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Cc: Juha-Pekka Heikkila <juhapekka.heikkila@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@gmail.com> Some nits below, but otherwise Reviewed-by: Jonathan Cavitt <jonathan.cavitt@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 > >
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) { 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; if (xe_bo_is_vram(bo) && ggtt->flags & XE_GGTT_FLAGS_64K) alignment = SZ_64K;
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@gmail.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(-)