Hi Ville,

On Wed, Nov 03, 2021 at 08:05:16PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 03, 2021 at 01:02:11PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 02, 2021 at 03:59:32PM +0100, Maxime Ripard wrote:
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
> > >  		u32 max_tmds_clock = hf_vsdb[5] * 5000;
> > >  		struct drm_scdc *scdc = &hdmi->scdc;
> > >  
> > > -		if (max_tmds_clock > 340000) {
> > > +		if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> > >  			display->max_tmds_clock = max_tmds_clock;
> > >  			DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
> > >  				display->max_tmds_clock);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > index d2e61f6c6e08..0666203d52b7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > @@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
> > >  		if (scdc->scrambling.low_rates)
> > >  			pipe_config->hdmi_scrambling = true;
> > >  
> > > -		if (pipe_config->port_clock > 340000) {
> > > +		if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> > >  			pipe_config->hdmi_scrambling = true;
> > >  			pipe_config->hdmi_high_tmds_clock_ratio = true;
> > >  		}
> > 
> > All of that is HDMI 2.0 stuff. So this just makes it all super
> > confusing IMO. Nak.
> 
> So reading throgh HDMI 1.4 again it does specify 340 MHz as some kind
> of upper limit for the physical cable. But nowhere else is that number
> really mentioned AFAICS. HDMI 2.0 does talk quite a bit about the 340
> Mcsc limit in various places.
> 
> I wonder what people would think of a couple of helpers like:
> - drm_hdmi_{can,must}_use_scrambling()
> - drm_hdmi_is_high_tmds_clock_ratio()
> or something along those lines? At least with those the code would
> read decently and I wouldn't have to wonder what this HDMI 1.4 TMDS
> clock limit really is.

Patch 2 introduces something along those lines.

It doesn't cover everything though, we're using this define in vc4 to
limit the available modes in mode_valid on HDMI controllers not
4k-capable

We could probably do better on the name, but I still believe a define
like this would be valuable.

Maxime