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 <jonas@xxxxxxxxx> > > Configure the correct mtmdsclock for deep colors to prepare support > for 10, 12 & 16bit output. > > Signed-off-by: Jonas Karlman <jonas@xxxxxxxxx> > Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel