[PATCH 4/9] drm/i915: Introduce fb->min_alignment

[Imre Deak]

On Tue, May 28, 2024 at 02:27:58PM +0300, Imre Deak wrote:
> [...]
> > +}
> > +
> >  static unsigned int
> >  intel_plane_fb_min_alignment(const struct intel_plane_state *plane_state)
> >  {
> >  	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
> > -	const struct drm_framebuffer *fb = plane_state->hw.fb;
> > +	const struct intel_framebuffer *fb = to_intel_framebuffer(plane_state->hw.fb);
> >  
> > -	return plane->min_alignment(plane, fb, 0);
> > +	/*
> > +	 * Only use plane specific alignment for binding
> > +	 * a per-plane gtt view (remapped or rotated),
> > +	 * otherwise make sure the alignment is suitable
> > +	 * for all planes.
> > +	 */
> > +	if (!gtt_view_is_per_plane(plane_state))
> > +		return fb->min_alignment;
> > +
> > +	if (intel_plane_needs_physical(plane))
> > +		return 0;
> 
> I guess the above is ok, though looks like an unrelated change: the
> cursor plane min_alignment() for relevant platforms is <= 4k, which will
> be rounded up to 4k anyway when binding the vma.

Hm, actually this would change the current 16k vma alignment for cursors on i830?

> 
> > +
> > +	return plane->min_alignment(plane, &fb->base, 0);
> 
> The commit could've had more details about the rational for the above.
> As I understand it avoids having to rebind the vma for different plane
> types, though this is already handled to some degree by
> i915_vma::display_alignment.
> 
> >  }
> >  
> >  static unsigned int
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > index ff685aebbd1a..124aac172acb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > @@ -46,7 +46,6 @@
> >  #include "gem/i915_gem_mman.h"
> >  
> >  #include "i915_drv.h"
> > -#include "intel_crtc.h"
> >  #include "intel_display_types.h"
> >  #include "intel_fb.h"
> >  #include "intel_fb_pin.h"
> > @@ -172,21 +171,6 @@ static const struct fb_ops intelfb_ops = {
> >  
> >  __diag_pop();
> >  
> > -static unsigned int intel_fbdev_min_alignment(const struct drm_framebuffer *fb)
> > -{
> > -	struct drm_i915_private *i915 = to_i915(fb->dev);
> > -	struct intel_plane *plane;
> > -	struct intel_crtc *crtc;
> > -
> > -	crtc = intel_first_crtc(i915);
> > -	if (!crtc)
> > -		return 0;
> > -
> > -	plane = to_intel_plane(crtc->base.primary);
> > -
> > -	return plane->min_alignment(plane, fb, 0);
> > -}
> > -
> >  static int intelfb_create(struct drm_fb_helper *helper,
> >  			  struct drm_fb_helper_surface_size *sizes)
> >  {
> > @@ -244,7 +228,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> >  	 * BIOS is suitable for own access.
> >  	 */
> >  	vma = intel_fb_pin_to_ggtt(&fb->base, &view,
> > -				   intel_fbdev_min_alignment(&fb->base), 0,
> > +				   fb->min_alignment, 0,
> >  				   false, &flags);
> >  	if (IS_ERR(vma)) {
> >  		ret = PTR_ERR(vma);
> > -- 
> > 2.43.2
> >