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? 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
