Re: [PATCH 1/8] drm/display: hdmi: Add common TMDS character rate constants

[Jani Nikula]

On Wed, 20 May 2026, Javier Martinez Canillas <javierm@xxxxxxxxxx> wrote:
> Jani Nikula <jani.nikula@xxxxxxxxx> writes:
>
> Hello Jani,
>
>> On Tue, 19 May 2026, Javier Martinez Canillas <javierm@xxxxxxxxxx> 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.

I guess I'm surprised those things were defined in terms of Hz then.

> If we define these in kHz, it means that drivers will have to * 1000 at
> every call site.

Yeah, not ideal.

>> 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.

Frankly I've never thought of the limits in terms of the spec versions,
but maybe it's just me. *shrug*

BR,
Jani.

>
> 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
>
>> BR,
>> Jani.
>>

-- 
Jani Nikula, Intel