Regression: Kernel 4.6 DisplayPort Audio

Regression: Kernel 4.6 DisplayPort Audio

[Daniel Exner]

[-- Attachment #1.1: Type: text/plain, Size: 904 bytes --]

Hi,

(Please keep me CC as I am currently not subscribed to LKML or any other
Linux Kernel Dev ML, thanks)

Since very first 4.6 rc1 the speakers integrated in my monitor (Dell
U3415W) stopped working. It is connected via DisplayPort from a ATI R7 270X.

Same setup works just fine in 4.5.3.

The bug report about this is:

https://bugzilla.kernel.org/show_bug.cgi?id=114981

I managed to bisect this down to:

commit 739ffee97ed550a2899a925ed3f260fa1e8fa955
Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Date:   Fri Mar 4 19:59:49 2016 +0530

    ALSA: hda - Add hdmi chmap verb programming ops to chmap object

But I cannot easily revert this from current rc7.
Please make me a happy DP Audio User again :)

I can provide further informations or test possible patches.

Greetings
Daniel
-- 
Daniel Exner
Public-Key: https://www.dragonslave.de/pub_key.asc


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Regression: Kernel 4.6 DisplayPort Audio

[Takashi Iwai]

On Wed, 11 May 2016 10:32:07 +0200,
Daniel Exner wrote:
> 
> Hi,
> 
> (Please keep me CC as I am currently not subscribed to LKML or any other
> Linux Kernel Dev ML, thanks)
> 
> Since very first 4.6 rc1 the speakers integrated in my monitor (Dell
> U3415W) stopped working. It is connected via DisplayPort from a ATI R7 270X.
> 
> Same setup works just fine in 4.5.3.
> 
> The bug report about this is:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=114981
> 
> I managed to bisect this down to:
> 
> commit 739ffee97ed550a2899a925ed3f260fa1e8fa955
> Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Date:   Fri Mar 4 19:59:49 2016 +0530
> 
>     ALSA: hda - Add hdmi chmap verb programming ops to chmap object
> 
> But I cannot easily revert this from current rc7.
> Please make me a happy DP Audio User again :)
> 
> I can provide further informations or test possible patches.

If the commit above alone breaks, does the patch below change the
behavior?  Through a quick look at it, some ops are overridden only
conditionally while the older code always overwrote them.


thanks,

Takashi

---
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index d2e57c72ba94..d0d5ad8beac5 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -3549,6 +3549,9 @@ static int patch_atihdmi(struct hda_codec *codec)
 	spec->ops.pin_hbr_setup = atihdmi_pin_hbr_setup;
 	spec->ops.setup_stream = atihdmi_setup_stream;
 
+	spec->chmap.ops.pin_get_slot_channel = atihdmi_pin_get_slot_channel;
+	spec->chmap.ops.pin_set_slot_channel = atihdmi_pin_set_slot_channel;
+
 	if (!has_amd_full_remap_support(codec)) {
 		/* override to ATI/AMD-specific versions with pairwise mapping */
 		spec->chmap.ops.chmap_cea_alloc_validate_get_type =
@@ -3556,10 +3559,6 @@ static int patch_atihdmi(struct hda_codec *codec)
 		spec->chmap.ops.cea_alloc_to_tlv_chmap =
 				atihdmi_paired_cea_alloc_to_tlv_chmap;
 		spec->chmap.ops.chmap_validate = atihdmi_paired_chmap_validate;
-		spec->chmap.ops.pin_get_slot_channel =
-				atihdmi_pin_get_slot_channel;
-		spec->chmap.ops.pin_set_slot_channel =
-				atihdmi_pin_set_slot_channel;
 	}
 
 	/* ATI/AMD converters do not advertise all of their capabilities */

Re: Regression: Kernel 4.6 DisplayPort Audio

[Daniel Exner]

[-- Attachment #1.1: Type: text/plain, Size: 437 bytes --]

Hi,

Am 11.05.2016 um 10:48 schrieb Takashi Iwai:
[..]

> If the commit above alone breaks, does the patch below change the
> behavior?  Through a quick look at it, some ops are overridden only
> conditionally while the older code always overwrote them.

Yes, the patch fixes it. Thanks!
I hope its possible to include that in 4.6.

Greetings
Daniel
-- 
Daniel Exner
Public-Key: https://www.dragonslave.de/pub_key.asc


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Regression: Kernel 4.6 DisplayPort Audio

[Takashi Iwai]

On Wed, 11 May 2016 14:52:22 +0200,
Daniel Exner wrote:
> 
> Hi,
> 
> Am 11.05.2016 um 10:48 schrieb Takashi Iwai:
> [..]
> 
> > If the commit above alone breaks, does the patch below change the
> > behavior?  Through a quick look at it, some ops are overridden only
> > conditionally while the older code always overwrote them.
> 
> Yes, the patch fixes it. Thanks!
> I hope its possible to include that in 4.6.

Good to hear!
FWIW, below is the proper patch I'm going to queue.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda - Fix regression on ATI HDMI audio

The HDMI/DP audio output on ATI/AMD chips got broken due to the recent
restructuring of chmap.  Fortunately, Daniel Exner could bisect, and
pointed the culprit commit [739ffee97ed5: ALSA: hda - Add hdmi chmap
verb programming ops to chmap object].

This commit moved some ops from hdmi_ops to chmap_ops, and reassigned
the ops in the embedded chmap object in hdmi_spec instead.
Unfortunately, the reassignment of these ops in patch_atihdmi() were
moved into an if block that is performed only for old chips.  Thus, on
newer chips, the generic ops is still used, which doesn't work for
such ATI/AMD chips.

This patch addresses the regression, simply by moving the assignment
of chmap ops to the right place.

Fixes: 739ffee97ed5 ('ALSA: hda - Add hdmi chmap verb programming ops to chmap object')
Reported-and-tested-by: Daniel Exner <dex@dragonslave.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_hdmi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 1483f85999ec..a010d704e0e2 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -3401,6 +3401,9 @@ static int patch_atihdmi(struct hda_codec *codec)
 	spec->ops.pin_hbr_setup = atihdmi_pin_hbr_setup;
 	spec->ops.setup_stream = atihdmi_setup_stream;
 
+	spec->chmap.ops.pin_get_slot_channel = atihdmi_pin_get_slot_channel;
+	spec->chmap.ops.pin_set_slot_channel = atihdmi_pin_set_slot_channel;
+
 	if (!has_amd_full_remap_support(codec)) {
 		/* override to ATI/AMD-specific versions with pairwise mapping */
 		spec->chmap.ops.chmap_cea_alloc_validate_get_type =
@@ -3408,10 +3411,6 @@ static int patch_atihdmi(struct hda_codec *codec)
 		spec->chmap.ops.cea_alloc_to_tlv_chmap =
 				atihdmi_paired_cea_alloc_to_tlv_chmap;
 		spec->chmap.ops.chmap_validate = atihdmi_paired_chmap_validate;
-		spec->chmap.ops.pin_get_slot_channel =
-				atihdmi_pin_get_slot_channel;
-		spec->chmap.ops.pin_set_slot_channel =
-				atihdmi_pin_set_slot_channel;
 	}
 
 	/* ATI/AMD converters do not advertise all of their capabilities */
-- 
2.8.2