All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
@ 2018-10-12 18:38 Ville Syrjala
  2018-10-12 18:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Ville Syrjala @ 2018-10-12 18:38 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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);
+}
+
 void intel_ddi_fdi_post_disable(struct intel_encoder *encoder,
 				const struct intel_crtc_state *old_crtc_state,
 				const struct drm_connector_state *old_conn_state)
@@ -3146,9 +3180,11 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	struct drm_connector *connector = conn_state->connector;
+	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
+							      dig_port->hdmi.ddc_bus);
 	enum port port = encoder->port;
 
-	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
+	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
 					       crtc_state->hdmi_high_tmds_clock_ratio,
 					       crtc_state->hdmi_scrambling))
 		DRM_ERROR("[CONNECTOR:%d:%s] Failed to configure sink scrambling/TMDS bit clock ratio\n",
@@ -3243,13 +3279,17 @@ static void intel_disable_ddi_hdmi(struct intel_encoder *encoder,
 				   const struct intel_crtc_state *old_crtc_state,
 				   const struct drm_connector_state *old_conn_state)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_connector *connector = old_conn_state->connector;
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
+							      dig_port->hdmi.ddc_bus);
 
 	if (old_crtc_state->has_audio)
 		intel_audio_codec_disable(encoder,
 					  old_crtc_state, old_conn_state);
 
-	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
+	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
 					       false, false))
 		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Failed to reset sink scrambling/TMDS bit clock ratio\n",
 			      connector->base.id, connector->name);
@@ -3862,17 +3902,21 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	}
 
 	if (init_lspcon) {
-		if (lspcon_init(intel_dig_port))
+		if (lspcon_init(intel_dig_port)) {
 			/* TODO: handle hdmi info frame part */
 			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
 				port_name(port));
-		else
+
+			intel_encoder->pre_enable = intel_ddi_pre_enable_lspcon;
+			intel_encoder->post_disable = intel_ddi_post_disable_lspcon;
+		} else {
 			/*
 			 * LSPCON init faied, but DP init was success, so
 			 * lets try to drive as DP++ port.
 			 */
 			DRM_ERROR("LSPCON init failed on port %c\n",
 				port_name(port));
+		}
 	}
 
 	return;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e321fc698ae1..d0a06fcc80c0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1866,6 +1866,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct drm_connector_state *conn_state);
 bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
 				       struct drm_connector *connector,
+				       struct i2c_adapter *adapter,
 				       bool high_tmds_clock_ratio,
 				       bool scrambling);
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 2c53efc463e6..bf6f571b674b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2114,6 +2114,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
  * intel_hdmi_handle_sink_scrambling: handle sink scrambling/clock ratio setup
  * @encoder: intel_encoder
  * @connector: drm_connector
+ * @adapter: i2c adapter for the ddc bus
  * @high_tmds_clock_ratio = bool to indicate if the function needs to set
  *  or reset the high tmds clock ratio for scrambling
  * @scrambling: bool to Indicate if the function needs to set or reset
@@ -2130,15 +2131,13 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
  */
 bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
 				       struct drm_connector *connector,
+				       struct i2c_adapter *adapter,
 				       bool high_tmds_clock_ratio,
 				       bool scrambling)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_scrambling *sink_scrambling =
 		&connector->display_info.hdmi.scdc.scrambling;
-	struct i2c_adapter *adapter =
-		intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
 
 	if (!sink_scrambling->supported)
 		return true;
-- 
2.18.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-12 18:38 [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail Ville Syrjala
@ 2018-10-12 18:47 ` Patchwork
  2018-10-12 18:54 ` [PATCH v2] " Ville Syrjala
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-12 18:47 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/lspcon: Fix Parade LSPCON scrambling fail
URL   : https://patchwork.freedesktop.org/series/50950/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/intel_hdmi.o
drivers/gpu/drm/i915/intel_hdmi.c: In function ‘intel_hdmi_handle_sink_scrambling’:
drivers/gpu/drm/i915/intel_hdmi.c:2138:27: error: unused variable ‘dev_priv’ [-Werror=unused-variable]
  struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
                           ^~~~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:305: recipe for target 'drivers/gpu/drm/i915/intel_hdmi.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_hdmi.o] Error 1
scripts/Makefile.build:546: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:546: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:546: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1050: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-12 18:38 [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail Ville Syrjala
  2018-10-12 18:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-10-12 18:54 ` Ville Syrjala
  2018-10-12 18:56 ` [PATCH] " Sharma, Shashank
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjala @ 2018-10-12 18:54 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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.

v2: Nuke unused variables/parameters

Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  | 52 ++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h  |  4 +--
 drivers/gpu/drm/i915/intel_hdmi.c | 10 ++----
 3 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 47960c92cbbf..a9f317dda2a8 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(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(old_conn_state->connector,
+					  &intel_dp->aux.ddc, false, false);
+
+	intel_ddi_post_disable(encoder, old_crtc_state, old_conn_state);
+}
+
 void intel_ddi_fdi_post_disable(struct intel_encoder *encoder,
 				const struct intel_crtc_state *old_crtc_state,
 				const struct drm_connector_state *old_conn_state)
@@ -3146,9 +3180,11 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	struct drm_connector *connector = conn_state->connector;
+	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
+							      dig_port->hdmi.ddc_bus);
 	enum port port = encoder->port;
 
-	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
+	if (!intel_hdmi_handle_sink_scrambling(connector, adapter,
 					       crtc_state->hdmi_high_tmds_clock_ratio,
 					       crtc_state->hdmi_scrambling))
 		DRM_ERROR("[CONNECTOR:%d:%s] Failed to configure sink scrambling/TMDS bit clock ratio\n",
@@ -3243,13 +3279,17 @@ static void intel_disable_ddi_hdmi(struct intel_encoder *encoder,
 				   const struct intel_crtc_state *old_crtc_state,
 				   const struct drm_connector_state *old_conn_state)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_connector *connector = old_conn_state->connector;
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
+							      dig_port->hdmi.ddc_bus);
 
 	if (old_crtc_state->has_audio)
 		intel_audio_codec_disable(encoder,
 					  old_crtc_state, old_conn_state);
 
-	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
+	if (!intel_hdmi_handle_sink_scrambling(connector, adapter,
 					       false, false))
 		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Failed to reset sink scrambling/TMDS bit clock ratio\n",
 			      connector->base.id, connector->name);
