[PATCH 2/4] drm/i915: Break intel_dbuf_mbus_update into 2 separate parts

[Ville Syrjälä]

On Sun, Mar 17, 2024 at 07:24:14PM +0200, Stanislav Lisovskiy wrote:
> We need to be able to update dbuf min tracker and mdclk ratio
> separately if mbus_join state didn't change, so lets add one
> degree of freedom and make it possible.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_watermark.c | 55 ++++++++++++--------
>  1 file changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 2d3b08c2f8d78..d7d2278fd201c 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -3570,16 +3570,38 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio
>  			     DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1));
>  }
>  
> +static void intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *i915 = to_i915(state->base.dev);
> +	const struct intel_dbuf_state *old_dbuf_state =
> +		intel_atomic_get_old_dbuf_state(state);
> +	const struct intel_dbuf_state *new_dbuf_state =
> +		intel_atomic_get_new_dbuf_state(state);
> +
> +	if (DISPLAY_VER(i915) >= 20 &&
> +	    old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) {
> +		/*
> +		 * For Xe2LPD and beyond, when there is a change in the ratio
> +		 * between MDCLK and CDCLK, updates to related registers need to
> +		 * happen at a specific point in the CDCLK change sequence. In
> +		 * that case, we defer to the call to
> +		 * intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic.
> +		 */
> +		return;
> +	}
> +
> +	intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio,
> +					    new_dbuf_state->joined_mbus);
> +}

Argh. The hardware is turning into a disaster with all these
links between different units.

This whole thing looks rather suspicious as the cdclk changes
and mbus joining changes don't happen in sync.

AFAICS the sequence should end up doing more or less like this:
1. disable pipes
2. increase cdclk
  2.1 reprogram cdclk
  2.2 update dbuf tracker value
3. enable mbus joining if necessary
 3.1 update mbus_ctl
 3.2 update dbuf tracker value
4. reallocate dbuf for planes on active pipes
5. disable mbus joining if necessary
 5.1 update dbuf tracker value
 5.2 update mbus_ctl
6. enable pipes
7. decrease cdclk, mbus joining is unchanged
  7.1 update dbuf tracker value
  7.2 reprogram cdclk

So step 2.2 should keep using the old mbus_join valued when
updating the ratio, and steps 3.2,5.1,7.1 should use the new
value. That's assuming there is actually some ordering
requirements between these steps (whch bspec does seem to
imply).

-- 
Ville Syrjälä
Intel