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

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

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.

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?

-- 
Ville Syrjälä
Intel