Re: [PATCH 04/11] drm/i915: Introduce pin_params.needs_cpu_lmem_access

Ville Syrjälä
2026-04-17T11:33:13Z

On Fri, Apr 17, 2026 at 12:39:54PM +0300, Jani Nikula wrote:
> On Thu, 16 Apr 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Add a new flag pin_params.neeeds_cpu_lmem_access so that the
> > low level pinning code doesn't need to peek into the display
> > driver's framebuffer structure.
> >
> > 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/i915_fb_pin.c | 4 +++-
> > drivers/gpu/drm/xe/display/xe_fb_pin.c | 3 ++-
> > 3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.h b/drivers/gpu/drm/i915/display/intel_fb_pin.h
> > index e6271437459d..bcf5a1f46092 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fb_pin.h
> > +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.h
> > @@ -19,6 +19,7 @@ struct intel_fb_pin_params {
> > unsigned int alignment;
> > unsigned int phys_alignment;
> > unsigned int vtd_guard;
> > + bool needs_cpu_lmem_access;
> > };
> >
> > struct i915_vma *
> > diff --git a/drivers/gpu/drm/i915/i915_fb_pin.c b/drivers/gpu/drm/i915/i915_fb_pin.c
> > index 780be25ad43b..96ffc4b0d809 100644
> > --- a/drivers/gpu/drm/i915/i915_fb_pin.c
> > +++ b/drivers/gpu/drm/i915/i915_fb_pin.c
> > @@ -61,7 +61,7 @@ intel_fb_pin_to_dpt(const struct drm_framebuffer *fb,
> > * ensure it is always in the mappable part of lmem, if this is
> > * a small-bar device.
> > */
> > - if (intel_fb_needs_cpu_access(fb))
> > + if (pin_params->needs_cpu_lmem_access)
> > flags &= ~I915_BO_ALLOC_GPU_ONLY;
> > ret = __i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0,
> > flags);
> > @@ -275,6 +275,7 @@ int intel_plane_pin_fb(struct intel_plane_state *plane_state,
> > .alignment = intel_plane_fb_min_alignment(plane_state),
> > .phys_alignment = intel_plane_fb_min_phys_alignment(plane_state),
> > .vtd_guard = intel_plane_fb_vtd_guard(plane_state),
> > + .needs_cpu_lmem_access = intel_fb_needs_cpu_access(&fb->base),
>
> IIUC this path never uses .needs_cpu_lmem_access, but you initialize it
> anyway. What I find confusing is initializing it here apparently
> unnecessarily, but then leaving it out on a number of other paths that
> don't need it.

Hmm, yeah looks like we don't have the explicit migrate stuff
in the DPT path for some reason.

I'm sure the migration is happening though because I've used
DG2 on a small BAR system quite a bit, and page flips with clear
color work fine on i915 (unlike on xe+small BAR where it just
fails). And I remember even seeing the migration popping up in
the profiles. Now I'm wondering what might be triggering it
before the vma has been pinned...

Ideally I'd like to get rid of this whole thing and instead have
userspace put the clear color into its own separate little BO.
That way we wouldn't have to waste the precious CPU visible LMEM
on the full framebuffers. The kernel part of that is pretty trivial
and I have the patches in some branch, but sadly the Mesa side
is much more complicated. That particular onion has at least four
layers through which the extra dmabuf would need to get plumbed :(

>
> Other than that,
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> > };
> > int fence_id = -1;
> >
> > @@ -289,6 +290,7 @@ int intel_plane_pin_fb(struct intel_plane_state *plane_state,
> > struct intel_fb_pin_params pin_params = {
> > .view = &plane_state->view.gtt,
> > .alignment = intel_plane_fb_min_alignment(plane_state),
> > + .needs_cpu_lmem_access = intel_fb_needs_cpu_access(&fb->base),
> > };
> >
> > vma = i915_dpt_pin_to_ggtt(fb->dpt, pin_params.alignment / 512);
> > diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > index 205492639dba..a4eb06cfa769 100644
> > --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > @@ -335,7 +335,7 @@ static struct i915_vma *__xe_pin_fb_vma(const struct intel_framebuffer *fb,
> >
> > refcount_set(&vma->ref, 1);
> > if (IS_DGFX(to_xe_device(bo->ttm.base.dev)) &&
> > - intel_fb_needs_cpu_access(&fb->base) &&
> > + pin_params->needs_cpu_lmem_access &&
> > !(bo->flags & XE_BO_FLAG_NEEDS_CPU_ACCESS)) {
> > struct xe_vram_region *vram = xe_device_get_root_tile(xe)->mem.vram;
> >
> > @@ -474,6 +474,7 @@ int intel_plane_pin_fb(struct intel_plane_state *new_plane_state,
> > struct intel_fb_pin_params pin_params = {
> > .view = &new_plane_state->view.gtt,
> > .alignment = plane->min_alignment(plane, fb, 0),
> > + .needs_cpu_lmem_access = intel_fb_needs_cpu_access(fb),
> > };
> >
> > if (reuse_vma(new_plane_state, old_plane_state))
>
> --
> Jani Nikula, Intel

--
Ville Syrjälä
Intel