Re: [PATCH] drm/i915/display: Implement wa_16011342517

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, May 30, 2025 at 02:42:40PM +0530, Nemesa Garg wrote:
> While doing voltage swing for type-c phy
> for DP 1.62 and HDMI write the
> LOADGEN_SHARING_PMD_DISABLE bit to 1.
> 
> -v2: Update commit message.
>      Add bspec[Suraj]
> 
> Bspec: 55359
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c         | 16 ++++++++++++++++
>  .../gpu/drm/i915/display/intel_dkl_phy_regs.h    |  4 ++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4c845dd410a2..2cdd51cdfe17 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -77,6 +77,7 @@
>  #include "intel_psr.h"
>  #include "intel_quirks.h"
>  #include "intel_snps_phy.h"
> +#include "intel_step.h"
>  #include "intel_tc.h"
>  #include "intel_vdsc.h"
>  #include "intel_vdsc_regs.h"
> @@ -1439,6 +1440,21 @@ static void tgl_dkl_phy_set_signal_levels(struct intel_encoder *encoder,
>  					  DKL_TX_DPCNTL2_CFG_LOADGENSELECT_TX2_MASK,
>  					  val);
>  		}
> +
> +		/* Wa_16011342517:adl-p */

That one is tagged as 'pre-prod stepping' in bspec. Can someone try
to figure out which steppings are actually pre-prod and which are not?
The bspec page that is supposed to have that information has become
completely useless for new platforms :(

> +		if (display->platform.alderlake_p &&
> +		    IS_DISPLAY_STEP(display, STEP_A0, STEP_D0)) {
> +			if ((intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
> +			     crtc_state->port_clock == 594000) ||

The w/a says to do it for HDMI in general. Hmm, Windows does seem to do the
link rate change for HDMI as well though. Shrug.

> +			     (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) &&

Insufficient to catch all DP cases.

> +			     crtc_state->port_clock == 162000)) {
> +				intel_dkl_phy_rmw(display, DKLP_PCS_GLUE_TX_DPCNTL2(tc_port),
> +						  LOADGEN_SHARING_PMD_DISABLE, 1);
> +			} else {
> +				intel_dkl_phy_rmw(display, DKLP_PCS_GLUE_TX_DPCNTL2(tc_port),
> +						  LOADGEN_SHARING_PMD_DISABLE, 0);
> +			}
> +		}
>  	}

Windows seems to do this w/a before the DKL_TX_PMD_LANE_SUS write.
No idea if the order is meaningful or not, if yes we should do the
same, if not we should just combine this with the DKL_TX_DPCNTL2
loadgen programming we already do.

>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dkl_phy_regs.h b/drivers/gpu/drm/i915/display/intel_dkl_phy_regs.h
> index 56085b32956d..70ad3f1b1289 100644
> --- a/drivers/gpu/drm/i915/display/intel_dkl_phy_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy_regs.h
> @@ -188,6 +188,10 @@ struct intel_dkl_phy_reg {
>  								 _DKL_CMN_UC_DW27)
>  #define  DKL_CMN_UC_DW27_UC_HEALTH			(0x1 << 15)
>  
> +#define _DKLP_PCS_GLUE_TX_DPCNTL2                       0xB68

No idea what these weird 0x168b?? addressed are that are listed in bspec.
The whole DKL register documentation is a complete mess, but this seems
to be just DKL_TX_DPCNTL2.

> +#define DKLP_PCS_GLUE_TX_DPCNTL2(tc_port)		_DKL_REG(tc_port, \
> +								 _DKLP_PCS_GLUE_TX_DPCNTL2)
> +#define LOADGEN_SHARING_PMD_DISABLE                     REG_BIT(12)
>  /*
>   * Each Dekel PHY is addressed through a 4KB aperture. Each PHY has more than
>   * 4KB of register space, so a separate index is programmed in HIP_INDEX_REG0
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel