On 6/12/26 4:43 PM, Maxime Ripard wrote:
> On Tue, Jun 02, 2026 at 01:44:04AM +0300, Cristian Ciocaltea wrote:
>> +/**
>> + * 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->scrambler_supported) {
>> + drm_scdc_dbg(connector, "Scrambler not supported, bailing.\n");
>> + return -EINVAL;
>> + }
>> +
>> + 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;
>> + }
>> +
>> + drm_scdc_dbg(connector, "Enabling scrambling\n");
>> +
>> + ret = drm_scdc_readb(connector->ddc, SCDC_SINK_VERSION, &ver);
>> + if (ret) {
>> + drm_scdc_dbg(connector, "Failed to read SCDC_SINK_VERSION:
>> %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = drm_scdc_writeb(connector->ddc, SCDC_SOURCE_VERSION,
>> + min_t(u8, ver, SCDC_MAX_SOURCE_VERSION));
>> + if (ret) {
>> + drm_scdc_dbg(connector, "Failed to write SCDC_SOURCE_VERSION:
>> %d\n", ret);
>> + return ret;
>> + }
>> +
>> + hdmi->scdc_cb = drm_scdc_monitor_scrambler;
>> + WRITE_ONCE(hdmi->scrambler_enabled, true);
>> +
>> + ret = drm_scdc_try_scrambling_setup(connector);
>> + if (!ret)
>> + ret = hdmi->funcs->scrambler_enable(connector);
>> +
>> + if (ret) {
>> + WRITE_ONCE(hdmi->scrambler_enabled, false);
>> + cancel_delayed_work_sync(&hdmi->scdc_work);
>> + hdmi->scdc_cb = NULL;
>> +
>> + drm_scdc_set_scrambling(connector, false);
>> + drm_scdc_set_high_tmds_clock_ratio(connector, false);
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(drm_scdc_start_scrambling);
>
> This is also pretty significantly different than what vc4 implemented. I
> don't necessarily mind, but claiming that it's functionally equivalent
> isn't true. I think it would be a much better strategy to turn the vc4
> code into helpers as is because it's been merged 4 years ago and we know
> it's solid.
As explained in patch 25, I think there's been a slight misunderstanding of how
the CRTC reset logic was actually implemented.
> Then, if you want to add additional checks and constraints (like the
> SCDC_SINK_VERSION) that's fine, but it should come on top, and with its
> own explanation.
I'll recheck if there's anything that I might have missed from the original
implementation, while moving the additional changes to separate patch(es).
Thanks,
Cristian
