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

[Imre Deak]

On Tue, Feb 06, 2024 at 12:47:22AM +0200, Ville Syrjälä wrote:
> 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...

The tunnel information, describing which group the tunnel belongs to and
so how much BW it can use is needed already during detect time: to
filter the connectors' mode list during connector probing and to
pass/fail an atomic check of connectors that go through a tunnel/group
based on the modes the connectors use, the BW these require vs. the
available BW of the tunnel group.

The atomic state for the tunnel - with the required BW through it - is
only created/added during a modeset.

> -- 
> Ville Syrjälä
> Intel