Hi Neil and Jonas,

Thank you for the patch.

On Thu, Feb 06, 2020 at 08:18:24PM +0100, Neil Armstrong wrote:
> From: Jonas Karlman <jo...@kwiboo.se>
> 
> Configure the correct mtmdsclock for deep colors to prepare support
> for 10, 12 & 16bit output.
> 
> Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
> Signed-off-by: Neil Armstrong <narmstr...@baylibre.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 67fca439bbfb..9e0927d22db6 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1818,9 +1818,26 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  
>       dev_dbg(hdmi->dev, "final pixclk = %d\n", vmode->mpixelclock);

Nitpicking a bit, I would change

-       vmode->mtmdsclock = vmode->mpixelclock = mode->clock * 1000;
+       vmode->mpixelclock = mode->clock * 1000;

above, and here add

        vmode->mtmdsclock = vmode->mpixelclock;

to keep all mtmdsclock calculation in a single place.

> +     if (!hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format)) {
> +             switch (hdmi_bus_fmt_color_depth(
> +                             hdmi->hdmi_data.enc_out_bus_format)) {
> +             case 16:
> +                     vmode->mtmdsclock = (u64)vmode->mpixelclock * 2;

Both mpixelclock and mtmdsclock are unsigned int. Is the cast to u64
needed ?

On a separate but related note, what does the 'm' in tmdsclock stand for
? It seems to originate from the 'm' prefix for mpixelclock, which has
been there from the start. Unless there's a good reason for the prefix,
renaming mtmdsclock to tmds_clock (and handling the other fields in the
hdmi_vmode structure similarly) would increase clarity I think.

> +                     break;
> +             case 12:
> +                     vmode->mtmdsclock = (u64)vmode->mpixelclock * 3 / 2;
> +                     break;
> +             case 10:
> +                     vmode->mtmdsclock = (u64)vmode->mpixelclock * 5 / 4;
> +                     break;
> +             }
> +     }
> +
>       if (hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format))
>               vmode->mtmdsclock /= 2;
>  
> +     dev_dbg(hdmi->dev, "final tmdsclk = %d\n", vmode->mtmdsclock);

s/tmdsclk/tmdsclock/ to match the field name ?

> +
>       /* Set up HDMI_FC_INVIDCONF */
>       inv_val = (hdmi->hdmi_data.hdcp_enable ||
>                  (dw_hdmi_support_scdc(hdmi) &&

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to