> Subject: RE: [PATCH 3/5] drm/{i915, xe}/bo: move display bo calls to parent
> interface
>
> > Subject: [PATCH 3/5] drm/{i915, xe}/bo: move display bo calls to
> > parent interface
> >
> > Continue i915 and xe separation from display by moving the bo calls to
> > the display parent interface. Instead of adding all these functions to
> > intel_parent.[ch], reuse the now vacated intel_bo.[ch], and avoid mass
> > renames to calls of these functions. This is similar to
> > intel_display_rpm.[ch].
> >
> > Make many of the hooks optional to avoid having to implement dummy
> > functions in xe. Indeed now we can remove many of the existing dummy
> > functions.
> >
> > Signed-off-by: Jani Nikula <[email protected]>
> > ---
> > drivers/gpu/drm/i915/Makefile | 1 +
> > drivers/gpu/drm/i915/display/intel_bo.c | 66 ++++++++++++++++++++
> > drivers/gpu/drm/i915/i915_bo.c | 32 +++++++---
> > drivers/gpu/drm/i915/i915_bo.h | 9 +++
> > drivers/gpu/drm/i915/i915_driver.c | 2 +
> > drivers/gpu/drm/xe/Makefile | 1 +
> > drivers/gpu/drm/xe/display/xe_display.c | 2 +
> > drivers/gpu/drm/xe/display/xe_display_bo.c | 45 +++----------
> > drivers/gpu/drm/xe/display/xe_display_bo.h | 9 +++
> > include/drm/intel/display_parent_interface.h | 16 +++++
> > 10 files changed, 138 insertions(+), 45 deletions(-) create mode
> > 100644 drivers/gpu/drm/i915/display/intel_bo.c
> > create mode 100644 drivers/gpu/drm/i915/i915_bo.h create mode 100644
> > drivers/gpu/drm/xe/display/xe_display_bo.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile index 52a82608b8b1..425933fb26a5
> > 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -240,6 +240,7 @@ i915-y += \
> > display/intel_atomic.o \
> > display/intel_audio.o \
> > display/intel_bios.o \
> > + display/intel_bo.o \
> > display/intel_bw.o \
> > display/intel_casf.o \
> > display/intel_cdclk.o \
> > diff --git a/drivers/gpu/drm/i915/display/intel_bo.c
> > b/drivers/gpu/drm/i915/display/intel_bo.c
> > new file mode 100644
> > index 000000000000..e356ab4e0640
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_bo.c
> > @@ -0,0 +1,66 @@
> > +// SPDX-License-Identifier: MIT
> > +/* Copyright © 2026 Intel Corporation */
> > +
> > +#include <drm/drm_gem.h>
> > +#include <drm/intel/display_parent_interface.h>
> > +
> > +#include "intel_bo.h"
> > +#include "intel_display_core.h"
> > +#include "intel_display_types.h"
> > +
> > +bool intel_bo_is_tiled(struct drm_gem_object *obj) {
> > + struct intel_display *display = to_intel_display(obj->dev);
> > +
> > + return display->parent->bo->is_tiled &&
> > +display->parent->bo->is_tiled(obj);
> > +}
> > +
> > +bool intel_bo_is_userptr(struct drm_gem_object *obj) {
> > + struct intel_display *display = to_intel_display(obj->dev);
> > +
> > + return display->parent->bo->is_userptr &&
> > +display->parent->bo->is_userptr(obj);
> > +}
> > +
> > +bool intel_bo_is_shmem(struct drm_gem_object *obj) {
> > + struct intel_display *display = to_intel_display(obj->dev);
> > +
> > + return display->parent->bo->is_shmem &&
> > +display->parent->bo->is_shmem(obj);
> > +}
> > +
> > +bool intel_bo_is_protected(struct drm_gem_object *obj) {
> > + struct intel_display *display = to_intel_display(obj->dev);
> > +
> > + return display->parent->bo->is_protected(obj);
> > +}
> > +
> > +int intel_bo_key_check(struct drm_gem_object *obj) {
> > + struct intel_display *display = to_intel_display(obj->dev);
> > +
> > + return display->parent->bo->key_check(obj);
> > +}
> > +
> > +int intel_bo_fb_mmap(struct drm_gem_object *obj, struct
> > +vm_area_struct
> > +*vma) {
> > + struct intel_display *display = to_intel_display(obj->dev);
> > +
> > + return display->parent->bo->fb_mmap(obj, vma); }
> > +
> > +int intel_bo_read_from_page(struct drm_gem_object *obj, u64 offset,
> > +void *dst, int size) {
> > + struct intel_display *display = to_intel_display(obj->dev);
> > +
> > + return display->parent->bo->read_from_page(obj, offset, dst, size);
> > +}
> > +
> > +void intel_bo_describe(struct seq_file *m, struct drm_gem_object
> > +*obj) {
> > + struct intel_display *display = to_intel_display(obj->dev);
> > +
> > + if (display->parent->bo->describe)
> > + display->parent->bo->describe(m, obj);
>
> Nit: Why not follow the same way of making this optional the way you have
> done on top Would look consistent.
> return display->parent->bo->describe && display->parent->bo-
> >describe(m, obj);
Ahh nevermind, I missed that it was a void function. In that case it make
perfect sense.
Regards,
Suraj Kandpal
>
> Otherwise LGTM,
> Reviewed-by: Suraj Kandpal <[email protected]>
>
> > diff --git a/drivers/gpu/drm/i915/i915_bo.c
> > b/drivers/gpu/drm/i915/i915_bo.c index 21a4533ba341..04fc0e3b7ef6
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_bo.c
> > +++ b/drivers/gpu/drm/i915/i915_bo.c
> > @@ -2,51 +2,63 @@
> > /* Copyright © 2024 Intel Corporation */
> >
> > #include <drm/drm_panic.h>
> > -
> > -#include "display/intel_bo.h"
> > +#include <drm/intel/display_parent_interface.h>
> >
> > #include "gem/i915_gem_mman.h"
> > #include "gem/i915_gem_object.h"
> > #include "gem/i915_gem_object_frontbuffer.h"
> > #include "pxp/intel_pxp.h"
> > +
> > +#include "i915_bo.h"
> > #include "i915_debugfs.h"
> >
> > -bool intel_bo_is_tiled(struct drm_gem_object *obj)
> > +static bool i915_bo_is_tiled(struct drm_gem_object *obj)
> > {
> > return i915_gem_object_is_tiled(to_intel_bo(obj));
> > }
> >
> > -bool intel_bo_is_userptr(struct drm_gem_object *obj)
> > +static bool i915_bo_is_userptr(struct drm_gem_object *obj)
> > {
> > return i915_gem_object_is_userptr(to_intel_bo(obj));
> > }
> >
> > -bool intel_bo_is_shmem(struct drm_gem_object *obj)
> > +static bool i915_bo_is_shmem(struct drm_gem_object *obj)
> > {
> > return i915_gem_object_is_shmem(to_intel_bo(obj));
> > }
> >
> > -bool intel_bo_is_protected(struct drm_gem_object *obj)
> > +static bool i915_bo_is_protected(struct drm_gem_object *obj)
> > {
> > return i915_gem_object_is_protected(to_intel_bo(obj));
> > }
> >
> > -int intel_bo_key_check(struct drm_gem_object *obj)
> > +static int i915_bo_key_check(struct drm_gem_object *obj)
> > {
> > return intel_pxp_key_check(obj, false); }
> >
> > -int intel_bo_fb_mmap(struct drm_gem_object *obj, struct
> > vm_area_struct
> > *vma)
> > +static int i915_bo_fb_mmap(struct drm_gem_object *obj, struct
> > +vm_area_struct *vma)
> > {
> > return i915_gem_fb_mmap(to_intel_bo(obj), vma); }
> >
> > -int intel_bo_read_from_page(struct drm_gem_object *obj, u64 offset,
> > void *dst, int size)
> > +static int i915_bo_read_from_page(struct drm_gem_object *obj, u64
> > +offset, void *dst, int size)
> > {
> > return i915_gem_object_read_from_page(to_intel_bo(obj), offset,
> dst,
> > size); }
> >
> > -void intel_bo_describe(struct seq_file *m, struct drm_gem_object
> > *obj)
> > +static void i915_bo_describe(struct seq_file *m, struct
> > +drm_gem_object
> > +*obj)
> > {
> > i915_debugfs_describe_obj(m, to_intel_bo(obj)); }
> > +
> > +const struct intel_display_bo_interface i915_display_bo_interface = {
> > + .is_tiled = i915_bo_is_tiled,
> > + .is_userptr = i915_bo_is_userptr,
> > + .is_shmem = i915_bo_is_shmem,
> > + .is_protected = i915_bo_is_protected,
> > + .key_check = i915_bo_key_check,
> > + .fb_mmap = i915_bo_fb_mmap,
> > + .read_from_page = i915_bo_read_from_page,
> > + .describe = i915_bo_describe,
> > +};
> > diff --git a/drivers/gpu/drm/i915/i915_bo.h
> > b/drivers/gpu/drm/i915/i915_bo.h new file mode 100644 index
> > 000000000000..57255d052dd9
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/i915_bo.h
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/* Copyright © 2026 Intel Corporation */
> > +
> > +#ifndef __I915_BO_H__
> > +#define __I915_BO_H__
> > +
> > +extern const struct intel_display_bo_interface
> > +i915_display_bo_interface;
> > +
> > +#endif /* __I915_BO_H__ */
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > b/drivers/gpu/drm/i915/i915_driver.c
> > index 7a8c59a8c865..385a634c3ed0 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -90,6 +90,7 @@
> > #include "pxp/intel_pxp_debugfs.h"
> > #include "pxp/intel_pxp_pm.h"
> >
> > +#include "i915_bo.h"
> > #include "i915_debugfs.h"
> > #include "i915_display_pc8.h"
> > #include "i915_dpt.h"
> > @@ -765,6 +766,7 @@ static bool vgpu_active(struct drm_device *drm) }
> >
> > static const struct intel_display_parent_interface parent = {
> > + .bo = &i915_display_bo_interface,
> > .dpt = &i915_display_dpt_interface,
> > .dsb = &i915_display_dsb_interface,
> > .frontbuffer = &i915_display_frontbuffer_interface,
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index aeede4423680..b16ed1ce2a85 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -235,6 +235,7 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
> > i915-display/intel_audio.o \
> > i915-display/intel_backlight.o \
> > i915-display/intel_bios.o \
> > + i915-display/intel_bo.o \
> > i915-display/intel_bw.o \
> > i915-display/intel_casf.o \
> > i915-display/intel_cdclk.o \
> > diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> > b/drivers/gpu/drm/xe/display/xe_display.c
> > index f1e1889a52d3..49b6f98e7391 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > @@ -35,6 +35,7 @@
> > #include "intel_hotplug.h"
> > #include "intel_opregion.h"
> > #include "skl_watermark.h"
> > +#include "xe_display_bo.h"
> > #include "xe_display_pcode.h"
> > #include "xe_display_rpm.h"
> > #include "xe_dsb_buffer.h"
> > @@ -541,6 +542,7 @@ static const struct intel_display_irq_interface
> > xe_display_irq_interface = { };
> >
> > static const struct intel_display_parent_interface parent = {
> > + .bo = &xe_display_bo_interface,
> > .dsb = &xe_display_dsb_interface,
> > .frontbuffer = &xe_display_frontbuffer_interface,
> > .hdcp = &xe_display_hdcp_interface,
> > diff --git a/drivers/gpu/drm/xe/display/xe_display_bo.c
> > b/drivers/gpu/drm/xe/display/xe_display_bo.c
> > index fa1f2c796b81..a53ba3f247ec 100644
> > --- a/drivers/gpu/drm/xe/display/xe_display_bo.c
> > +++ b/drivers/gpu/drm/xe/display/xe_display_bo.c
> > @@ -2,52 +2,27 @@
> > /* Copyright © 2024 Intel Corporation */
> >
> > #include <drm/drm_gem.h>
> > +#include <drm/intel/display_parent_interface.h>
> >
> > -#include "intel_bo.h"
> > -#include "intel_frontbuffer.h"
> > #include "xe_bo.h"
> > +#include "xe_display_bo.h"
> > #include "xe_pxp.h"
> >
> > -bool intel_bo_is_tiled(struct drm_gem_object *obj) -{
> > - /* legacy tiling is unused */
> > - return false;
> > -}
> > -
> > -bool intel_bo_is_userptr(struct drm_gem_object *obj) -{
> > - /* xe does not have userptr bos */
> > - return false;
> > -}
> > -
> > -bool intel_bo_is_shmem(struct drm_gem_object *obj) -{
> > - return false;
> > -}
> > -
> > -bool intel_bo_is_protected(struct drm_gem_object *obj)
> > +static bool xe_display_bo_is_protected(struct drm_gem_object *obj)
> > {
> > return xe_bo_is_protected(gem_to_xe_bo(obj));
> > }
> >
> > -int intel_bo_key_check(struct drm_gem_object *obj) -{
> > - return xe_pxp_obj_key_check(obj);
> > -}
> > -
> > -int intel_bo_fb_mmap(struct drm_gem_object *obj, struct
> > vm_area_struct
> > *vma) -{
> > - return drm_gem_prime_mmap(obj, vma);
> > -}
> > -
> > -int intel_bo_read_from_page(struct drm_gem_object *obj, u64 offset,
> > void *dst, int size)
> > +static int xe_display_bo_read_from_page(struct drm_gem_object *obj,
> > +u64 offset, void *dst, int size)
> > {
> > struct xe_bo *bo = gem_to_xe_bo(obj);
> >
> > return xe_bo_read(bo, offset, dst, size); }
> >
> > -void intel_bo_describe(struct seq_file *m, struct drm_gem_object *obj) -{
> > - /* FIXME */
> > -}
> > +const struct intel_display_bo_interface xe_display_bo_interface = {
> > + .is_protected = xe_display_bo_is_protected,
> > + .key_check = xe_pxp_obj_key_check,
> > + .fb_mmap = drm_gem_prime_mmap,
> > + .read_from_page = xe_display_bo_read_from_page, };
> > diff --git a/drivers/gpu/drm/xe/display/xe_display_bo.h
> > b/drivers/gpu/drm/xe/display/xe_display_bo.h
> > new file mode 100644
> > index 000000000000..6879c104b0b1
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/display/xe_display_bo.h
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/* Copyright © 2026 Intel Corporation */
> > +
> > +#ifndef __XE_DISPLAY_BO_H__
> > +#define __XE_DISPLAY_BO_H__
> > +
> > +extern const struct intel_display_bo_interface
> > +xe_display_bo_interface;
> > +
> > +#endif
> > diff --git a/include/drm/intel/display_parent_interface.h
> > b/include/drm/intel/display_parent_interface.h
> > index c044472b9400..2b53d12b0e0a 100644
> > --- a/include/drm/intel/display_parent_interface.h
> > +++ b/include/drm/intel/display_parent_interface.h
> > @@ -23,9 +23,22 @@ struct intel_initial_plane_config; struct
> > intel_panic; struct intel_stolen_node; struct ref_tracker;
> > +struct seq_file;
> > +struct vm_area_struct;
> >
> > /* Keep struct definitions sorted */
> >
> > +struct intel_display_bo_interface {
> > + bool (*is_tiled)(struct drm_gem_object *obj); /* Optional */
> > + bool (*is_userptr)(struct drm_gem_object *obj); /* Optional */
> > + bool (*is_shmem)(struct drm_gem_object *obj); /* Optional */
> > + bool (*is_protected)(struct drm_gem_object *obj);
> > + int (*key_check)(struct drm_gem_object *obj);
> > + int (*fb_mmap)(struct drm_gem_object *obj, struct vm_area_struct
> > *vma);
> > + int (*read_from_page)(struct drm_gem_object *obj, u64 offset, void
> > *dst, int size);
> > + void (*describe)(struct seq_file *m, struct drm_gem_object *obj); /*
> > +Optional */ };
> > +
> > struct intel_display_dpt_interface {
> > struct intel_dpt *(*create)(struct drm_gem_object *obj, size_t size);
> > void (*destroy)(struct intel_dpt *dpt); @@ -174,6 +187,9 @@ struct
> > intel_display_vma_interface {
> > * check the optional pointers.
> > */
> > struct intel_display_parent_interface {
> > + /** @bo: BO interface */
> > + const struct intel_display_bo_interface *bo;
> > +
> > /** @dpt: DPT interface. Optional. */
> > const struct intel_display_dpt_interface *dpt;
> >
> > --
> > 2.47.3
