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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Apr 17, 2026 at 02:33:07PM +0300, Ville Syrjälä wrote:
> 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.

Doh. Misread it. It is exactly the DPT path where we have this.
And that makes sense since DG2+ require DPT for tiled buffers
and clear color is only a thing with certain tiled modifiers.
And DG1, which doesn't have DPT, also doesn't support small-BAR
operation, so the migration is never needed there.

I guess the only situation where we might need the migration
in the ggtt path is if we run DG2 with DPT disable via the
modparam. But I've never actually tried that so not sure it
even works. Should probably give it a go at some point...

-- 
Ville Syrjälä
Intel