[PATCH v2 4/5] drm/vc4: hdmi: Enable the scrambler

[Maxime Ripard]

Hi Dave,

On Thu, Apr 01, 2021 at 12:30:45PM +0100, Dave Stevenson wrote:
> > Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> > ---
> >  drivers/gpu/drm/vc4/vc4_hdmi.c      | 56 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/vc4/vc4_hdmi_regs.h |  3 ++
> >  2 files changed, 59 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index 0924a1b9e186..530c83097b1a 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -35,6 +35,7 @@
> >  #include <drm/drm_edid.h>
> >  #include <drm/drm_probe_helper.h>
> >  #include <drm/drm_simple_kms_helper.h>
> > +#include <drm/drm_scdc_helper.h>
> >  #include <linux/clk.h>
> >  #include <linux/component.h>
> >  #include <linux/i2c.h>
> > @@ -76,6 +77,8 @@
> >  #define VC5_HDMI_VERTB_VSPO_SHIFT              16
> >  #define VC5_HDMI_VERTB_VSPO_MASK               VC4_MASK(29, 16)
> >
> > +#define VC5_HDMI_SCRAMBLER_CTL_ENABLE          BIT(0)
> > +
> >  #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT     8
> >  #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK      VC4_MASK(10, 8)
> >
> > @@ -457,6 +460,56 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder)
> >                 vc4_hdmi_set_audio_infoframe(encoder);
> >  }
> >
> > +static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
> > +                                        struct drm_display_mode *mode)
> > +{
> > +       struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
> > +       struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> > +       struct drm_display_info *display = &vc4_hdmi->connector.display_info;
> > +
> > +       if (!vc4_encoder->hdmi_monitor)
> > +               return false;
> > +
> > +       if (!display->hdmi.scdc.supported ||
> > +           !display->hdmi.scdc.scrambling.supported)
> > +               return false;
> > +
> 
> I think I made this comment last time, but possibly not very clearly.

I must have missed it then, sorry :/

> Up to this point in the function is whether the display/hdmi interface
> *supports* scrambling. The bit after is whether it is *required* to be
> enabled by the mode.

Thomas made a suggestion to move the mode clock check to a helper, so
we'll end up with essentially a helper about whether we support
scrambling or not and if the mode requires it.

> At the moment, if the display/interface supports scrambling but the
> mode doesn't need it, then the scrambling setup is never touched. That
> makes a few assumptions about the default settings.
> Boot with the firmware selecting 4k60 (ie scrambling on), but
> video=HDMI-1:1920x1080 in the kernel command line and you'll have a
> mess with scrambling enabled but not signalled.
> 
> I'd be happier if the display/interface says scrambling is supported
> then we always call drm_scdc_set_high_tmds_clock_ratio,
> drm_scdc_set_scrambling and set the VC5_HDMI_SCRAMBLER_CTL_ENABLE
> register bit appropriately for the mode. Feel free to disagree with me
> though.

I think part of it is due to our custom helpers never being called
currently during the boot process. Once that is fixed, the disable
helpers will be called and will disable the scrambling so we should be
good there.

This creates another issue though. That function takes the mode as the
argument and at boot time the mode pointer will be null. I think we can
work around it by assuming that we need to disable it at boot all the
time (and thus ignore the test if our pointer is null).

Would that work for you?

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210409/1ef92392/attachment-0001.sig>