Re: [PATCH 1/5] drm/i915: Move VT-d alignment into plane->min_alignment()

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

On Tue, Jan 28, 2025 at 11:46:23AM +0200, Jani Nikula wrote:
> On Mon, 27 Jan 2025, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Mon, Jan 27, 2025 at 06:44:21PM +0200, Ville Syrjälä wrote:
> >> On Mon, Jan 27, 2025 at 11:50:34AM +0200, Jani Nikula wrote:
> >> > On Wed, 22 Jan 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> >> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > >
> >> > > Currently we don't account for the VT-d alignment w/a in
> >> > > plane->min_alignment() which means that panning inside a larger
> >> > > framebuffer can still cause the plane SURF to be misaligned.
> >> > > Fix the issue by moving the VT-d alignment w/a into
> >> > > plane->min_alignment() itself (for the affected platforms).
> >> > >
> >> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > > ---
> >> > >  drivers/gpu/drm/i915/display/i9xx_plane.c          | 10 ++++++++++
> >> > >  drivers/gpu/drm/i915/display/intel_cursor.c        |  5 +++++
> >> > >  drivers/gpu/drm/i915/display/intel_fb_pin.c        |  8 --------
> >> > >  drivers/gpu/drm/i915/display/intel_sprite.c        |  5 +++++
> >> > >  drivers/gpu/drm/i915/display/skl_universal_plane.c |  4 ++++
> >> > >  5 files changed, 24 insertions(+), 8 deletions(-)
> >> > >
> >> > > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
> >> > > index ed171fbf8720..19cc34babef3 100644
> >> > > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> >> > > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> >> > > @@ -780,9 +780,14 @@ unsigned int vlv_plane_min_alignment(struct intel_plane *plane,
> >> > >  				     const struct drm_framebuffer *fb,
> >> > >  				     int color_plane)
> >> > >  {
> >> > > +	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> >> > > +
> >> > >  	if (intel_plane_can_async_flip(plane, fb->modifier))
> >> > >  		return 256 * 1024;
> >> > >  
> >> > > +	if (intel_scanout_needs_vtd_wa(i915))
> >> > > +		return 256 * 1024;
> >> > 
> >> > Nitpick, would be great to convert intel_scanout_needs_vtd_wa() to
> >> > struct intel_display first, so we wouldn't have to introduce so many new
> >> > struct drm_i915_private uses.
> >> 
> >> I didn't really want to add intel_display stuff to the
> >> gem side (where this is being used currently). Once
> >> its all moved into the display code (patch 3) then it
> >> makes more sense.
> 
> Roger.
> 
> > BTW I do have full conversion of all the low level
> > plane code (+ a bunch of other stuff including
> > intel_display_power_{put,get}() stuff) done locally.
> > But I have quite a few series on the list already
> > needing review, so not sure I should also post that
> > one right now. I can, if you especially want something
> > mundane to read?
> 
> I might be up for just that, actually.

Apparently I already posted that. Go figure.
https://patchwork.freedesktop.org/series/143942/

-- 
Ville Syrjälä
Intel