Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail

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

 



Regards

Shashank


On 10/13/2018 12:58 AM, Ville Syrjälä wrote:
On Fri, Oct 12, 2018 at 10:17:57PM +0300, Ville Syrjälä wrote:
On Sat, Oct 13, 2018 at 12:26:57AM +0530, Sharma, Shashank wrote:
Regards

Shashank


On 10/13/2018 12:08 AM, Ville Syrjala wrote:
From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

The Parade LSPCON on KBL NUCs forgets to turn off scrambling/bit clock
rate when switching from a mode that needs them to a mode that does
not. This manifests as a "no signal" on my TV when I try to go from
4k to 1080p for example. Resetting the SCDC register bits with
i2cset is sufficient to restore the picture to the screen.

Here's the OUI/fw revision for the LSPCON chip in question:
DP branch: OUI 00-1c-f8 dev-ID 175IB0 HW-rev 1.0 SW-rev 7.32 quirks 0x0000

Asking users to poke at SCDC with i2cset is a bit much, so
let's work around this in the driver. We don't need to go all
out here and compute whether scrambling is needed or not as
LSPCON will do that itself. If scrambling is actually
required LSPCON does not forget to enable it.

Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx>
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
   drivers/gpu/drm/i915/intel_ddi.c  | 52 ++++++++++++++++++++++++++++---
   drivers/gpu/drm/i915/intel_drv.h  |  1 +
   drivers/gpu/drm/i915/intel_hdmi.c |  5 ++-
   3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 47960c92cbbf..ef502fc9add1 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2984,6 +2984,23 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
   		intel_ddi_pre_enable_dp(encoder, crtc_state, conn_state);
   }
+static void intel_ddi_pre_enable_lspcon(struct intel_encoder *encoder,
+					const struct intel_crtc_state *crtc_state,
+					const struct drm_connector_state *conn_state)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	intel_ddi_pre_enable(encoder, crtc_state, conn_state);
+
+	/*
+	 * Parade LSPCON forgets to turn off scrambling/bit clock rate
+	 * when switching from a mode that needs them to a mode that
+	 * does not.
+	 */
+	intel_hdmi_handle_sink_scrambling(encoder, conn_state->connector,
+					  &intel_dp->aux.ddc, false, false);
+}
+
   static void intel_disable_ddi_buf(struct intel_encoder *encoder)
   {
   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -3086,6 +3103,23 @@ static void intel_ddi_post_disable(struct intel_encoder *encoder,
   					  old_crtc_state, old_conn_state);
   }
+static void intel_ddi_post_disable_lspcon(struct intel_encoder *encoder,
+					  const struct intel_crtc_state *old_crtc_state,
+					  const struct drm_connector_state *old_conn_state)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	/*
+	 * Parade LSPCON forgets to turn off scrambling/bit clock rate
+	 * when switching from a mode that needs them to a mode that
+	 * does not.
+	 */
+	intel_hdmi_handle_sink_scrambling(encoder, old_conn_state->connector,
+					  &intel_dp->aux.ddc, false, false);
+
+	intel_ddi_post_disable(encoder, old_crtc_state, old_conn_state);
+}
Few thoughts:
- Would it make more sense to move these 2 functions to intel_lspcon.c ?
Would require more non-statics. But I guess it might be nice to attempt
isolating it as much as possible.

-  And then add a lspcon->vendor == VENDOR_PARADE check, so that we will
run the code only when needed.
Maybe. The ->vendor thing isn't in yet, and we'd have to backport it
as well. Is it big?

Also at this time I have no idea whether Megachips LSPCON is similarly
bugged. Would need to find one and test it.
Oh, and another open question is what happens if one of these chips is
in a DP->HDMI 2.0 dongle. In that case we might end up needing the same
workaround in the normal DP codepath as well :(

You are right, that would be a LSPCON in a dongle configuration (this one is motherboard down configuration), which we are not officially supporting yet. That might come with its own complexity. I think we already have a BZ for such a Parade dongle adapter.

- Shashank
_______________________________________________
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