On Fri, Jan 21, 2022 at 10:06:15AM +0200, Stanislav Lisovskiy wrote:
> In terms of async flip optimization we don't to allocate
> extra ddb space, so lets skip it.
>
> v2: - Extracted min ddb async flip check to separate function
> (Ville Syrjälä)
> - Used this function to prevent false positive WARN
> to be triggered(Ville Syrjälä)
>
> v3: - Renamed dg2_need_min_ddb to need_min_ddb thus making
> it more universal.
> - Also used DISPLAY_VER instead of IS_DG2(Ville Syrjälä)
> - Use rate = 0 instead of just setting extra = 0, thus
> letting other planes to use extra ddb and avoiding WARN
> (Ville Syrjälä)
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5fb022a2a4d7..18fb35c480ef 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5118,6 +5118,12 @@ static bool icl_need_wm1_wa(struct drm_i915_private *i915,
> (IS_DISPLAY_VER(i915, 12, 13) && plane_id == PLANE_CURSOR);
> }
>
> +static bool needs_min_ddb(struct drm_i915_private *i915,
> + struct intel_crtc_state *crtc_state)
s/needs/use/ to match the wm0 counterpart?
Could use a comment as well perhaps, or maybe just put this right
next to the wm0 counterpart so the reader can see both together and
make the connection.
Hmm. Actually I think this would also need the plane->async_flip
check here too or else we'll drop all the planes to min ddb
instead of just the plane doing async flips.
Oh, and I think we need this same thing when calculating the
total_data_rate or else the numbers won't match.
> +{
> + return DISPLAY_VER(i915) >= 13 && crtc_state->uapi.async_flip;
> +}
> +
> static int
> skl_allocate_plane_ddb(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> @@ -5225,9 +5231,14 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
> break;
>
> rate = crtc_state->plane_data_rate[plane_id];
> +
> + if (needs_min_ddb(dev_priv, crtc_state))
> + rate = 0;
> +
> extra = min_t(u16, alloc_size,
> DIV64_U64_ROUND_UP(alloc_size * rate,
> total_data_rate));
> +
> total[plane_id] = wm->wm[level].min_ddb_alloc + extra;
> alloc_size -= extra;
> total_data_rate -= rate;
> @@ -5236,13 +5247,19 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
> break;
>
> rate = crtc_state->uv_plane_data_rate[plane_id];
> +
> + if (needs_min_ddb(dev_priv, crtc_state))
> + rate = 0;
> +
> extra = min_t(u16, alloc_size,
> DIV64_U64_ROUND_UP(alloc_size * rate,
> total_data_rate));
> +
> uv_total[plane_id] = wm->uv_wm[level].min_ddb_alloc + extra;
> alloc_size -= extra;
> total_data_rate -= rate;
> }
> +
> drm_WARN_ON(&dev_priv->drm, alloc_size != 0 || total_data_rate != 0);
>
> /* Set the actual DDB start/end points for each plane */
> --
> 2.24.1.485.gad05a3d8e5
--
Ville Syrjälä
Intel
