On Mon, Nov 08, 2021 at 07:55:00PM +0200, Ville Syrjälä wrote: > 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. > > I think involving state objects and the like is just going to make > it harder to share code between all drivers, if that is the goal. > Just a few tiny helpers I think is what would allow the broadest > reuse. I guess you could build additional midlayer on top of those > for some drivers if you wish. > > As for the bus_format stuff, that looks at the same time overkill, > and insufficiently documented. I guess its main purpose is to exactly > defines how some digtal bus works, which makes sense when you're > chaining a bunch of random chips together. But looks overly complicated > to me for defining what to output from a HDMI encoder. Looking at the > defines I wouldn't even know what to use for HDMI actually. All we > really want is RGB 4:4:4 vs. YCbCr 4:4:4 vs. YCbCr 4:2:2 vs. YCbCr 4:2:0. > Beyond that level of detail we don't care how each bit gets transferred > etc. Hence enum intel_output_format in i915. I was thinking about this some more, can we leverage struct hdmi_colorspace for this? Chances are it's already used by any driver that supports YCbCr output to setup the infoframes. Maxime
Attachment:
signature.asc
Description: PGP signature