[Intel-gfx] [PATCH v2 2/2] drm/i915/hdcp: Fix modeset locking issue in hdcp mst
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Fri May 12 11:26:15 UTC 2023
On 5/11/2023 3:26 PM, Suraj Kandpal 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 hence we fill in stream info if dpmst encoder is found before
> enabling hdcp.
>
> --v2
> -move prepare streams to beginning of intel_hdcp_enable to avoid
> checking of mst encoder twice [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 | 39 ++++++++++-------------
> 1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index e2c5781ad0ab..bf40d1c7aaa1 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,10 +43,10 @@ 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))
> + if (!payload)
> goto out;
>
> vcpi = payload->vcpi;
> @@ -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;
> }
>
> @@ -69,7 +69,8 @@ static int intel_conn_to_vcpi(struct intel_connector *connector)
> * policy to mark different content_types for different streams.
> */
> static int
> -intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
> +intel_hdcp_required_content_stream(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;
> @@ -81,9 +82,6 @@ intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
>
> data->k = 0;
>
> - if (dig_port->hdcp_auth_status)
> - return 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 +97,8 @@ intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
> if (!enforce_type0 && !dig_port->hdcp_mst_type1_capable)
> enforce_type0 = true;
Although not related to this patch, this seems to be off.
This does not seem to belong in loop, as dig_port is passed in the function.
Seems like we are already setting dig_port->hdcp_mst_type1_capable based
on the topology earlier. (even if any branch/leaf is not hdcp2.2 this is
set to 0).
So type1 content cannot be shown if dig_port->hdcp_mst_type1_capable is 0.
This would cause an issue with existing patch, as
dig_port->hdcp_mst_type1_capable is currently set later during
authentication.
Regards,
Ankit
>
> - 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 */
> @@ -122,7 +121,8 @@ intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
> return 0;
> }
>
> -static int intel_hdcp_prepare_streams(struct intel_connector *connector)
> +static int intel_hdcp_prepare_streams(struct intel_atomic_state *state,
> + 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;
> @@ -133,7 +133,7 @@ static int intel_hdcp_prepare_streams(struct intel_connector *connector)
> data->k = 1;
> data->streams[0].stream_type = hdcp->content_type;
> } else {
> - ret = intel_hdcp_required_content_stream(dig_port);
> + ret = intel_hdcp_required_content_stream(dig_port, state);
> if (ret)
> return ret;
> }
> @@ -1917,14 +1917,6 @@ 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;
> - }
> -
> ret = hdcp2_propagate_stream_management_info(connector);
> if (ret) {
> drm_dbg_kms(&i915->drm,
> @@ -2375,9 +2367,12 @@ 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;
> + if (!intel_hdcp_prepare_streams(state, connector)) {
> + ret = _intel_hdcp2_enable(connector);
> + if (!ret)
> + check_link_interval =
> + DRM_HDCP2_CHECK_PERIOD_MS;
> + }
> }
>
> /*
More information about the Intel-gfx
mailing list