Re: [PATCH v10 07/69] drm/connector: Add HDMI 2.0 scrambler infrastructure

On Fri, Jul 31, 2026 at 07:19:14PM +0300, Cristian Ciocaltea wrote:
> Add the connector-level infrastructure to support HDMI 2.0 scrambling:
> 
> - A drm_connector_hdmi_scrambler_supported() helper to report whether
>   the source supports the scrambling capability, based on the presence
>   of the newly introduced .scrambler_{enable|disable}() callbacks in
>   drm_connector_hdmi_funcs are mandatory
> - A scrambler_needed flag to be managed by the hdmi state helpers based
>   on the negotiated TMDS character rate and the source/sink scrambling
>   capabilities
> - A scrambler_enabled flag to track whether scrambling is currently
>   active
> - A delayed work item (scdc_work) to monitor sink-side scrambling status
>   and retry the setup if the sink resets it
> - A scdc_work_initialized flag to support lazy initialization of the
>   work item on the first scrambling enable and guard the teardown paths
> 
> These are intended to be used by SCDC scrambling helpers to coordinate
> scrambling setup and teardown between the source driver and the DRM
> core.
> 
> Tested-by: Maud Spierings <[email protected]>
> Tested-by: Diederik de Haas <[email protected]>  # NanoPC-T6 LTS, Rock 5B
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
>  drivers/gpu/drm/drm_connector.c | 31 ++++++++++++---
>  include/drm/drm_connector.h     | 83 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 109 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4721cdeafc84..a18410faf040 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -622,12 +622,29 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
>  	 * default with the actual controller capability. A value of zero keeps
>  	 * the limit inferred from supported_hdmi_ver.
>  	 */
> -	if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_2_0)
> +	if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_2_0) {
> +		if (!hdmi_funcs->scrambler_enable || !hdmi_funcs->scrambler_disable) {
> +			drm_err(dev, "Scrambler callbacks missing for HDMI 2.x\n");
> +			return -EINVAL;
> +		}
> +
>  		connector->hdmi.max_tmds_char_rate = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
> -	else if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_3)
> -		connector->hdmi.max_tmds_char_rate = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> -	else if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_0)
> -		connector->hdmi.max_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
> +	} else {
> +		/*
> +		 * Scrambler callbacks are only valid for connectors advertising
> +		 * HDMI 2.0 capability. drm_connector_hdmi_scrambler_supported()
> +		 * relies on their presence to report scrambling support.
> +		 */
> +		if (hdmi_funcs->scrambler_enable || hdmi_funcs->scrambler_disable) {
> +			drm_err(dev, "Scrambler callbacks unexpected for HDMI 1.x\n");
> +			return -EINVAL;
> +		}
> +
> +		if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_3)
> +			connector->hdmi.max_tmds_char_rate = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> +		else if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_0)
> +			connector->hdmi.max_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
> +	}

I'd put it into a separate test (possibly earlier). Merging both the
tmds rate default and the scrambler callbacks check makes it messier
than it would be if we had two separate tests.

>  	if (hdmi_funcs->supported_tmds_char_rate) {
>  		if (hdmi_funcs->supported_tmds_char_rate > connector->hdmi.max_tmds_char_rate) {
> @@ -635,6 +652,7 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
>  				connector->hdmi.max_tmds_char_rate);
>  			return -EINVAL;
>  		}
> +
>  		connector->hdmi.max_tmds_char_rate = hdmi_funcs->supported_tmds_char_rate;
>  	}
>  
> @@ -918,6 +936,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
>  		    DRM_CONNECTOR_REGISTERED))
>  		drm_connector_unregister(connector);
>  
> +	if (connector->hdmi.scdc_work_initialized)
> +		cancel_delayed_work_sync(&connector->hdmi.scdc_work);
> +
>  	platform_device_unregister(connector->hdmi_audio.codec_pdev);
>  
>  	if (connector->privacy_screen) {
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index a6de3e63b462..89a140d6f064 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -28,6 +28,7 @@
>  #include <linux/ctype.h>
>  #include <linux/hdmi.h>
>  #include <linux/notifier.h>
> +#include <linux/workqueue.h>
>  #include <drm/drm_mode_object.h>
>  #include <drm/drm_util.h>
>  #include <drm/drm_property.h>
> @@ -1131,6 +1132,17 @@ struct drm_connector_hdmi_state {
>  	 * @tmds_char_rate: TMDS Character Rate, in Hz.
>  	 */
>  	unsigned long long tmds_char_rate;
> +
> +	/**
> +	 * @scrambler_needed: Whether HDMI 2.0 SCDC scrambling is required
> +	 * for the negotiated mode/bpc/format.
> +	 *
> +	 * Computed by drm_atomic_helper_connector_hdmi_check() according to
> +	 * the HDMI 2.0 specification: scrambling is mandatory above a 340 MHz
> +	 * TMDS character rate. Optional scrambling at lower rates is
> +	 * deliberately not requested by the helper.
> +	 */
> +	bool scrambler_needed;
>  };
>  
>  /**
> @@ -1481,6 +1493,36 @@ struct drm_connector_hdmi_funcs {
>  	 */
>  	const struct drm_edid *(*read_edid)(struct drm_connector *connector);
>  
> +	/**
> +	 * @scrambler_enable:
> +	 *
> +	 * The callback is invoked via @drm_connector_hdmi_enable_scrambling
> +	 * during commit to setup SCDC scrambling and high TMDS clock ratio on
> +	 * the source side.
> +	 *
> +	 * The @scrambler_enable callback is mandatory if HDMI 2.0 is to be
> +	 * supported.
> +	 *
> +	 * Returns:
> +	 * 0 on success, a negative error code otherwise
> +	 */
> +	int (*scrambler_enable)(struct drm_connector *connector);
> +
> +	/**
> +	 * @scrambler_disable:
> +	 *
> +	 * The callback is invoked via @drm_connector_hdmi_disable_scrambling
> +	 * during commit to tear down SCDC scrambling and high TMDS clock ratio
> +	 * on the source side.
> +	 *
> +	 * The @scrambler_disable callback is mandatory if HDMI 2.0 is to be
> +	 * supported.
> +	 *
> +	 * Returns:
> +	 * 0 on success, a negative error code otherwise
> +	 */
> +	int (*scrambler_disable)(struct drm_connector *connector);
> +
>  	/**
>  	 * @avi:
>  	 *
> @@ -2111,6 +2153,25 @@ struct drm_connector_hdmi {
>  	 */
>  	unsigned long long max_tmds_char_rate;
>  
> +	/**
> +	 * @scrambler_enabled: Tracks whether HDMI 2.0 scrambler is currently enabled.
> +	 */
> +	bool scrambler_enabled;
> +
> +	/**
> +	 * @scdc_work: Work item currently used to monitor sink-side scrambling
> +	 * status and retry setup if the sink resets it.
> +	 */
> +	struct delayed_work scdc_work;
> +
> +	/**
> +	 * @scdc_work_initialized: Tracks whether @scdc_work has been set up via
> +	 * INIT_DELAYED_WORK(). The work item is initialized lazily on the first
> +	 * scrambling enable, so this guards the teardown paths against touching
> +	 * an uninitialized work item.
> +	 */
> +	bool scdc_work_initialized;
> +

Why should we track whether it's initialized or not? I'd always
initialize it, but only ever schedule something if we're using the
scrambler.

Maxime

-----BEGIN PGP SIGNATURE-----

iJUEABMJAB0WIQTkHFbLp4ejekA/qfgnX84Zoj2+dgUCaoWBbwAKCRAnX84Zoj2+
dlhmAX97RqIPyEXaFYpYG3l7TZ7Us8mqlWL1MjtrGnTqAM40X0uT+VOsdVcLawCQ
NS8yiHwBegO4ZITksuu0ciIrn6hDl8gqAvu/jj1nD+9BLfD0FYKvYonhxbmknMuw
4ggX/lyOBw==
=E2Mz
-----END PGP SIGNATURE-----