[PATCH 3/9] drm/i915: Introduce plane->can_async_flip()
Borah, Chaitanya Kumar
chaitanya.kumar.borah at intel.com
Wed Jan 8 07:54:39 UTC 2025
Hello Ville,
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, October 9, 2024 11:52 PM
> To: intel-gfx at lists.freedesktop.org
> Subject: [PATCH 3/9] drm/i915: Introduce plane->can_async_flip()
>
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Move the "does this modifier support async flips?" check to be handled by the
> platform specific plane code instead of having a big mess in common code.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/i9xx_plane.c | 9 +++
> .../gpu/drm/i915/display/intel_atomic_plane.c | 5 ++
> .../gpu/drm/i915/display/intel_atomic_plane.h | 1 +
> drivers/gpu/drm/i915/display/intel_display.c | 53 +-----------
> .../drm/i915/display/intel_display_types.h | 1 +
> .../drm/i915/display/skl_universal_plane.c | 80 +++++++++++++++++++
> 6 files changed, 97 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
> b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index 17a1e3801a85..7455da863a26 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -672,6 +672,11 @@ vlv_primary_disable_flip_done(struct intel_plane
> *plane)
> spin_unlock_irq(&i915->irq_lock);
> }
>
> +static bool i9xx_plane_can_async_flip(u64 modifier) {
> + return modifier == I915_FORMAT_MOD_X_TILED; }
> +
> static bool i9xx_plane_get_hw_state(struct intel_plane *plane,
> enum pipe *pipe)
> {
> @@ -958,19 +963,23 @@ intel_primary_plane_create(struct
> drm_i915_private *dev_priv, enum pipe pipe)
> 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/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index b7e462075ded..596f8662200a 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -155,6 +155,11 @@ bool intel_plane_needs_physical(struct intel_plane
> *plane)
> DISPLAY_INFO(i915)->cursor_needs_physical;
> }
>
> +bool intel_plane_can_async_flip(struct intel_plane *plane, u64
> +modifier) {
> + return plane->can_async_flip && plane->can_async_flip(modifier); }
> +
> unsigned int intel_adjusted_rate(const struct drm_rect *src,
> const struct drm_rect *dst,
> unsigned int rate)
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> index 0f982f452ff3..fb87b3353ab0 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> @@ -19,6 +19,7 @@ struct intel_plane;
> struct intel_plane_state;
> enum plane_id;
>
> +bool intel_plane_can_async_flip(struct intel_plane *plane, u64
> +modifier);
> unsigned int intel_adjusted_rate(const struct drm_rect *src,
> const struct drm_rect *dst,
> unsigned int rate);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 70a5e5357a14..2afd10bbe7b8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6486,58 +6486,7 @@ static int intel_async_flip_check_hw(struct
> intel_atomic_state *state, struct in
> if (!plane->async_flip)
> continue;
>
> - /*
> - * FIXME: This check is kept generic for all platforms.
> - * Need to verify this for all gen9 platforms to enable
> - * this selectively if required.
> - */
> - switch (new_plane_state->hw.fb->modifier) {
> - case DRM_FORMAT_MOD_LINEAR:
> - /*
> - * FIXME: Async on Linear buffer is supported on ICL as
> - * but with additional alignment and fbc restrictions
> - * need to be taken care of. These aren't applicable for
> - * gen12+.
> - */
> - if (DISPLAY_VER(i915) < 12) {
> - drm_dbg_kms(&i915->drm,
> - "[PLANE:%d:%s] Modifier 0x%llx
> does not support async flip on display ver %d\n",
> - plane->base.base.id, plane-
> >base.name,
> - new_plane_state->hw.fb->modifier,
> DISPLAY_VER(i915));
> - return -EINVAL;
> - }
> - break;
> - case I915_FORMAT_MOD_Y_TILED_CCS:
> - case I915_FORMAT_MOD_Yf_TILED_CCS:
> - /*
> - * Display WA #0731: skl
> - * WaDisableRCWithAsyncFlip: skl
> - * "When render decompression is enabled, hardware
> - * internally converts the Async flips to Sync flips."
> - *
> - * Display WA #1159: glk
> - * "Async flip with render compression may result in
> - * intermittent underrun corruption."
> - */
> - if (DISPLAY_VER(i915) < 11) {
> - drm_dbg_kms(&i915->drm,
> - "[PLANE:%d:%s] Modifier 0x%llx
> does not support async flip on display ver %d\n",
> - plane->base.base.id, plane-
> >base.name,
> - new_plane_state->hw.fb->modifier,
> DISPLAY_VER(i915));
> - return -EINVAL;
> - }
> - break;
> - case I915_FORMAT_MOD_X_TILED:
> - case I915_FORMAT_MOD_Y_TILED:
> - case I915_FORMAT_MOD_Yf_TILED:
> - case I915_FORMAT_MOD_4_TILED:
> - 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:
> - case I915_FORMAT_MOD_4_TILED_BMG_CCS:
> - case I915_FORMAT_MOD_4_TILED_LNL_CCS:
> - break;
> - default:
> + if (!intel_plane_can_async_flip(plane,
> +new_plane_state->hw.fb->modifier)) {
> drm_dbg_kms(&i915->drm,
> "[PLANE:%d:%s] Modifier 0x%llx does not
> support async flip\n",
> plane->base.base.id, plane->base.name, diff
> --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 2bb1fa64da2f..3af2864c7885 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1456,6 +1456,7 @@ struct intel_plane {
> unsigned int (*max_stride)(struct intel_plane *plane,
> u32 pixel_format, u64 modifier,
> unsigned int rotation);
> + bool (*can_async_flip)(u64 modifier);
Few questions.
Is it better to name it something like can_async_flip_with_modifier because it specifically checks only if modifier is supported?
Or may be we can add a more generic can_async_flip with the plane state passed to it, making it possible to extend it later if needed.
Should we check for a format and modifier pair like it is done for sync flips? Can we run into scenarios in future(if already not existing) where modifiers are supported for some pixel format and not others?
Regards
Chaitanya
> /* Write all non-self arming plane registers */
> void (*update_noarm)(struct intel_dsb *dsb,
> struct intel_plane *plane,
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 0c09f76f8369..ca82cc55802e 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -504,6 +504,79 @@ skl_plane_max_stride(struct intel_plane *plane,
> max_pixels, max_bytes);
> }
>
> +static bool tgl_plane_can_async_flip(u64 modifier) {
> + switch (modifier) {
> + case DRM_FORMAT_MOD_LINEAR:
> + case I915_FORMAT_MOD_X_TILED:
> + case I915_FORMAT_MOD_Y_TILED:
> + case I915_FORMAT_MOD_4_TILED:
> + 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:
> + case I915_FORMAT_MOD_4_TILED_BMG_CCS:
> + case I915_FORMAT_MOD_4_TILED_LNL_CCS:
> + return true;
> + 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:
> + case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> + case I915_FORMAT_MOD_4_TILED_MTL_RC_CCS_CC:
> + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
> + return false;
> + default:
> + return false;
> + }
> +}
> +
> +static bool icl_plane_can_async_flip(u64 modifier) {
> + switch (modifier) {
> + case DRM_FORMAT_MOD_LINEAR:
> + /*
> + * FIXME: Async on Linear buffer is supported on ICL
> + * but with additional alignment and fbc restrictions
> + * need to be taken care of.
> + */
> + return false;
> + case I915_FORMAT_MOD_X_TILED:
> + case I915_FORMAT_MOD_Y_TILED:
> + case I915_FORMAT_MOD_Yf_TILED:
> + case I915_FORMAT_MOD_Y_TILED_CCS:
> + case I915_FORMAT_MOD_Yf_TILED_CCS:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static bool skl_plane_can_async_flip(u64 modifier) {
> + switch (modifier) {
> + case DRM_FORMAT_MOD_LINEAR:
> + return false;
> + case I915_FORMAT_MOD_X_TILED:
> + case I915_FORMAT_MOD_Y_TILED:
> + case I915_FORMAT_MOD_Yf_TILED:
> + return true;
> + case I915_FORMAT_MOD_Y_TILED_CCS:
> + case I915_FORMAT_MOD_Yf_TILED_CCS:
> + /*
> + * Display WA #0731: skl
> + * WaDisableRCWithAsyncFlip: skl
> + * "When render decompression is enabled, hardware
> + * internally converts the Async flips to Sync flips."
> + *
> + * Display WA #1159: glk
> + * "Async flip with render compression may result in
> + * intermittent underrun corruption."
> + */
> + return false;
> + default:
> + return false;
> + }
> +}
> +
> static u32 tgl_plane_min_alignment(struct intel_plane *plane,
> const struct drm_framebuffer *fb,
> int color_plane)
> @@ -2621,6 +2694,13 @@ skl_universal_plane_create(struct
> drm_i915_private *dev_priv,
> plane->async_flip = skl_plane_async_flip;
> plane->enable_flip_done = skl_plane_enable_flip_done;
> plane->disable_flip_done = skl_plane_disable_flip_done;
> +
> + if (DISPLAY_VER(dev_priv) >= 12)
> + plane->can_async_flip = tgl_plane_can_async_flip;
> + else if (DISPLAY_VER(dev_priv) == 11)
> + plane->can_async_flip = icl_plane_can_async_flip;
> + else
> + plane->can_async_flip = skl_plane_can_async_flip;
> }
>
> if (DISPLAY_VER(dev_priv) >= 11)
> --
> 2.45.2
More information about the Intel-gfx
mailing list