Re: [PATCH v5 01/16] drm/i915/display: remove enum macro magic in intel_display_wa()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 06 Mar 2026, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 05 Mar 2026, Luca Coelho <luciano.coelho@intel.com> wrote:
>> There's not much use in passing a number to the macro and let it
>> convert that into the enum and a string.  It just hides the symbols.
>
> There was a point, though. Passing a random number to intel_display_wa()
> would fail the build. Now you'll only find out runtime. I'll concede
> there's value, perhaps more so, in having the symbols not hidden.
>
>> Remove the number to enum conversion magic in intel_display_wa().
>>
>> This has the side-effect of changing the print in the drm_WARN() that
>> is issued when the number is not implemented, but that is moot anyway
>> and can be changed later to something cleaner if needed.
>
> I actually wonder if having the names there is really worth it. It's
> just a bunch of rodata bloat for not much benefit. It should be easy
> enough to find which enumerator isn't handled.
>
> Maybe check the objdump bloat reduction with the intel_display_wa()
> wrapper macro and the const char *name parameter removed?

PS. All of this can be done in follow-up too, to not hold this series
back any longer.

>
> BR,
> Jani.
>
>>
>> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cdclk.c              | 5 +++--
>>  drivers/gpu/drm/i915/display/intel_display.c            | 2 +-
>>  drivers/gpu/drm/i915/display/intel_display_power_well.c | 4 ++--
>>  drivers/gpu/drm/i915/display/intel_display_wa.c         | 2 +-
>>  drivers/gpu/drm/i915/display/intel_display_wa.h         | 2 +-
>>  drivers/gpu/drm/i915/display/intel_fbc.c                | 8 ++++----
>>  drivers/gpu/drm/i915/display/intel_gmbus.c              | 6 +++---
>>  drivers/gpu/drm/i915/display/skl_scaler.c               | 2 +-
>>  8 files changed, 16 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index 38331e899519..7767f8c198da 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -1870,7 +1870,7 @@ static void icl_cdclk_pll_disable(struct intel_display *display)
>>  	 *      after the PLL is enabled (which is already done as part of the
>>  	 *      normal flow of _bxt_set_cdclk()).
>>  	 */
>> -	if (intel_display_wa(display, 13012396614))
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_13012396614))
>>  		intel_de_rmw(display, CDCLK_CTL, MDCLK_SOURCE_SEL_MASK, MDCLK_SOURCE_SEL_CD2XCLK);
>>  
>>  	intel_de_rmw(display, BXT_DE_PLL_ENABLE,
>> @@ -2186,7 +2186,8 @@ static u32 bxt_cdclk_ctl(struct intel_display *display,
>>  		 * icl_cdclk_pll_disable().  Here we are just making sure
>>  		 * we keep the expected value.
>>  		 */
>> -		if (intel_display_wa(display, 13012396614) && vco == 0)
>> +		if (intel_display_wa(display, INTEL_DISPLAY_WA_13012396614) &&
>> +		    vco == 0)
>>  			val |= MDCLK_SOURCE_SEL_CD2XCLK;
>>  		else
>>  			val |= xe2lpd_mdclk_source_sel(display);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 138ee7dd1977..15edf609fff4 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -1070,7 +1070,7 @@ static void intel_post_plane_update(struct intel_atomic_state *state,
>>  	if (audio_enabling(old_crtc_state, new_crtc_state))
>>  		intel_encoders_audio_enable(state, crtc);
>>  
>> -	if (intel_display_wa(display, 14011503117)) {
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_14011503117)) {
>>  		if (old_crtc_state->pch_pfit.enabled != new_crtc_state->pch_pfit.enabled)
>>  			adl_scaler_ecc_unmask(new_crtc_state);
>>  	}
>> 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 9c8d29839caf..1e03187dbd38 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> @@ -249,7 +249,7 @@ static void hsw_power_well_post_enable(struct intel_display *display,
>>  	if (irq_pipe_mask) {
>>  		gen8_irq_power_well_post_enable(display, irq_pipe_mask);
>>  
>> -		if (intel_display_wa(display, 22021048059))
>> +		if (intel_display_wa(display, INTEL_DISPLAY_WA_22021048059))
>>  			dss_pipe_gating_enable_disable(display, irq_pipe_mask, false);
>>  	}
>>  }
>> @@ -258,7 +258,7 @@ static void hsw_power_well_pre_disable(struct intel_display *display,
>>  				       u8 irq_pipe_mask)
>>  {
>>  	if (irq_pipe_mask) {
>> -		if (intel_display_wa(display, 22021048059))
>> +		if (intel_display_wa(display, INTEL_DISPLAY_WA_22021048059))
>>  			dss_pipe_gating_enable_disable(display, irq_pipe_mask, true);
>>  
>>  		gen8_irq_power_well_pre_disable(display, irq_pipe_mask);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c b/drivers/gpu/drm/i915/display/intel_display_wa.c
>> index c2ccdca2c2f3..1d8340b36c01 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_wa.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
>> @@ -87,7 +87,7 @@ bool __intel_display_wa(struct intel_display *display, enum intel_display_wa wa,
>>  	case INTEL_DISPLAY_WA_22021048059:
>>  		return IS_DISPLAY_VER(display, 14, 35);
>>  	default:
>> -		drm_WARN(display->drm, 1, "Missing Wa number: %s\n", name);
>> +		drm_WARN(display->drm, 1, "Missing Wa: %s\n", name);
>>  		break;
>>  	}
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h b/drivers/gpu/drm/i915/display/intel_display_wa.h
>> index 767420d5f406..06c1f62c0f6d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
>> @@ -40,6 +40,6 @@ enum intel_display_wa {
>>  bool __intel_display_wa(struct intel_display *display, enum intel_display_wa wa, const char *name);
>>  
>>  #define intel_display_wa(__display, __wa) \
>> -	__intel_display_wa((__display), INTEL_DISPLAY_WA_##__wa, __stringify(__wa))
>> +	__intel_display_wa((__display), __wa, __stringify(__wa))
>>  
>>  #endif
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
>> index 3e9b3e532499..5d0d7c1027f5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>> @@ -957,7 +957,7 @@ static void intel_fbc_program_workarounds(struct intel_fbc *fbc)
>>  	 * Fixes: Screen flicker with FBC and Package C state enabled
>>  	 * Workaround: Forced SLB invalidation before start of new frame.
>>  	 */
>> -	if (intel_display_wa(display, 22014263786))
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_22014263786))
>>  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
>>  			     0, DPFC_CHICKEN_FORCE_SLB_INVALIDATION);
>>  
>> @@ -979,7 +979,7 @@ static void fbc_sys_cache_update_config(struct intel_display *display, u32 reg,
>>  	 * Fixes: SoC hardware issue in read caching
>>  	 * Workaround: disable cache read setting which is enabled by default.
>>  	 */
>> -	if (!intel_display_wa(display, 14025769978))
>> +	if (!intel_display_wa(display, INTEL_DISPLAY_WA_14025769978))
>>  		/* Cache read enable is set by default */
>>  		reg |= FBC_SYS_CACHE_READ_ENABLE;
>>  
>> @@ -1612,7 +1612,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state,
>>  		return 0;
>>  	}
>>  
>> -	if (intel_display_wa(display, 16023588340)) {
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_16023588340)) {
>>  		plane_state->no_fbc_reason = "Wa_16023588340";
>>  		return 0;
>>  	}
>> @@ -1622,7 +1622,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state,
>>  	 * Fixes: Underrun during media decode
>>  	 * Workaround: Do not enable FBC
>>  	 */
>> -	if (intel_display_wa(display, 15018326506)) {
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_15018326506)) {
>>  		plane_state->no_fbc_reason = "Wa_15018326506";
>>  		return 0;
>>  	}
>> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
>> index 38706017c0c6..df48f27f1cc1 100644
>> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
>> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
>> @@ -250,7 +250,7 @@ static u32 get_reserved(struct intel_gmbus *bus)
>>  	preserve_bits |= GPIO_DATA_PULLUP_DISABLE | GPIO_CLOCK_PULLUP_DISABLE;
>>  
>>  	/* Wa_16025573575: the masks bits need to be preserved through out */
>> -	if (intel_display_wa(display, 16025573575))
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_16025573575))
>>  		preserve_bits |= GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_VAL_MASK |
>>  				 GPIO_DATA_DIR_MASK | GPIO_DATA_VAL_MASK;
>>  
>> @@ -342,7 +342,7 @@ intel_gpio_pre_xfer(struct i2c_adapter *adapter)
>>  	if (display->platform.pineview)
>>  		pnv_gmbus_clock_gating(display, false);
>>  
>> -	if (intel_display_wa(display, 16025573575))
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_16025573575))
>>  		ptl_handle_mask_bits(bus, true);
>>  
>>  	set_data(bus, 1);
>> @@ -363,7 +363,7 @@ intel_gpio_post_xfer(struct i2c_adapter *adapter)
>>  	if (display->platform.pineview)
>>  		pnv_gmbus_clock_gating(display, true);
>>  
>> -	if (intel_display_wa(display, 16025573575))
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_16025573575))
>>  		ptl_handle_mask_bits(bus, false);
>>  }
>>  
>> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
>> index 4c4deac7f9c8..7c5cb188ebf0 100644
>> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
>> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
>> @@ -823,7 +823,7 @@ void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>>  			crtc_state->scaler_state.scaler_id < 0))
>>  		return;
>>  
>> -	if (intel_display_wa(display, 14011503117))
>> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_14011503117))
>>  		adl_scaler_ecc_mask(crtc_state);
>>  
>>  	drm_rect_init(&src, 0, 0,

-- 
Jani Nikula, Intel