On Thu, Nov 04, 2021 at 05:41:13PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 04, 2021 at 09:48:41AM +0100, Maxime Ripard wrote:
> > 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
>
> I wouldn't want to use this kind of define for those kinds of checks
> anyway. If the hardware has specific limits in what kind of clocks it
> can generate (or what it was validated for) IMO you should spell
> those out explicitly instead of assuming they happen to match
> some standard defined max value.
AFAIK, in the vc4 case, this is the hardware limit.
And there's other cases where it still seems to make sense to have that
define:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_edid.c#n4978
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_encoders.c#n385
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c#n1174
etc..
Maxime
Attachment:
signature.asc
Description: PGP signature
