Re: [PATCH 01/16] drm/i915: Introduce intel_parent_fb_pin_get_map()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 23 Apr 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Introduce the "fb_pin" parent interface, as the first trivial step
> move the *_get_map() stuff there.
>
> The whole "fb_pin" as an interface might not really make sense,
> and perhaps this (and other stuff) should just be collected into
> some kind of "bo" interface. But let's go with "fb_pin" for now
> to match where things are implemented, and possibly restructure
> it later.

Yeah, I've also come to the conclusion that trying to always get things
like this right in advance is somewhat of a waste of time. You'll only
see things clearly once everything is in place.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fb_pin.h  | 1 -
>  drivers/gpu/drm/i915/display/intel_fbdev.c   | 7 +++++--
>  drivers/gpu/drm/i915/display/intel_fbdev.h   | 4 ++--
>  drivers/gpu/drm/i915/display/intel_parent.c  | 7 +++++++
>  drivers/gpu/drm/i915/display/intel_parent.h  | 5 +++++
>  drivers/gpu/drm/i915/display/intel_plane.c   | 4 ++--
>  drivers/gpu/drm/i915/i915_driver.c           | 2 ++
>  drivers/gpu/drm/i915/i915_fb_pin.c           | 8 +++++++-
>  drivers/gpu/drm/i915/i915_fb_pin.h           | 9 +++++++++
>  drivers/gpu/drm/xe/display/xe_display.c      | 2 ++
>  drivers/gpu/drm/xe/display/xe_fb_pin.c       | 8 +++++++-
>  drivers/gpu/drm/xe/display/xe_fb_pin.h       | 9 +++++++++
>  include/drm/intel/display_parent_interface.h | 8 ++++++++
>  13 files changed, 65 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_fb_pin.h
>  create mode 100644 drivers/gpu/drm/xe/display/xe_fb_pin.h
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.h b/drivers/gpu/drm/i915/display/intel_fb_pin.h
> index 5825503c38ea..805e23067004 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.h
> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.h
> @@ -35,6 +35,5 @@ void intel_fb_unpin_vma(struct i915_vma *vma, int fence_id);
>  int intel_plane_pin_fb(struct intel_plane_state *new_plane_state,
>  		       const struct intel_plane_state *old_plane_state);
>  void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state);
> -void intel_fb_get_map(struct i915_vma *vma, struct iosys_map *map);
>  
>  #endif
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 9ab0ac49abb7..ce4082da30e8 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -55,6 +55,7 @@
>  #include "intel_fb_pin.h"
>  #include "intel_fbdev.h"
>  #include "intel_frontbuffer.h"
> +#include "intel_parent.h"
>  #include "intel_plane.h"
>  
>  struct intel_fbdev {
> @@ -563,7 +564,9 @@ struct i915_vma *intel_fbdev_vma_pointer(struct intel_fbdev *fbdev)
>  	return fbdev ? fbdev->vma : NULL;
>  }
>  
> -void intel_fbdev_get_map(struct intel_fbdev *fbdev, struct iosys_map *map)
> +void intel_fbdev_get_map(struct intel_display *display, struct iosys_map *map)
>  {
> -	intel_fb_get_map(fbdev->vma, map);
> +	struct intel_fbdev *fbdev = display->fbdev.fbdev;
> +
> +	intel_parent_fb_pin_get_map(display, fbdev->vma, map);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h
> index 150cc5f45bb3..edaf5d16a456 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
> @@ -23,7 +23,7 @@ int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
>  void intel_fbdev_setup(struct intel_display *display);
>  struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev);
>  struct i915_vma *intel_fbdev_vma_pointer(struct intel_fbdev *fbdev);
> -void intel_fbdev_get_map(struct intel_fbdev *fbdev, struct iosys_map *map);
> +void intel_fbdev_get_map(struct intel_display *display, struct iosys_map *map);
>  #else
>  #define INTEL_FBDEV_DRIVER_OPS \
>  	.fbdev_probe = NULL
> @@ -40,7 +40,7 @@ static inline struct i915_vma *intel_fbdev_vma_pointer(struct intel_fbdev *fbdev
>  	return NULL;
>  }
>  
> -static inline void intel_fbdev_get_map(struct intel_fbdev *fbdev, struct iosys_map *map)
> +static inline void intel_fbdev_get_map(struct intel_display *display, struct iosys_map *map)
>  {
>  }
>  #endif
> diff --git a/drivers/gpu/drm/i915/display/intel_parent.c b/drivers/gpu/drm/i915/display/intel_parent.c
> index 67b21a42d354..070cc60f49b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_parent.c
> +++ b/drivers/gpu/drm/i915/display/intel_parent.c
> @@ -52,6 +52,13 @@ void intel_parent_dpt_resume(struct intel_display *display, struct intel_dpt *dp
>  		display->parent->dpt->resume(dpt);
>  }
>  
> +/* fb_pin */
> +void intel_parent_fb_pin_get_map(struct intel_display *display,
> +				 struct i915_vma *vma, struct iosys_map *map)
> +{
> +	return display->parent->fb_pin->get_map(vma, map);
> +}
> +
>  /* frontbuffer */
>  struct intel_frontbuffer *intel_parent_frontbuffer_get(struct intel_display *display, struct drm_gem_object *obj)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_parent.h b/drivers/gpu/drm/i915/display/intel_parent.h
> index 5f0c835b9416..2b9e829e7f47 100644
> --- a/drivers/gpu/drm/i915/display/intel_parent.h
> +++ b/drivers/gpu/drm/i915/display/intel_parent.h
> @@ -18,6 +18,7 @@ struct intel_frontbuffer;
>  struct intel_hdcp_gsc_context;
>  struct intel_panic;
>  struct intel_stolen_node;
> +struct iosys_map;
>  
>  /* dpt */
>  struct intel_dpt *intel_parent_dpt_create(struct intel_display *display,
> @@ -26,6 +27,10 @@ void intel_parent_dpt_destroy(struct intel_display *display, struct intel_dpt *d
>  void intel_parent_dpt_suspend(struct intel_display *display, struct intel_dpt *dpt);
>  void intel_parent_dpt_resume(struct intel_display *display, struct intel_dpt *dpt);
>  
> +/* fb_pin */
> +void intel_parent_fb_pin_get_map(struct intel_display *display,
> +				 struct i915_vma *vma, struct iosys_map *map);
> +
>  /* frontbuffer */
>  struct intel_frontbuffer *intel_parent_frontbuffer_get(struct intel_display *display, struct drm_gem_object *obj);
>  void intel_parent_frontbuffer_ref(struct intel_display *display, struct intel_frontbuffer *front);
> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
> index c2b58d3b9c23..e50e1a15410a 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane.c
> @@ -1404,7 +1404,7 @@ static void intel_panic_flush(struct drm_plane *_plane)
>  	if (fb == intel_fbdev_framebuffer(display->fbdev.fbdev)) {
>  		struct iosys_map map;
>  
> -		intel_fbdev_get_map(display->fbdev.fbdev, &map);
> +		intel_fbdev_get_map(display, &map);
>  		drm_clflush_virt_range(map.vaddr, fb->base.pitches[0] * fb->base.height);
>  		return;
>  	}
> @@ -1462,7 +1462,7 @@ static int intel_get_scanout_buffer(struct drm_plane *plane,
>  		return -ENODEV;
>  
>  	if (fb == intel_fbdev_framebuffer(display->fbdev.fbdev)) {
> -		intel_fbdev_get_map(display->fbdev.fbdev, &sb->map[0]);
> +		intel_fbdev_get_map(display, &sb->map[0]);
>  	} else {
>  		int ret;
>  		/* Can't disable tiling if DPT is in use */
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index d31819758f3d..c23436be5973 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -99,6 +99,7 @@
>  #include "i915_drv.h"
>  #include "i915_dsb_buffer.h"
>  #include "i915_edram.h"
> +#include "i915_fb_pin.h"
>  #include "i915_file_private.h"
>  #include "i915_getparam.h"
>  #include "i915_gmch.h"
> @@ -769,6 +770,7 @@ static const struct intel_display_parent_interface parent = {
>  	.bo = &i915_display_bo_interface,
>  	.dpt = &i915_display_dpt_interface,
>  	.dsb = &i915_display_dsb_interface,
> +	.fb_pin = &i915_display_fb_pin_interface,
>  	.frontbuffer = &i915_display_frontbuffer_interface,
>  	.hdcp = &i915_display_hdcp_interface,
>  	.initial_plane = &i915_display_initial_plane_interface,
> diff --git a/drivers/gpu/drm/i915/i915_fb_pin.c b/drivers/gpu/drm/i915/i915_fb_pin.c
> index b0e121462ca3..b800b55530a5 100644
> --- a/drivers/gpu/drm/i915/i915_fb_pin.c
> +++ b/drivers/gpu/drm/i915/i915_fb_pin.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include <drm/drm_print.h>
> +#include <drm/intel/display_parent_interface.h>
>  
>  #include "display/intel_display_core.h"
>  #include "display/intel_display_types.h"
> @@ -18,6 +19,7 @@
>  #include "gem/i915_gem_domain.h"
>  #include "gem/i915_gem_object.h"
>  
> +#include "i915_fb_pin.h"
>  #include "i915_dpt.h"
>  #include "i915_drv.h"
>  #include "i915_vma.h"
> @@ -352,7 +354,11 @@ void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
>  	}
>  }
>  
> -void intel_fb_get_map(struct i915_vma *vma, struct iosys_map *map)
> +static void i915_fb_pin_get_map(struct i915_vma *vma, struct iosys_map *map)
>  {
>  	iosys_map_set_vaddr_iomem(map, i915_vma_get_iomap(vma));
>  }
> +
> +const struct intel_display_fb_pin_interface i915_display_fb_pin_interface = {
> +	.get_map = i915_fb_pin_get_map,
> +};
> diff --git a/drivers/gpu/drm/i915/i915_fb_pin.h b/drivers/gpu/drm/i915/i915_fb_pin.h
> new file mode 100644
> index 000000000000..18c6c0b61ab8
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_fb_pin.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: MIT */
> +/* Copyright © 2026 Intel Corporation */
> +
> +#ifndef __I915_FB_PIN_H__
> +#define __I915_FB_PIN_H__
> +
> +extern const struct intel_display_fb_pin_interface i915_display_fb_pin_interface;
> +
> +#endif /* __I915_FB_PIN_H__ */
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 0747044f7c2a..aa73023b7398 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -40,6 +40,7 @@
>  #include "xe_display_pcode.h"
>  #include "xe_display_rpm.h"
>  #include "xe_dsb_buffer.h"
> +#include "xe_fb_pin.h"
>  #include "xe_frontbuffer.h"
>  #include "xe_hdcp_gsc.h"
>  #include "xe_initial_plane.h"
> @@ -553,6 +554,7 @@ static bool has_auxccs(struct drm_device *drm)
>  static const struct intel_display_parent_interface parent = {
>  	.bo = &xe_display_bo_interface,
>  	.dsb = &xe_display_dsb_interface,
> +	.fb_pin = &xe_display_fb_pin_interface,
>  	.frontbuffer = &xe_display_frontbuffer_interface,
>  	.hdcp = &xe_display_hdcp_interface,
>  	.initial_plane = &xe_display_initial_plane_interface,
> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> index 948446346c53..afb6cdda537e 100644
> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> @@ -3,6 +3,7 @@
>   * Copyright © 2021 Intel Corporation
>   */
>  
> +#include <drm/intel/display_parent_interface.h>
>  #include <drm/ttm/ttm_bo.h>
>  
>  #include "intel_display_core.h"
> @@ -13,6 +14,7 @@
>  #include "xe_bo.h"
>  #include "xe_device.h"
>  #include "xe_display_vma.h"
> +#include "xe_fb_pin.h"
>  #include "xe_ggtt.h"
>  #include "xe_pat.h"
>  #include "xe_pm.h"
> @@ -498,7 +500,11 @@ void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
>  	old_plane_state->ggtt_vma = NULL;
>  }
>  
> -void intel_fb_get_map(struct i915_vma *vma, struct iosys_map *map)
> +static void xe_fb_pin_get_map(struct i915_vma *vma, struct iosys_map *map)
>  {
>  	*map = vma->bo->vmap;
>  }
> +
> +const struct intel_display_fb_pin_interface xe_display_fb_pin_interface = {
> +	.get_map = xe_fb_pin_get_map,
> +};
> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.h b/drivers/gpu/drm/xe/display/xe_fb_pin.h
> new file mode 100644
> index 000000000000..8a42d4009f5d
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: MIT */
> +/* Copyright © 2026 Intel Corporation */
> +
> +#ifndef __XE_FB_PIN_H__
> +#define __XE_FB_PIN_H__
> +
> +extern const struct intel_display_fb_pin_interface xe_display_fb_pin_interface;
> +
> +#endif /* __XE_FB_PIN_H__ */
> diff --git a/include/drm/intel/display_parent_interface.h b/include/drm/intel/display_parent_interface.h
> index 270dc028bec2..ee2e9572bfca 100644
> --- a/include/drm/intel/display_parent_interface.h
> +++ b/include/drm/intel/display_parent_interface.h
> @@ -24,6 +24,7 @@ struct intel_hdcp_gsc_context;
>  struct intel_initial_plane_config;
>  struct intel_panic;
>  struct intel_stolen_node;
> +struct iosys_map;
>  struct ref_tracker;
>  struct seq_file;
>  struct vm_area_struct;
> @@ -69,6 +70,10 @@ struct intel_display_dsb_interface {
>  	void (*flush_map)(struct intel_dsb_buffer *dsb_buf);
>  };
>  
> +struct intel_display_fb_pin_interface {
> +	void (*get_map)(struct i915_vma *vma, struct iosys_map *map);
> +};
> +
>  struct intel_display_frontbuffer_interface {
>  	struct intel_frontbuffer *(*get)(struct drm_gem_object *obj);
>  	void (*ref)(struct intel_frontbuffer *front);
> @@ -211,6 +216,9 @@ struct intel_display_parent_interface {
>  	/** @dsb: DSB buffer interface */
>  	const struct intel_display_dsb_interface *dsb;
>  
> +	/** @fb_pin: Framebuffer pin interface */
> +	const struct intel_display_fb_pin_interface *fb_pin;
> +
>  	/** @frontbuffer: Frontbuffer interface */
>  	const struct intel_display_frontbuffer_interface *frontbuffer;

-- 
Jani Nikula, Intel