[PATCH v10 07/69] drm/connector: Add HDMI 2.0 scrambler infrastructure
Maxime Ripard
mripard at kernel.org
Thu Aug 20 09:56:18 PDT 2026
On Wed, Aug 19, 2026 at 10:33:04PM +0300, Cristian Ciocaltea wrote:
> On 8/19/26 1:12 PM, Maxime Ripard wrote:
> > On Fri, Jul 31, 2026 at 07:19:14PM +0300, Cristian Ciocaltea wrote:
> >> Add the connector-level infrastructure to support HDMI 2.0 scrambling:
> >>
> >> - A drm_connector_hdmi_scrambler_supported() helper to report whether
> >> the source supports the scrambling capability, based on the presence
> >> of 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) to monitor sink-side scrambling status
> >> and retry the setup if the sink resets it
> >> - A scdc_work_initialized flag to support lazy initialization of the
> >> work item on the first scrambling enable and guard the teardown paths
> >>
> >> These are intended to be used by SCDC scrambling helpers to coordinate
> >> scrambling setup and teardown between the source driver and the DRM
> >> core.
> >>
> >> Tested-by: Maud Spierings <maud_spierings at hotmail.com>
> >> Tested-by: Diederik de Haas <diederik at cknow-tech.com> # NanoPC-T6 LTS, Rock 5B
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea at collabora.com>
> >> ---
> >> drivers/gpu/drm/drm_connector.c | 31 ++++++++++++---
> >> include/drm/drm_connector.h | 83 +++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 109 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> >> index 4721cdeafc84..a18410faf040 100644
> >> --- a/drivers/gpu/drm/drm_connector.c
> >> +++ b/drivers/gpu/drm/drm_connector.c
> >> @@ -622,12 +622,29 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
> >> * default with the actual controller capability. A value of zero keeps
> >> * the limit inferred from supported_hdmi_ver.
> >> */
> >> - if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_2_0)
> >> + if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_2_0) {
> >> + if (!hdmi_funcs->scrambler_enable || !hdmi_funcs->scrambler_disable) {
> >> + drm_err(dev, "Scrambler callbacks missing for HDMI 2.x\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> connector->hdmi.max_tmds_char_rate = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
> >> - else if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_3)
> >> - connector->hdmi.max_tmds_char_rate = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> >> - else if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_0)
> >> - connector->hdmi.max_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
> >> + } else {
> >> + /*
> >> + * Scrambler callbacks are only valid for connectors advertising
> >> + * HDMI 2.0 capability. drm_connector_hdmi_scrambler_supported()
> >> + * relies on their presence to report scrambling support.
> >> + */
> >> + if (hdmi_funcs->scrambler_enable || hdmi_funcs->scrambler_disable) {
> >> + drm_err(dev, "Scrambler callbacks unexpected for HDMI 1.x\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_3)
> >> + connector->hdmi.max_tmds_char_rate = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> >> + else if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_0)
> >> + connector->hdmi.max_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
> >> + }
> >
> > I'd put it into a separate test (possibly earlier). Merging both the
> > tmds rate default and the scrambler callbacks check makes it messier
> > than it would be if we had two separate tests.
>
> Ack. How about the following?
>
> if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_2_0)
> connector->hdmi.max_tmds_char_rate = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
> else if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_3)
> connector->hdmi.max_tmds_char_rate = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> else if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_1_0)
> connector->hdmi.max_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
>
> if (hdmi_funcs->supported_tmds_char_rate) {
> if (hdmi_funcs->supported_tmds_char_rate > connector->hdmi.max_tmds_char_rate) {
> drm_err(dev, "Enforced max_tmds_char_rate exceeds %llu spec limit\n",
> connector->hdmi.max_tmds_char_rate);
> return -EINVAL;
> }
>
> connector->hdmi.max_tmds_char_rate = hdmi_funcs->supported_tmds_char_rate;
> }
>
> if (hdmi_funcs->supported_hdmi_ver >= HDMI_VERSION_2_0) {
> if (!hdmi_funcs->scrambler_enable || !hdmi_funcs->scrambler_disable) {
> drm_err(dev, "Scrambler callbacks missing for HDMI 2.x\n");
> return -EINVAL;
> }
> } else {
> /*
> * Scrambler callbacks are only valid for connectors advertising
> * HDMI 2.0 capability. drm_connector_hdmi_scrambler_supported()
> * relies on their presence to report scrambling support.
> */
> if (hdmi_funcs->scrambler_enable || hdmi_funcs->scrambler_disable) {
> drm_err(dev, "Scrambler callbacks unexpected for HDMI 1.x\n");
> return -EINVAL;
> }
> }
I don't think we need the else clause at all. It's not valid, but it's
also not creating any issue.
I'd move that second check earlier together with the infoframe callbacks
checks and so on too.
>
> >> if (hdmi_funcs->supported_tmds_char_rate) {
> >> if (hdmi_funcs->supported_tmds_char_rate > connector->hdmi.max_tmds_char_rate) {
> >> @@ -635,6 +652,7 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
> >> connector->hdmi.max_tmds_char_rate);
> >> return -EINVAL;
> >> }
> >> +
> >> connector->hdmi.max_tmds_char_rate = hdmi_funcs->supported_tmds_char_rate;
> >> }
>
> [...]
>
> >> + /**
> >> + * @scdc_work: Work item currently used to monitor sink-side scrambling
> >> + * status and retry setup if the sink resets it.
> >> + */
> >> + struct delayed_work scdc_work;
> >> +
> >> + /**
> >> + * @scdc_work_initialized: Tracks whether @scdc_work has been set up via
> >> + * INIT_DELAYED_WORK(). The work item is initialized lazily on the first
> >> + * scrambling enable, so this guards the teardown paths against touching
> >> + * an uninitialized work item.
> >> + */
> >> + bool scdc_work_initialized;
> >> +
> >
> > Why should we track whether it's initialized or not? I'd always
> > initialize it, but only ever schedule something if we're using the
> > scrambler.
>
> Having this initialized in the connector would lead to a module dependency
> cycle.
>
> Currently the work function lives in drm_hdmi_helper.c, which is built into
> drm_display_helper module:
>
> static void drm_connector_hdmi_scdc_work(struct work_struct *work)
> {
> [...]
> if (READ_ONCE(connector->hdmi.scrambler_enabled) &&
> !drm_scdc_get_scrambling_status(connector))
> drm_connector_hdmi_try_scrambling_setup(connector);
> [...]
> }
>
> int drm_connector_hdmi_enable_scrambling(struct drm_connector *connector,
> const struct drm_connector_state *conn_state)
> {
>
> [...]
> if (!hdmi->scdc_work_initialized) {
> INIT_DELAYED_WORK(&hdmi->scdc_work,
> drm_connector_hdmi_scdc_work);
> hdmi->scdc_work_initialized = true;
> }
> [...]
> }
>
> If we move INIT_DELAYED_WORK() into the connector (i.e. in drm.ko), the work
> function has to be reachable from there. The following attempts to accomplish
> that would fail:
>
> - Keep the work function in drm_hdmi_helper.c and export it from
> drm_display_helper.
>
> - Move the work function into drm_connector.c and export
> drm_connector_hdmi_try_scrambling_setup(), or a wrapper function, from
> drm_display_helper.
An alternative could be to move drm_connector_hdmi_init to
drm_hdmi_helper.c, no?
But yeah, if we can't let's keep it like that
Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 273 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20260820/4823fdac/attachment.sig>
More information about the Linux-rockchip
mailing list