Re: [PATCH v2 13/15] drm/i915/display: convert W/As in intel_psr.c to new framework

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

On Thu, Feb 12, 2026 at 08:46:11PM +0200, Luca Coelho wrote:
> Convert the low-hanging fruits of workaround checks to the workaround
> framework.  Instead of having display structure checks for the
> workarounds all over, concentrate the checks in intel_wa.c.
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_display_wa.c   | 15 ++++++++++++--
>  .../gpu/drm/i915/display/intel_display_wa.h   |  4 ++++
>  drivers/gpu/drm/i915/display/intel_psr.c      | 20 ++++++++-----------
>  3 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c b/drivers/gpu/drm/i915/display/intel_display_wa.c
> index 77ea2e5b8144..783e1383ff89 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_wa.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
> @@ -112,6 +112,13 @@ bool __intel_display_wa(struct intel_display *display, enum intel_display_wa wa,
>  		return DISPLAY_VER(display) == 20;
>  	case INTEL_DISPLAY_WA_15018326506:
>  		return display->platform.battlemage;
> +	case INTEL_DISPLAY_WA_16011303918:
> +	case INTEL_DISPLAY_WA_22011320316:
> +		return display->platform.alderlake_p &&
> +			IS_DISPLAY_STEP(display, STEP_A0, STEP_B0);
> +	case INTEL_DISPLAY_WA_16011181250:
> +		return display->platform.rocketlake || display->platform.alderlake_s ||
> +			display->platform.dg2;
>  	case INTEL_DISPLAY_WA_16011342517:
>  		return display->platform.alderlake_p &&
>  			IS_DISPLAY_STEP(display, STEP_A0, STEP_D0);
> @@ -121,15 +128,19 @@ bool __intel_display_wa(struct intel_display *display, enum intel_display_wa wa,
>  		return intel_display_needs_wa_16023588340(display);
>  	case INTEL_DISPLAY_WA_16025573575:
>  		return intel_display_needs_wa_16025573575(display);
> +	case INTEL_DISPLAY_WA_16025596647:
> +		return DISPLAY_VER(display) != 20 &&
> +			!IS_DISPLAY_VERx100_STEP(display, 3000,
> +						 STEP_A0, STEP_B0);

This one is nuts. It declarates (incorrectly) which platforms don't
need the w/a. I don't think this sort of thing should be allowed here
ever.

Presumably the only reason it was OK in the old place is because 
those codepaths were only executed on some new platforms. But
__intel_display_wa() is so generic that is is clearly meant to
give correct answers regardless of where it gets called.

>  	case INTEL_DISPLAY_WA_18034343758:
>  		return DISPLAY_VER(display) == 20 ||
>  			(display->platform.pantherlake &&
>  			 IS_DISPLAY_STEP(display, STEP_A0, STEP_B0));
>  	case INTEL_DISPLAY_WA_22010178259:
>  		return DISPLAY_VER(display) == 12;
> -	case INTEL_DISPLAY_WA_22011320316:
> +	case INTEL_DISPLAY_WA_22012278275:
>  		return display->platform.alderlake_p &&
> -			IS_DISPLAY_STEP(display, STEP_A0, STEP_B0);
> +			IS_DISPLAY_STEP(display, STEP_A0, STEP_E0);
>  	case INTEL_DISPLAY_WA_22014263786:
>  		return IS_DISPLAY_VERx100(display, 1100, 1400);
>  	case INTEL_DISPLAY_WA_22021048059:
> diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h b/drivers/gpu/drm/i915/display/intel_display_wa.h
> index 3d2cf05ffacc..35d8df4c75a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
> @@ -44,13 +44,17 @@ enum intel_display_wa {
>  	INTEL_DISPLAY_WA_14025769978,
>  	INTEL_DISPLAY_WA_15013987218,
>  	INTEL_DISPLAY_WA_15018326506,
> +	INTEL_DISPLAY_WA_16011181250,
> +	INTEL_DISPLAY_WA_16011303918,
>  	INTEL_DISPLAY_WA_16011342517,
>  	INTEL_DISPLAY_WA_16011863758,
>  	INTEL_DISPLAY_WA_16023588340,
>  	INTEL_DISPLAY_WA_16025573575,
> +	INTEL_DISPLAY_WA_16025596647,
>  	INTEL_DISPLAY_WA_18034343758,
>  	INTEL_DISPLAY_WA_22010178259,
>  	INTEL_DISPLAY_WA_22011320316,
> +	INTEL_DISPLAY_WA_22012278275,
>  	INTEL_DISPLAY_WA_22012358565,
>  	INTEL_DISPLAY_WA_22014263786,
>  	INTEL_DISPLAY_WA_22021048059,
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 5bea2eda744b..b21e52f0c461 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -40,6 +40,7 @@
>  #include "intel_display_rpm.h"
>  #include "intel_display_types.h"
>  #include "intel_display_utils.h"
> +#include "intel_display_wa.h"
>  #include "intel_dmc.h"
>  #include "intel_dp.h"
>  #include "intel_dp_aux.h"
> @@ -1082,7 +1083,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	}
>  
>  	/* Wa_22012278275:adl-p */
> -	if (display->platform.alderlake_p && IS_DISPLAY_STEP(display, STEP_A0, STEP_E0)) {
> +	if (intel_display_wa(display, 22012278275)) {
>  		static const u8 map[] = {
>  			2, /* 5 lines */
>  			1, /* 6 lines */
> @@ -1263,7 +1264,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp,
>  		return;
>  
>  	/* Wa_16011303918:adl-p */
> -	if (display->platform.alderlake_p && IS_DISPLAY_STEP(display, STEP_A0, STEP_B0))
> +	if (intel_display_wa(display, 16011303918))
>  		return;
>  
>  	/*
> @@ -1540,8 +1541,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>  	}
>  
>  	/* Wa_16011181250 */
> -	if (display->platform.rocketlake || display->platform.alderlake_s ||
> -	    display->platform.dg2) {
> +	if (intel_display_wa(display, 16011181250)) {
>  		drm_dbg_kms(display->drm,
>  			    "PSR2 is defeatured for this platform\n");
>  		return false;
> @@ -1823,8 +1823,7 @@ void intel_psr_set_non_psr_pipes(struct intel_dp *intel_dp,
>  	u8 active_pipes = 0;
>  
>  	/* Wa_16025596647 */
> -	if (DISPLAY_VER(display) != 20 &&
> -	    !IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0, STEP_B0))
> +	if (intel_display_wa(display, 16025596647))
>  		return;
>  
>  	/* Not needed by Panel Replay  */
> @@ -3973,8 +3972,7 @@ static void psr_dc5_dc6_wa_work(struct work_struct *work)
>   */
>  void intel_psr_notify_dc5_dc6(struct intel_display *display)
>  {
> -	if (DISPLAY_VER(display) != 20 &&
> -	    !IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0, STEP_B0))
> +	if (intel_display_wa(display, 16025596647))
>  		return;
>  
>  	schedule_work(&display->psr_dc5_dc6_wa_work);
> @@ -3989,8 +3987,7 @@ void intel_psr_notify_dc5_dc6(struct intel_display *display)
>   */
>  void intel_psr_dc5_dc6_wa_init(struct intel_display *display)
>  {
> -	if (DISPLAY_VER(display) != 20 &&
> -	    !IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0, STEP_B0))
> +	if (intel_display_wa(display, 16025596647))
>  		return;
>  
>  	INIT_WORK(&display->psr_dc5_dc6_wa_work, psr_dc5_dc6_wa_work);
> @@ -4011,8 +4008,7 @@ void intel_psr_notify_pipe_change(struct intel_atomic_state *state,
>  	struct intel_display *display = to_intel_display(state);
>  	struct intel_encoder *encoder;
>  
> -	if (DISPLAY_VER(display) != 20 &&
> -	    !IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0, STEP_B0))
> +	if (intel_display_wa(display, 16025596647))
>  		return;
>  
>  	for_each_intel_encoder_with_psr(display->drm, encoder) {
> -- 
> 2.51.0

-- 
Ville Syrjälä
Intel