> 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);

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

Reply via email to