diff mbox series

[H,1/1] drm/i915/ilk-glk: Fix link training on links with LTTPRs

Message ID 20210817061851.6258-2-aaron.ma@canonical.com
State New
Headers show
Series Fix no external DP output on some monitors | expand

Commit Message

Aaron Ma Aug. 17, 2021, 6:18 a.m. UTC 
From: Imre Deak <imre.deak@intel.com>

BugLink: https://bugs.launchpad.net/bugs/1938999

The spec requires to use at least 3.2ms for the AUX timeout period if
there are LT-tunable PHY Repeaters on the link (2.11.2). An upcoming
spec update makes this more specific, by requiring a 3.2ms minimum
timeout period for the LTTPR detection reading the 0xF0000-0xF0007
range (3.6.5.1).

Accordingly disable LTTPR detection until GLK, where the maximum timeout
we can set is only 1.6ms.

Link training in the non-transparent mode is known to fail at least on
some SKL systems with a WD19 dock on the link, which exposes an LTTPR
(see the References below). While this could have different reasons
besides the too short AUX timeout used, not detecting LTTPRs (and so not
using the non-transparent LT mode) fixes link training on these systems.

While at it add a code comment about the platform specific maximum
timeout values.

v2: Add a comment about the g4x maximum timeout as well. (Ville)

Reported-by: Takashi Iwai <tiwai@suse.de>
Reported-and-tested-by: Santiago Zarate <santiago.zarate@suse.com>
Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
References: https://gitlab.freedesktop.org/drm/intel/-/issues/3166
Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link training")
Cc: <stable@vger.kernel.org> # v5.11
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210317184901.4029798-2-imre.deak@intel.com
(backported from commit 984982f3ef7b240cd24c2feb2762d81d9d8da3c2)
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c           |  7 +++++++
 .../gpu/drm/i915/display/intel_dp_link_training.c | 15 ++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

Comments

Tim Gardner Aug. 17, 2021, 11:52 a.m. UTC | #1 
Acked-by: Tim Gardner <tim.gardner@canonical.com>

You could have mentioned as part of the backport explanation that 
drivers/gpu/drm/i915/display/intel_dp_aux.c does not exist in this 
version of the kernel. In fact, its the reason this is a backport and 
not a cherry-pick.

