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

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

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.

> 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.

Regards,
Cristian