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
> >
