On Wed, 17 Nov 2021 09:31:25 +0100, sujith kumar reddy wrote: > > Hi Takashi, > > By HBR definition macro its non-pcm right? > > /* HBR should be Non-PCM, 8 channels > > */#define is_hbr_format(format) \ ((format & AC_FMT_TYPE_NON_PCM) && (format & > AC_FMT_CHAN_MASK) == 7) > > if we restrict channel_max for LPCM does it break it for other formats in > passthrough mode? It's the format for HD-audio codec, and it's set by the driver when IEC958 status bits contains the non-audio bit. So this format value is basically independent from SAD contents. But, the change I proposed was utterly untested, and I'm not sure whether it works at all. Takashi > > Thanks > Sujith > > On Wed, Nov 17, 2021 at 1:19 PM Takashi Iwai <tiwai@xxxxxxx> wrote: > > 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) > >