On Tue, 19 May 2026, Luca Coelho <[email protected]> wrote: > HSW_AUD_PIN_ELD_CP_VLD has a 4-bit field per transcoder for > TRANSCODER_A..TRANSCODER_D only (bits 0..15). Any other transcoder > value (TRANSCODER_EDP, TRANSCODER_DSI_*, INVALID_TRANSCODER) is not > valid here. > > This is not a problem with the current implementation, because trans > is always valid when these macros are called, but it's more robust to > mask the index to the low 2 bits so the shift is always well-defined. > > Signed-off-by: Luca Coelho <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_audio_regs.h | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_audio_regs.h > b/drivers/gpu/drm/i915/display/intel_audio_regs.h > index 4c31844d21df..25df7af4f67f 100644 > --- a/drivers/gpu/drm/i915/display/intel_audio_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_audio_regs.h > @@ -109,12 +109,20 @@ > #define _HSW_AUD_EDID_DATA_B 0x65150 > #define HSW_AUD_EDID_DATA(trans) _MMIO_TRANS(trans, > _HSW_AUD_EDID_DATA_A, _HSW_AUD_EDID_DATA_B) > > +/* > + * HSW_AUD_PIN_ELD_CP_VLD has a 4-bit field per transcoder for > + * TRANSCODER_A..TRANSCODER_D only (bits 0..15). Any other transcoder > + * value (TRANSCODER_EDP, TRANSCODER_DSI_*, INVALID_TRANSCODER) is not > + * valid here. Mask the index to the low 2 bits so the shift is > + * always well-defined. > + */ > #define HSW_AUD_PIPE_CONV_CFG _MMIO(0x6507c) > #define HSW_AUD_PIN_ELD_CP_VLD _MMIO(0x650c0) > -#define AUDIO_INACTIVE(trans) ((1 << 3) << ((trans) * 4)) > -#define AUDIO_OUTPUT_ENABLE(trans) ((1 << 2) << ((trans) * 4)) > -#define AUDIO_CP_READY(trans) ((1 << 1) << ((trans) * 4)) > -#define AUDIO_ELD_VALID(trans) ((1 << 0) << ((trans) * 4)) > +#define __AUDIO_MASK_TRANS(trans) (((unsigned int)(trans) & 0x3) * 4) > +#define AUDIO_INACTIVE(trans) ((u32)BIT(3) << > __AUDIO_MASK_TRANS(trans)) > +#define AUDIO_OUTPUT_ENABLE(trans) ((u32)BIT(2) << > __AUDIO_MASK_TRANS(trans)) > +#define AUDIO_CP_READY(trans) ((u32)BIT(1) << > __AUDIO_MASK_TRANS(trans)) > +#define AUDIO_ELD_VALID(trans) ((u32)BIT(0) << > __AUDIO_MASK_TRANS(trans))
Please use REG_BIT() instead of (u32)BIT(). All of this would be cleaner with the REG_GENMASK() stuff too. BR, Jani. > > #define _AUD_TCA_DP_2DOT0_CTRL 0x650bc > #define _AUD_TCB_DP_2DOT0_CTRL 0x651bc -- Jani Nikula, Intel
