On Fri, 06 Mar 2026, Jani Nikula <[email protected]> wrote: > On Thu, 05 Mar 2026, Luca Coelho <[email protected]> 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 <[email protected]> >> Signed-off-by: Luca Coelho <[email protected]> >> --- >> 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
