Re: [PATCH 09/11] drm/i915: Introduce pin_params.needs_fence

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

On Fri, Apr 17, 2026 at 12:58:26PM +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.needs_fencel to inform the pinning
> 
> *needs_fence
> 
> > code that the display needs a fence for tiled scanout.
> >
> > The goal is to eliminate all display specific stuff from
> > the low level pinning code.
> 
> Again, I find it just a little magical that .needs_fence is only
> initialized in certain code paths, with the implementation detail
> knowledge where the member is used.

I think in the end we could more or less set all the pin_params
members identically in all the codepaths. Though in the end we
should only have three codepaths (plane ggtt pin, plane dpt pin,
fbdev ggtt pin), so the xe vs. i915 differences here will just
go away with that.

> E.g. in this case out_fence_id !=
> NULL.

I suppose for that particular thing I could also add a
.uses_fence and just always require the &fence_id to be
passed in. Although I guess then I'd need to add the
fence_id tracking to to the fbdev path as well.

Hmm, I think fences might disappear on runtime suspend
so it might not really work to have a fence being tracked
for the fbdev perma-pin and expect it to survive runtime
suspend. So it may be that we never want to request a fence
in the fbdev codepath. But if the fence disappears then how
would a tiled fbdev framebuffer even work? I need to check
this...

And for the DPT path we probably shouldn't set any fence flags
(nor even have the *out_fence_id) since fence+DPT is just
nonsense.

> 
> Regardless,
> 
> 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/i915_fb_pin.c          | 4 ++--
> >  2 files changed, 3 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 3e37e9874f50..95f83bf7411f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fb_pin.h
> > +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.h
> > @@ -22,6 +22,7 @@ struct intel_fb_pin_params {
> >  	bool needs_cpu_lmem_access;
> >  	bool needs_low_address;
> >  	bool needs_physical;
> > +	bool needs_fence;
> >  };
> >  
> >  struct i915_vma *
> > diff --git a/drivers/gpu/drm/i915/i915_fb_pin.c b/drivers/gpu/drm/i915/i915_fb_pin.c
> > index a8ed888183cb..5060ec8c76ca 100644
> > --- a/drivers/gpu/drm/i915/i915_fb_pin.c
> > +++ b/drivers/gpu/drm/i915/i915_fb_pin.c
> > @@ -112,7 +112,6 @@ intel_fb_pin_to_ggtt(const struct drm_framebuffer *fb,
> >  		     const struct intel_fb_pin_params *pin_params,
> >  		     int *out_fence_id)
> >  {
> > -	struct intel_display *display = to_intel_display(fb->dev);
> >  	struct drm_i915_private *i915 = to_i915(fb->dev);
> >  	struct drm_gem_object *_obj = intel_fb_bo(fb);
> >  	struct drm_i915_gem_object *obj = to_intel_bo(_obj);
> > @@ -188,7 +187,7 @@ intel_fb_pin_to_ggtt(const struct drm_framebuffer *fb,
> >  		 * mode that matches the user configuration.
> >  		 */
> >  		ret = i915_vma_pin_fence(vma);
> > -		if (ret != 0 && intel_plane_needs_fence(display)) {
> > +		if (ret != 0 && pin_params->needs_fence) {
> >  			i915_vma_unpin(vma);
> >  			goto err_unpin;
> >  		}
> > @@ -272,6 +271,7 @@ int intel_plane_pin_fb(struct intel_plane_state *plane_state,
> >  			.needs_cpu_lmem_access = intel_fb_needs_cpu_access(&fb->base),
> >  			.needs_low_address = intel_plane_needs_low_address(display),
> >  			.needs_physical = intel_plane_needs_physical(plane),
> > +			.needs_fence = intel_plane_needs_fence(display),
> >  		};
> >  		int fence_id = -1;
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel