On 6/12/26 11:52 AM, Maxime Ripard wrote:
> On Tue, Jun 02, 2026 at 01:44:11AM +0300, Cristian Ciocaltea wrote:
>> Connect the bridge connector's .scrambler_{enable|disable} callbacks to
>> the underlying bridge's .hdmi_scrambler_{enable|disable} funcs when
>> DRM_BRIDGE_OP_HDMI_SCRAMBLER is advertised.
>>
>> This completes the bridge connector plumbing so that the SCDC
>> scrambling helpers can control source-side scrambling through the
>> bridge chain.
>>
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>> drivers/gpu/drm/display/drm_bridge_connector.c | 41
>> +++++++++++++++++++++++++-
>> 1 file changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c
>> b/drivers/gpu/drm/display/drm_bridge_connector.c
>> index 9d21b1b57b0d..d048ab49eade 100644
>> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
>> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
>> @@ -555,6 +555,32 @@ static int
>> drm_bridge_connector_write_spd_infoframe(struct drm_connector *connec
>> return bridge->funcs->hdmi_write_spd_infoframe(bridge, buffer, len);
>> }
>>
>> +static int drm_bridge_connector_scrambler_enable(struct drm_connector
>> *connector)
>> +{
>> + struct drm_bridge_connector *bridge_connector =
>> + to_drm_bridge_connector(connector);
>> + struct drm_bridge *bridge;
>> +
>> + bridge = bridge_connector->bridge_hdmi;
>> + if (!bridge)
>> + return -EINVAL;
>> +
>> + return bridge->funcs->hdmi_scrambler_enable(bridge);
>> +}
>> +
>> +static int drm_bridge_connector_scrambler_disable(struct drm_connector
>> *connector)
>> +{
>> + struct drm_bridge_connector *bridge_connector =
>> + to_drm_bridge_connector(connector);
>> + struct drm_bridge *bridge;
>> +
>> + bridge = bridge_connector->bridge_hdmi;
>> + if (!bridge)
>> + return -EINVAL;
>> +
>> + return bridge->funcs->hdmi_scrambler_disable(bridge);
>> +}
>> +
>> static const struct drm_edid *
>> drm_bridge_connector_read_edid(struct drm_connector *connector)
>> {
>> @@ -580,7 +606,7 @@ static const struct drm_connector_hdmi_funcs
>> drm_bridge_connector_hdmi_funcs = {
>> .clear_infoframe = drm_bridge_connector_clear_hdmi_infoframe,
>> .write_infoframe = drm_bridge_connector_write_hdmi_infoframe,
>> },
>> - /* audio, hdr_drm and spd are set dynamically during init */
>> + /* scrambler, audio, hdr_drm and spd are set dynamically during init */
>> };
>>
>> static const struct drm_connector_infoframe_funcs
>> drm_bridge_connector_hdmi_audio_infoframe = {
>> @@ -886,6 +912,11 @@ struct drm_connector *drm_bridge_connector_init(struct
>> drm_device *drm,
>> !bridge->funcs->hdmi_clear_spd_infoframe))
>> return ERR_PTR(-EINVAL);
>>
>> + if (bridge->ops & DRM_BRIDGE_OP_HDMI_SCRAMBLER &&
>> + (!bridge->funcs->hdmi_scrambler_enable ||
>> + !bridge->funcs->hdmi_scrambler_disable))
>> + return ERR_PTR(-EINVAL);
>> +
>> bridge_connector->bridge_hdmi = drm_bridge_get(bridge);
>>
>> if (bridge->supported_formats)
>> @@ -990,6 +1021,14 @@ struct drm_connector *drm_bridge_connector_init(struct
>> drm_device *drm,
>> bridge_connector->hdmi_funcs.spd =
>> drm_bridge_connector_hdmi_spd_infoframe;
>>
>> + if (bridge_connector->bridge_hdmi->ops &
>> DRM_BRIDGE_OP_HDMI_SCRAMBLER) {
>> + bridge_connector->hdmi_funcs.scrambler_enable =
>> + drm_bridge_connector_scrambler_enable;
>> + bridge_connector->hdmi_funcs.scrambler_disable =
>> + drm_bridge_connector_scrambler_disable;
>> + connector->hdmi.scrambler_supported = true;
>> + }
>> +
>
> I think we're taking this backwards. The scrambler support isn't
> optional: either the controller supports HDMI < 2.0, and then it doesn't
> exist, or it supports >= 2.0 and then it's mandatory.
>
> You're considering it optional here, when it's never actually optional
> (unlike YUV420 for example)
>
> I still think we should list, somehow, the capabilities of the
> controller to the helpers, like max tmds rate supported, formats, etc.
> We've so far put everything as an argument to drmm_connector_hdmi_init
> but it becomes a bit overloaded, and I wonder if introducing a callback
> wouldn't solve this, kind of like what we have for planes and formats.
What about introducing something like:
struct drm_connector_hdmi_caps {
...
unsigned int supported_formats;
enum hdmi_version supported_hdmi_ver;
};
struct drm_connector_hdmi_funcs {
...
int (*get_caps)(struct drm_connector *connector,
struct drm_connector_hdmi_caps *caps);
...
};
int drmm_connector_hdmi_init(struct drm_device *dev, ...)
{
...
if (hdmi_funcs->get_caps) {
struct drm_connector_hdmi_caps caps = { };
ret = hdmi_funcs->get_caps(connector, &caps);
if (ret)
return ret;
connector->hdmi.supported_formats = caps.supported_formats;
...
if (caps.supported_hdmi_ver > HDMI_2_0)
connector->hdmi.frl_supported = true;
else if (caps.supported_hdmi_ver == HDMI_2_0)
connector->hdmi.scrambler_supported = true;
}
...
}
Not sure if max_tmds_char_rate should be listed as a capability, as we already
have the .tmds_char_rate_valid() callback.
Cristian
