Re: [PATCH v6 04/22] drm/display: scdc_helper: Add HDMI 2.0 scrambling management helpers

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 6904 bytes --]

On Wed, May 20, 2026 at 09:38:15PM +0300, Cristian Ciocaltea wrote:
> Add drm_scdc_start_scrambling(), drm_scdc_stop_scrambling() and
> drm_scdc_sync_status() helpers to manage the full lifecycle of HDMI 2.0
> SCDC scrambling on both source and sink sides.
> 
> drm_scdc_start_scrambling() configures SCDC scrambling and high TMDS
> clock ratio and starts a periodic work item that monitors the sink's
> SCDC scrambling status, retrying setup when the sink loses state.
> 
> drm_scdc_stop_scrambling() tears down scrambling on both sides and
> cancels the monitoring work.
> 
> drm_scdc_sync_status() triggers a CRTC reset on reconnection to restore
> SCDC state lost during sink disconnects within an active display
> pipeline.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_scdc_helper.c | 235 +++++++++++++++++++++++++++++-
>  include/drm/display/drm_scdc_helper.h     |   6 +-
>  2 files changed, 236 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> index cb6632346aad..5bacb886d373 100644
> --- a/drivers/gpu/drm/display/drm_scdc_helper.c
> +++ b/drivers/gpu/drm/display/drm_scdc_helper.c
> @@ -21,16 +21,22 @@
>   * DEALINGS IN THE SOFTWARE.
>   */
>  
> +#include <linux/delay.h>
>  #include <linux/export.h>
>  #include <linux/i2c.h>
> +#include <linux/minmax.h>
>  #include <linux/slab.h>
> -#include <linux/delay.h>
>  
> -#include <drm/display/drm_scdc_helper.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge_helper.h>
>  #include <drm/drm_connector.h>
> +#include <drm/drm_crtc.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_print.h>
>  
> +#include <drm/display/drm_scdc_helper.h>
> +
>  /**
>   * DOC: scdc helpers
>   *
> @@ -50,10 +56,14 @@
>   * has to track the connector status changes using interrupts and
>   * restore the SCDC status. The typical solution for this is to trigger an
>   * empty modeset in drm_connector_helper_funcs.detect_ctx(), like what vc4 does
> - * in vc4_hdmi_reset_link().
> + * in vc4_hdmi_reset_link(). Alternatively, use the HDMI connector framework
> + * which ensures drm_scdc_sync_status() is called in the context of
> + * drm_atomic_helper_connector_hdmi_hotplug_ctx().
>   */
>  
> -#define SCDC_I2C_SLAVE_ADDRESS 0x54
> +#define SCDC_I2C_SLAVE_ADDRESS		0x54
> +#define SCDC_MAX_SOURCE_VERSION		0x1
> +#define SCDC_STATUS_POLL_DELAY_MS	3000
>  
>  #define drm_scdc_dbg(connector, fmt, ...)					\
>  	drm_dbg_kms((connector)->dev, "[CONNECTOR:%d:%s] " fmt,			\
> @@ -270,3 +280,220 @@ bool drm_scdc_set_high_tmds_clock_ratio(struct drm_connector *connector,
>  	return true;
>  }
>  EXPORT_SYMBOL(drm_scdc_set_high_tmds_clock_ratio);
> +
> +static int drm_scdc_setup_scrambler(struct drm_connector *connector)
> +{
> +	bool done;
> +
> +	done = drm_scdc_set_high_tmds_clock_ratio(connector, true);
> +	if (!done)
> +		return -EIO;
> +
> +	done = drm_scdc_set_scrambling(connector, true);
> +	if (!done)
> +		return -EIO;
> +
> +	schedule_delayed_work(&connector->hdmi.scdc_work,
> +			      msecs_to_jiffies(SCDC_STATUS_POLL_DELAY_MS));
> +	return 0;
> +}

There's a very tight deadline in the spec to enable the scrambler
relative to the video. Debouncing (I assume?) for three seconds break
it. Drivers might not be able to do better, but it's really not
something we should entertain at the framework level and we should call
the callback straight-away.

> +static void drm_scdc_monitor_scrambler(struct drm_connector *connector)
> +{
> +	if (READ_ONCE(connector->hdmi.scrambler_enabled) &&
> +	    !drm_scdc_get_scrambling_status(connector))
> +		drm_scdc_setup_scrambler(connector);
> +}
> +
> +static int drm_scdc_reset_crtc(struct drm_connector *connector,
> +			       struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_crtc *crtc;
> +	u8 config;
> +	int ret;
> +
> +	if (!ctx || !connector->state)
> +		return 0;
> +
> +	crtc = connector->state->crtc;
> +	if (!crtc || !crtc->state || !crtc->state->active)
> +		return 0;
> +
> +	ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
> +	if (ret) {
> +		drm_scdc_dbg(connector, "Failed to read TMDS config: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if ((config & SCDC_SCRAMBLING_ENABLE) &&
> +	    (config & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40))
> +		return 0;
> +
> +	/*
> +	 * Reset the CRTC to suspend TMDS transmission, conforming to HDMI 2.0
> +	 * spec which requires scrambled data not to be sent before the sink is
> +	 * configured, and TMDS clock to be suspended while changing the clock
> +	 * ratio.  The disable/re-enable cycle triggered by the reset should
> +	 * call drm_scdc_start_scrambling() during re-enable, properly
> +	 * configuring the sink before data transmission resumes.
> +	 */
> +
> +	drm_scdc_dbg(connector, "Resetting CRTC to restore SCDC status\n");
> +
> +	ret = drm_atomic_helper_reset_crtc(crtc, ctx);
> +	if (ret && ret != -EDEADLOCK)
> +		drm_scdc_dbg(connector, "Failed to reset CRTC: %d\n", ret);
> +
> +	return ret;
> +}

locking is a bit more involved than that, I'd suggest to take
vc4_hdmi_reset_link() and turn it into a helper.

> +/**
> + * drm_scdc_start_scrambling - activate scrambling and monitor SCDC status
> + * @connector: connector
> + *
> + * Enables scrambling and high TMDS clock ratio on both source and sink sides.
> + * Additionally, use a delayed work item to monitor the scrambling status on
> + * the sink side and retry the operation, as some displays refuse to set the
> + * scrambling bit right away.
> + *
> + * Returns:
> + * Zero if scrambling is set successfully, an error code otherwise.
> + */
> +int drm_scdc_start_scrambling(struct drm_connector *connector)
> +{
> +	struct drm_display_info *info = &connector->display_info;
> +	struct drm_connector_hdmi *hdmi = &connector->hdmi;
> +	int ret;
> +	u8 ver;
> +
> +	if (!hdmi->funcs ||
> +	    !hdmi->funcs->scrambler_src_enable ||
> +	    !hdmi->funcs->scrambler_src_disable) {
> +		drm_scdc_dbg(connector, "Function not implemented, bailing.\n");
> +		return -EINVAL;
> +	}

This case shouldn't be a thing. The driver must not probe if it's not set.

> +	if (!info->is_hdmi ||
> +	    !info->hdmi.scdc.supported ||
> +	    !info->hdmi.scdc.scrambling.supported) {
> +		drm_scdc_dbg(connector, "Sink doesn't support scrambling.\n");
> +		return -EINVAL;
> +	}

You should also compute whether we actually need to enable the scrambler
here, based on the mode, format and bpc.

I'd also like to see what a !bridge user of this would look like. The
vc4 driver has all the infrastructure you need already, so converting it
to it would be great.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]