Re: [PATCH v2] ASoC: hdmi-codec: Add option for ELD bypass

[Maxime Ripard <maxime@cerno.tech>]

[-- Attachment #1: Type: text/plain, Size: 2821 bytes --]

Hi,

On Sat, Apr 30, 2022 at 01:41:18PM +0800, Sugar Zhang wrote:
> This patch allow users to enable "ELD Bypass" who don't
> care anything from EDID Link Data.
> 
> Currently, this driver gets ELD(from EDID) to constraint
> channels and rates.
> 
> Unfortunately, EDID is not always valid, maybe caused by
> the fragile HDMI port or cable, in this situation, the max
> features are limited to 48kHz stereo.
> 
> So, add this option to allow user to select the manual way
> to output audio as expected. such as multi-channels LPCM(7.1),
> or HBR bitstream for these sink devices.
> 
> Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>

I think some more documentation is needed there to describe how it's
going to be used.

Like, you mention that it's relevant when the EDID is not valid. But if
the EDID is valid, is bypass still allowed or not?

> ---
> 
> Changes in v2:
> - Use MACRO SOC_SINGLE_BOOL_EXT to simplify code.
>   Fix event_missing checked by mixer-test.
>   Add suffix "Switch" for "ELD Bypass".
> 
>  sound/soc/codecs/hdmi-codec.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
> index b07607a..be46fbd 100644
> --- a/sound/soc/codecs/hdmi-codec.c
> +++ b/sound/soc/codecs/hdmi-codec.c
> @@ -275,6 +275,7 @@ struct hdmi_codec_priv {
>  	unsigned int chmap_idx;
>  	struct mutex lock;
>  	bool busy;
> +	bool eld_bypass;
>  	struct snd_soc_jack *jack;
>  	unsigned int jack_status;
>  	u8 iec_status[AES_IEC958_STATUS_SIZE];
> @@ -427,6 +428,31 @@ static int hdmi_codec_iec958_mask_get(struct snd_kcontrol *kcontrol,
>  	return 0;
>  }
>  
> +static int hdmi_codec_eld_bypass_get(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
> +
> +	ucontrol->value.integer.value[0] = hcp->eld_bypass;
> +
> +	return 0;
> +}
> +
> +static int hdmi_codec_eld_bypass_put(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
> +
> +	if (hcp->eld_bypass == ucontrol->value.integer.value[0])
> +		return 0;
> +
> +	hcp->eld_bypass = ucontrol->value.integer.value[0];
> +
> +	return 1;
> +}

If the ELD bypass is set, how does it affect the hdmi_codec_params being
passed to the codec?

Also, what is being returned to the userspace by hdmi_eld_ctl_get once
the bypass is enabled?

And shouldn't we call get_eld when we remove the bypass?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]