On 8/17/21 12:18 AM, Aaron Ma wrote:
> From: Imre Deak <imre.deak@intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1938999
> 
> The spec requires to use at least 3.2ms for the AUX timeout period if
> there are LT-tunable PHY Repeaters on the link (2.11.2). An upcoming
> spec update makes this more specific, by requiring a 3.2ms minimum
> timeout period for the LTTPR detection reading the 0xF0000-0xF0007
> range (3.6.5.1).
> 
> Accordingly disable LTTPR detection until GLK, where the maximum timeout
> we can set is only 1.6ms.
> 
> Link training in the non-transparent mode is known to fail at least on
> some SKL systems with a WD19 dock on the link, which exposes an LTTPR
> (see the References below). While this could have different reasons
> besides the too short AUX timeout used, not detecting LTTPRs (and so not
> using the non-transparent LT mode) fixes link training on these systems.
> 
> While at it add a code comment about the platform specific maximum
> timeout values.
> 
> v2: Add a comment about the g4x maximum timeout as well. (Ville)
> 
> Reported-by: Takashi Iwai <tiwai@suse.de>
> Reported-and-tested-by: Santiago Zarate <santiago.zarate@suse.com>
> Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link training")
> Cc: <stable@vger.kernel.org> # v5.11
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20210317184901.4029798-2-imre.deak@intel.com
> (backported from commit 984982f3ef7b240cd24c2feb2762d81d9d8da3c2)
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dp.c           |  7 +++++++
>   .../gpu/drm/i915/display/intel_dp_link_training.c | 15 ++++++++++++---
>   2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 63e73fbe7a9e..28ead8a3843f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1407,6 +1407,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>   	else
>   		precharge = 5;
>   
> +	/* Max timeout value on G4x-BDW: 1.6ms */
>   	if (IS_BROADWELL(dev_priv))
>   		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
>   	else
> @@ -1433,6 +1434,12 @@ static u32 skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>   	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
>   	u32 ret;
>   
> +	/*
> +	 * Max timeout values:
> +	 * SKL-GLK: 1.6ms
> +	 * CNL: 3.2ms
> +	 * ICL+: 4ms
> +	 */
>   	ret = DP_AUX_CH_CTL_SEND_BUSY |
>   	      DP_AUX_CH_CTL_DONE |
>   	      DP_AUX_CH_CTL_INTERRUPT |
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index fad9e9874c7b..0359d5936901 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -81,6 +81,18 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
>   
>   static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
>   {
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +
> +	if (intel_dp_is_edp(intel_dp))
> +		return false;
> +
> +	/*
> +	 * Detecting LTTPRs must be avoided on platforms with an AUX timeout
> +	 * period < 3.2ms. (see DP Standard v2.0, 2.11.2, 3.6.6.1).
> +	 */
> +	if (INTEL_GEN(i915) < 10)
> +		return false;
> +
>   	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
>   					  intel_dp->lttpr_common_caps) < 0) {
>   		memset(intel_dp->lttpr_common_caps, 0,
> @@ -126,9 +138,6 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
>   	bool ret;
>   	int i;
>   
> -	if (intel_dp_is_edp(intel_dp))
> -		return 0;
> -
>   	ret = intel_dp_read_lttpr_common_caps(intel_dp);
>   	if (!ret)
>   		return 0;
>
Aaron Ma Aug. 17, 2021, 11:57 a.m. UTC | #2 
On 8/17/21 7:52 PM, Tim Gardner wrote:
> 
> You could have mentioned as part of the backport explanation that drivers/gpu/drm/i915/display/intel_dp_aux.c does not exist in this version of the kernel. In fact, its the reason this is a backport and not a cherry-pick.

Backport due to the file name intel_dp.c is changed to intel_dp_aux.c in the original patch.
Sorry for missing that explanation, I will keep that in mind next time.

Thanks for pointing out.
Aaron
Stefan Bader Aug. 24, 2021, 7:19 a.m. UTC | #3 
On 17.08.21 08:18, Aaron Ma wrote:
> From: Imre Deak <imre.deak@intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1938999
> 
> The spec requires to use at least 3.2ms for the AUX timeout period if
> there are LT-tunable PHY Repeaters on the link (2.11.2). An upcoming
> spec update makes this more specific, by requiring a 3.2ms minimum
> timeout period for the LTTPR detection reading the 0xF0000-0xF0007
> range (3.6.5.1).
> 
> Accordingly disable LTTPR detection until GLK, where the maximum timeout
> we can set is only 1.6ms.
> 
> Link training in the non-transparent mode is known to fail at least on
> some SKL systems with a WD19 dock on the link, which exposes an LTTPR
> (see the References below). While this could have different reasons
> besides the too short AUX timeout used, not detecting LTTPRs (and so not
> using the non-transparent LT mode) fixes link training on these systems.
> 
> While at it add a code comment about the platform specific maximum
> timeout values.
> 
> v2: Add a comment about the g4x maximum timeout as well. (Ville)
> 
> Reported-by: Takashi Iwai <tiwai@suse.de>
> Reported-and-tested-by: Santiago Zarate <santiago.zarate@suse.com>
> Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link training")
> Cc: <stable@vger.kernel.org> # v5.11
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20210317184901.4029798-2-imre.deak@intel.com
> (backported from commit 984982f3ef7b240cd24c2feb2762d81d9d8da3c2)
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---

Overall the real change is bailing out of something when on older(? assuming 
lower gen is older) hardware. Which looks to be reasonably tested agains.

-Stefan

>   drivers/gpu/drm/i915/display/intel_dp.c           |  7 +++++++
>   .../gpu/drm/i915/display/intel_dp_link_training.c | 15 ++++++++++++---
>   2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 63e73fbe7a9e..28ead8a3843f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1407,6 +1407,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>   	else
>   		precharge = 5;
>   
> +	/* Max timeout value on G4x-BDW: 1.6ms */
>   	if (IS_BROADWELL(dev_priv))
>   		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
>   	else
> @@ -1433,6 +1434,12 @@ static u32 skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>   	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
>   	u32 ret;
>   
> +	/*
> +	 * Max timeout values:
> +	 * SKL-GLK: 1.6ms
> +	 * CNL: 3.2ms
> +	 * ICL+: 4ms
> +	 */
>   	ret = DP_AUX_CH_CTL_SEND_BUSY |
>   	      DP_AUX_CH_CTL_DONE |
>   	      DP_AUX_CH_CTL_INTERRUPT |
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index fad9e9874c7b..0359d5936901 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -81,6 +81,18 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
>   
>   static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
>   {
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +
> +	if (intel_dp_is_edp(intel_dp))
> +		return false;
> +
> +	/*
> +	 * Detecting LTTPRs must be avoided on platforms with an AUX timeout
> +	 * period < 3.2ms. (see DP Standard v2.0, 2.11.2, 3.6.6.1).
> +	 */
> +	if (INTEL_GEN(i915) < 10)
> +		return false;
> +
>   	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
>   					  intel_dp->lttpr_common_caps) < 0) {
>   		memset(intel_dp->lttpr_common_caps, 0,
> @@ -126,9 +138,6 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
>   	bool ret;
>   	int i;
>   
> -	if (intel_dp_is_edp(intel_dp))
> -		return 0;
> -
>   	ret = intel_dp_read_lttpr_common_caps(intel_dp);
>   	if (!ret)
>   		return 0;
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 63e73fbe7a9e..28ead8a3843f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1407,6 +1407,7 @@  static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
 	else
 		precharge = 5;
 
+	/* Max timeout value on G4x-BDW: 1.6ms */
 	if (IS_BROADWELL(dev_priv))
 		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
 	else
@@ -1433,6 +1434,12 @@  static u32 skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
 	u32 ret;
 
+	/*
+	 * Max timeout values:
+	 * SKL-GLK: 1.6ms
+	 * CNL: 3.2ms
+	 * ICL+: 4ms
+	 */
 	ret = DP_AUX_CH_CTL_SEND_BUSY |
 	      DP_AUX_CH_CTL_DONE |
 	      DP_AUX_CH_CTL_INTERRUPT |
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index fad9e9874c7b..0359d5936901 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -81,6 +81,18 @@  static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
 
 static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
 {
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+	if (intel_dp_is_edp(intel_dp))
+		return false;
+
+	/*
+	 * Detecting LTTPRs must be avoided on platforms with an AUX timeout
+	 * period < 3.2ms. (see DP Standard v2.0, 2.11.2, 3.6.6.1).
+	 */
+	if (INTEL_GEN(i915) < 10)
+		return false;
+
 	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
 					  intel_dp->lttpr_common_caps) < 0) {
 		memset(intel_dp->lttpr_common_caps, 0,
@@ -126,9 +138,6 @@  int intel_dp_lttpr_init(struct intel_dp *intel_dp)
 	bool ret;
 	int i;
 
-	if (intel_dp_is_edp(intel_dp))
-		return 0;
-
 	ret = intel_dp_read_lttpr_common_caps(intel_dp);
 	if (!ret)
 		return 0;