> Subject: [PATCH v3] drm/i915/hdcp: Fail fast when HDCP Type 1 is unsupported > via MST
Do not update this fast comments given in v1 are still not addressed this way comments will end up getting lost Regards, Suraj Kandpal > > HDCP Content Type 1 requires HDCP 2.x support from both the source and the > downstream MST sink. A request for a sink that supports HDCP 1.x but cannot > use > HDCP 2.x currently passes atomic validation and fails later in the HDCP enable > path. > > Query the downstream sink through its MST remote AUX channel during HDCP > initialization. Cache the result only when the sink reports HDCP support and > either the source or sink lacks HDCP 2.x. Treat a missing callback, probe > failure, or > no reported HDCP support as unknown and non-fatal. > > Use the cached result to reject unsupported Content Type 1 requests with - > EOPNOTSUPP in intel_hdcp_atomic_check(), and propagate the error through the > digital connector atomic check. This keeps remote AUX transactions out of the > atomic check path. > > Signed-off-by: George D. Sworo <[email protected]> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > b/drivers/gpu/drm/i915/display/intel_atomic.c > index b66c2d4ba2b3..c6383e27e62b 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic.c > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > @@ -123,8 +123,12 @@ int intel_digital_connector_atomic_check(struct > drm_connector *conn, > struct intel_digital_connector_state *old_conn_state = > to_intel_digital_connector_state(old_state); > struct drm_crtc_state *crtc_state; > + /* Propagate HDCP capability failures during atomic validation. */ > + int ret; > > - intel_hdcp_atomic_check(conn, old_state, new_state); > + ret = intel_hdcp_atomic_check(conn, old_state, new_state); > + if (ret) > + return ret; > > if (!new_state->crtc) > return 0; > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 79f30660c2b6..1c169849e502 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -605,6 +605,8 @@ struct intel_connector { > struct { > struct drm_dp_mst_port *port; > struct intel_dp *dp; > + /* Used to reject unsupported HDCP Type1 requests during > atomic check. */ > + bool type1_unsupported; > } mst; > > struct { > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 7023619aaaba..da54699fb121 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -1838,6 +1838,7 @@ mst_topology_add_connector(struct > drm_dp_mst_topology_mgr *mgr, > connector->sync_state = intel_dp_connector_sync_state; > connector->mst.dp = intel_dp; > connector->mst.port = port; > + connector->mst.type1_unsupported = false; > drm_dp_mst_get_port_malloc(port); > > ret = drm_connector_dynamic_init(display->drm, &connector->base, > &mst_connector_funcs, diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > index e56df337dc6d..b6f28dc3e5c4 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -312,6 +312,39 @@ static void intel_hdcp_get_remote_capability(struct > intel_connector *connector, > *hdcp2_capable = false; > } > > +/* Determine whether atomic check should reject Type1 for this MST > +sink. */ static bool intel_hdcp_mst_type1_is_unsupported(struct > +intel_connector *connector, > + const struct intel_hdcp_shim *shim) { > + struct intel_hdcp *hdcp = &connector->hdcp; > + bool hdcp_capable = false, hdcp2_capable = false; > + int ret; > + > + if (!shim->get_remote_hdcp_capability) > + return false; > + > + ret = shim->get_remote_hdcp_capability(connector, &hdcp_capable, > + &hdcp2_capable); > + if (ret) > + return false; > + > + return hdcp_capable && (!hdcp->hdcp2_supported || !hdcp2_capable); } > + > +static void > +intel_hdcp_mst_update_type1_capability(struct intel_connector *connector, > + const struct intel_hdcp_shim > *shim) > { > + connector->mst.type1_unsupported = false; > + > + if (!shim || !shim->get_remote_hdcp_capability) > + return; > + > + connector->mst.type1_unsupported = > + intel_hdcp_mst_type1_is_unsupported(connector, shim); } > + > static bool intel_hdcp_in_use(struct intel_display *display, > enum transcoder cpu_transcoder, enum port port) { > @@ -2421,6 +2454,9 @@ int intel_hdcp_init(struct intel_connector *connector, > if (is_hdcp2_supported(display)) > intel_hdcp2_init(connector, dig_port, shim); > > + /* Cache the capability for the currently attached MST sink only. */ > + intel_hdcp_mst_update_type1_capability(connector, shim); > + > ret = drm_connector_attach_content_protection_property(&connector- > >base, > hdcp- > >hdcp2_supported); > if (ret) { > @@ -2652,6 +2688,8 @@ void intel_hdcp_cleanup(struct intel_connector > *connector) { > struct intel_hdcp *hdcp = &connector->hdcp; > > + connector->mst.type1_unsupported = false; > + > if (!hdcp->shim) > return; > > @@ -2684,10 +2722,11 @@ void intel_hdcp_cleanup(struct intel_connector > *connector) > mutex_unlock(&hdcp->mutex); > } > > -void intel_hdcp_atomic_check(struct drm_connector *connector, > - struct drm_connector_state *old_state, > - struct drm_connector_state *new_state) > +int intel_hdcp_atomic_check(struct drm_connector *connector, > + struct drm_connector_state *old_state, > + struct drm_connector_state *new_state) > { > + struct intel_connector *intel_connector = > +to_intel_connector(connector); > u64 old_cp = old_state->content_protection; > u64 new_cp = new_state->content_protection; > struct drm_crtc_state *crtc_state; > @@ -2700,7 +2739,20 @@ void intel_hdcp_atomic_check(struct drm_connector > *connector, > if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED) > new_state->content_protection = > > DRM_MODE_CONTENT_PROTECTION_DESIRED; > - return; > + return 0; > + } > + > + /* > + * Fail fast if userspace asks for Type1 but neither the platform nor > + * the downstream sink can do HDCP 2.x, instead of only discovering > + * this once intel_hdcp_enable() is reached. > + */ > + if (new_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED && > + new_state->hdcp_content_type == > DRM_MODE_HDCP_CONTENT_TYPE1 && > + intel_connector->mst.type1_unsupported) { > + drm_dbg_kms(connector->dev, > + "HDCP Type1 requested without HDCP 2.x > support\n"); > + return -EOPNOTSUPP; > } > > crtc_state = drm_atomic_get_new_crtc_state(new_state->state, > @@ -2725,10 +2777,12 @@ void intel_hdcp_atomic_check(struct > drm_connector *connector, > new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) { > if (old_state->hdcp_content_type == > new_state->hdcp_content_type) > - return; > + return 0; > } > > crtc_state->mode_changed = true; > + > + return 0; > } > > /* Handles the CP_IRQ raised from the DP HDCP sink */ diff --git > a/drivers/gpu/drm/i915/display/intel_hdcp.h > b/drivers/gpu/drm/i915/display/intel_hdcp.h > index efe86808e17e..34661b074784 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h > @@ -22,9 +22,9 @@ struct intel_hdcp_shim; struct seq_file; enum port; > > -void intel_hdcp_atomic_check(struct drm_connector *connector, > - struct drm_connector_state *old_state, > - struct drm_connector_state *new_state); > +int intel_hdcp_atomic_check(struct drm_connector *connector, > + struct drm_connector_state *old_state, > + struct drm_connector_state *new_state); > int intel_hdcp_init(struct intel_connector *connector, > struct intel_digital_port *dig_port, > const struct intel_hdcp_shim *hdcp_shim); > -- > 2.43.0 > > > Thanks for the review. > > You are right: the cached Type1 capability is only valid for the current MST > sink, > and it must not outlive a hotplug or topology change. I've updated the patch > to > clear the cached value when a new MST connector is created and when it is torn > down, and to refresh it only for the active sink. > > This keeps the atomic check AUX-free while avoiding stale results across MST > hotplug events.
