On Thu, Jul 02, 2026 at 05:46:17PM +0300, Cristian Ciocaltea wrote:
> Add the connector-level infrastructure to support HDMI 2.0 scrambling:
>
> - A scrambler_supported flag to indicate whether the source supports the
> scrambling capability, in which case the newly introduced
> .scrambler_{enable|disable}() callbacks in drm_connector_hdmi_funcs
> are mandatory
Do we need a flag? What would it mean if the flag is set, but the
callbacks are not? Can we drop the flag and use the presence of the
callbacks as a way to identify that scrambler is enabled?
> - 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) with an associated callback (scdc_cb)
> to monitor sink-side scrambling status and retry the setup if the sink
> resets it
>
> 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 <[email protected]>
> ---
> drivers/gpu/drm/drm_connector.c | 26 ++++++++++++--
> include/drm/drm_connector.h | 77
> +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 100 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index c7ce6b7bd8b0..deecfc582f09 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)
> }
> }
>
> +static void drm_connector_hdmi_scdc_work(struct work_struct *work)
> +{
> + struct drm_connector *connector;
> + struct drm_connector_hdmi *hdmi;
> +
> + hdmi = container_of(to_delayed_work(work), struct drm_connector_hdmi,
> + scdc_work);
> + connector = container_of(hdmi, struct drm_connector, hdmi);
> +
> + if (hdmi->scdc_cb)
> + hdmi->scdc_cb(connector);
> +}
> +
> static int drm_connector_init_only(struct drm_device *dev,
> struct drm_connector *connector,
> const struct drm_connector_funcs *funcs,
> @@ -285,6 +298,7 @@ static int drm_connector_init_only(struct drm_device *dev,
> mutex_init(&connector->edid_override_mutex);
> mutex_init(&connector->hdmi.infoframes.lock);
> mutex_init(&connector->hdmi_audio.lock);
> + INIT_DELAYED_WORK(&connector->hdmi.scdc_work,
> drm_connector_hdmi_scdc_work);
> connector->edid_blob_ptr = NULL;
> connector->epoch_counter = 0;
> connector->tile_blob_ptr = NULL;
> @@ -624,12 +638,18 @@ int drmm_connector_hdmi_init_with_caps(struct
> drm_device *dev,
> * inferred limit with the actual controller capability. A value of
> * zero keeps the default limit inferred from supported_hdmi_ver.
> */
Here we need to check that the driver doesn't set only one callback of
the pair.
> - if (caps->supported_hdmi_ver >= HDMI_VERSION_2_0)
> + if (caps->supported_hdmi_ver >= HDMI_VERSION_2_0) {
> + if (!hdmi_funcs->scrambler_enable ||
> + !hdmi_funcs->scrambler_disable)
> + return -EINVAL;
> +
> + connector->hdmi.scrambler_supported = true;
> connector->hdmi.max_tmds_char_rate =
> HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
> - else if (caps->supported_hdmi_ver >= HDMI_VERSION_1_3)
> + } else if (caps->supported_hdmi_ver >= HDMI_VERSION_1_3) {
> connector->hdmi.max_tmds_char_rate =
> HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> - else if (caps->supported_hdmi_ver >= HDMI_VERSION_1_0)
> + } else if (caps->supported_hdmi_ver >= HDMI_VERSION_1_0) {
> connector->hdmi.max_tmds_char_rate =
> HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
> + }
>
> if (caps->max_tmds_char_rate) {
> if (caps->max_tmds_char_rate >
> connector->hdmi.max_tmds_char_rate)
> +
> + /** @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_connector_hdmi_enable_scrambling() helper.
No such helper. Also, please explain in the commit message, why do we
need to keep it configurable. Why the default is not enough?
> + */
> + void (*scdc_cb)(struct drm_connector *connector);
> +
> /**
> * @funcs: HDMI connector Control Functions
> */
>
> --
> 2.54.0
>
--
With best wishes
Dmitry
