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

[Cristian Ciocaltea]

On 5/21/26 11:10 AM, Maxime Ripard wrote:
> 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 at 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.

In drm_scdc_start_scrambling() we call this function directly, the callback is
only meant to retry the call if the sink didn't set the scrambling for whatever
reason.

Or do you mean we shouldn't wait that long before retrying? vc4 uses 1s, I got
some mixed results in a few occasions and experimented with increased values,
but I'll revert or we can adjust to whatever you think it'd be more sensible.

>> +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.

The current non-NULL ctx call chain goes through drm_helper_probe_detect_ctx()
-> .detect_ctx -> ... -> drm_scdc_sync_status() -> drm_scdc_reset_crtc(), and
the connection mutex is already held by drm_helper_probe_detect() in that path. 

I'll make the helper safe under any caller by taking the connection_mutex via
the supplied ctx, since re-acquiring it with the same context when it is already
held should be a no-op.  Moreover, the racy crtc->state->active early out could
be dropped: when the CRTC is inactive, drm_atomic_helper_reset_crtc() should
result in a no-op commit anyway, so the check is at best an optimisation. 
This mirrors what drm_bridge_helper_reset_crtc() does.

>> +/**
>> + * 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.

When advertising DRM_BRIDGE_OP_HDMI_SCRAMBLER, drm_bridge_connector_init() will
fail if the scrambler hooks are not provided.  However, the check would be still
useful if one missed to set DRM_BRIDGE_OP_HDMI_SCRAMBLER before adding the
bridge.  I'll try to also handle non-bridge usecases - pls see the proposal
below.

>> +	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'm thinking to mirror the existing supported_formats / ycbcr_420_allowed
plumbing:

- Add a 'scrambling_supported' flag to struct drm_connector_hdmi and let
  drm_bridge_connector_init() to propagates it from the HDMI bridge before
  calling drmm_connector_hdmi_init().

- drmm_connector_hdmi_init() validates that the capability is consistent with
  the provided callbacks: when scrambling_supported is true, both
  scrambler_src_enable() and scrambler_src_disable() must be provided.

- Add a 'scrambling_needed' flag to struct drm_connector_hdmi_state to be set by
  hdmi_compute_clock() based on the TMDS character rate and the source/sink
  scrambling capabilities.

- Ensure hdmi_clock_valid() rejects modes having TMDS character rate exceeding
  340 MHz when either the source or the sink lacks the required SCDC scrambling
  capability.

> 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.

Sure, will do.  I've just ordered a RPI5 board, so I should be able to properly
validate this.

Thanks,
Cristian