Hi Geraldo, and apologies for resurrecting a 2 year old thread...
On 17/10/2022 13:04, Geraldo Nascimento wrote:
Hi Mark, resending this as it failed to apply in my last submission. Added Neil Armstrong to Cc: as hopefully he will be able to better review this.
Thanks, Geraldo Nascimento
Starting with version 2.10a of Synopsys DesignWare HDMI controller the insert_pcuv bit was introduced. On RK3399pro SoM (Radxa Rock Pi N10), for example, if we neglect to set this bit and proceed to enable hdmi_sound and i2s2 on the device tree there will be extreme clipping of sound output, to the point that music sounds like white noise. Problem could also manifest as just mild cracking depending of HDMI audio implementation of sink. Setting insert_pcuv bit (bit 2 of aud_conf2 Audio Sample register) fixes this.
Signed-off-by: Geraldo Nascimento geraldogabriel@gmail.com
I also had the HDMI audio clipping issue described here, on a RK3399. This was on a 6.1.23 kernel based on the one used by LibreELEC.tv with their out-of-tree patches for video decoding, 4k HDMI support, etc. When testing this patch I also updated my kernel tree to 6.10.3, and found that even without this patch, on 6.10.3 the problem no longer happens.
I added printk to show the value of AUD_CONF2, and found that on 6.1.23, the value is 0 before the code in this patch sets the insert_pcuv bit. On 6.10.3 the value is 4, i.e. insert_pcuv is already set.
According to the RK3399 TRM, the value-after-reset of the insert_pcuv bit is 1, so apparently on the 6.1.23 kernel something is clearing the bit after HW reset but before this driver sets the hw_params, and this patch sets it back to the correct value. On 6.10.3 the bit is not cleared, i.e. this patch is seemingly no longer necessary (but is a harmless no-op).
v1->v2: SoC->SoM on description, better commenting, minor style changes, conditional application of fix for L-PCM only
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio-20221017.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio-20221017.c @@ -42,6 +42,7 @@ static int dw_hdmi_i2s_hw_params(struct device *dev, void *data, struct dw_hdmi *hdmi = audio->hdmi; u8 conf0 = 0; u8 conf1 = 0;
u8 conf2 = 0; u8 inputclkfs = 0;
/* it cares I2S only */
@@ -101,6 +102,28 @@ static int dw_hdmi_i2s_hw_params(struct device *dev, void *data, return -EINVAL; }
- /*
* dw-hdmi introduced insert_pcuv bit in* version 2.10a.** This single bit (bit 2 of HDMI_AUD_CONF2)* when set to 1 will enable the insertion of the PCUV* (Parity, Channel Status, User bit and Validity)* bits on the incoming audio stream.** Support is limited to Linear PCM audio. If* neglected, the lack of valid PCUV bits* on L-PCM streams will cause anything from* mild cracking to full blown extreme* clipping depending on the HDMI audio* implementation of the sink.**/- if (hdmi_read(audio, HDMI_DESIGN_ID) >= 0x21 &&
!(hparms->iec.status[0] & IEC958_AES0_NONAUDIO))conf2 = HDMI_AUD_CONF2_INSERT_PCUV;- dw_hdmi_set_sample_rate(hdmi, hparms->sample_rate); dw_hdmi_set_channel_status(hdmi, hparms->iec.status); dw_hdmi_set_channel_count(hdmi, hparms->channels);
@@ -109,6 +120,7 @@ static int dw_hdmi_i2s_hw_params(struct device *dev, void *data, hdmi_write(audio, inputclkfs, HDMI_AUD_INPUTCLKFS); hdmi_write(audio, conf0, HDMI_AUD_CONF0); hdmi_write(audio, conf1, HDMI_AUD_CONF1);
hdmi_write(audio, conf2, HDMI_AUD_CONF2);
return 0;
} --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-20221017.h +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-20221017.h
btw, this patch doesn't apply without edits as these filenames are incorrect.
@@ -931,6 +931,11 @@ enum { HDMI_AUD_CONF1_WIDTH_16 = 0x10, HDMI_AUD_CONF1_WIDTH_24 = 0x18,
+/* AUD_CONF2 field values */
- HDMI_AUD_CONF2_HBR = 0x01,
- HDMI_AUD_CONF2_NLPCM = 0x02,
- HDMI_AUD_CONF2_INSERT_PCUV = 0x04,
/* AUD_CTS3 field values */ HDMI_AUD_CTS3_N_SHIFT_OFFSET = 5, HDMI_AUD_CTS3_N_SHIFT_MASK = 0xe0,
Best regards, Hugh