On Wed, 22 Jan 2025, Ville Syrjala <[email protected]> wrote:
> From: Ville Syrjälä <[email protected]>
>
> 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ä <[email protected]>
> ---
>  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.

BR,
Jani.

> +
>       switch (fb->modifier) {
>       case I915_FORMAT_MOD_X_TILED:
>               return 4 * 1024;
> @@ -798,9 +803,14 @@ static unsigned int g4x_primary_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;
> +
>       switch (fb->modifier) {
>       case I915_FORMAT_MOD_X_TILED:
>       case DRM_FORMAT_MOD_LINEAR:
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c 
> b/drivers/gpu/drm/i915/display/intel_cursor.c
> index ae7243ad6e0c..9157825e370b 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -372,6 +372,11 @@ static unsigned int i9xx_cursor_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_scanout_needs_vtd_wa(i915))
> +             return 256 * 1024;
> +
>       return 4 * 1024; /* physical for i915/i945 */
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c 
> b/drivers/gpu/drm/i915/display/intel_fb_pin.c
> index dd3ac7f98dfc..2b9ad46eaef7 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
> @@ -126,14 +126,6 @@ intel_fb_pin_to_ggtt(const struct drm_framebuffer *fb,
>       if (drm_WARN_ON(dev, alignment && !is_power_of_2(alignment)))
>               return ERR_PTR(-EINVAL);
>  
> -     /* Note that the w/a also requires 64 PTE of padding following the
> -      * bo. We currently fill all unused PTE with the shadow page and so
> -      * we should always have valid PTE following the scanout preventing
> -      * the VT-d warning.
> -      */
> -     if (intel_scanout_needs_vtd_wa(dev_priv) && alignment < 256 * 1024)
> -             alignment = 256 * 1024;
> -
>       /*
>        * Global gtt pte registers are special registers which actually forward
>        * writes to a chunk of system memory. Which means that there is no risk
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c 
> b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 13996d7059ad..d63e71fe469e 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -980,6 +980,11 @@ static unsigned int g4x_sprite_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_scanout_needs_vtd_wa(i915))
> +             return 256 * 1024;
> +
>       return 4 * 1024;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c 
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 450dd8c64e0c..5cec2df0baca 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -649,6 +649,10 @@ static u32 skl_plane_min_alignment(struct intel_plane 
> *plane,
>       if (color_plane != 0)
>               return 4 * 1024;
>  
> +     /*
> +      * VT-d needs at least 256k alignment,
> +      * but that's already covered below.
> +      */
>       switch (fb->modifier) {
>       case DRM_FORMAT_MOD_LINEAR:
>       case I915_FORMAT_MOD_X_TILED:

-- 
Jani Nikula, Intel

Reply via email to