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

Suraj Kandpal suraj.kandpal at intel.com
Mon May 15 10:32:24 UTC 2023


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]

--v4
-change return types for intel_hdcp_prepare_stream and
intel_hdcp_required content_stream [Ankit]
-rename intel_hdcp_set_content_stream to
intel_hdcp_set_stream [Ankit]
-place intel_hdcp_set_streams above caller [Ankit]

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 | 116 ++++++++++++----------
 1 file changed, 62 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index f2d258a72c59..6c3cd3b7db69 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;
 }
 
@@ -68,49 +68,19 @@ static int intel_conn_to_vcpi(struct intel_connector *connector)
  * DP MST topology. Though it is not compulsory, security fw should change its
  * policy to mark different content_types for different streams.
  */
-static int
+static void
 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;
+		return;
 
 	if (!dig_port->hdcp_mst_type1_capable)
 		enforce_type0 = true;
 
-	drm_connector_list_iter_begin(&i915->drm, &conn_iter);
-	for_each_intel_connector_iter(connector, &conn_iter) {
-		if (connector->base.status == connector_status_disconnected)
-			continue;
-
-		if (!intel_encoder_is_mst(intel_attached_encoder(connector)))
-			continue;
-
-		conn_dig_port = intel_attached_dig_port(connector);
-		if (conn_dig_port != dig_port)
-			continue;
-
-		data->streams[data->k].stream_id = intel_conn_to_vcpi(connector);
-		data->k++;
-
-		/* if there is only one active stream */
-		if (dig_port->dp.active_mst_links <= 1)
-			break;
-	}
-	drm_connector_list_iter_end(&conn_iter);
-
-	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.
@@ -118,27 +88,20 @@ intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
 	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_prepare_streams(struct intel_connector *connector)
+static void intel_hdcp_prepare_streams(struct intel_connector *connector)
 {
 	struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
 	struct hdcp_port_data *data = &dig_port->hdcp_port_data;
 	struct intel_hdcp *hdcp = &connector->hdcp;
-	int ret;
 
 	if (!intel_encoder_is_mst(intel_attached_encoder(connector))) {
 		data->k = 1;
 		data->streams[0].stream_type = hdcp->content_type;
 	} else {
-		ret = intel_hdcp_required_content_stream(dig_port);
-		if (ret)
-			return ret;
+		intel_hdcp_required_content_stream(dig_port);
 	}
-
-	return 0;
 }
 
 static
@@ -1917,13 +1880,7 @@ static int hdcp2_authenticate_and_encrypt(struct intel_connector *connector)
 	for (i = 0; i < tries && !dig_port->hdcp_auth_status; i++) {
 		ret = hdcp2_authenticate_sink(connector);
 		if (!ret) {
-			ret = intel_hdcp_prepare_streams(connector);
-			if (ret) {
-				drm_dbg_kms(&i915->drm,
-					    "Prepare streams failed.(%d)\n",
-					    ret);
-				break;
-			}
+			intel_hdcp_prepare_streams(connector);
 
 			ret = hdcp2_propagate_stream_management_info(connector);
 			if (ret) {
@@ -2330,6 +2287,49 @@ int intel_hdcp_init(struct intel_connector *connector,
 	return 0;
 }
 
+static int
+intel_hdcp_set_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)
+			continue;
+
+		if (!intel_encoder_is_mst(intel_attached_encoder(connector)))
+			continue;
+
+		conn_dig_port = intel_attached_dig_port(connector);
+		if (conn_dig_port != dig_port)
+			continue;
+
+		data->streams[data->k].stream_id =
+			intel_conn_to_vcpi(&state->base, connector);
+		data->k++;
+
+		/* if there is only one active stream */
+		if (dig_port->dp.active_mst_links <= 1)
+			break;
+	}
+	drm_connector_list_iter_end(&conn_iter);
+
+	if (drm_WARN_ON(&i915->drm, data->k > INTEL_NUM_PIPES(i915) || data->k == 0))
+		return -EINVAL;
+
+	return 0;
+}
+
 int intel_hdcp_enable(struct intel_atomic_state *state,
 		      struct intel_encoder *encoder,
 		      const struct intel_crtc_state *pipe_config,
@@ -2375,9 +2375,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_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);
+		}
 	}
 
 	/*
-- 
2.25.1



More information about the Intel-gfx mailing list