Re: [PATCH v3] drm/i915/hdcp: Fail fast when HDCP Type 1 is unsupported via MST

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/i915/hdcp: stale type1_unsupported cache on MST hotplug
- [Medium] drm/i915/hdcp: bypass of Type 1 fast-fail validation
--

commit fb3d7b407b5fff7eb8ca9312ac9476d293a32b63
Author: George D. Sworo <george.d.sworo@intel.com>

drm/i915/hdcp: Fail fast when HDCP Type 1 is unsupported via MST

This commit attempts to fail early in the atomic check phase when userspace
requests HDCP Content Type 1 for a downstream MST sink that lacks HDCP 2.x
support. It queries and caches the sink's capability during initialization
and uses the cached result to reject unsupported configurations.

> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c

[ ... ]

> @@ -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);
> +

[Severity: High]
Could this cached capability become permanently stale after a hotplug event?

Since DRM MST connectors represent topological branch ports rather than the
sinks themselves, they persist across sink hotplug events. If a user connects
an HDCP 1.x monitor, type1_unsupported will be cached as true here in
intel_hdcp_init(). If the user then unplugs that monitor and connects an
HDCP 2.x capable monitor to the same port, the cached value is never updated.

Will this cause valid HDCP Type 1 requests to be permanently and incorrectly
rejected for the new monitor?

[ ... ]

> @@ -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) {

[Severity: Medium]
Does this condition inadvertently allow invalid configurations to bypass the
fast-fail validation?

If userspace has already enabled HDCP Type 0, the content_protection
property will be in the ENABLED state. If a subsequent atomic commit
changes only the hdcp_content_type to TYPE1, new_cp will inherit the
ENABLED state.

Since this check strictly requires new_cp to be DESIRED, it appears it will
evaluate to false, allowing the invalid request to proceed and fail
asynchronously during hardware enablement later. Should this check also
account for the ENABLED state?

> +		drm_dbg_kms(connector->dev,
> +			    "HDCP Type1 requested without HDCP 2.x support\n");
> +		return -EOPNOTSUPP;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925052703.2068117-1-george.d.sworo@intel.com?part=1