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@xxxxxxxxx wrote: > >> From: Clint Taylor <clinton.a.taylor@xxxxxxxxx> > >> > >> 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@xxxxxxxxx> > >> --- > >> 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@xxxxxxxxxxxxxxxxxxxxx > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
