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@xxxxxxxxx>
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
