On Wed, May 20, 2026 at 09:40:17AM +0200, Javier Martinez Canillas wrote: > Jani Nikula <[email protected]> writes: > > Hello Jani, > > > On Tue, 19 May 2026, Javier Martinez Canillas <[email protected]> wrote: > > [...] > > >> > >> +/* HDMI spec TMDS character rate limits (in Hz) */ > >> +#define DRM_HDMI_TMDS_CHAR_RATE_MIN 25000000 > >> +#define DRM_HDMI_TMDS_CHAR_RATE_MAX_1_0 165000000 > >> +#define DRM_HDMI_TMDS_CHAR_RATE_MAX_1_3 340000000 > >> +#define DRM_HDMI_TMDS_CHAR_RATE_MAX_2_0 600000000 > > > > Usually everything in DRM is in kHz, and Hz is the exception. > > > > That is correct but in this case these constants are to be used with the > HDMI helpers. Both struct drm_connector_hdmi_state.tmds_char_rate and > the struct drm_bridge_funcs.hdmi_tmds_char_rate_valid() callback expect > the TMDS char rate to be defined in Hz. > > If we define these in kHz, it means that drivers will have to * 1000 at > every call site.
As long as the unit is obvious from the name, I think we'll be fine, and we can even have both Hz and kHz defines if we want to. > > I'm also not sure the 1_0, 1_3, and 2_0 really help anyone reading the > > code. I won't remember what they mean in Hz or kHz, and I'll have to > > look them up every single time. > > > > I discussed this with Maxime before posting the patches since I wondered > the same. He suggested that the max TMDS character rate was linked to the > HDMI spec versions and that it would be more readable to name it using > the spec version rather than the resolution. As usual, it's both the spec and hardware capabilities. But some like the HDMI 1.3/1.4 max is used to know where to setup the scrambler for example, and is used everywhere. I still think it has value, but we don't have to force anyone to use them either. I'd prefer to use the DRM_HDMI_$SPEC_TMDS_CHAR_RATE_MAX_HZ though > The other naming I suggested was: > > /* HDMI spec TMDS character rate limits (in Hz) */ > #define DRM_HDMI_TMDS_CHAR_RATE_MIN 25000000 > #define DRM_HDMI_TMDS_CHAR_RATE_74_25MHZ 74250000 > #define DRM_HDMI_TMDS_CHAR_RATE_148_5MHZ 148500000 > #define DRM_HDMI_TMDS_CHAR_RATE_297MHZ 297000000 > #define DRM_HDMI_TMDS_CHAR_RATE_MAX_1_4 340000000 Honestly, I'm not sure if it's worth defining if we'll have the frequency in the define name. Maxime
signature.asc
Description: PGP signature
