[Intel-gfx] [PATCH] /drm/i915/hdmi: SCDC Scrambling enable without CTS mode
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Oct 10 11:41:50 UTC 2018
On Tue, Oct 09, 2018 at 04:30:45PM -0700, Clint Taylor wrote:
>
>
> On 10/08/2018 03:33 AM, Ville Syrjälä wrote:
> > On Fri, Oct 05, 2018 at 03:18:44PM -0700, clinton.a.taylor at intel.com wrote:
> >> From: Clint Taylor <clinton.a.taylor at intel.com>
> >>
> >> Setting the SCDC scrambling CTS mode causes HDMI Link Layer protocol tests
> >> HF1-12 and HF1-13 to fail. Added "Source Shall" entries from SCDC
> >> section before enabling scrambling.
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107895
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107896
> >> Signed-off-by: Clint Taylor <clinton.a.taylor at intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
> >> drivers/gpu/drm/i915/intel_hdmi.c | 8 ++++++++
> >> 2 files changed, 11 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> index 9e82281..a1b877f 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -1872,7 +1872,7 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
> >> temp |= TRANS_DDI_MODE_SELECT_DVI;
> >>
> >> if (crtc_state->hdmi_scrambling)
> >> - temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> >> + temp |= TRANS_DDI_HDMI_SCRAMBLING;
> >> if (crtc_state->hdmi_high_tmds_clock_ratio)
> >> temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> >> } else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
> >> @@ -3394,8 +3394,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >> if (intel_dig_port->infoframe_enabled(encoder, pipe_config))
> >> pipe_config->has_infoframe = true;
> >>
> >> - if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> >> - TRANS_DDI_HDMI_SCRAMBLING_MASK)
> >> + if ((temp & TRANS_DDI_HDMI_SCRAMBLING) ==
> >> + TRANS_DDI_HDMI_SCRAMBLING)
> > It's a single bit so 'temp & TRANS_DDI_HDMI_SCRAMBLING' will do.
> I will optimize the statement.
> > The spec isn't particularly clear about the CTS enable bit, but judging
> > from the name I guess you should only enable it when doing compliance
> > testing.
> Section 6.1.2.4.1 of the HDMI 2.1 specification contains some
> information about the CTS testing. In normal video transmission there is
> one SSCP transmitted per field. Of course the HDMI 2.0 CTS doesn't
> mention a need for per line SSCP's that this bit enables.
> >> pipe_config->hdmi_scrambling = true;
> >> if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> >> pipe_config->hdmi_high_tmds_clock_ratio = true;
> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >> index 454f570..d181d67 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >> @@ -2148,6 +2148,14 @@ bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
> >> connector->base.id, connector->name,
> >> yesno(scrambling), high_tmds_clock_ratio ? 40 : 10);
> >>
> >> + /* SCDC source version 10.4.1.2 */
> >> + if (drm_scdc_writeb(adapter, SCDC_SOURCE_VERSION, 0x01) < 0)
> >> + DRM_DEBUG_KMS("Unable to set SCDC Source Version register\n");
> > These look unrelated to the scrambler fix, so should be a separate patch.
> Agreed. There are more fixes on the way to correct enable/disable
> scrambling and the SCDC registers.
> > I don't think the spec section numbers are particularly helpful without
> > some indication as to which specification they refer to.
> Will add specification version information.
> >> +
> >> + /* Clear SCDC CONFIG_0 10.4.1.6 - RR_Enable Polling Only */
> >> + if (drm_scdc_writeb(adapter, SCDC_CONFIG_0, 0x00) < 0)
> >> + DRM_DEBUG_KMS("Unable to set SCDC CONFIG_0 register\n");
> > The spec is unfortunately vague about this stuff. It sort of implies
> > that polling is optional, but then it says that if either source or sink
> > doesn't set the RR bit then polling must be used, which to me seems
> > like polling is in fact mandatory.
> I'm experimenting with an HDMI hotplug handler specifically for HDMI 2.0
> sinks. I would prefer not to wake up every 250ms, powering up the DDC
> lines, doing a single byte read, and sleeping again.
> >
> > The spec allows for a max polling interval of 250 ms. I don't particulary
> > cherish waking up every 250ms whenver a HDMI 2.0 sink is hooked up. I
> > guess maybe we could limit it to times when the link is actually active,
> > but it still feels very wasteful to poll for something that should
> > basically never happen.
> >
> > This is rather like the eDP dpcd polling when hpd isn't support.
> > Except IIRC the eDP polling is actually opitonal and we haven't
> > bothered to implement it. But I'm not even sure whether there are
> > any machines w/o eDP hpd hooked up.
> >
> > Anyway, back to the patch itself. It seems to me that we should
> > probably be configuring this stuff during detect rather than
> > during crtc enable.
> The SCDC scramble_enable bit must be enabled within 100ms of sending
> scrambled data. Maybe compute_config would work.
Scramble enable we already configure during enable. That is fine.
>
> -Clint
>
>
> >> +
> >> /* Set TMDS bit clock ratio to 1/40 or 1/10, and enable/disable scrambling */
> >> return drm_scdc_set_high_tmds_clock_ratio(adapter,
> >> high_tmds_clock_ratio) &&
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list