On Mon, Feb 14, 2022 at 12:55:29PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> data_rate==0 no longer means a plane is disabled, it could
> also mean we want to use the minimum ddb allocation for it.
> Hence we can't bail out early during ddb allocation or
> else we'll simply forget to allocate any ddb for such planes.
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> Fixes: 6a4d8cc6bbbf ("drm/i915: Don't allocate extra ddb during async flip for DG2")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 30 ++++++++++++------------------
> 1 file changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1179bf31f743..ec2de4f13b5e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5114,12 +5114,15 @@ skl_allocate_plane_ddb(struct skl_plane_ddb_iter *iter,
> const struct skl_wm_level *wm,
> u64 data_rate)
> {
> - u16 extra;
> + u16 extra = 0;
>
> - extra = min_t(u16, iter->size,
> - DIV64_U64_ROUND_UP(iter->size * data_rate, iter->data_rate));
> - iter->size -= extra;
> - iter->data_rate -= data_rate;
> + if (data_rate) {
> + extra = min_t(u16, iter->size,
> + DIV64_U64_ROUND_UP(iter->size * data_rate,
> + iter->data_rate));
> + iter->size -= extra;
> + iter->data_rate -= data_rate;
> + }
>
> return wm->min_ddb_alloc + extra;
> }
> @@ -5162,9 +5165,6 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
> skl_ddb_entry_init(&crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR],
> alloc->end - iter.total[PLANE_CURSOR], alloc->end);
>
> - if (iter.data_rate == 0)
> - return 0;
> -
> /*
> * Find the highest watermark level for which we can satisfy the block
> * requirement of active planes.
> @@ -5203,6 +5203,10 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
> return -EINVAL;
> }
>
> + /* avoid the WARN later when we don't allocate any extra DDB */
> + if (iter.data_rate == 0)
> + iter.size = 0;
> +
> /*
> * Grant each plane the blocks it requires at the highest achievable
> * watermark level, plus an extra share of the leftover blocks
> @@ -5215,20 +5219,10 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
> if (plane_id == PLANE_CURSOR)
> continue;
>
> - /*
> - * We've accounted for all active planes; remaining planes are
> - * all disabled.
> - */
> - if (iter.data_rate == 0)
> - break;
> -
> iter.total[plane_id] =
> skl_allocate_plane_ddb(&iter, &wm->wm[level],
> crtc_state->plane_data_rate[plane_id]);
>
> - if (iter.data_rate == 0)
> - break;
> -
> iter.uv_total[plane_id] =
> skl_allocate_plane_ddb(&iter, &wm->uv_wm[level],
> crtc_state->uv_plane_data_rate[plane_id]);
> --
> 2.34.1
>
