Re: Missing surround channels with Nvidia HDMI & snd-hda-intel driver

[VDR User <user.vdr@gmail.com>]

Hi Takashi,

I can confirm that your patch does fix the bug:

[ao/alsa] trying to set ALSA channel map: FL FR FC LFE RL RR
[ao/alsa] channel map reported by ALSA: FL FR FC LFE RL RR
[ao/alsa] which we understand as: 5.1

/usr/src/linux$ uname -r
4.14.0-amd
/usr/src/linux$ speaker-test -c 6 -t wav

speaker-test 1.1.3

Playback device is default
Stream parameters are 48000Hz, S16_LE, 6 channels
WAV file(s)
Rate set to 48000Hz (requested 48000Hz)
Buffer size range from 64 to 5440
Period size range from 32 to 2720
Using max buffer size 5440
Periods = 4
was set period_size = 1088
was set buffer_size = 5440
 0 - Front Left
 4 - Front Center
 1 - Front Right
 3 - Rear Right
 2 - Rear Left
 5 - LFE
Time per period = 8.525875

Thank you for all your help to resolve this bug!
-Derek

On Thu, Nov 16, 2017 at 11:33 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Thu, 16 Nov 2017 22:28:17 +0100,
> VDR User wrote:
>>
>> > What you can try is to bisect like
>> >    git bisect start -- sound/hda sound/pci/hda
>> >    git bisect good v4.5
>> >    git bisect bad v4.6-rc1
>>
>> Ok, did more bisect'ing. I had to skip a commit because the compile
>> failed but other than that, this is what I wound up with:
>>
>> git bisect start '--' 'sound/hda' 'sound/pci/hda'
>> # good: [fe0d128c57bf927a713159f60a18d9f315d4d91d] ALSA: jack: Allow
>> building the jack layer without input device
>> git bisect good fe0d128c57bf927a713159f60a18d9f315d4d91d
>> # bad: [9b3dc8aa3fb1a5f38ca9501f20f6ea4dced10fa0] ALSA: hda - Register
>> chmap obj as priv data instead of codec
>> git bisect bad 9b3dc8aa3fb1a5f38ca9501f20f6ea4dced10fa0
>> # skip: [d61b04f801e6005182d432ebe4a0211c1d6feadd] Merge branch
>> 'for-linus' into for-next
>> git bisect skip d61b04f801e6005182d432ebe4a0211c1d6feadd
>> # good: [ec75a940b1037e877efd9a5a9e94eab1e464f73b] ALSA: hda - hdmi
>> add wmb barrier for audio component
>> git bisect good ec75a940b1037e877efd9a5a9e94eab1e464f73b
>> # good: [d10a80de04a3a8c0d7c1567cbc0a8d2e1181c10a] ALSA: hda -
>> hdmi_find_pcm_slot return value bug fix
>> git bisect good d10a80de04a3a8c0d7c1567cbc0a8d2e1181c10a
>> # good: [6defb60ae4ffe92c29291ed49695daa255b8f559] Merge branch
>> 'for-linus' into for-next
>> git bisect good 6defb60ae4ffe92c29291ed49695daa255b8f559
>> # good: [67b90cb84be8fde0e51f71834e15c32fbec08562] ALSA: hda - Create
>> common chmap object
>> git bisect good 67b90cb84be8fde0e51f71834e15c32fbec08562
>> # first bad commit: [9b3dc8aa3fb1a5f38ca9501f20f6ea4dced10fa0] ALSA:
>> hda - Register chmap obj as priv data instead of codec
>>
>> 9b3dc8aa3fb1a5f38ca9501f20f6ea4dced10fa0 is the first bad commit
>> commit 9b3dc8aa3fb1a5f38ca9501f20f6ea4dced10fa0
>> Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
>> Date:   Fri Mar 4 19:59:47 2016 +0530
>>
>>     ALSA: hda - Register chmap obj as priv data instead of codec
>>
>>     With this chmap object is added as private data and new ops are
>>     added to access driver specific chmap.
>>
>>     Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
>>     Signed-off-by: Vinod Koul <vinod.koul@intel.com>
>>     Signed-off-by: Takashi Iwai <tiwai@suse.de>
>>
>> :040000 040000 11dc1a68fd79182d4e38c2a7fd73fd699dbd0685
>> 69c05f445bc255fc02441742b091247af99fb177 M      include
>> :040000 040000 8946088a30757dfcbc1ea6badbe6ea8366b2b39f
>> 6fcf7ce7986ffd29d3b5686ebe63168713b866ed M      sound
>>
>> Does this sound like it could be where the bug resides?.
>
> Thanks for the effort!
> Yes, it looks more likely than before.
>
>> I created a
>> patch with `git format-patch -1
>> 9b3dc8aa3fb1a5f38ca9501f20f6ea4dced10fa0` and tried to apply it to
>> kernel 4.6-rc1 but I failed. I didn't bother trying to hand-reverse it
>> because it looks like it would just cause breakage. Not sure what to
>> do next here so I'll wait and see what you think..
>
> I guess the bug is at copying the chmap data where calculating the
> size wrongly.  A typical error of sizeof() usage.
>
> The fix would be to correct sizeof(chmap) to ARRAY_SIZE(pcm_chmap).
> At the commit 9b3dc8aa3fb1a5f3, it's in hdmi_chmap_ctl_get() in
> patch_hdmi.c, correct like:
>
> -       for (i = 0; i < sizeof(chmap); i++)
> +       for (i = 0; i < ARRAY_SIZE(pcm_chmap); i++)
>
>
> For the latest upstream, the relevant code was moved to
> sound/hda/hdmi_chmap.c, and the fix would be a patch like below.
>
>
> Could you check it?
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/sound/hda/hdmi_chmap.c b/sound/hda/hdmi_chmap.c
> index 81acc20c2535..f21633cd9b38 100644
> --- a/sound/hda/hdmi_chmap.c
> +++ b/sound/hda/hdmi_chmap.c
> @@ -746,7 +746,7 @@ static int hdmi_chmap_ctl_get(struct snd_kcontrol *kcontrol,
>         memset(pcm_chmap, 0, sizeof(pcm_chmap));
>         chmap->ops.get_chmap(chmap->hdac, pcm_idx, pcm_chmap);
>
> -       for (i = 0; i < sizeof(chmap); i++)
> +       for (i = 0; i < ARRAY_SIZE(pcm_chmap); i++)
>                 ucontrol->value.integer.value[i] = pcm_chmap[i];
>
>         return 0;