@@ -3862,17 +3902,21 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	}
 
 	if (init_lspcon) {
-		if (lspcon_init(intel_dig_port))
+		if (lspcon_init(intel_dig_port)) {
 			/* TODO: handle hdmi info frame part */
 			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
 				port_name(port));
-		else
+
+			intel_encoder->pre_enable = intel_ddi_pre_enable_lspcon;
+			intel_encoder->post_disable = intel_ddi_post_disable_lspcon;
+		} else {
 			/*
 			 * LSPCON init faied, but DP init was success, so
 			 * lets try to drive as DP++ port.
 			 */
 			DRM_ERROR("LSPCON init failed on port %c\n",
 				port_name(port));
+		}
 	}
 
 	return;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e321fc698ae1..8f3022529990 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1864,8 +1864,8 @@ struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_state *pipe_config,
 			       struct drm_connector_state *conn_state);
-bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
-				       struct drm_connector *connector,
+bool intel_hdmi_handle_sink_scrambling(struct drm_connector *connector,
+				       struct i2c_adapter *adapter,
 				       bool high_tmds_clock_ratio,
 				       bool scrambling);
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 2c53efc463e6..878a33a0cef2 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2112,8 +2112,8 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 
 /*
  * intel_hdmi_handle_sink_scrambling: handle sink scrambling/clock ratio setup
- * @encoder: intel_encoder
  * @connector: drm_connector
+ * @adapter: i2c adapter for the ddc bus
  * @high_tmds_clock_ratio = bool to indicate if the function needs to set
  *  or reset the high tmds clock ratio for scrambling
  * @scrambling: bool to Indicate if the function needs to set or reset
@@ -2128,17 +2128,13 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
  * Returns:
  * True on success, false on failure.
  */
-bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
-				       struct drm_connector *connector,
+bool intel_hdmi_handle_sink_scrambling(struct drm_connector *connector,
+				       struct i2c_adapter *adapter,
 				       bool high_tmds_clock_ratio,
 				       bool scrambling)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_scrambling *sink_scrambling =
 		&connector->display_info.hdmi.scdc.scrambling;
-	struct i2c_adapter *adapter =
-		intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
 
 	if (!sink_scrambling->supported)
 		return true;
-- 
2.18.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-12 18:38 [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail Ville Syrjala
  2018-10-12 18:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2018-10-12 18:54 ` [PATCH v2] " Ville Syrjala
@ 2018-10-12 18:56 ` Sharma, Shashank
  2018-10-12 19:17   ` Ville Syrjälä
  2018-10-12 19:35 ` ✓ Fi.CI.BAT: success for drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2) Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Sharma, Shashank @ 2018-10-12 18:56 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Regards

Shashank


On 10/13/2018 12:08 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   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 ?
-  And then add a lspcon->vendor == VENDOR_PARADE check, so that we will 
run the code only when needed.
-  Also, we should check if scrambling is enabled, there might be a case 
where we are driving a HDMI 2.0 display (scrambling->supported = 1) but 
current mode is 1080 P.

