On Wed, 2024-10-09 at 21:22 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Async flips often require bigger alignment that sync flips.
> Currently we have HAS_ASYNC_FLIPS() checks strewn about to
> inidcate that async flips are generally supported and thus
> we want more alignment. Switch that over to using
> intel_plane_can_async_flip() so that we can handle these
> in a slightly less messy way. Currently we don't have cases
> where async flips would require different alignment for
> different modifiers on the same plane.
>
> We'll also move the HAS_ASYNC_FLIPS() check to the plane init
> code so that we can still use that as a quick way to disable
> the async flips workarounds for testing purposes.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/i9xx_plane.c | 55 +++++++++--------
> --
> .../drm/i915/display/skl_universal_plane.c | 21 ++++---
> 2 files changed, 37 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
> b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index 7455da863a26..8d3346199645 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -779,12 +779,11 @@ static unsigned int
> vlv_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;
>
> switch (fb->modifier) {
> case I915_FORMAT_MOD_X_TILED:
> - if (HAS_ASYNC_FLIPS(i915))
> - return 256 * 1024;
> return 4 * 1024;
> case DRM_FORMAT_MOD_LINEAR:
> return 128 * 1024;
> @@ -798,13 +797,11 @@ 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;
>
> switch (fb->modifier) {
> case I915_FORMAT_MOD_X_TILED:
> - if (HAS_ASYNC_FLIPS(i915))
> - return 256 * 1024;
> - return 4 * 1024;
> case DRM_FORMAT_MOD_LINEAR:
> return 4 * 1024;
> default:
> @@ -959,27 +956,29 @@ intel_primary_plane_create(struct
> drm_i915_private *dev_priv, enum pipe pipe)
> plane->get_hw_state = i9xx_plane_get_hw_state;
> plane->check_plane = i9xx_plane_check;
>
> - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> - plane->async_flip = vlv_primary_async_flip;
> - plane->enable_flip_done =
> vlv_primary_enable_flip_done;
> - plane->disable_flip_done =
> vlv_primary_disable_flip_done;
> - plane->can_async_flip = i9xx_plane_can_async_flip;
> - } else if (IS_BROADWELL(dev_priv)) {
> - plane->need_async_flip_toggle_wa = true;
> - plane->async_flip = g4x_primary_async_flip;
> - plane->enable_flip_done =
> bdw_primary_enable_flip_done;
> - plane->disable_flip_done =
> bdw_primary_disable_flip_done;
> - plane->can_async_flip = i9xx_plane_can_async_flip;
> - } else if (DISPLAY_VER(dev_priv) >= 7) {
> - plane->async_flip = g4x_primary_async_flip;
> - plane->enable_flip_done =
> ivb_primary_enable_flip_done;
> - plane->disable_flip_done =
> ivb_primary_disable_flip_done;
> - plane->can_async_flip = i9xx_plane_can_async_flip;
> - } else if (DISPLAY_VER(dev_priv) >= 5) {
> - plane->async_flip = g4x_primary_async_flip;
> - plane->enable_flip_done =
> ilk_primary_enable_flip_done;
> - plane->disable_flip_done =
> ilk_primary_disable_flip_done;
> - plane->can_async_flip = i9xx_plane_can_async_flip;
> + if (HAS_ASYNC_FLIPS(dev_priv)) {
Check for HAS_ASYNC_FLIPS could be dropped. I assume you were
considering having it would be more clear? Anyways:
Reviewed-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> + if (IS_VALLEYVIEW(dev_priv) ||
> IS_CHERRYVIEW(dev_priv)) {
> + plane->async_flip = vlv_primary_async_flip;
> + plane->enable_flip_done =
> vlv_primary_enable_flip_done;
> + plane->disable_flip_done =
> vlv_primary_disable_flip_done;
> + plane->can_async_flip =
> i9xx_plane_can_async_flip;
> + } else if (IS_BROADWELL(dev_priv)) {
> + plane->need_async_flip_toggle_wa = true;
> + plane->async_flip = g4x_primary_async_flip;
> + plane->enable_flip_done =
> bdw_primary_enable_flip_done;
> + plane->disable_flip_done =
> bdw_primary_disable_flip_done;
> + plane->can_async_flip =
> i9xx_plane_can_async_flip;
> + } else if (DISPLAY_VER(dev_priv) >= 7) {
> + plane->async_flip = g4x_primary_async_flip;
> + plane->enable_flip_done =
> ivb_primary_enable_flip_done;
> + plane->disable_flip_done =
> ivb_primary_disable_flip_done;
> + plane->can_async_flip =
> i9xx_plane_can_async_flip;
> + } else if (DISPLAY_VER(dev_priv) >= 5) {
> + plane->async_flip = g4x_primary_async_flip;
> + plane->enable_flip_done =
> ilk_primary_enable_flip_done;
> + plane->disable_flip_done =
> ilk_primary_disable_flip_done;
> + plane->can_async_flip =
> i9xx_plane_can_async_flip;
> + }
> }
>
> modifiers = intel_fb_plane_get_modifiers(dev_priv,
> INTEL_PLANE_CAP_TILING_X);
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index ca82cc55802e..ea72c54e8329 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -589,25 +589,24 @@ static u32 tgl_plane_min_alignment(struct
> intel_plane *plane,
> if (intel_fb_is_ccs_aux_plane(fb, color_plane))
> return mult * 4 * 1024;
>
> + /*
> + * FIXME ADL sees GGTT/DMAR faults with async
> + * flips unless we align to 16k at least.
> + * Figure out what's going on here...
> + */
> + if (IS_ALDERLAKE_P(i915) &&
> + intel_plane_can_async_flip(plane, fb->modifier))
> + return mult * 16 * 1024;
> +
> switch (fb->modifier) {
> case DRM_FORMAT_MOD_LINEAR:
> case I915_FORMAT_MOD_X_TILED:
> case I915_FORMAT_MOD_Y_TILED:
> case I915_FORMAT_MOD_4_TILED:
> - /*
> - * FIXME ADL sees GGTT/DMAR faults with async
> - * flips unless we align to 16k at least.
> - * Figure out what's going on here...
> - */
> - if (IS_ALDERLAKE_P(i915) && HAS_ASYNC_FLIPS(i915))
> - return mult * 16 * 1024;
> return mult * 4 * 1024;
> case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> case I915_FORMAT_MOD_4_TILED_MTL_RC_CCS:
> case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
> - if (IS_ALDERLAKE_P(i915) && HAS_ASYNC_FLIPS(i915))
> - return mult * 16 * 1024;
> - fallthrough;
> case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> case I915_FORMAT_MOD_4_TILED_MTL_MC_CCS:
> case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
> @@ -2689,7 +2688,7 @@ skl_universal_plane_create(struct
> drm_i915_private *dev_priv,
> plane->get_hw_state = skl_plane_get_hw_state;
> plane->check_plane = skl_plane_check;
>
> - if (plane_id == PLANE_1) {
> + if (HAS_ASYNC_FLIPS(dev_priv) && plane_id == PLANE_1) {
> plane->need_async_flip_toggle_wa =
> IS_DISPLAY_VER(dev_priv, 9, 10);
> plane->async_flip = skl_plane_async_flip;
> plane->enable_flip_done = skl_plane_enable_flip_done;
