Hi Maxime,
Thanks for taking the time to review this!
On 6/12/26 3:06 PM, Maxime Ripard wrote:
> On Tue, Jun 02, 2026 at 01:44:02AM +0300, Cristian Ciocaltea wrote:
>> Add the connector-level infrastructure to support HDMI 2.0 scrambling:
>>
>> - A scrambler_supported flag to indicate whether the source supports the
>> scrambling capability, in which case 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) with an associated callback (scdc_cb)
>> to monitor sink-side scrambling status and retry the setup if the sink
>> resets it
>>
>> These are intended to be used by SCDC scrambling helpers to coordinate
>> scrambling setup and teardown between the source driver and the DRM
>> core.
>>
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>> drivers/gpu/drm/drm_connector.c | 18 +++++++++
>> include/drm/drm_connector.h | 81
>> +++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 99 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c
>> b/drivers/gpu/drm/drm_connector.c
>> index a5d13b92b665..526dc2931b8a 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -220,6 +220,19 @@ void drm_connector_free_work_fn(struct work_struct
>> *work)
>> }
>> }
>>
>> +static void drm_connector_hdmi_scdc_work(struct work_struct *work)
>> +{
>> + struct drm_connector *connector;
>> + struct drm_connector_hdmi *hdmi;
>> +
>> + hdmi = container_of(to_delayed_work(work), struct drm_connector_hdmi,
>> + scdc_work);
>> + connector = container_of(hdmi, struct drm_connector, hdmi);
>> +
>> + if (hdmi->scdc_cb)
>> + hdmi->scdc_cb(connector);
>> +}
>> +
>> static int drm_connector_init_only(struct drm_device *dev,
>> struct drm_connector *connector,
>> const struct drm_connector_funcs *funcs,
>> @@ -285,6 +298,7 @@ static int drm_connector_init_only(struct drm_device
>> *dev,
>> mutex_init(&connector->edid_override_mutex);
>> mutex_init(&connector->hdmi.infoframes.lock);
>> mutex_init(&connector->hdmi_audio.lock);
>> + INIT_DELAYED_WORK(&connector->hdmi.scdc_work,
>> drm_connector_hdmi_scdc_work);
>> connector->edid_blob_ptr = NULL;
>> connector->epoch_counter = 0;
>> connector->tile_blob_ptr = NULL;
>> @@ -606,6 +620,10 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
>> !hdmi_funcs->hdmi.write_infoframe)
>> return -EINVAL;
>>
>> + if (connector->hdmi.scrambler_supported &&
>> + (!hdmi_funcs->scrambler_enable || !hdmi_funcs->scrambler_disable))
>> + return -EINVAL;
>> +
>> ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
>> if (ret)
>> return ret;
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 529755c2e862..f1c5c15a6cce 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>
>> @@ -1057,6 +1058,19 @@ 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() from
>> + * @tmds_char_rate and the source/sink scrambling capabilities.
>> + *
>> + * Per HDMI 2.0, scrambling is mandatory above 340 MHz TMDS
>> + * character rate. Optional scrambling at lower rates is
>> + * deliberately not requested by the helper.
>> + */
>> + bool scrambler_needed;
>> };
>>
>> /**
>> @@ -1358,6 +1372,36 @@ struct drm_connector_hdmi_funcs {
>> */
>> const struct drm_edid *(*read_edid)(struct drm_connector *connector);
>>
>> + /**
>> + * @scrambler_enable:
>> + *
>> + * This callback is invoked through @drm_scdc_start_scrambling during
>> + * a commit to setup SCDC scrambling and high TMDS clock ratio on
>> + * 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:
>> + *
>> + * This callback is invoked through @drm_scdc_stop_scrambling during
>> + * a commit to disable SCDC scrambling and high TMDS clock ratio on
>> + * 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:
>> *
>> @@ -1960,6 +2004,43 @@ struct drm_connector_hdmi {
>> */
>> unsigned long supported_formats;
>>
>> + /**
>> + * @scrambler_supported: Indicates whether the HDMI controller
>> + * (source) supports HDMI 2.0 SCDC scrambling.
>> + *
>> + * When true, @drm_connector_hdmi_funcs.scrambler_enable and
>> + * @drm_connector_hdmi_funcs.scrambler_disable are mandatory.
>> + * This is enforced by drmm_connector_hdmi_init().
>> + *
>> + * For HDMI bridge based drivers using drm_bridge_connector_init(),
>> + * this is propagated automatically from bridges that set the
>> + * DRM_BRIDGE_OP_HDMI_SCRAMBLER flag in their &drm_bridge->ops.
>> + * Other drivers must set this field on @connector->hdmi before calling
>> + * drmm_connector_hdmi_init().
>> + */
>> + bool scrambler_supported;
>> +
>> + /**
>> + * @scrambler_enabled: Tracks whether HDMI 2.0 scrambler is currently
>> enabled.
>> + */
>> + bool scrambler_enabled;
>
> This is already in the state. Why do you need to copy it over here, and
This was supposed to be used exclusively by drm_scdc_{start,stop}_scrambling()
and drm_scdc_sync_status() helpers to control the work item and conditionally
trigger CRTC resets. However, it's now also set by vc4_hdmi_connector_init() to
force disable the scrambler in the vc4_crtc_disable_at_boot() execution path.
For this purpose, I think we should add a "force" argument to
drm_scdc_stop_scrambling(), or maybe a dedicated helper?!
> if you do, you'll need a lock.
Currently it's managed via READ_ONCE()/WRITE_ONCE(). Do you think locking is
really necessary?
Regards,
Cristian
