On Mon, May 13, 2024 at 08:59:41PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Currently we still use the SKL+ PLANE_SURF alignment even
> for TGL+ even though the hardware no longer needs it.
> Introduce a separate tgl_plane_min_alignment() and update
> it to more accurately reflect the hardware requirements.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> .../drm/i915/display/skl_universal_plane.c | 103 ++++++++++--------
> 1 file changed, 55 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 1ecd7c691317..ca7fc9fae990 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -502,75 +502,79 @@ skl_plane_max_stride(struct intel_plane *plane,
> max_pixels, max_bytes);
> }
>
> -static unsigned int skl_plane_min_alignment(struct intel_plane *plane,
> - const struct drm_framebuffer *fb,
> - int color_plane)
> +static u32 tgl_plane_min_alignment(struct intel_plane *plane,
> + const struct drm_framebuffer *fb,
> + int color_plane)
> {
> - struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> -
> - if (intel_fb_uses_dpt(fb)) {
> - /* AUX_DIST needs only 4K alignment */
> - if (intel_fb_is_ccs_aux_plane(fb, color_plane))
> - return 512 * 4096;
> -
> - /*
> - * 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(dev_priv) &&
> - !intel_fb_is_ccs_modifier(fb->modifier) &&
> - HAS_ASYNC_FLIPS(dev_priv))
> - return 512 * 16 * 1024;
> -
> - return 512 * 4096;
> - }
> + struct drm_i915_private *i915 = to_i915(plane->base.dev);
> + /* PLANE_SURF GGTT -> DPT alignment */
> + int mult = intel_fb_uses_dpt(fb) ? 512 : 1;
>
> /* AUX_DIST needs only 4K alignment */
> if (intel_fb_is_ccs_aux_plane(fb, color_plane))
> - return 4096;
> + return mult * 4 * 1024;
>
> if (is_semiplanar_uv_plane(fb, color_plane)) {
> /*
> * TODO: cross-check wrt. the bspec stride in bytes * 64 bytes
> * alignment for linear UV planes on all platforms.
> */
> - if (DISPLAY_VER(dev_priv) >= 12) {
> - if (fb->modifier == DRM_FORMAT_MOD_LINEAR)
> - return 256 * 1024;
> -
> - return intel_tile_row_size(fb, color_plane);
> - }
> -
> - return 4096;
> - }
> -
> - drm_WARN_ON(&dev_priv->drm, color_plane != 0);
> -
> - switch (fb->modifier) {
> - case DRM_FORMAT_MOD_LINEAR:
> - return 256 * 1024;
> - case I915_FORMAT_MOD_X_TILED:
> - if (HAS_ASYNC_FLIPS(dev_priv))
> + if (fb->modifier == DRM_FORMAT_MOD_LINEAR)
> return 256 * 1024;
> - return 0;
> +
> + return intel_tile_row_size(fb, color_plane);
> + }
> +
> + 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))
On ADL HAS_ASYNC_FLIPS() is always true, otherwise looks ok:
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> + return mult * 16 * 1024;
> + return mult * 4 * 1024;
> case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
> + case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
> + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
> + case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
> case I915_FORMAT_MOD_4_TILED_MTL_MC_CCS:
> case I915_FORMAT_MOD_4_TILED_MTL_RC_CCS:
> case I915_FORMAT_MOD_4_TILED_MTL_RC_CCS_CC:
> - return 16 * 1024;
> + /* 4x1 main surface tiles (16K) match 64B of AUX */
> + return max(mult * 4 * 1024, 16 * 1024);
> + default:
> + MISSING_CASE(fb->modifier);
> + return 0;
> + }
> +}
> +
> +static u32 skl_plane_min_alignment(struct intel_plane *plane,
> + const struct drm_framebuffer *fb,
> + int color_plane)
> +{
> + /*
> + * AUX_DIST needs only 4K alignment,
> + * as does ICL UV PLANE_SURF.
> + */
> + if (color_plane != 0)
> + return 4 * 1024;
> +
> + switch (fb->modifier) {
> + case DRM_FORMAT_MOD_LINEAR:
> + case I915_FORMAT_MOD_X_TILED:
> + return 256 * 1024;
> case I915_FORMAT_MOD_Y_TILED_CCS:
> case I915_FORMAT_MOD_Yf_TILED_CCS:
> case I915_FORMAT_MOD_Y_TILED:
> - case I915_FORMAT_MOD_4_TILED:
> case I915_FORMAT_MOD_Yf_TILED:
> return 1 * 1024 * 1024;
> - case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
> - case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
> - case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
> - return 16 * 1024;
> default:
> MISSING_CASE(fb->modifier);
> return 0;
> @@ -2442,7 +2446,10 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
> else
> plane->max_stride = skl_plane_max_stride;
>
> - plane->min_alignment = skl_plane_min_alignment;
> + if (DISPLAY_VER(dev_priv) >= 12)
> + plane->min_alignment = tgl_plane_min_alignment;
> + else
> + plane->min_alignment = skl_plane_min_alignment;
>
> if (DISPLAY_VER(dev_priv) >= 11) {
> plane->update_noarm = icl_plane_update_noarm;
> --
> 2.43.2
>
