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.

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.

Maxime

Attachment: signature.asc
Description: PGP signature

Reply via email to