[alsa-devel] Missing surround channels with Nvidia HDMI & snd-hda-intel driver

[Takashi Iwai]

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 at 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 at intel.com>
>     Signed-off-by: Vinod Koul <vinod.koul at intel.com>
>     Signed-off-by: Takashi Iwai <tiwai at 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;