[RESEND PATCH v2] drm/bridge: dw-hdmi-i2s: set insert_pcuv bit if hardware supports it

[Neil Armstrong]

Hi,


On 17/10/2022 14: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.


I did some research and this insert_pcuv is already present in the 1.40a version
of the dw-hdmi databook, so I wonder why suddenly this is needed.

The insert_pcuv is documented as:
-------------------------------------------------------
When set (1'b1), it enables the insertion of the PCUV (Parity, Channel Status, User
bit and Validity) bits on the incoming audio stream (support limited to Linear PCM
audio).
If disabled, the incomming audio stream must contain the PCUV bits, mapped
acording to 2.6.4.2 Data Mapping Examples
--------------------------------------------------------


What's interesting is this register is only present if thre DW-HDMI IP is configured
as GPAUD or GDOUBLE, meaning it must have GPAUD enabled. So it has
something to do with it, so what's value of it when GPAUD isn't present in the IP ?

And HDMI2 spec added this, even PCVU were required before:
--------------------------------------------------------
Note that PCUV refers to the parity bit (P), channel status (C), user data (U), and validity bit (V) as defined in IEC
60958-1.
--------------------------------------------------------

So it has something to do with IEC60958-1 stream format, do maybe this
insert_pcuv should only be enforced when the input stream is _not_ IEC60958-1 ?

Neil

> 
> Signed-off-by: Geraldo Nascimento <geraldogabriel at gmail.com>
> 
> ---
> 
> 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
> @@ -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,