Re: [PATCH v6 02/22] drm/connector: Add HDMI 2.0 scrambler infrastructure

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3244 bytes --]

On Fri, May 22, 2026 at 01:57:16PM +0300, Cristian Ciocaltea wrote:
> Hi Maxime,
> 
> On 5/21/26 10:52 AM, Maxime Ripard wrote:
> > Hi,
> > 
> > Thanks for working on this!
> 
> Thanks for your quick review! :-)
> 
> > 
> > On Wed, May 20, 2026 at 09:38:13PM +0300, Cristian Ciocaltea wrote:
> >> Add the connector-level infrastructure to support HDMI 2.0 scrambling:
> >>
> >> - .scrambler_src_{enable|disable}() callbacks in
> >>   drm_connector_hdmi_funcs for source-side scrambling control
> >> - A delayed work item (scdc_work) with an associated callback (scdc_cb)
> >>   for periodic monitoring of sink-side scrambling status
> >> - A scrambler_enabled flag to track whether scrambling is currently
> >>   active
> >>
> >> 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 <cristian.ciocaltea@collabora.com>
> >> ---
> >>  drivers/gpu/drm/drm_connector.c | 14 +++++++++++
> >>  include/drm/drm_connector.h     | 52 +++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 66 insertions(+)
> > 
> > So we would need kunit tests for this.
> 
> Ack.
> 
> > 
> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> >> index 3fa4d2082cd7..91e58362fbc0 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)
> >>  	}
> >>  }
> 
> [...]
> 
> >> +	/**
> >> +	 * @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_cb: Callback to be invoked as part of @scdc_work.
> >> +	 *
> >> +	 * Currently used to monitor sink-side scrambling status and retry
> >> +	 * setup if the sink resets it.
> >> +	 *
> >> +	 * This is assigned by the framework when making use of
> >> +	 * drm_scdc_start_scrambling() helper.
> >> +	 */
> >> +	void (*scdc_cb)(struct drm_connector *connector);
> >> +
> > 
> > I'm really not sure what the monitor thing is about. If we have setup
> > the scrambler at enable time, and we set it again on hotplugging, why
> > would we need to monitor anything?
> 
> This is similar to 257d36d493e9 ("drm/vc4: hdmi: Add a workqueue to set
> scrambling") in order to deal with some displays needing a retry on scrambling
> setup.

Ah, I see, sorry :)

It looks pretty generic though, should we have a helper for this then?

> > Also, scrambling is only relevant for HDMI 2.0. We need a way to expose
> > that somehow and make sure that HDMI 2.0 drivers actually have
> > scrambling setup.
> 
> I assumed drivers that do not reject the HDMI 2.0 rates in their
> .tmds_char_rate_valid() are implicitly expected to support scrambling. 
> Thinking again we could extend the framework to provide some additional checks,
> but let's discuss the topic on the SCDC patch.

HDMI 2.0 isn't just about scrambling though, YUV420 is another big
addition, and iirc you have more audio formats too.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]