> -----Original Message-----
> From: Intel-xe <[email protected]> On Behalf Of Jani
> Nikula
> Sent: Wednesday, November 26, 2025 4:41 PM
> To: [email protected]; [email protected]
> Cc: Nikula, Jani <[email protected]>; [email protected]
> Subject: [RESEND 2/4] drm/{i915, xe}/dsb: allocate struct intel_dsb_buffer
> dynamically
>
> Prepare for hiding the struct intel_dsb_buffer implementation details from
> the generic DSB code.
>
> Signed-off-by: Jani Nikula <[email protected]>
LGTM.
Reviewed-by: Animesh Manna <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++---------
> .../gpu/drm/i915/display/intel_dsb_buffer.c | 34 +++++++++++----
> .../gpu/drm/i915/display/intel_dsb_buffer.h | 3 +-
> drivers/gpu/drm/xe/display/xe_dsb_buffer.c | 28 ++++++++++---
> 4 files changed, 72 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 6d546f9ff316..ec2a3fb171ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -26,7 +26,7 @@
> struct intel_dsb {
> enum intel_dsb_id id;
>
> - struct intel_dsb_buffer dsb_buf;
> + struct intel_dsb_buffer *dsb_buf;
> struct intel_crtc *crtc;
>
> /*
> @@ -211,10 +211,10 @@ static void intel_dsb_dump(struct intel_dsb *dsb)
> for (i = 0; i < ALIGN(dsb->free_pos, 64 / 4); i += 4)
> drm_dbg_kms(display->drm,
> " 0x%08x: 0x%08x 0x%08x 0x%08x 0x%08x\n", i * 4,
> - intel_dsb_buffer_read(&dsb->dsb_buf, i),
> - intel_dsb_buffer_read(&dsb->dsb_buf, i + 1),
> - intel_dsb_buffer_read(&dsb->dsb_buf, i + 2),
> - intel_dsb_buffer_read(&dsb->dsb_buf, i + 3));
> + intel_dsb_buffer_read(dsb->dsb_buf, i),
> + intel_dsb_buffer_read(dsb->dsb_buf, i + 1),
> + intel_dsb_buffer_read(dsb->dsb_buf, i + 2),
> + intel_dsb_buffer_read(dsb->dsb_buf, i + 3));
> drm_dbg_kms(display->drm, "}\n");
> }
>
> @@ -231,12 +231,12 @@ unsigned int intel_dsb_size(struct intel_dsb *dsb)
>
> unsigned int intel_dsb_head(struct intel_dsb *dsb) {
> - return intel_dsb_buffer_ggtt_offset(&dsb->dsb_buf);
> + return intel_dsb_buffer_ggtt_offset(dsb->dsb_buf);
> }
>
> static unsigned int intel_dsb_tail(struct intel_dsb *dsb) {
> - return intel_dsb_buffer_ggtt_offset(&dsb->dsb_buf) +
> intel_dsb_size(dsb);
> + return intel_dsb_buffer_ggtt_offset(dsb->dsb_buf) +
> +intel_dsb_size(dsb);
> }
>
> static void intel_dsb_ins_align(struct intel_dsb *dsb) @@ -263,8 +263,8 @@
> static void intel_dsb_emit(struct intel_dsb *dsb, u32 ldw, u32 udw)
> dsb->ins[0] = ldw;
> dsb->ins[1] = udw;
>
> - intel_dsb_buffer_write(&dsb->dsb_buf, dsb->free_pos++, dsb-
> >ins[0]);
> - intel_dsb_buffer_write(&dsb->dsb_buf, dsb->free_pos++, dsb-
> >ins[1]);
> + intel_dsb_buffer_write(dsb->dsb_buf, dsb->free_pos++, dsb-
> >ins[0]);
> + intel_dsb_buffer_write(dsb->dsb_buf, dsb->free_pos++, dsb-
> >ins[1]);
> }
>
> static bool intel_dsb_prev_ins_is_write(struct intel_dsb *dsb, @@ -335,13
> +335,13 @@ void intel_dsb_reg_write_indexed(struct intel_dsb *dsb,
>
> /* Update the count */
> dsb->ins[0]++;
> - intel_dsb_buffer_write(&dsb->dsb_buf, dsb->ins_start_offset + 0,
> + intel_dsb_buffer_write(dsb->dsb_buf, dsb->ins_start_offset + 0,
> dsb->ins[0]);
>
> - intel_dsb_buffer_write(&dsb->dsb_buf, dsb->free_pos++, val);
> + intel_dsb_buffer_write(dsb->dsb_buf, dsb->free_pos++, val);
> /* if number of data words is odd, then the last dword should be
> 0.*/
> if (dsb->free_pos & 0x1)
> - intel_dsb_buffer_write(&dsb->dsb_buf, dsb->free_pos, 0);
> + intel_dsb_buffer_write(dsb->dsb_buf, dsb->free_pos, 0);
> }
>
> void intel_dsb_reg_write(struct intel_dsb *dsb, @@ -521,7 +521,7 @@ static
> void intel_dsb_align_tail(struct intel_dsb *dsb)
> aligned_tail = ALIGN(tail, CACHELINE_BYTES);
>
> if (aligned_tail > tail)
> - intel_dsb_buffer_memset(&dsb->dsb_buf, dsb->free_pos, 0,
> + intel_dsb_buffer_memset(dsb->dsb_buf, dsb->free_pos, 0,
> aligned_tail - tail);
>
> dsb->free_pos = aligned_tail / 4;
> @@ -541,7 +541,7 @@ static void intel_dsb_gosub_align(struct intel_dsb
> *dsb)
> * "Ensure GOSUB is not placed in cacheline QW slot 6 or 7
> (numbered 0-7)"
> */
> if (aligned_tail - tail <= 2 * 8)
> - intel_dsb_buffer_memset(&dsb->dsb_buf, dsb->free_pos, 0,
> + intel_dsb_buffer_memset(dsb->dsb_buf, dsb->free_pos, 0,
> aligned_tail - tail);
>
> dsb->free_pos = aligned_tail / 4;
> @@ -606,14 +606,14 @@ void intel_dsb_gosub_finish(struct intel_dsb *dsb)
> */
> intel_dsb_noop(dsb, 8);
>
> - intel_dsb_buffer_flush_map(&dsb->dsb_buf);
> + intel_dsb_buffer_flush_map(dsb->dsb_buf);
> }
>
> void intel_dsb_finish(struct intel_dsb *dsb) {
> intel_dsb_align_tail(dsb);
>
> - intel_dsb_buffer_flush_map(&dsb->dsb_buf);
> + intel_dsb_buffer_flush_map(dsb->dsb_buf);
> }
>
> static u32 dsb_error_int_status(struct intel_display *display) @@ -888,7
> +888,7 @@ void intel_dsb_wait(struct intel_dsb *dsb)
> !is_busy,
> 100, 1000, false);
> if (ret) {
> - u32 offset = intel_dsb_buffer_ggtt_offset(&dsb->dsb_buf);
> + u32 offset = intel_dsb_buffer_ggtt_offset(dsb->dsb_buf);
>
> intel_de_write_fw(display, DSB_CTRL(pipe, dsb->id),
> DSB_ENABLE | DSB_HALT);
> @@ -934,6 +934,7 @@ struct intel_dsb *intel_dsb_prepare(struct
> intel_atomic_state *state,
> unsigned int max_cmds)
> {
> struct intel_display *display = to_intel_display(state);
> + struct intel_dsb_buffer *dsb_buf;
> struct ref_tracker *wakeref;
> struct intel_dsb *dsb;
> unsigned int size;
> @@ -953,9 +954,12 @@ struct intel_dsb *intel_dsb_prepare(struct
> intel_atomic_state *state,
> /* ~1 qword per instruction, full cachelines */
> size = ALIGN(max_cmds * 8, CACHELINE_BYTES);
>
> - if (!intel_dsb_buffer_create(display->drm, &dsb->dsb_buf, size))
> + dsb_buf = intel_dsb_buffer_create(display->drm, size);
> + if (IS_ERR(dsb_buf))
> goto out_put_rpm;
>
> + dsb->dsb_buf = dsb_buf;
> +
> intel_display_rpm_put(display, wakeref);
>
> dsb->id = dsb_id;
> @@ -988,7 +992,7 @@ struct intel_dsb *intel_dsb_prepare(struct
> intel_atomic_state *state,
> */
> void intel_dsb_cleanup(struct intel_dsb *dsb) {
> - intel_dsb_buffer_cleanup(&dsb->dsb_buf);
> + intel_dsb_buffer_cleanup(dsb->dsb_buf);
> kfree(dsb);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb_buffer.c
> b/drivers/gpu/drm/i915/display/intel_dsb_buffer.c
> index 1eafcb2dedcb..fc1f0e6031ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb_buffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb_buffer.c
> @@ -31,48 +31,66 @@ void intel_dsb_buffer_memset(struct
> intel_dsb_buffer *dsb_buf, u32 idx, u32 val,
> memset(&dsb_buf->cmd_buf[idx], val, size); }
>
> -bool intel_dsb_buffer_create(struct drm_device *drm, struct
> intel_dsb_buffer *dsb_buf, size_t size)
> +struct intel_dsb_buffer *intel_dsb_buffer_create(struct drm_device
> +*drm, size_t size)
> {
> struct drm_i915_private *i915 = to_i915(drm);
> + struct intel_dsb_buffer *dsb_buf;
> struct drm_i915_gem_object *obj;
> struct i915_vma *vma;
> u32 *buf;
> + int ret;
> +
> + dsb_buf = kzalloc(sizeof(*dsb_buf), GFP_KERNEL);
> + if (!dsb_buf)
> + return ERR_PTR(-ENOMEM);
>
> if (HAS_LMEM(i915)) {
> obj = i915_gem_object_create_lmem(i915,
> PAGE_ALIGN(size),
>
> I915_BO_ALLOC_CONTIGUOUS);
> - if (IS_ERR(obj))
> - return false;
> + if (IS_ERR(obj)) {
> + ret = PTR_ERR(obj);
> + goto err;
> + }
> } else {
> obj = i915_gem_object_create_internal(i915,
> PAGE_ALIGN(size));
> - if (IS_ERR(obj))
> - return false;
> + if (IS_ERR(obj)) {
> + ret = PTR_ERR(obj);
> + goto err;
> + }
>
> i915_gem_object_set_cache_coherency(obj,
> I915_CACHE_NONE);
> }
>
> vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
> if (IS_ERR(vma)) {
> + ret = PTR_ERR(vma);
> i915_gem_object_put(obj);
> - return false;
> + goto err;
> }
>
> buf = i915_gem_object_pin_map_unlocked(vma->obj,
> I915_MAP_WC);
> if (IS_ERR(buf)) {
> + ret = PTR_ERR(buf);
> i915_vma_unpin_and_release(&vma,
> I915_VMA_RELEASE_MAP);
> - return false;
> + goto err;
> }
>
> dsb_buf->vma = vma;
> dsb_buf->cmd_buf = buf;
> dsb_buf->buf_size = size;
>
> - return true;
> + return dsb_buf;
> +
> +err:
> + kfree(dsb_buf);
> +
> + return ERR_PTR(ret);
> }
>
> void intel_dsb_buffer_cleanup(struct intel_dsb_buffer *dsb_buf) {
> i915_vma_unpin_and_release(&dsb_buf->vma,
> I915_VMA_RELEASE_MAP);
> + kfree(dsb_buf);
> }
>
> void intel_dsb_buffer_flush_map(struct intel_dsb_buffer *dsb_buf) diff --git
> a/drivers/gpu/drm/i915/display/intel_dsb_buffer.h
> b/drivers/gpu/drm/i915/display/intel_dsb_buffer.h
> index 496ece42b4a1..2cf639fae47a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb_buffer.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb_buffer.h
> @@ -21,8 +21,7 @@ u32 intel_dsb_buffer_ggtt_offset(struct
> intel_dsb_buffer *dsb_buf); void intel_dsb_buffer_write(struct
> intel_dsb_buffer *dsb_buf, u32 idx, u32 val);
> u32 intel_dsb_buffer_read(struct intel_dsb_buffer *dsb_buf, u32 idx); void
> intel_dsb_buffer_memset(struct intel_dsb_buffer *dsb_buf, u32 idx, u32 val,
> size_t size); -bool intel_dsb_buffer_create(struct drm_device *drm, struct
> intel_dsb_buffer *dsb_buf,
> - size_t size);
> +struct intel_dsb_buffer *intel_dsb_buffer_create(struct drm_device
> +*drm, size_t size);
> void intel_dsb_buffer_cleanup(struct intel_dsb_buffer *dsb_buf); void
> intel_dsb_buffer_flush_map(struct intel_dsb_buffer *dsb_buf);
>
> diff --git a/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
> b/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
> index 1bbebc0313c8..ee7717b1980f 100644
> --- a/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
> +++ b/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
> @@ -31,15 +31,23 @@ void intel_dsb_buffer_memset(struct
> intel_dsb_buffer *dsb_buf, u32 idx, u32 val,
> iosys_map_memset(&dsb_buf->vma->bo->vmap, idx * 4, val, size); }
>
> -bool intel_dsb_buffer_create(struct drm_device *drm, struct
> intel_dsb_buffer *dsb_buf, size_t size)
> +struct intel_dsb_buffer *intel_dsb_buffer_create(struct drm_device
> +*drm, size_t size)
> {
> struct xe_device *xe = to_xe_device(drm);
> + struct intel_dsb_buffer *dsb_buf;
> struct xe_bo *obj;
> struct i915_vma *vma;
> + int ret;
> +
> + dsb_buf = kzalloc(sizeof(*dsb_buf), GFP_KERNEL);
> + if (!dsb_buf)
> + return ERR_PTR(-ENOMEM);
>
> vma = kzalloc(sizeof(*vma), GFP_KERNEL);
> - if (!vma)
> - return false;
> + if (!vma) {
> + ret = -ENOMEM;
> + goto err_vma;
> + }
>
> /* Set scanout flag for WC mapping */
> obj = xe_bo_create_pin_map_novm(xe, xe_device_get_root_tile(xe),
> @@ -48,21 +56,29 @@ bool intel_dsb_buffer_create(struct drm_device
> *drm, struct intel_dsb_buffer *ds
>
> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) |
> XE_BO_FLAG_SCANOUT |
> XE_BO_FLAG_GGTT, false);
> if (IS_ERR(obj)) {
> - kfree(vma);
> - return false;
> + ret = PTR_ERR(obj);
> + goto err_pin_map;
> }
>
> vma->bo = obj;
> dsb_buf->vma = vma;
> dsb_buf->buf_size = size;
>
> - return true;
> + return dsb_buf;
> +
> +err_pin_map:
> + kfree(vma);
> +err_vma:
> + kfree(dsb_buf);
> +
> + return ERR_PTR(ret);
> }
>
> void intel_dsb_buffer_cleanup(struct intel_dsb_buffer *dsb_buf) {
> xe_bo_unpin_map_no_vm(dsb_buf->vma->bo);
> kfree(dsb_buf->vma);
> + kfree(dsb_buf);
> }
>
> void intel_dsb_buffer_flush_map(struct intel_dsb_buffer *dsb_buf)
> --
> 2.47.3
