[Intel-gfx] [PATCH v3 3/4] drm/i915/hdcp: Fix modeset locking issue in hdcp mst

[Jani Nikula]

On Mon, 15 May 2023, Suraj Kandpal <suraj.kandpal at intel.com> wrote:
> Since topology state is being added to drm_atomic_state now all
> drm_modeset_lock required are being taken from core. This raises
> an issue when we try to loop over connector and assign vcpi id to
> our streams as we did not have atomic state to derive acquire_ctx
> from. We fill in stream info if dpmst encoder is found before
> enabling hdcp. intel_hdcp_required_stream will be broken which
> will only set the content type.
>
> --v2
> -move prepare streams to beginning of intel_hdcp_enable to avoid
> checking of mst encoder twice [Ankit]
>
> --v3
> -break intel_required_content_stream to two part and set the stream_id
> at the beginning [Ankit]

Patches 3-4 look fine to me, but I'd like Ankit's review on them.

Thanks,
Jani.

>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 63 +++++++++++++++--------
>  1 file changed, 42 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index f2d258a72c59..64875c33832c 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -30,7 +30,8 @@
>  #define KEY_LOAD_TRIES	5
>  #define HDCP2_LC_RETRY_CNT			3
>  
> -static int intel_conn_to_vcpi(struct intel_connector *connector)
> +static int intel_conn_to_vcpi(struct drm_atomic_state *state,
> +			      struct intel_connector *connector)
>  {
>  	struct drm_dp_mst_topology_mgr *mgr;
>  	struct drm_dp_mst_atomic_payload *payload;
> @@ -42,7 +43,7 @@ static int intel_conn_to_vcpi(struct intel_connector *connector)
>  		return 0;
>  	mgr = connector->port->mgr;
>  
> -	drm_modeset_lock(&mgr->base.lock, NULL);
> +	drm_modeset_lock(&mgr->base.lock, state->acquire_ctx);
>  	mst_state = to_drm_dp_mst_topology_state(mgr->base.state);
>  	payload = drm_atomic_get_mst_payload_state(mst_state, connector->port);
>  	if (drm_WARN_ON(mgr->dev, !payload))
> @@ -54,7 +55,6 @@ static int intel_conn_to_vcpi(struct intel_connector *connector)
>  		goto out;
>  	}
>  out:
> -	drm_modeset_unlock(&mgr->base.lock);
>  	return vcpi;
>  }
>  
> @@ -71,22 +71,42 @@ static int intel_conn_to_vcpi(struct intel_connector *connector)
>  static int
>  intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
>  {
> -	struct drm_connector_list_iter conn_iter;
> -	struct intel_digital_port *conn_dig_port;
> -	struct intel_connector *connector;
> -	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>  	struct hdcp_port_data *data = &dig_port->hdcp_port_data;
>  	bool enforce_type0 = false;
>  	int k;
>  
> -	data->k = 0;
> -
>  	if (dig_port->hdcp_auth_status)
>  		return 0;
>  
>  	if (!dig_port->hdcp_mst_type1_capable)
>  		enforce_type0 = true;
>  
> +	/*
> +	 * Apply common protection level across all streams in DP MST Topology.
> +	 * Use highest supported content type for all streams in DP MST Topology.
> +	 */
> +	for (k = 0; k < data->k; k++)
> +		data->streams[k].stream_type =
> +			enforce_type0 ? DRM_MODE_HDCP_CONTENT_TYPE0 : DRM_MODE_HDCP_CONTENT_TYPE1;
> +
> +	return 0;
> +}
> +
> +static int
> +intel_hdcp_set_content_streams(struct intel_digital_port *dig_port,
> +			       struct intel_atomic_state *state)
> +{
> +	struct drm_connector_list_iter conn_iter;
> +	struct intel_digital_port *conn_dig_port;
> +	struct intel_connector *connector;
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	struct hdcp_port_data *data = &dig_port->hdcp_port_data;
> +
> +	if (!intel_encoder_is_mst(&dig_port->base))
> +		return 0;
> +
> +	data->k = 0;
> +
>  	drm_connector_list_iter_begin(&i915->drm, &conn_iter);
>  	for_each_intel_connector_iter(connector, &conn_iter) {
>  		if (connector->base.status == connector_status_disconnected)
> @@ -99,7 +119,8 @@ intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
>  		if (conn_dig_port != dig_port)
>  			continue;
>  
> -		data->streams[data->k].stream_id = intel_conn_to_vcpi(connector);
> +		data->streams[data->k].stream_id =
> +			intel_conn_to_vcpi(&state->base, connector);
>  		data->k++;
>  
>  		/* if there is only one active stream */
> @@ -111,14 +132,6 @@ intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
>  	if (drm_WARN_ON(&i915->drm, data->k > INTEL_NUM_PIPES(i915) || data->k == 0))
>  		return -EINVAL;
>  
> -	/*
> -	 * Apply common protection level across all streams in DP MST Topology.
> -	 * Use highest supported content type for all streams in DP MST Topology.
> -	 */
> -	for (k = 0; k < data->k; k++)
> -		data->streams[k].stream_type =
> -			enforce_type0 ? DRM_MODE_HDCP_CONTENT_TYPE0 : DRM_MODE_HDCP_CONTENT_TYPE1;
> -
>  	return 0;
>  }
>  
> @@ -2375,9 +2388,17 @@ int intel_hdcp_enable(struct intel_atomic_state *state,
>  	 * is capable of HDCP2.2, it is preferred to use HDCP2.2.
>  	 */
>  	if (intel_hdcp2_capable(connector)) {
> -		ret = _intel_hdcp2_enable(connector);
> -		if (!ret)
> -			check_link_interval = DRM_HDCP2_CHECK_PERIOD_MS;
> +		ret = intel_hdcp_set_content_streams(dig_port, state);
> +		if (!ret) {
> +			ret = _intel_hdcp2_enable(connector);
> +			if (!ret)
> +				check_link_interval =
> +					DRM_HDCP2_CHECK_PERIOD_MS;
> +		} else {
> +			drm_dbg_kms(&dev_priv->drm,
> +				    "Set content streams failed: (%d)\n",
> +				    ret);
> +		}
>  	}
>  
>  	/*

-- 
Jani Nikula, Intel Open Source Graphics Center