On Tue, 9 Jun 2026, Luca Coelho 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
nasty nit: double space sign: s/ / /
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.
Reviewed-by: Jani Nikula <[email protected]>
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..f3d2a99c03d0 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
double space: s/ / /
+ * value (TRANSCODER_EDP, TRANSCODER_DSI_*, INVALID_TRANSCODER) is not
+ * valid here. Mask the index to the low 2 bits so the shift is
double space: s/ / /
+ * 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_TRANS_SHIFT(trans) (((trans) & 0x3) * 4)
+#define AUDIO_INACTIVE(trans)
REG_BIT(__AUDIO_TRANS_SHIFT(trans) + 3)
+#define AUDIO_OUTPUT_ENABLE(trans) REG_BIT(__AUDIO_TRANS_SHIFT(trans) + 2)
+#define AUDIO_CP_READY(trans)
REG_BIT(__AUDIO_TRANS_SHIFT(trans) + 1)
+#define AUDIO_ELD_VALID(trans) REG_BIT(__AUDIO_TRANS_SHIFT(trans))
I'm thinking about something like:
#define AUDIO_TRANS_MASK REG_GENMASK(1, 0)
#define __AUDIO_TRANS(trans, bit) (REG_FIELD_GET(AUDIO_TRANS_MASK, trans)
* 4 + (bit))
#define AUDIO_INACTIVE(trans) REG_BIT(__AUDIO_TRANS(trans, 3))
#define AUDIO_OUTPUT_ENABLE(trans) REG_BIT(__AUDIO_TRANS(trans, 2))
#define AUDIO_CP_READY(trans) REG_BIT(__AUDIO_TRANS(trans, 1))
#define AUDIO_ELD_VALID(trans) REG_BIT(__AUDIO_TRANS(trans, 0))
or, on a second thought, maybe even:
#define AUDIO_TRANS_MASK REG_GENMASK(3, 2)
#define __AUDIO_TRANS_SHIFT(trans) (trans << 2)
#define __AUDIO_TRANS(trans, bit) (REG_FIELD_GET(AUDIO_TRANS_MASK,
__AUDIO_TRANS_SHIFT(trans)) + (bit))
...
since multiplying by 4 should be effectively left-shifting by 2,
and it shouldn't matter whether we first do the left-shift or bitwise
AND.
But no idea if it makes any sense. Anyways:
Reviewed-by: Michał Grzelak <[email protected]>
BR,
Michał
#define _AUD_TCA_DP_2DOT0_CTRL 0x650bc
#define _AUD_TCB_DP_2DOT0_CTRL 0x651bc
