Re: [PATCH] drm/i915/display: Increase AUX timeout for Type-C

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

 



Hi Suraj,

Thanks for the patch, please find my comments inline:


On 4/3/2023 1:31 PM, Suraj Kandpal wrote:
Type-C PHYs are taking longer than expected for Aux IO Power Enabling.
Workaround: Increase the timeout.

WA: 14017271110

Lets use Wa_#### as per convention.

Also I am wondering if we should keep the original number 14017248603.

Bspec: 55480

Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
---
  .../drm/i915/display/intel_display_power_well.c   | 15 +++++++++++++++
  1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
index 62bafcbc7937..357617b9b725 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -252,6 +252,7 @@ static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
  					   bool timeout_expected)
  {
  	const struct i915_power_well_regs *regs = power_well->desc->ops->regs;
+	enum phy phy = icl_aux_pw_to_phy(dev_priv, power_well);
  	int pw_idx = i915_power_well_instance(power_well)->hsw.idx;
/*
@@ -264,6 +265,20 @@ static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
  		return;
  	}
+ /*
+	 * WA: 14017271110

Wa_<number>:adlp


+	 * Type-C Phy are taking longer than expected for AUX IO Power Enabling.
+	 * Increase timeout to 500ms.
+	 */
+	if (IS_ALDERLAKE_P(dev_priv) && intel_phy_is_tc(dev_priv, phy)) {
+		if (intel_de_wait_for_set(dev_priv, regs->driver,
+					  HSW_PWR_WELL_CTL_STATE(pw_idx), 500)) {
+			drm_dbg_kms(&dev_priv->drm, "%s power well enable timeout\n",
+				    intel_power_well_name(power_well));
+			return;
+		}
+	}
+

Though not part of this workaround, as per Bspec:49294, the power well enable timeout value for Gen 12 seems to be 1.5 ms : ~2ms

May be have another change for that as well?

In my opinion, we can have an enable_timeout_ms which is set to different values based on platform and just

use that in intel_de_wait _for_set.


Regards,

Ankit

  	/* Timeout for PW1:10 us, AUX:not specified, other PWs:20 us. */
  	if (intel_de_wait_for_set(dev_priv, regs->driver,
  				  HSW_PWR_WELL_CTL_STATE(pw_idx), 1)) {



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

  Powered by Linux