On Thu, Jul 16, 2026 at 06:07:09PM +0300, Cristian Ciocaltea wrote:
> On 7/16/26 4:50 PM, Maxime Ripard wrote:
> > On Wed, Jul 15, 2026 at 01:46:12PM +0300, Cristian Ciocaltea wrote:
> >> On 7/15/26 11:55 AM, Maxime Ripard wrote:
> >>> On Mon, Jul 13, 2026 at 01:23:00PM +0300, Cristian Ciocaltea wrote:
> >>>> On 7/13/26 11:50 AM, Maxime Ripard wrote:
> >>>>> On Thu, Jul 09, 2026 at 10:25:54PM +0300, Cristian Ciocaltea wrote:
> >>>>>> On 7/3/26 11:54 PM, Cristian Ciocaltea wrote:
> >>>>>>> On 7/3/26 5:34 PM, Dmitry Baryshkov wrote:
> >>>>>>>> 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?
> >>>>>>>
> >>>>>>> The flag is intended to be set only within
> >>>>>>> drmm_connector_hdmi_init_with_caps()
> >>>>>>> when drivers advertise HDMI 2.x capability, in which case it also
> >>>>>>> ensures the
> >>>>>>> callbacks are provided.
> >>>>>>>
> >>>>>>> We could drop the flag and instead have the init helper clear the
> >>>>>>> callbacks if
> >>>>>>> they were provided for HDMI 1.x. This might slightly reduce code
> >>>>>>> readability,
> >>>>>>> as it relies on checking the presence of individual callbacks -
> >>>>>>> especially since
> >>>>>>> we plan to extend this further with HDMI 2.1 support, providing four
> >>>>>>> or five
> >>>>>>> additional FRL-specific callbacks.
> >>>>>>
> >>>>>> I tried to replace the flag with a helper that checks the presence of
> >>>>>> (one of)
> >>>>>> the callbacks, but it's not straightforward to unset those for
> >>>>>> non-HDMI 2.x
> >>>>>> cases since the hdmi_funcs argument is immutable.
> >>>>>
> >>>>> I'm not sure why we would need to unset them. If the driver states that
> >>>>> it support HDMI 2.0, then it needs to be there, if it doesn't, then who
> >>>>> cares? it's not going to be used. We can log a warning that it's
> >>>>> inconsistent I guess, but there's no need to actively remove it.
> >>>>
> >>>> I was trying to address the use case where drivers provide the scrambler
> >>>> callbacks despite not supporting HDMI 2.0.
> >>>
> >>> Scrambling got introduced with HDMI 2.0. That doesn't make sense, but
> >>> it's not a total deal breaker, it's just going to be here unused. Hence
> >>> why I was suggesting to put a warning there if you wanted to.
> >>>
> >>>> If we replace the scrambler_supported flag with a helper checking the
> >>>> presence of the scrambler callbacks, then we would need to ensure the
> >>>> callbacks do not exist in this case.
> >>>
> >>> Keep it simple:
> >>>
> >>> if (hdmi_version >= HDMI_VERSION_2_0)
> >>> if (funcs->scrambler_enable)
> >>> hdmi->scramblers_supported = true
> >>> else
> >>> return -EINVAL
> >>> else
> >>> drm_warn(warn, "Inconsistent HDMI version");
> >>>
> >>> We don't need anything more than that.
> >>
> >> I dropped the scrambler_supported flag and introduced a helper:
> >>
> >> static inline bool
> >> drm_connector_hdmi_scrambler_supported(struct drm_connector *connector)
> >> {
> >> return connector->hdmi.funcs && connector->hdmi.funcs->scrambler_enable;
> >
> > I'd add disable to that test
>
> If scrambler_enable is present, scrambler_disable is also present, as
> guaranteed
> by the init validation below. I assume you'd like to have it added
> regardless,
> to improve code readability, which is a fair point.
>
> >> }
> >>
> >> Therefore we need to ensure the callbacks are not set in the HDMI 1.x
> >> cases:
> >>
> >> int drmm_connector_hdmi_init_with_caps()
> >> {
> >> ...
> >> if (caps->supported_hdmi_ver >= HDMI_VERSION_2_0) {
> >> if (!hdmi_funcs->scrambler_enable ||
> >> !hdmi_funcs->scrambler_disable)
> >> return -EINVAL;
> >>
> >> connector->hdmi.max_tmds_char_rate =
> >> HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
> >> } else {
> >> /*
> >> * Scrambler callbacks are only valid for connectors advertising
> >> * HDMI 2.0 capability. drm_connector_hdmi_scrambler_supported()
> >> * relies on their presence to report scrambling support.
> >> */
> >> if (hdmi_funcs->scrambler_enable ||
> >> hdmi_funcs->scrambler_disable)
> >> return -EINVAL;
> >>
> >> 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) {
> >> connector->hdmi.max_tmds_char_rate =
> >> HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
> >> }
> >> }
> >
> > Drivers might have a lower limit than the max allowed by the spec. It
> > should be provided by the driver, possibly optionally with a fallback to
> > what the spec states?
>
> Yes, this has been already part of patch 2 (also added an error message to
> help
> with debugging):
>
> if (caps->max_tmds_char_rate) {
> if (caps->max_tmds_char_rate >
> connector->hdmi.max_tmds_char_rate) {
> drm_err(dev, "Enforced max_tmds_char_rate exceeds %llu
> spec limit\n",
> connector->hdmi.max_tmds_char_rate);
> return -EINVAL;
> }
> connector->hdmi.max_tmds_char_rate = caps->max_tmds_char_rate;
> }Ah right, my bad. Thanks! Maxime
signature.asc
Description: PGP signature