- Shashank
> +
>   void intel_ddi_fdi_post_disable(struct intel_encoder *encoder,
>   				const struct intel_crtc_state *old_crtc_state,
>   				const struct drm_connector_state *old_conn_state)
> @@ -3146,9 +3180,11 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>   	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>   	struct drm_connector *connector = conn_state->connector;
> +	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
> +							      dig_port->hdmi.ddc_bus);
>   	enum port port = encoder->port;
>   
> -	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> +	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
>   					       crtc_state->hdmi_high_tmds_clock_ratio,
>   					       crtc_state->hdmi_scrambling))
>   		DRM_ERROR("[CONNECTOR:%d:%s] Failed to configure sink scrambling/TMDS bit clock ratio\n",
> @@ -3243,13 +3279,17 @@ static void intel_disable_ddi_hdmi(struct intel_encoder *encoder,
>   				   const struct intel_crtc_state *old_crtc_state,
>   				   const struct drm_connector_state *old_conn_state)
>   {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>   	struct drm_connector *connector = old_conn_state->connector;
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
> +							      dig_port->hdmi.ddc_bus);
>   
>   	if (old_crtc_state->has_audio)
>   		intel_audio_codec_disable(encoder,
>   					  old_crtc_state, old_conn_state);
>   
> -	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> +	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
>   					       false, false))
>   		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Failed to reset sink scrambling/TMDS bit clock ratio\n",
>   			      connector->base.id, connector->name);
> @@ -3862,17 +3902,21 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>   	}
>   
>   	if (init_lspcon) {
> -		if (lspcon_init(intel_dig_port))
> +		if (lspcon_init(intel_dig_port)) {
>   			/* TODO: handle hdmi info frame part */
>   			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
>   				port_name(port));
> -		else
> +
> +			intel_encoder->pre_enable = intel_ddi_pre_enable_lspcon;
> +			intel_encoder->post_disable = intel_ddi_post_disable_lspcon;
> +		} else {
>   			/*
>   			 * LSPCON init faied, but DP init was success, so
>   			 * lets try to drive as DP++ port.
>   			 */
>   			DRM_ERROR("LSPCON init failed on port %c\n",
>   				port_name(port));
> +		}
>   	}
>   
>   	return;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e321fc698ae1..d0a06fcc80c0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1866,6 +1866,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>   			       struct drm_connector_state *conn_state);
>   bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
>   				       struct drm_connector *connector,
> +				       struct i2c_adapter *adapter,
>   				       bool high_tmds_clock_ratio,
>   				       bool scrambling);
>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 2c53efc463e6..bf6f571b674b 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2114,6 +2114,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>    * intel_hdmi_handle_sink_scrambling: handle sink scrambling/clock ratio setup
>    * @encoder: intel_encoder
>    * @connector: drm_connector
> + * @adapter: i2c adapter for the ddc bus
>    * @high_tmds_clock_ratio = bool to indicate if the function needs to set
>    *  or reset the high tmds clock ratio for scrambling
>    * @scrambling: bool to Indicate if the function needs to set or reset
> @@ -2130,15 +2131,13 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>    */
>   bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
>   				       struct drm_connector *connector,
> +				       struct i2c_adapter *adapter,
>   				       bool high_tmds_clock_ratio,
>   				       bool scrambling)
>   {
>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>   	struct drm_scrambling *sink_scrambling =
>   		&connector->display_info.hdmi.scdc.scrambling;
> -	struct i2c_adapter *adapter =
> -		intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
>   
>   	if (!sink_scrambling->supported)
>   		return true;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-12 18:56 ` [PATCH] " Sharma, Shashank
@ 2018-10-12 19:17   ` Ville Syrjälä
  2018-10-12 19:28     ` Ville Syrjälä
  2018-10-16  2:52     ` Sharma, Shashank
  0 siblings, 2 replies; 12+ messages in thread
From: Ville Syrjälä @ 2018-10-12 19:17 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: intel-gfx

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@linux.intel.com>
> >
> > 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@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   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.

> -  Also, we should check if scrambling is enabled, there might be a case 
> where we are driving a HDMI 2.0 display (scrambling->supported = 1) but 
> current mode is 1080 P.

As explained in the commit message this is not needed. And currently we
have no idea whether LSPCON will enable scrambling or not. Adding code
to determine that would mean second guessing what LSPCON will do. I
don't see any benefit in doing that.

> 
> - Shashank
> > +
> >   void intel_ddi_fdi_post_disable(struct intel_encoder *encoder,
> >   				const struct intel_crtc_state *old_crtc_state,
> >   				const struct drm_connector_state *old_conn_state)
> > @@ -3146,9 +3180,11 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> >   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >   	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >   	struct drm_connector *connector = conn_state->connector;
> > +	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
> > +							      dig_port->hdmi.ddc_bus);
> >   	enum port port = encoder->port;
> >   
> > -	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> > +	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
> >   					       crtc_state->hdmi_high_tmds_clock_ratio,
> >   					       crtc_state->hdmi_scrambling))
> >   		DRM_ERROR("[CONNECTOR:%d:%s] Failed to configure sink scrambling/TMDS bit clock ratio\n",
> > @@ -3243,13 +3279,17 @@ static void intel_disable_ddi_hdmi(struct intel_encoder *encoder,
> >   				   const struct intel_crtc_state *old_crtc_state,
> >   				   const struct drm_connector_state *old_conn_state)
> >   {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >   	struct drm_connector *connector = old_conn_state->connector;
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> > +	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
> > +							      dig_port->hdmi.ddc_bus);
> >   
> >   	if (old_crtc_state->has_audio)
> >   		intel_audio_codec_disable(encoder,
> >   					  old_crtc_state, old_conn_state);
> >   
> > -	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> > +	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
> >   					       false, false))
> >   		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Failed to reset sink scrambling/TMDS bit clock ratio\n",
> >   			      connector->base.id, connector->name);
> > @@ -3862,17 +3902,21 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >   	}
> >   
> >   	if (init_lspcon) {
> > -		if (lspcon_init(intel_dig_port))
> > +		if (lspcon_init(intel_dig_port)) {
> >   			/* TODO: handle hdmi info frame part */
> >   			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
> >   				port_name(port));
> > -		else
> > +
> > +			intel_encoder->pre_enable = intel_ddi_pre_enable_lspcon;
> > +			intel_encoder->post_disable = intel_ddi_post_disable_lspcon;
> > +		} else {
> >   			/*
> >   			 * LSPCON init faied, but DP init was success, so
> >   			 * lets try to drive as DP++ port.
> >   			 */
> >   			DRM_ERROR("LSPCON init failed on port %c\n",
> >   				port_name(port));
> > +		}
> >   	}
> >   
> >   	return;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index e321fc698ae1..d0a06fcc80c0 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1866,6 +1866,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >   			       struct drm_connector_state *conn_state);
> >   bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
> >   				       struct drm_connector *connector,
> > +				       struct i2c_adapter *adapter,
> >   				       bool high_tmds_clock_ratio,
> >   				       bool scrambling);
> >   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 2c53efc463e6..bf6f571b674b 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -2114,6 +2114,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> >    * intel_hdmi_handle_sink_scrambling: handle sink scrambling/clock ratio setup
> >    * @encoder: intel_encoder
> >    * @connector: drm_connector
> > + * @adapter: i2c adapter for the ddc bus
> >    * @high_tmds_clock_ratio = bool to indicate if the function needs to set
> >    *  or reset the high tmds clock ratio for scrambling
> >    * @scrambling: bool to Indicate if the function needs to set or reset
> > @@ -2130,15 +2131,13 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> >    */
> >   bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
> >   				       struct drm_connector *connector,
> > +				       struct i2c_adapter *adapter,
> >   				       bool high_tmds_clock_ratio,
> >   				       bool scrambling)
> >   {
> >   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> >   	struct drm_scrambling *sink_scrambling =
> >   		&connector->display_info.hdmi.scdc.scrambling;
> > -	struct i2c_adapter *adapter =
> > -		intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
> >   
> >   	if (!sink_scrambling->supported)
> >   		return true;

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-12 19:17   ` Ville Syrjälä
@ 2018-10-12 19:28     ` Ville Syrjälä
  2018-10-16  2:55       ` Sharma, Shashank
  2018-10-16  2:52     ` Sharma, Shashank
  1 sibling, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2018-10-12 19:28 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: intel-gfx

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@linux.intel.com>
> > >
> > > 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@intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >   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 :(

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2)
  2018-10-12 18:38 [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail Ville Syrjala
                   ` (2 preceding siblings ...)
  2018-10-12 18:56 ` [PATCH] " Sharma, Shashank
@ 2018-10-12 19:35 ` Patchwork
  2018-10-12 21:39 ` [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail kbuild test robot
  2018-10-12 22:36 ` ✓ Fi.CI.IGT: success for drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2) Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-12 19:35 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2)
URL   : https://patchwork.freedesktop.org/series/50950/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4978 -> Patchwork_10447 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50950/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10447 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-reload:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106248, fdo#106725)

    igt@kms_flip@basic-flip-vs-dpms:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS +1

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
  fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (44 -> 42) ==

  Additional (1): fi-icl-u 
  Missing    (3): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


== Build changes ==

    * Linux: CI_DRM_4978 -> Patchwork_10447

  CI_DRM_4978: ca98b2681a49a1417f8157af2d94a4f2d0bd0e47 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4674: 93871c6fb3c25e5d350c9faf36ded917174214de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10447: 1fb5c7237e151105877baa2f44a114d0b41e4790 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1fb5c7237e15 drm/i915/lspcon: Fix Parade LSPCON scrambling fail

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10447/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-12 18:38 [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail Ville Syrjala
                   ` (3 preceding siblings ...)
  2018-10-12 19:35 ` ✓ Fi.CI.BAT: success for drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2) Patchwork
@ 2018-10-12 21:39 ` kbuild test robot
  2018-10-12 22:36 ` ✓ Fi.CI.IGT: success for drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2) Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2018-10-12 21:39 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 13077 bytes --]

Hi Ville,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.19-rc7 next-20181012]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-i915-lspcon-Fix-Parade-LSPCON-scrambling-fail/20181013-044614
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x005-201840 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/i915/intel_hdmi.c: In function 'intel_hdmi_handle_sink_scrambling':
>> drivers/gpu//drm/i915/intel_hdmi.c:2144:27: error: unused variable 'dev_priv' [-Werror=unused-variable]
     struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
                              ^~~~~~~~
   Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls64
   Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u64
   Cyclomatic Complexity 4 include/linux/string.h:memcpy
   Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
   Cyclomatic Complexity 3 include/linux/ktime.h:ktime_compare
   Cyclomatic Complexity 1 include/linux/ktime.h:ktime_after
   Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index
   Cyclomatic Complexity 68 include/linux/slab.h:kmalloc_large
   Cyclomatic Complexity 5 include/linux/slab.h:kmalloc
   Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
   Cyclomatic Complexity 1 include/drm/drm_modeset_helper_vtables.h:drm_connector_helper_add
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/i915_utils.h:yesno
   Cyclomatic Complexity 7 drivers/gpu//drm/i915/intel_display.h:port_identifier
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/i915_drv.h:to_i915
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_drv.h:intel_info
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/i915_drv.h:intel_gmbus_is_forced_bit
   Cyclomatic Complexity 1 include/media/cec-notifier.h:cec_notifier_phys_addr_invalidate
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:intel_get_crtc_for_pipe
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_drv.h:intel_attached_encoder
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_drv.h:intel_encoder_is_dig_port
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_drv.h:enc_to_dig_port
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_drv.h:hdmi_to_dig_port
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:intel_wait_for_vblank
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_drv.h:intel_wait_for_vblank_if_active
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_to_dev
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:g4x_infoframe_enabled
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:hsw_infoframe_enabled
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:hdmi_sink_is_deep_color
   Cyclomatic Complexity 12 drivers/gpu//drm/i915/intel_hdmi.c:gcp_default_phase_possible
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:vlv_enable_hdmi
   Cyclomatic Complexity 7 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_source_max_tmds_clock
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:hdmi_port_clock_limit
   Cyclomatic Complexity 8 drivers/gpu//drm/i915/intel_hdmi.c:hdmi_port_clock_valid
   Cyclomatic Complexity 23 drivers/gpu//drm/i915/intel_hdmi.c:hdmi_deep_color_possible
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_ycbcr420_config
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:cpt_infoframe_enabled
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:assert_hdmi_port_disabled
   Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_hdmi.c:g4x_infoframe_index
   Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_hdmi.c:g4x_infoframe_enable
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:assert_hdmi_transcoder_func_disabled
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:hsw_dip_data_reg
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:hsw_infoframe_enable
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:hsw_write_infoframe
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:g4x_write_infoframe
   Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_hdmi.c:chv_port_to_ddc_pin
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:bxt_port_to_ddc_pin
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:cnp_port_to_ddc_pin
   Cyclomatic Complexity 8 drivers/gpu//drm/i915/intel_hdmi.c:icl_port_to_ddc_pin
   Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_hdmi.c:g4x_port_to_ddc_pin
   Cyclomatic Complexity 7 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_ddc_pin
   Cyclomatic Complexity 7 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_set_gcp_infoframe
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_write_infoframe
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_set_spd_infoframe
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_set_hdmi_infoframe
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:cpt_write_infoframe
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:ibx_infoframe_enabled
   Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_hdmi.c:ibx_write_infoframe
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:vlv_infoframe_enabled
   Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_hdmi.c:vlv_write_infoframe
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_toggle_signalling
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_read
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_read_v_prime_part
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_read_ksv_fifo
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_read_ksv_ready
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_read_ri_prime
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_check_link
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_repeater_present
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_read_bstatus
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_read_bksv
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_write
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_hdcp_write_an_aksv
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_get_modes
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_add_properties
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:intel_enable_hdmi_audio
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:vlv_hdmi_post_disable
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:chv_hdmi_post_pll_disable
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:chv_hdmi_post_disable
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:pch_disable_hdmi
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:enc_to_intel_hdmi
   Cyclomatic Complexity 27 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_compute_config
   Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_set_avi_infoframe
   Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_hdmi.c:cpt_set_infoframes
   Cyclomatic Complexity 7 drivers/gpu//drm/i915/intel_hdmi.c:ibx_set_infoframes
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:hsw_set_infoframes
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:g4x_set_infoframes
   Cyclomatic Complexity 7 drivers/gpu//drm/i915/intel_hdmi.c:vlv_set_infoframes
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:intel_attached_hdmi
   Cyclomatic Complexity 13 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_mode_valid
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_destroy
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_unset_edid
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_dp_dual_mode_detect
   Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_set_edid
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_force
   Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_hdmi.c:intel_hdmi_detect
   Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_hdmi.c:g4x_enable_hdmi
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:vlv_hdmi_pre_enable
   Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_hdmi.c:chv_hdmi_pre_enable

vim +/dev_priv +2144 drivers/gpu//drm/i915/intel_hdmi.c

55b7d6e8c Chris Wilson    2010-09-19  2118  
159536378 Shashank Sharma 2017-03-13  2119  /*
159536378 Shashank Sharma 2017-03-13  2120   * intel_hdmi_handle_sink_scrambling: handle sink scrambling/clock ratio setup
159536378 Shashank Sharma 2017-03-13  2121   * @encoder: intel_encoder
159536378 Shashank Sharma 2017-03-13  2122   * @connector: drm_connector
98706eef6 Ville Syrjälä   2018-10-12  2123   * @adapter: i2c adapter for the ddc bus
159536378 Shashank Sharma 2017-03-13  2124   * @high_tmds_clock_ratio = bool to indicate if the function needs to set
159536378 Shashank Sharma 2017-03-13  2125   *  or reset the high tmds clock ratio for scrambling
159536378 Shashank Sharma 2017-03-13  2126   * @scrambling: bool to Indicate if the function needs to set or reset
159536378 Shashank Sharma 2017-03-13  2127   *  sink scrambling
159536378 Shashank Sharma 2017-03-13  2128   *
159536378 Shashank Sharma 2017-03-13  2129   * This function handles scrambling on HDMI 2.0 capable sinks.
159536378 Shashank Sharma 2017-03-13  2130   * If required clock rate is > 340 Mhz && scrambling is supported by sink
159536378 Shashank Sharma 2017-03-13  2131   * it enables scrambling. This should be called before enabling the HDMI
159536378 Shashank Sharma 2017-03-13  2132   * 2.0 port, as the sink can choose to disable the scrambling if it doesn't
159536378 Shashank Sharma 2017-03-13  2133   * detect a scrambled clock within 100 ms.
277ab5abc Ville Syrjälä   2018-03-22  2134   *
277ab5abc Ville Syrjälä   2018-03-22  2135   * Returns:
277ab5abc Ville Syrjälä   2018-03-22  2136   * True on success, false on failure.
159536378 Shashank Sharma 2017-03-13  2137   */
277ab5abc Ville Syrjälä   2018-03-22  2138  bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
159536378 Shashank Sharma 2017-03-13  2139  				       struct drm_connector *connector,
98706eef6 Ville Syrjälä   2018-10-12  2140  				       struct i2c_adapter *adapter,
159536378 Shashank Sharma 2017-03-13  2141  				       bool high_tmds_clock_ratio,
159536378 Shashank Sharma 2017-03-13  2142  				       bool scrambling)
159536378 Shashank Sharma 2017-03-13  2143  {
277ab5abc Ville Syrjälä   2018-03-22 @2144  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
159536378 Shashank Sharma 2017-03-13  2145  	struct drm_scrambling *sink_scrambling =
159536378 Shashank Sharma 2017-03-13  2146  		&connector->display_info.hdmi.scdc.scrambling;
159536378 Shashank Sharma 2017-03-13  2147  
159536378 Shashank Sharma 2017-03-13  2148  	if (!sink_scrambling->supported)
277ab5abc Ville Syrjälä   2018-03-22  2149  		return true;
159536378 Shashank Sharma 2017-03-13  2150  
277ab5abc Ville Syrjälä   2018-03-22  2151  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] scrambling=%s, TMDS bit clock ratio=1/%d\n",
277ab5abc Ville Syrjälä   2018-03-22  2152  		      connector->base.id, connector->name,
277ab5abc Ville Syrjälä   2018-03-22  2153  		      yesno(scrambling), high_tmds_clock_ratio ? 40 : 10);
159536378 Shashank Sharma 2017-03-13  2154  
277ab5abc Ville Syrjälä   2018-03-22  2155  	/* Set TMDS bit clock ratio to 1/40 or 1/10, and enable/disable scrambling */
277ab5abc Ville Syrjälä   2018-03-22  2156  	return drm_scdc_set_high_tmds_clock_ratio(adapter,
277ab5abc Ville Syrjälä   2018-03-22  2157  						  high_tmds_clock_ratio) &&
277ab5abc Ville Syrjälä   2018-03-22  2158  		drm_scdc_set_scrambling(adapter, scrambling);
159536378 Shashank Sharma 2017-03-13  2159  }
159536378 Shashank Sharma 2017-03-13  2160  

:::::: The code at line 2144 was first introduced by commit
:::::: 277ab5abc68df2f6f8fac7a46e50105b6648f432 drm/i915: Don't spew errors when resetting HDMI scrambling/bit clock ratio fails

:::::: TO: Ville Syrjälä <ville.syrjala@linux.intel.com>
:::::: CC: Ville Syrjälä <ville.syrjala@linux.intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35372 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2)
  2018-10-12 18:38 [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail Ville Syrjala
                   ` (4 preceding siblings ...)
  2018-10-12 21:39 ` [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail kbuild test robot
@ 2018-10-12 22:36 ` Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-12 22:36 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2)
URL   : https://patchwork.freedesktop.org/series/50950/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4978_full -> Patchwork_10447_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10447_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10447_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10447_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10447_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540, fdo#106886)
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, k.org#198133, fdo#106886)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039) +1

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_chv_cursor_fail@pipe-a-64x64-right-edge:
      shard-skl:          PASS -> FAIL (fdo#104671)

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-apl:          PASS -> FAIL (fdo#103232, fdo#103191)

    igt@kms_cursor_crc@cursor-64x21-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +2

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538)

    igt@kms_fbcon_fbt@psr:
      shard-skl:          NOTRUN -> FAIL (fdo#107882)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
      shard-apl:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-glk:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-wc:
      shard-skl:          PASS -> FAIL (fdo#103167)

    igt@kms_plane@pixel-format-pipe-b-planes:
      shard-skl:          NOTRUN -> DMESG-FAIL (fdo#103166, fdo#106885)

    igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +1

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#103166) +1

    igt@kms_vblank@pipe-c-ts-continuation-suspend:
      shard-apl:          PASS -> DMESG-WARN (fdo#105602, fdo#103558) +2

    igt@perf@short-reads:
      shard-skl:          NOTRUN -> FAIL (fdo#103183)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries_display_off:
      shard-skl:          INCOMPLETE (fdo#104108) -> PASS

    igt@gem_userptr_blits@readonly-unsync:
      shard-skl:          INCOMPLETE (fdo#108074) -> PASS

    igt@kms_busy@extended-modeset-hang-newfb-render-b:
      shard-hsw:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_cursor_crc@cursor-128x42-onscreen:
      shard-apl:          FAIL (fdo#103232) -> PASS +4

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-apl:          FAIL (fdo#103232, fdo#103191) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_setmode@basic:
      shard-hsw:          FAIL (fdo#99912) -> PASS

    igt@pm_rpm@modeset-non-lpsp:
      shard-skl:          INCOMPLETE (fdo#107807) -> SKIP

    
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103183 https://bugs.freedesktop.org/show_bug.cgi?id=103183
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107882 https://bugs.freedesktop.org/show_bug.cgi?id=107882
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4978 -> Patchwork_10447

  CI_DRM_4978: ca98b2681a49a1417f8157af2d94a4f2d0bd0e47 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4674: 93871c6fb3c25e5d350c9faf36ded917174214de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10447: 1fb5c7237e151105877baa2f44a114d0b41e4790 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10447/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-12 19:17   ` Ville Syrjälä
  2018-10-12 19:28     ` Ville Syrjälä
@ 2018-10-16  2:52     ` Sharma, Shashank
  2018-10-22 14:27       ` Ville Syrjälä
  1 sibling, 1 reply; 12+ messages in thread
From: Sharma, Shashank @ 2018-10-16  2:52 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Regards

Shashank


On 10/13/2018 12:47 AM, 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@linux.intel.com>
>>>
>>> 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@intel.com>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    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?
The LSPCON patches are merged now, we can directly use the vendor check 
now.
> Also at this time I have no idea whether Megachips LSPCON is similarly
> bugged. Would need to find one and test it.
That would be best, we will have to do that anyways for compliance 
issues sooner or later :-)
>> -  Also, we should check if scrambling is enabled, there might be a case
>> where we are driving a HDMI 2.0 display (scrambling->supported = 1) but
>> current mode is 1080 P.
> As explained in the commit message this is not needed. And currently we
> have no idea whether LSPCON will enable scrambling or not. Adding code
> to determine that would mean second guessing what LSPCON will do. I
> don't see any benefit in doing that.
Humm, I guess this can be determined with few checks, and HDMI spec 
mendates:
- clock above 340Mhz ? LSPCON must enable scrambling
- clock below 340Mhz && monitor supports scrambling below 340Mhz ? 
LSPCON should enable scrambling : LSPCON mustn't enable scrambling.

This will make sure that we are doing what's recommended by spec.

- Shashank
>> - Shashank
>>> +
>>>    void intel_ddi_fdi_post_disable(struct intel_encoder *encoder,
>>>    				const struct intel_crtc_state *old_crtc_state,
>>>    				const struct drm_connector_state *old_conn_state)
>>> @@ -3146,9 +3180,11 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>>>    	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>>    	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>>>    	struct drm_connector *connector = conn_state->connector;
>>> +	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
>>> +							      dig_port->hdmi.ddc_bus);
>>>    	enum port port = encoder->port;
>>>    
>>> -	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
>>> +	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
>>>    					       crtc_state->hdmi_high_tmds_clock_ratio,
>>>    					       crtc_state->hdmi_scrambling))
>>>    		DRM_ERROR("[CONNECTOR:%d:%s] Failed to configure sink scrambling/TMDS bit clock ratio\n",
>>> @@ -3243,13 +3279,17 @@ static void intel_disable_ddi_hdmi(struct intel_encoder *encoder,
>>>    				   const struct intel_crtc_state *old_crtc_state,
>>>    				   const struct drm_connector_state *old_conn_state)
>>>    {
>>> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>>    	struct drm_connector *connector = old_conn_state->connector;
>>> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>>> +	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
>>> +							      dig_port->hdmi.ddc_bus);
>>>    
>>>    	if (old_crtc_state->has_audio)
>>>    		intel_audio_codec_disable(encoder,
>>>    					  old_crtc_state, old_conn_state);
>>>    
>>> -	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
>>> +	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
>>>    					       false, false))
>>>    		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Failed to reset sink scrambling/TMDS bit clock ratio\n",
>>>    			      connector->base.id, connector->name);
>>> @@ -3862,17 +3902,21 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>>    	}
>>>    
>>>    	if (init_lspcon) {
>>> -		if (lspcon_init(intel_dig_port))
>>> +		if (lspcon_init(intel_dig_port)) {
>>>    			/* TODO: handle hdmi info frame part */
>>>    			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
>>>    				port_name(port));
>>> -		else
>>> +
>>> +			intel_encoder->pre_enable = intel_ddi_pre_enable_lspcon;
>>> +			intel_encoder->post_disable = intel_ddi_post_disable_lspcon;
>>> +		} else {
>>>    			/*
>>>    			 * LSPCON init faied, but DP init was success, so
>>>    			 * lets try to drive as DP++ port.
>>>    			 */
>>>    			DRM_ERROR("LSPCON init failed on port %c\n",
>>>    				port_name(port));
>>> +		}
>>>    	}
>>>    
>>>    	return;
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index e321fc698ae1..d0a06fcc80c0 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -1866,6 +1866,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>>>    			       struct drm_connector_state *conn_state);
>>>    bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
>>>    				       struct drm_connector *connector,
>>> +				       struct i2c_adapter *adapter,
>>>    				       bool high_tmds_clock_ratio,
>>>    				       bool scrambling);
>>>    void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index 2c53efc463e6..bf6f571b674b 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -2114,6 +2114,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>>>     * intel_hdmi_handle_sink_scrambling: handle sink scrambling/clock ratio setup
>>>     * @encoder: intel_encoder
>>>     * @connector: drm_connector
>>> + * @adapter: i2c adapter for the ddc bus
>>>     * @high_tmds_clock_ratio = bool to indicate if the function needs to set
>>>     *  or reset the high tmds clock ratio for scrambling
>>>     * @scrambling: bool to Indicate if the function needs to set or reset
>>> @@ -2130,15 +2131,13 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>>>     */
>>>    bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
>>>    				       struct drm_connector *connector,
>>> +				       struct i2c_adapter *adapter,
>>>    				       bool high_tmds_clock_ratio,
>>>    				       bool scrambling)
>>>    {
>>>    	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>>    	struct drm_scrambling *sink_scrambling =
>>>    		&connector->display_info.hdmi.scdc.scrambling;
>>> -	struct i2c_adapter *adapter =
>>> -		intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
>>>    
>>>    	if (!sink_scrambling->supported)
>>>    		return true;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-12 19:28     ` Ville Syrjälä
@ 2018-10-16  2:55       ` Sharma, Shashank
  0 siblings, 0 replies; 12+ messages in thread
From: Sharma, Shashank @ 2018-10-16  2:55 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

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@linux.intel.com>
>>>>
>>>> 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@intel.com>
>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> ---
>>>>    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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail
  2018-10-16  2:52     ` Sharma, Shashank
@ 2018-10-22 14:27       ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2018-10-22 14:27 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: intel-gfx

On Tue, Oct 16, 2018 at 08:22:26AM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 10/13/2018 12:47 AM, 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@linux.intel.com>
> >>>
> >>> 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@intel.com>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>    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?
> The LSPCON patches are merged now, we can directly use the vendor check 
> now.
> > Also at this time I have no idea whether Megachips LSPCON is similarly
> > bugged. Would need to find one and test it.
> That would be best, we will have to do that anyways for compliance 
> issues sooner or later :-)
> >> -  Also, we should check if scrambling is enabled, there might be a case
> >> where we are driving a HDMI 2.0 display (scrambling->supported = 1) but
> >> current mode is 1080 P.
> > As explained in the commit message this is not needed. And currently we
> > have no idea whether LSPCON will enable scrambling or not. Adding code
> > to determine that would mean second guessing what LSPCON will do. I
> > don't see any benefit in doing that.
> Humm, I guess this can be determined with few checks, and HDMI spec 
> mendates:
> - clock above 340Mhz ? LSPCON must enable scrambling
> - clock below 340Mhz && monitor supports scrambling below 340Mhz ? 
> LSPCON should enable scrambling : LSPCON mustn't enable scrambling.

Which means we have to guess as to which clock LSPCON will choose. Ie.
just another guess that we can get wrong at which point the bug will
be back. This patch has no such achilles heel.

> 
> This will make sure that we are doing what's recommended by spec.
> 
> - Shashank
> >> - Shashank
> >>> +
> >>>    void intel_ddi_fdi_post_disable(struct intel_encoder *encoder,
> >>>    				const struct intel_crtc_state *old_crtc_state,
> >>>    				const struct drm_connector_state *old_conn_state)
> >>> @@ -3146,9 +3180,11 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> >>>    	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>>    	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >>>    	struct drm_connector *connector = conn_state->connector;
> >>> +	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
> >>> +							      dig_port->hdmi.ddc_bus);
> >>>    	enum port port = encoder->port;
> >>>    
> >>> -	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> >>> +	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
> >>>    					       crtc_state->hdmi_high_tmds_clock_ratio,
> >>>    					       crtc_state->hdmi_scrambling))
> >>>    		DRM_ERROR("[CONNECTOR:%d:%s] Failed to configure sink scrambling/TMDS bit clock ratio\n",
> >>> @@ -3243,13 +3279,17 @@ static void intel_disable_ddi_hdmi(struct intel_encoder *encoder,
> >>>    				   const struct intel_crtc_state *old_crtc_state,
> >>>    				   const struct drm_connector_state *old_conn_state)
> >>>    {
> >>> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>>    	struct drm_connector *connector = old_conn_state->connector;
> >>> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >>> +	struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv,
> >>> +							      dig_port->hdmi.ddc_bus);
> >>>    
> >>>    	if (old_crtc_state->has_audio)
> >>>    		intel_audio_codec_disable(encoder,
> >>>    					  old_crtc_state, old_conn_state);
> >>>    
> >>> -	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> >>> +	if (!intel_hdmi_handle_sink_scrambling(encoder, connector, adapter,
> >>>    					       false, false))
> >>>    		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Failed to reset sink scrambling/TMDS bit clock ratio\n",
> >>>    			      connector->base.id, connector->name);
> >>> @@ -3862,17 +3902,21 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >>>    	}
> >>>    
> >>>    	if (init_lspcon) {
> >>> -		if (lspcon_init(intel_dig_port))
> >>> +		if (lspcon_init(intel_dig_port)) {
> >>>    			/* TODO: handle hdmi info frame part */
> >>>    			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
> >>>    				port_name(port));
> >>> -		else
> >>> +
> >>> +			intel_encoder->pre_enable = intel_ddi_pre_enable_lspcon;
> >>> +			intel_encoder->post_disable = intel_ddi_post_disable_lspcon;
> >>> +		} else {
> >>>    			/*
> >>>    			 * LSPCON init faied, but DP init was success, so
> >>>    			 * lets try to drive as DP++ port.
> >>>    			 */
> >>>    			DRM_ERROR("LSPCON init failed on port %c\n",
> >>>    				port_name(port));
> >>> +		}
> >>>    	}
> >>>    
> >>>    	return;
> >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>> index e321fc698ae1..d0a06fcc80c0 100644
> >>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>> @@ -1866,6 +1866,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >>>    			       struct drm_connector_state *conn_state);
> >>>    bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
> >>>    				       struct drm_connector *connector,
> >>> +				       struct i2c_adapter *adapter,
> >>>    				       bool high_tmds_clock_ratio,
> >>>    				       bool scrambling);
> >>>    void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
> >>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >>> index 2c53efc463e6..bf6f571b674b 100644
> >>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>> @@ -2114,6 +2114,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> >>>     * intel_hdmi_handle_sink_scrambling: handle sink scrambling/clock ratio setup
> >>>     * @encoder: intel_encoder
> >>>     * @connector: drm_connector
> >>> + * @adapter: i2c adapter for the ddc bus
> >>>     * @high_tmds_clock_ratio = bool to indicate if the function needs to set
> >>>     *  or reset the high tmds clock ratio for scrambling
> >>>     * @scrambling: bool to Indicate if the function needs to set or reset
> >>> @@ -2130,15 +2131,13 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> >>>     */
> >>>    bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
> >>>    				       struct drm_connector *connector,
> >>> +				       struct i2c_adapter *adapter,
> >>>    				       bool high_tmds_clock_ratio,
> >>>    				       bool scrambling)
> >>>    {
> >>>    	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> >>>    	struct drm_scrambling *sink_scrambling =
> >>>    		&connector->display_info.hdmi.scdc.scrambling;
> >>> -	struct i2c_adapter *adapter =
> >>> -		intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
> >>>    
> >>>    	if (!sink_scrambling->supported)
> >>>    		return true;

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-10-22 14:27 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-12 18:38 [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail Ville Syrjala
2018-10-12 18:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-10-12 18:54 ` [PATCH v2] " Ville Syrjala
2018-10-12 18:56 ` [PATCH] " Sharma, Shashank
2018-10-12 19:17   ` Ville Syrjälä
2018-10-12 19:28     ` Ville Syrjälä
2018-10-16  2:55       ` Sharma, Shashank
2018-10-16  2:52     ` Sharma, Shashank
2018-10-22 14:27       ` Ville Syrjälä
2018-10-12 19:35 ` ✓ Fi.CI.BAT: success for drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2) Patchwork
2018-10-12 21:39 ` [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail kbuild test robot
2018-10-12 22:36 ` ✓ Fi.CI.IGT: success for drm/i915/lspcon: Fix Parade LSPCON scrambling fail (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.