Re: [PATCH 7/7] drm/vc4: hdmi: Defer pixel clock validation to HDMI helpers

[Dave Stevenson]

Hi Cristian

Sorry for coming in late on this one.

On Thu, 1 Oct 2026 at 01:27, Cristian Ciocaltea
<cristian.ciocaltea@xxxxxxxxxxxxx> wrote:
>
> drm_atomic_helper_connector_hdmi_check() rejects modes whose computed
> TMDS character rate exceeds the connector limit before invoking the
> driver's .tmds_char_rate_valid() hook.
>
> BCM2835 is capped at 162 MHz, slightly below the 165 MHz limit of HDMI
> 1.2.

Nothing I have says that BCM2835 is HDMI capped at 162MHz.
Working back through the history, originally the HSM clock was always
set to 163.7MHz and the pixel rate had to be >1% below that, hence
below 162MHz.

However that was reworked so that the HSM clock rate is now set to the
pixel clock * 101/100.
The HSM clock can run at 166.65MHz without issue, so a pixel clock of
165MHz is fine.

  Dave

> Set supported_tmds_char_rate in vc4_hdmi_connector_funcs_hdmi10 so
> that the limit inferred from supported_hdmi_ver is overridden by the
> lower hardware constraint.  All other chip variants rely on the standard
> HDMI 1.4/2.0 limits, so the default validation suffices.
>
> This allows vc4_hdmi_connector_clock_valid() to be simplified by
> dropping the now-redundant max_pixel_clock field from struct
> vc4_hdmi_variant.
>
> Reviewed-by: Maxime Ripard <mripard@xxxxxxxxxx>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 10 +---------
>  drivers/gpu/drm/vc4/vc4_hdmi.h |  3 ---
>  2 files changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 2451ff85e759..6d3db0d24c38 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1529,12 +1529,8 @@ vc4_hdmi_connector_clock_valid(const struct drm_connector *connector,
>                                const struct drm_display_mode *mode,
>                                unsigned long long clock)
>  {
> -       const struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
>         struct vc4_dev *vc4 = to_vc4_dev(connector->dev);
>
> -       if (clock > vc4_hdmi->variant->max_pixel_clock)
> -               return MODE_CLOCK_HIGH;
> -
>         if (!vc4->hvs->vc5_hdmi_enable_hdmi_20 && clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ)
>                 return MODE_CLOCK_HIGH;
>
> @@ -1579,6 +1575,7 @@ static const struct drm_connector_hdmi_funcs vc4_hdmi_connector_funcs_hdmi12 = {
>         VC4_HDMI_CONNECTOR_FUNCS_COMMON,
>         .max_bpc                = 8,
>         .supported_hdmi_ver     = HDMI_VERSION_1_2,
> +       .supported_tmds_char_rate = 162000000,
>  };
>
>  static const struct drm_connector_hdmi_funcs vc4_hdmi_connector_funcs_hdmi14 = {
> @@ -3185,7 +3182,6 @@ static const struct vc4_hdmi_variant bcm2835_variant = {
>         .encoder_type           = VC4_ENCODER_TYPE_HDMI0,
>         .debugfs_name           = "hdmi_regs",
>         .card_name              = "vc4-hdmi",
> -       .max_pixel_clock        = 162000000,
>         .registers              = vc4_hdmi_fields,
>         .num_registers          = ARRAY_SIZE(vc4_hdmi_fields),
>
> @@ -3205,7 +3201,6 @@ static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
>         .encoder_type           = VC4_ENCODER_TYPE_HDMI0,
>         .debugfs_name           = "hdmi0_regs",
>         .card_name              = "vc4-hdmi-0",
> -       .max_pixel_clock        = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ,
>         .registers              = vc5_hdmi_hdmi0_fields,
>         .num_registers          = ARRAY_SIZE(vc5_hdmi_hdmi0_fields),
>         .phy_lane_mapping       = {
> @@ -3234,7 +3229,6 @@ static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
>         .encoder_type           = VC4_ENCODER_TYPE_HDMI1,
>         .debugfs_name           = "hdmi1_regs",
>         .card_name              = "vc4-hdmi-1",
> -       .max_pixel_clock        = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ,
>         .registers              = vc5_hdmi_hdmi1_fields,
>         .num_registers          = ARRAY_SIZE(vc5_hdmi_hdmi1_fields),
>         .phy_lane_mapping       = {
> @@ -3263,7 +3257,6 @@ static const struct vc4_hdmi_variant bcm2712_hdmi0_variant = {
>         .encoder_type           = VC4_ENCODER_TYPE_HDMI0,
>         .debugfs_name           = "hdmi0_regs",
>         .card_name              = "vc4-hdmi-0",
> -       .max_pixel_clock        = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ,
>         .registers              = vc6_hdmi_hdmi0_fields,
>         .num_registers          = ARRAY_SIZE(vc6_hdmi_hdmi0_fields),
>         .phy_lane_mapping       = {
> @@ -3290,7 +3283,6 @@ static const struct vc4_hdmi_variant bcm2712_hdmi1_variant = {
>         .encoder_type           = VC4_ENCODER_TYPE_HDMI1,
>         .debugfs_name           = "hdmi1_regs",
>         .card_name              = "vc4-hdmi-1",
> -       .max_pixel_clock        = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ,
>         .registers              = vc6_hdmi_hdmi1_fields,
>         .num_registers          = ARRAY_SIZE(vc6_hdmi_hdmi1_fields),
>         .phy_lane_mapping       = {
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> index f6159c9e6144..61486e7b4ba2 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> @@ -29,9 +29,6 @@ struct vc4_hdmi_variant {
>         /* Filename to expose the registers in debugfs */
>         const char *debugfs_name;
>
> -       /* Maximum pixel clock supported by the controller (in Hz) */
> -       unsigned long long max_pixel_clock;
> -
>         /* List of the registers available on that variant */
>         const struct vc4_hdmi_register *registers;
>
>
> --
> 2.55.0
>