Re: [PATCH] /drm/i915/hdmi: SCDC Scrambling enable without CTS mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux