On Fri, Nov 05, 2021 at 08:14:04PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 05, 2021 at 07:02:30PM +0100, Daniel Vetter wrote:
> > On Thu, Nov 04, 2021 at 05:37:21PM +0200, Ville Syrjälä wrote:
> > > On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote:
> > > > --- a/include/drm/drm_modes.h
> > > > +++ b/include/drm/drm_modes.h
> > > > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode)
> > > >  	return mode->flags & DRM_MODE_FLAG_3D_MASK;
> > > >  }
> > > >  
> > > > +/**
> > > > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling
> > > > + * @mode: DRM display mode
> > > > + *
> > > > + * Checks if a given display mode requires the scrambling to be enabled.
> > > > + *
> > > > + * Returns:
> > > > + * A boolean stating whether it's required or not.
> > > > + */
> > > > +static inline bool
> > > > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode)
> > > > +{
> > > > +	return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ;
> > > > +}
> > > 
> > > That's only correct for 8bpc. The actual limit is on the TMDS clock (or
> > > rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4
> > > magic for the actually transmitted TMDS clock).
> > 
> > Yeah we might need to add the bus format for the cable here too, to make
> > this complete.
> 
> I don't think we have a usable thing for that on the drm level, so
> would need to invent something. Oh, and the mode->clock vs. 
> mode->crtc_clock funny business also limits the usability of this
> approach. So probably just easiest to pass in the the driver calculated
> TMDS clock instead.

If we look at all (I think?) the existing users of scrambling in KMS as
of 5.15, only i915 seems to use crtc_clock (which, in retrospect, seems
to be the right thing to do), and only i915 and dw-hdmi use an output
format, i915 rolling its own, and dw-hdmi using the mbus formats.

I think using the mbus format here makes the most sense: i915 already is
rolling a whole bunch of other code, and we use the mbus defines for the
bridge format enumeration as well which is probably going to have some
interaction.

I'm not quite sure what to do next though. The whole point of that
series is to streamline as much as possible the scrambling and TMDS
ratio setup to avoid the duplication we already have in the drivers that
support it, every one using the mostly-the-same-but-slightly-different
logic to configure it.

The mode is fortunately stored in generic structures so it's easy to
make that decision. Having to take into account the output format
however makes it mandatory to move the bus format in the
drm_connector_state(?) structure too.

It's already in the bridge_state though, so should we take the final
bridge format as the cable format if it's tied to a bridge?

Maxime