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

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

 





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.

-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

_______________________________________________
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