Re: [PATCH 11/19] drm/i915/dp: Add support for DP tunnel BW allocation

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jan 23, 2024 at 12:28:42PM +0200, Imre Deak wrote:
> +static int check_inherited_tunnel_state(struct intel_atomic_state *state,
> +					struct intel_dp *intel_dp,
> +					const struct intel_digital_connector_state *old_conn_state)
> +{
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> +	const struct intel_connector *connector =
> +		to_intel_connector(old_conn_state->base.connector);
> +	struct intel_crtc *old_crtc;
> +	const struct intel_crtc_state *old_crtc_state;
> +
> +	/*
> +	 * If a BWA tunnel gets detected only after the corresponding
> +	 * connector got enabled already without a BWA tunnel, or a different
> +	 * BWA tunnel (which was removed meanwhile) the old CRTC state won't
> +	 * contain the state of the current tunnel. This tunnel still has a
> +	 * reserved BW, which needs to be released, add the state for such
> +	 * inherited tunnels separately only to this atomic state.
> +	 */
> +	if (!intel_dp_tunnel_bw_alloc_is_enabled(intel_dp))
> +		return 0;
> +
> +	if (!old_conn_state->base.crtc)
> +		return 0;
> +
> +	old_crtc = to_intel_crtc(old_conn_state->base.crtc);
> +	old_crtc_state = intel_atomic_get_old_crtc_state(state, old_crtc);
> +
> +	if (!old_crtc_state->hw.active ||
> +	    old_crtc_state->dp_tunnel_ref.tunnel == intel_dp->tunnel)
> +		return 0;
> +
> +	drm_dbg_kms(&i915->drm,
> +		    "[DPTUN %s][CONNECTOR:%d:%s][ENCODER:%d:%s][CRTC:%d:%s] Adding state for inherited tunnel %p\n",
> +		    drm_dp_tunnel_name(intel_dp->tunnel),
> +		    connector->base.base.id,
> +		    connector->base.name,
> +		    encoder->base.base.id,
> +		    encoder->base.name,
> +		    old_crtc->base.base.id,
> +		    old_crtc->base.name,
> +		    intel_dp->tunnel);
> +
> +	return add_inherited_tunnel_state(state, intel_dp->tunnel, old_crtc);

I still strongly dislike this "tunnels are magically created by detect
behind our back" approach. IMO in an ideal world we'd only ever create the
tunnels during modeset/sanitize. What was the reason that didn't work again?
I think you explained it to me in person at least once already, but can't
remember anymore...

-- 
Ville Syrjälä
Intel