[Intel-gfx] [PATCH v7 4/4] drm/dp: Track MST link bandwidth

[Maarten Lankhorst]

On 21-04-17 07:51, Dhinakaran Pandiyan wrote:
> From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan at intel.com>
>
> Use the added helpers to track MST link bandwidth for atomic modesets.
> Link bw is acquired in the ->atomic_check() phase when CRTCs are being
> enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().
> Similarly, link bw is released during ->atomic_check() with the connector
> helper callback ->atomic_release() when CRTCs are disabled.
>
> v5: Implement connector->atomic_check() in place of atomic_release()
> v4: Return an int from intel_dp_mst_atomic_release() (Maarten)
> v3:
>  Handled the case where ->atomic_release() is called more than once
>  during an atomic_check() for the same state.
> v2:
>  Squashed atomic_release() implementation and caller (Daniel)
>  Fixed logic for connector-crtc switching case (Daniel)
>  Fixed logic for suspend-resume case.
>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Archit Taneja <architt at codeaurora.org>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Harry Wentland <Harry.wentland at amd.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 57 +++++++++++++++++++++++++++++++++----
>  1 file changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 5af22a7..20c557c 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  	struct intel_connector *connector =
>  		to_intel_connector(conn_state->connector);
> -	struct drm_atomic_state *state;
> +	struct drm_atomic_state *state = pipe_config->base.state;
>  	int bpp;
> -	int lane_count, slots;
> +	int lane_count, slots = 0;
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	int mst_pbn;
>  
> @@ -63,24 +63,68 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	pipe_config->pipe_bpp = bpp;
>  	pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
>  
> -	state = pipe_config->base.state;
> -
>  	if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port))
>  		pipe_config->has_audio = true;
> -	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
>  
> +	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
>  	pipe_config->pbn = mst_pbn;
> -	slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn);
>  
>  	intel_link_compute_m_n(bpp, lane_count,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n);
>  
> +	if (pipe_config->base.active) {
Is this check needed? If so it needs a comment as to why.

I know that the output won't actually turn on, but if the crtc is configured vcpi slots should be allocated if possible.

Else the following will fail in the wrong place:
1. Configure CRTC with a valid mode and DP-MST connector, but set active=false. connectors_changed = true, mode_changed = true. Not enough VCPI slots to enable all crtc's at the same time now, but this returns OK.
2. Commit with active property set to true. There are too few vcpi slots to make this work, this atomic commit returns -EINVAL. This might happen on a different crtc than the newly configured one in stap 1, if they were all previously set to !active.

> +		slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr,
> +					      connector->port, mst_pbn);
> +		if (slots < 0) {
> +			DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);
> +			return false;
> +		}
> +	}
>  	pipe_config->dp_m_n.tu = slots;
>  
>  	return true;
> +}
>  
> +static inline bool release_resources(struct drm_crtc_state *crtc_state)
> +{
> +	return (crtc_state->connectors_changed ||
> +		crtc_state->mode_changed ||
> +		(crtc_state->active_changed && !crtc_state->active));
> +}
> +
> +static int intel_dp_mst_atomic_check(struct drm_connector *connector,
> +		struct drm_connector_state *new_conn_state)
> +{
> +	struct drm_atomic_state *state = new_conn_state->state;
> +	struct drm_connector_state *old_conn_state;
> +	struct drm_crtc *old_crtc;
> +	struct drm_crtc_state *crtc_state;
> +	int slots, ret = 0;
> +
> +	old_conn_state = drm_atomic_get_old_connector_state(state, connector);
> +	old_crtc = old_conn_state->crtc;
> +	if (!old_crtc)
> +		return 0;
> +
> +	crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc);
> +	slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu;
> +
> +	if (release_resources(crtc_state) && slots > 0) {
> +		struct drm_dp_mst_topology_mgr *mgr;
> +		struct drm_encoder *old_encoder;
> +
> +		old_encoder = old_conn_state->best_encoder;
> +		mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr;
> +
> +		ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots);
> +		if (ret)
> +			DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret);
> +		else
> +			to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0;
> +	}
> +	return ret;
>  }
>  
>  static void intel_mst_disable_dp(struct intel_encoder *encoder,
> @@ -378,6 +422,7 @@ static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun
>  	.mode_valid = intel_dp_mst_mode_valid,
>  	.atomic_best_encoder = intel_mst_atomic_best_encoder,
>  	.best_encoder = intel_mst_best_encoder,
> +	.atomic_check = intel_dp_mst_atomic_check,
>  };
>  
>  static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder)