HDMI driver format channel mismatch bug

[Takashi Iwai]

On Fri, 12 Nov 2021 15:32:41 +0100,
sujith kumar reddy wrote:
> 
> Hi Alsa Team,
> 
> Sound is not coming in sony tv . which has below supported formats and
> channels.
> 
> please find the attached  edid information and hw_params dump.
> 
> with intel platform, same monitor populating 2 channels as max in hw_params
> in dump.
> but with AMD card, its populating as 6 channels.
> 
> 
> When we digged the code, we found that snd_hdmi_eld_update_pcm_info   API
> setting hinfo->channel_max as 8 and as channels max retrieved from sad info
> as 6.
> 
> 
> for LPCM , why the channel max assignment logic is not safeguared with
> audio format check as LPCM ?
> 
> 
> snippet code:
> 
> /* update PCM info based on ELD */void
> snd_hdmi_eld_update_pcm_info(struct parsed_hdmi_eld *e,
> 			      struct hda_pcm_stream *hinfo){
> 	u32 rates;
> 	u64 formats;
> 	unsigned int maxbps;
> 	unsigned int channels_max;
> 	int i;
> 
> 	/* assume basic audio support (the basic audio flag is not in ELD;	 *
> however, all audio capable sinks are required to support basic	 *
> audio) */
> 	rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |
> 		SNDRV_PCM_RATE_48000;
> 	formats = SNDRV_PCM_FMTBIT_S16_LE;
> 	maxbps = 16;
> 	channels_max = 2;
> 	for (i = 0; i < e->sad_count; i++) {
> 		struct cea_sad *a = &e->sad[i];
> 		rates |= a->rates;
> 		if (a->channels > channels_max)
> 			channels_max = a->channels;
> 		if (a->format == AUDIO_CODING_TYPE_LPCM) {
> 			if (a->sample_bits & AC_SUPPCM_BITS_20) {
> 				formats |= SNDRV_PCM_FMTBIT_S32_LE;
> 				if (maxbps < 20)
> 					maxbps = 20;
> 			}
> 			if (a->sample_bits & AC_SUPPCM_BITS_24) {
> 				formats |= SNDRV_PCM_FMTBIT_S32_LE;
> 				if (maxbps < 24)
> 					maxbps = 24;
> 			}
> 		}
> 	}
> 
> 	/* restrict the parameters by the values the codec provides */
> 	hinfo->rates &= rates;
> 	hinfo->formats &= formats;
> 	hinfo->maxbps = min(hinfo->maxbps, maxbps);
> 	hinfo->channels_max = min(hinfo->channels_max, channels_max);
> 
> >>>>>>>>>>the above statement channels_max is the maximum of channels supported of different formats.
> 
> For example: the below attached edid is of sony tv. which supports two
> formats(LPCM and AC3)
> 
>             LPCM --->supports 2 channels
> 
>             AC3 ----->Supports 6 channels you can see in the attached edid info.
> 
>        As AMD supports LPCM ...>when we keep logs here we got
> channels_max =6 .in which sound is not observed on tv .when we
> hardcode to 2 channels
> 
>        the sound is heard from monitor. As you see the above code
> ..for loop is not distinguishing channels_max for different formats.
> 
> }

Hm, the number of channels advertised from SAD makes little sense for
the hw_params that is rather for the PCM stream.  So the patch like
below would work, at least, for your examples.

The remaining question is whether it's only LPCM that we'd have to
take care of the channels.  For HBR, we'd have to set 8 channels.

Is HBR always tied with LPCM?  Or may it be with
AUDIO_CODING_TYPE_DTS_HD that advertises the channels?


thanks,

Takashi

--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -572,9 +572,9 @@ void snd_hdmi_eld_update_pcm_info(struct parsed_hdmi_eld *e,
 	for (i = 0; i < e->sad_count; i++) {
 		struct cea_sad *a = &e->sad[i];
 		rates |= a->rates;
-		if (a->channels > channels_max)
-			channels_max = a->channels;
 		if (a->format == AUDIO_CODING_TYPE_LPCM) {
+			if (a->channels > channels_max)
+				channels_max = a->channels;
 			if (a->sample_bits & AC_SUPPCM_BITS_20) {
 				formats |= SNDRV_PCM_FMTBIT_S32_LE;
 				if (maxbps < 20)