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

[Nautiyal, Ankit K]

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.kand...@intel.com>
---
  .../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)) {