On Mon, Nov 08, 2021 at 04:58:34PM +0100, Maxime Ripard wrote:
> 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?
Maybe as a default, it nothing is set. Also if nothing is set in the
connector then just assume 8bpc rgb, and drivers can be fixed as we go.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
