HDMI driver format channel mismatch bug

[Takashi Iwai]

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 at suse.de> 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)
> 
>