Re: [Intel-gfx] [PATCH 1/4] drm/i915: Don't skip ddb allocation if data_rate==0

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Mon, Feb 14, 2022 at 12:55:29PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 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@intel.com>

> 
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Fixes: 6a4d8cc6bbbf ("drm/i915: Don't allocate extra ddb during async flip for DG2")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  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
>