Re: [PATCH v7 11/30] drm/display: bridge_connector: Wire up HDMI 2.0 scrambler callbacks

On Fri, Jun 19, 2026 at 03:58:43PM +0200, Maxime Ripard wrote:
> On Fri, Jun 12, 2026 at 11:42:06PM +0300, Cristian Ciocaltea wrote:
> > 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(-)
> > >>
> > > 
> > > 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;
> >     }
> >     ...
> > }
> 
> That's what I initially had in mind, but it feels a bit over-the-top
> when looking at it. I think I'd really like something that drivers
> cannot forget about, screw up and/or mess with, so for example report
> HDMI 2.1 but force disable FRL support, or report HDMI 1.4 but support
> YUV420.
> 
> Adding more arguments to drm_connector_hdmi_init could work I guess, but
> it won't scale to everything we need. Expecting the callers to fill
> drm_connector_hdmi won't work either. So I somewhat think a get_caps
> like we discussed is the less bad solution, but I'm definitely open to
> suggestions.

Works for me. Then we can migrate some of the drm_connector_hdmi_init
args to the caps too.

> 
> > Not sure if max_tmds_char_rate should be listed as a capability, as we already
> > have the .tmds_char_rate_valid() callback.
> 
> I guess it would make sense to move it there and consolidate
> atomic_check / mode_valid checks, but I don't think it should be a
> prerequisite for this patch either.
> 
> Maxime



-- 
With best wishes
Dmitry

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip