From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH 13/16] drm/i915: Introduce the main fb_pin parent interface
Date: Fri, 24 Apr 2026 13:55:52 +0300 [thread overview]
Message-ID: <aetMOEagKgSCvVnv@intel.com> (raw)
In-Reply-To: <28d5b5722ab0005103050a70e04b4e6b7ff691de@intel.com>
On Fri, Apr 24, 2026 at 01:48:02PM +0300, Jani Nikula wrote:
> On Thu, 23 Apr 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Introduce the main part of the new fb_pin parent interface:
> > - intel_parent_fb_pin_ggtt_(un)pin()
> > - intel_parent_fb_pin_dpt_(un)pin()
> > - intel_parent_fb_pin_reuse_vma()
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> This does introduce the silly intermediate step where i915 and xe
> specific code calls the generic parent interface. But I'm not sure if
> it's even feasible to do this some other route.
Yeah. I had all in one lump initially, but the result was a mess.
So in the end I figured it's cleaner to separate this non-functional
part out, even if calling the parent interface in the core driver
code is silly.
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> > ---
> > drivers/gpu/drm/i915/display/intel_parent.c | 53 ++++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_parent.h | 29 +++++++++++
> > drivers/gpu/drm/i915/i915_fb_pin.c | 29 +++++++----
> > drivers/gpu/drm/xe/display/xe_fb_pin.c | 40 +++++++++------
> > include/drm/intel/display_parent_interface.h | 22 ++++++++
> > 5 files changed, 148 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_parent.c b/drivers/gpu/drm/i915/display/intel_parent.c
> > index 070cc60f49b3..a5816561be40 100644
> > --- a/drivers/gpu/drm/i915/display/intel_parent.c
> > +++ b/drivers/gpu/drm/i915/display/intel_parent.c
> > @@ -53,6 +53,59 @@ void intel_parent_dpt_resume(struct intel_display *display, struct intel_dpt *dp
> > }
> >
> > /* fb_pin */
> > +int intel_parent_fb_pin_ggtt_pin(struct intel_display *display,
> > + struct drm_gem_object *obj,
> > + const struct intel_fb_pin_params *pin_params,
> > + struct i915_vma **out_ggtt_vma,
> > + u32 *out_offset,
> > + int *out_fence_id)
> > +{
> > + return display->parent->fb_pin->ggtt_pin(obj, pin_params,
> > + out_ggtt_vma, out_offset, out_fence_id);
> > +}
> > +
> > +void intel_parent_fb_pin_ggtt_unpin(struct intel_display *display,
> > + struct i915_vma *ggtt_vma,
> > + int fence_id)
> > +{
> > + return display->parent->fb_pin->ggtt_unpin(ggtt_vma, fence_id);
> > +}
> > +
> > +int intel_parent_fb_pin_dpt_pin(struct intel_display *display,
> > + struct drm_gem_object *obj,
> > + struct intel_dpt *dpt,
> > + const struct intel_fb_pin_params *pin_params,
> > + struct i915_vma **out_dpt_vma,
> > + struct i915_vma **out_ggtt_vma,
> > + u32 *out_offset)
> > +{
> > + return display->parent->fb_pin->dpt_pin(obj, dpt, pin_params,
> > + out_dpt_vma, out_ggtt_vma, out_offset);
> > +}
> > +
> > +void intel_parent_fb_pin_dpt_unpin(struct intel_display *display,
> > + struct intel_dpt *dpt,
> > + struct i915_vma *dpt_vma,
> > + struct i915_vma *ggtt_vma)
> > +{
> > + return display->parent->fb_pin->dpt_unpin(dpt, dpt_vma, ggtt_vma);
> > +}
> > +
> > +struct i915_vma *intel_parent_fb_pin_reuse_vma(struct intel_display *display,
> > + struct i915_vma *old_ggtt_vma,
> > + struct drm_gem_object *old_obj,
> > + const struct i915_gtt_view *old_view,
> > + struct drm_gem_object *new_obj,
> > + const struct i915_gtt_view *new_view,
> > + u32 *out_offset)
> > +{
> > + if (!display->parent->fb_pin->reuse_vma)
> > + return NULL;
> > +
> > + return display->parent->fb_pin->reuse_vma(old_ggtt_vma, old_obj, old_view,
> > + new_obj, new_view, out_offset);
> > +}
> > +
> > void intel_parent_fb_pin_get_map(struct intel_display *display,
> > struct i915_vma *vma, struct iosys_map *map)
> > {
> > diff --git a/drivers/gpu/drm/i915/display/intel_parent.h b/drivers/gpu/drm/i915/display/intel_parent.h
> > index 2b9e829e7f47..27e35f891a6b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_parent.h
> > +++ b/drivers/gpu/drm/i915/display/intel_parent.h
> > @@ -11,9 +11,11 @@ struct dma_fence;
> > struct drm_file;
> > struct drm_gem_object;
> > struct drm_scanout_buffer;
> > +struct i915_gtt_view;
> > struct i915_vma;
> > struct intel_display;
> > struct intel_dpt;
> > +struct intel_fb_pin_params;
> > struct intel_frontbuffer;
> > struct intel_hdcp_gsc_context;
> > struct intel_panic;
> > @@ -28,6 +30,33 @@ void intel_parent_dpt_suspend(struct intel_display *display, struct intel_dpt *d
> > void intel_parent_dpt_resume(struct intel_display *display, struct intel_dpt *dpt);
> >
> > /* fb_pin */
> > +int intel_parent_fb_pin_ggtt_pin(struct intel_display *display,
> > + struct drm_gem_object *obj,
> > + const struct intel_fb_pin_params *pin_params,
> > + struct i915_vma **out_ggtt_vma,
> > + u32 *out_offset,
> > + int *out_fence_id);
> > +void intel_parent_fb_pin_ggtt_unpin(struct intel_display *display,
> > + struct i915_vma *ggtt_vma,
> > + int fence_id);
> > +int intel_parent_fb_pin_dpt_pin(struct intel_display *display,
> > + struct drm_gem_object *obj,
> > + struct intel_dpt *dpt,
> > + const struct intel_fb_pin_params *pin_params,
> > + struct i915_vma **out_dpt_vma,
> > + struct i915_vma **out_ggtt_vma,
> > + u32 *out_offset);
> > +void intel_parent_fb_pin_dpt_unpin(struct intel_display *display,
> > + struct intel_dpt *dpt,
> > + struct i915_vma *dpt_vma,
> > + struct i915_vma *ggtt_vma);
> > +struct i915_vma *intel_parent_fb_pin_reuse_vma(struct intel_display *display,
> > + struct i915_vma *old_ggtt_vma,
> > + struct drm_gem_object *old_obj,
> > + const struct i915_gtt_view *old_view,
> > + struct drm_gem_object *new_obj,
> > + const struct i915_gtt_view *new_view,
> > + u32 *out_offset);
> > void intel_parent_fb_pin_get_map(struct intel_display *display,
> > struct i915_vma *vma, struct iosys_map *map);
> >
> > diff --git a/drivers/gpu/drm/i915/i915_fb_pin.c b/drivers/gpu/drm/i915/i915_fb_pin.c
> > index e14d2c1fb81f..1f08e364d569 100644
> > --- a/drivers/gpu/drm/i915/i915_fb_pin.c
> > +++ b/drivers/gpu/drm/i915/i915_fb_pin.c
> > @@ -14,6 +14,7 @@
> > #include "display/intel_display_types.h"
> > #include "display/intel_fb.h"
> > #include "display/intel_fb_pin.h"
> > +#include "display/intel_parent.h"
> > #include "display/intel_plane.h"
> >
> > #include "gem/i915_gem_domain.h"
> > @@ -360,9 +361,9 @@ int intel_plane_pin_fb(struct intel_plane_state *plane_state,
> > .needs_fence = intel_plane_needs_fence(display),
> > };
> >
> > - ret = i915_fb_pin_ggtt_pin(intel_fb_bo(&fb->base),
> > - &pin_params, &ggtt_vma, &offset,
> > - intel_plane_uses_fence(plane_state) ? &fence_id : NULL);
> > + ret = intel_parent_fb_pin_ggtt_pin(display, intel_fb_bo(&fb->base),
> > + &pin_params, &ggtt_vma, &offset,
> > + intel_plane_uses_fence(plane_state) ? &fence_id : NULL);
> > if (ret)
> > return ret;
> > } else {
> > @@ -372,9 +373,9 @@ int intel_plane_pin_fb(struct intel_plane_state *plane_state,
> > .needs_cpu_lmem_access = intel_fb_needs_cpu_access(&fb->base),
> > };
> >
> > - ret = i915_fb_pin_dpt_pin(intel_fb_bo(&fb->base), fb->dpt,
> > - &pin_params, &dpt_vma,
> > - &ggtt_vma, &offset);
> > + ret = intel_parent_fb_pin_dpt_pin(display, intel_fb_bo(&fb->base),
> > + fb->dpt, &pin_params,
> > + &dpt_vma, &ggtt_vma, &offset);
> > if (ret)
> > return ret;
> > }
> > @@ -389,19 +390,21 @@ int intel_plane_pin_fb(struct intel_plane_state *plane_state,
> >
> > void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
> > {
> > + struct intel_display *display = to_intel_display(old_plane_state);
> > const struct intel_framebuffer *fb =
> > to_intel_framebuffer(old_plane_state->hw.fb);
> >
> > if (!intel_fb_uses_dpt(&fb->base)) {
> > - i915_fb_pin_ggtt_unpin(old_plane_state->ggtt_vma,
> > - old_plane_state->fence_id);
> > + intel_parent_fb_pin_ggtt_unpin(display,
> > + old_plane_state->ggtt_vma,
> > + old_plane_state->fence_id);
> >
> > old_plane_state->ggtt_vma = NULL;
> > old_plane_state->fence_id = -1;
> > } else {
> > - i915_fb_pin_dpt_unpin(fb->dpt,
> > - old_plane_state->dpt_vma,
> > - old_plane_state->ggtt_vma);
> > + intel_parent_fb_pin_dpt_unpin(display, fb->dpt,
> > + old_plane_state->dpt_vma,
> > + old_plane_state->ggtt_vma);
> >
> > old_plane_state->dpt_vma = NULL;
> > old_plane_state->ggtt_vma = NULL;
> > @@ -414,5 +417,9 @@ static void i915_fb_pin_get_map(struct i915_vma *vma, struct iosys_map *map)
> > }
> >
> > const struct intel_display_fb_pin_interface i915_display_fb_pin_interface = {
> > + .ggtt_pin = i915_fb_pin_ggtt_pin,
> > + .ggtt_unpin = i915_fb_pin_ggtt_unpin,
> > + .dpt_pin = i915_fb_pin_dpt_pin,
> > + .dpt_unpin = i915_fb_pin_dpt_unpin,
> > .get_map = i915_fb_pin_get_map,
> > };
> > diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > index cd287efded28..e2b6ce3686a3 100644
> > --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > @@ -10,6 +10,7 @@
> > #include "intel_display_types.h"
> > #include "intel_fb.h"
> > #include "intel_fb_pin.h"
> > +#include "intel_parent.h"
> > #include "xe_bo.h"
> > #include "xe_device.h"
> > #include "xe_display_vma.h"
> > @@ -509,6 +510,7 @@ xe_fb_pin_reuse_vma(struct i915_vma *old_ggtt_vma,
> > int intel_plane_pin_fb(struct intel_plane_state *new_plane_state,
> > const struct intel_plane_state *old_plane_state)
> > {
> > + struct intel_display *display = to_intel_display(new_plane_state);
> > const struct intel_framebuffer *fb = to_intel_framebuffer(new_plane_state->hw.fb);
> > const struct intel_framebuffer *old_fb = to_intel_framebuffer(old_plane_state->hw.fb);
> > struct drm_gem_object *obj = intel_fb_bo(&fb->base);
> > @@ -524,23 +526,25 @@ int intel_plane_pin_fb(struct intel_plane_state *new_plane_state,
> > u32 offset;
> > int ret;
> >
> > - ggtt_vma = xe_fb_pin_reuse_vma(old_plane_state->ggtt_vma,
> > - intel_fb_bo(&old_fb->base),
> > - &old_plane_state->view.gtt,
> > - intel_fb_bo(&fb->base),
> > - &new_plane_state->view.gtt,
> > - &offset);
> > + ggtt_vma = intel_parent_fb_pin_reuse_vma(display,
> > + old_plane_state->ggtt_vma,
> > + intel_fb_bo(&old_fb->base),
> > + &old_plane_state->view.gtt,
> > + intel_fb_bo(&fb->base),
> > + &new_plane_state->view.gtt,
> > + &offset);
> > if (ggtt_vma)
> > goto got_vma;
> >
> > if (!intel_fb_uses_dpt(&fb->base)) {
> > - ret = xe_fb_pin_ggtt_pin(obj, &pin_params,
> > - &ggtt_vma, &offset, NULL);
> > + ret = intel_parent_fb_pin_ggtt_pin(display, obj, &pin_params,
> > + &ggtt_vma, &offset, NULL);
> > if (ret)
> > return ret;
> > } else {
> > - ret = xe_fb_pin_dpt_pin(obj, fb->dpt, &pin_params,
> > - &dpt_vma, &ggtt_vma, &offset);
> > + ret = intel_parent_fb_pin_dpt_pin(display, obj, fb->dpt,
> > + &pin_params, &dpt_vma,
> > + &ggtt_vma, &offset);
> > if (ret)
> > return ret;
> > }
> > @@ -556,17 +560,20 @@ int intel_plane_pin_fb(struct intel_plane_state *new_plane_state,
> >
> > void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
> > {
> > + struct intel_display *display = to_intel_display(old_plane_state);
> > const struct intel_framebuffer *fb = to_intel_framebuffer(old_plane_state->hw.fb);
> >
> > if (!intel_fb_uses_dpt(&fb->base)) {
> > - xe_fb_pin_ggtt_unpin(old_plane_state->ggtt_vma,
> > - old_plane_state->fence_id);
> > + intel_parent_fb_pin_ggtt_unpin(display,
> > + old_plane_state->ggtt_vma,
> > + old_plane_state->fence_id);
> >
> > old_plane_state->ggtt_vma = NULL;
> > old_plane_state->fence_id = -1;
> > } else {
> > - xe_fb_pin_dpt_unpin(fb->dpt, old_plane_state->dpt_vma,
> > - old_plane_state->ggtt_vma);
> > + intel_parent_fb_pin_dpt_unpin(display, fb->dpt,
> > + old_plane_state->dpt_vma,
> > + old_plane_state->ggtt_vma);
> >
> > old_plane_state->dpt_vma = NULL;
> > old_plane_state->ggtt_vma = NULL;
> > @@ -579,5 +586,10 @@ static void xe_fb_pin_get_map(struct i915_vma *vma, struct iosys_map *map)
> > }
> >
> > const struct intel_display_fb_pin_interface xe_display_fb_pin_interface = {
> > + .ggtt_pin = xe_fb_pin_ggtt_pin,
> > + .ggtt_unpin = xe_fb_pin_ggtt_unpin,
> > + .dpt_pin = xe_fb_pin_dpt_pin,
> > + .dpt_unpin = xe_fb_pin_dpt_unpin,
> > + .reuse_vma = xe_fb_pin_reuse_vma,
> > .get_map = xe_fb_pin_get_map,
> > };
> > diff --git a/include/drm/intel/display_parent_interface.h b/include/drm/intel/display_parent_interface.h
> > index b363d6a85dfe..39991afeb173 100644
> > --- a/include/drm/intel/display_parent_interface.h
> > +++ b/include/drm/intel/display_parent_interface.h
> > @@ -83,6 +83,28 @@ struct intel_display_dsb_interface {
> > };
> >
> > struct intel_display_fb_pin_interface {
> > + int (*ggtt_pin)(struct drm_gem_object *obj,
> > + const struct intel_fb_pin_params *pin_params,
> > + struct i915_vma **out_ggtt_vma,
> > + u32 *out_offset,
> > + int *out_fence_id);
> > + void (*ggtt_unpin)(struct i915_vma *ggtt_vma,
> > + int fence_id);
> > + int (*dpt_pin)(struct drm_gem_object *obj,
> > + struct intel_dpt *dpt,
> > + const struct intel_fb_pin_params *pin_params,
> > + struct i915_vma **out_dpt_vma,
> > + struct i915_vma **out_ggtt_vma,
> > + u32 *out_offset);
> > + void (*dpt_unpin)(struct intel_dpt *dpt,
> > + struct i915_vma *dpt_vma,
> > + struct i915_vma *ggtt_vma);
> > + struct i915_vma *(*reuse_vma)(struct i915_vma *old_ggtt_vma,
> > + struct drm_gem_object *old_obj,
> > + const struct i915_gtt_view *old_view,
> > + struct drm_gem_object *new_obj,
> > + const struct i915_gtt_view *new_view,
> > + u32 *out_offset);
> > void (*get_map)(struct i915_vma *vma, struct iosys_map *map);
> > };
>
> --
> Jani Nikula, Intel
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2026-04-24 10:56 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-23 16:53 [PATCH 00/16] drm/i915: Introduce 'fb_pin' parent interface Ville Syrjala
2026-04-23 16:53 ` [PATCH 01/16] drm/i915: Introduce intel_parent_fb_pin_get_map() Ville Syrjala
2026-04-24 9:14 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 02/16] drm/i915: Move intel_fb_pin_params to the parent interface Ville Syrjala
2026-04-24 9:19 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 03/16] drm/i915: Move the i915_dpt_offset()==0 assert Ville Syrjala
2026-04-24 9:19 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 04/16] drm/i915: Reorganize intel_plane_pin_fb() a bit Ville Syrjala
2026-04-24 9:55 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 05/16] drm/i915: Introduce i915_fb_pin_dpt_(un)pin() Ville Syrjala
2026-04-24 9:56 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 06/16] drm/i915: Introduce i915_fb_pin_ggtt_(un)pin() Ville Syrjala
2026-04-24 9:59 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 07/16] drm/xe: Move the FORCE_WC assert into __xe_pin_fb_vma() Ville Syrjala
2026-04-24 10:02 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 08/16] drm/xe: Kill the fbdev vma reuse hack Ville Syrjala
2026-04-24 10:06 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 09/16] drm/xe: Reorganize intel_plane_pin_fb() a bit Ville Syrjala
2026-04-24 10:07 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 10/16] drm/xe: Introduce xe_fb_pin_dpt_(un)pin() Ville Syrjala
2026-04-24 10:12 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 11/16] drm/xe: Introduce xe_fb_pin_ggtt_(un)pin() Ville Syrjala
2026-04-24 10:13 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 12/16] drm/xe: Restructure reuse_vma() Ville Syrjala
2026-04-24 10:30 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 13/16] drm/i915: Introduce the main fb_pin parent interface Ville Syrjala
2026-04-24 10:48 ` Jani Nikula
2026-04-24 10:55 ` Ville Syrjälä [this message]
2026-04-23 16:53 ` [PATCH 14/16] drm/i915/fbdev: Use intel_parent_fb_pin_ggtt_(un)pin() Ville Syrjala
2026-04-24 12:54 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 15/16] drm/xe: Use xe_fb_pin_ggtt_pin() for the initial FB pin Ville Syrjala
2026-04-24 12:58 ` Jani Nikula
2026-04-23 16:53 ` [PATCH 16/16] drm/i915: Consolidate the intel_plane_(un)pin_fb() implementations Ville Syrjala
2026-04-24 13:30 ` Jani Nikula
2026-04-23 17:01 ` ✗ CI.checkpatch: warning for drm/i915: Introduce 'fb_pin' parent interface Patchwork
2026-04-23 17:02 ` ✓ CI.KUnit: success " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aetMOEagKgSCvVnv@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox