Hi Neil, On Friday, 23 November 2018 16:29:15 EET Neil Armstrong wrote: > On 23/11/2018 15:25, Laurent Pinchart wrote: > > On Friday, 23 November 2018 16:02:14 EET Neil Armstrong wrote: > >> Add support for SCDC Setup for TMDS Clock > 3.4GHz and enable TMDS > >> Scrambling when supported or mandatory. > >> > >> This patch also adds an helper to setup the control bit to support > >> the hight TMDS Bit Period/TMDS Clock-Period Ratio as required with > > > > s/hight/high/ ? > > Thanks for catching ! > > >> TMDS Clock > 3.4GHz for HDMI2.0 3840x2160@60/50 modes. > > > > Why do you need a helper for this, is there no way it could be handled > > internally ? > > I could, but all the platforms would also do it internally... seems better > to have common helper, no ? And it will be usable by the PHY models handler > in the dw-hdmi driver aswell. I meant internally in the dw-hdmi driver, not in the glue layer. > >> These changes were based on work done by Huicong Xu <xhc@xxxxxxxxxxxxxx> > >> and Nickey Yang <nickey.yang@xxxxxxxxxxxxxx> to support HDMI2.0 modes > >> on the Rockchip 4.4 BSP kernel at [1] > >> > >> [1] https://github.com/rockchip-linux/kernel/tree/release-4.4 > >> > >> Cc: Nickey Yang <nickey.yang@xxxxxxxxxxxxxx> > >> Cc: Huicong Xu <xhc@xxxxxxxxxxxxxx> > >> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > >> --- > >> > >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++++++++-- > >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 1 + > >> include/drm/bridge/dw_hdmi.h | 1 + > >> 3 files changed, 44 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index > >> 5971976284bf..523508af70b0 100644 > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c [snip] > >> @@ -1562,6 +1581,26 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, > >> vsync_len /= 2; > >> } > >> > >> + /* Scrambling Control */ > >> + if (hdmi_info->scdc.supported) { > >> + if (vmode->mpixelclock > 340000000 || > >> + hdmi_info->scdc.scrambling.low_rates) { > >> + drm_scdc_readb(&hdmi->i2c->adap, SCDC_SINK_VERSION, > >> + &bytes); > >> + drm_scdc_writeb(&hdmi->i2c->adap, SCDC_SOURCE_VERSION, > >> + bytes); > > > > Shouldn't the source version be min(sink version, highest supported source > > version) ? > > How should the "highest supported source version" be defined ? That's the highest version supported by the DW HDMI TX controller, and is an intrinsic property of the IP core. With the above code, a sink newer than the DW HDMI TX will incorrectly be told that the source supports the same version as the sink. > >> + drm_scdc_set_scrambling(&hdmi->i2c->adap, 1); > >> + hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ, > >> + HDMI_MC_SWRSTZ); > >> + hdmi_writeb(hdmi, 1, HDMI_FC_SCRAMBLER_CTRL); > >> + } else { > >> + hdmi_writeb(hdmi, 0, HDMI_FC_SCRAMBLER_CTRL); > >> + hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ, > >> + HDMI_MC_SWRSTZ); > >> + drm_scdc_set_scrambling(&hdmi->i2c->adap, 0); > >> + } > >> + } > >> + > >> /* Set up horizontal active pixel width */ > >> hdmi_writeb(hdmi, mode->hdisplay >> 8, HDMI_FC_INHACTV1); > >> hdmi_writeb(hdmi, mode->hdisplay, HDMI_FC_INHACTV0); [snip] -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel