Hi Maxime
On Fri, 9 Apr 2021 at 13:36, Maxime Ripard maxime@cerno.tech wrote:
Hi Dave,
On Thu, Apr 01, 2021 at 12:30:45PM +0100, Dave Stevenson wrote:
Signed-off-by: Maxime Ripard maxime@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?
Yes, that all sounds reasonable to me.
Dave