[PATCH] ALSA: hda - HDMI Audio init all connectors

[PATCH] ALSA: hda - HDMI Audio init all connectors

[Steven Newbury]

[-- Attachment #1: Type: text/plain, Size: 470 bytes --]

When VGA_SWITCHEROO support is enabled hda_intel initialises the HDMI audio device on the current VGA device.  When it's not enabled it only initialises the HDMI device on the default VGA adaptor, this means secondary cards get no audio support which is very unhelpful for multi-seat!

With this patch, when SUPPORT_VGA_SWITCHEROO is disabled hda_intel initialises all HDMI audio devices, not just the default VGA.

Signed-off-by: Steven Newbury <steve@snewbury.org.uk>

[-- Attachment #2: hdmi-audio-init.patch --]
[-- Type: text/x-patch, Size: 1610 bytes --]

commit 8989f0e53f7108d099fb7c5fdb76f58d3a63f85a
Author: Steven Newbury <steve@snewbury.org.uk>
Date:   Wed Jun 6 23:52:13 2012 +0100

ALSA: hda - Always initialise all HDMI audio connectors
              when SUPPORT_VGA_SWITCHEROO is disabled

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index e924722..f207549 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2484,9 +2484,9 @@ static void azx_notifier_unregister(struct azx *chip)
 static int DELAYED_INIT_MARK azx_first_init(struct azx *chip);
 static int DELAYED_INIT_MARK azx_probe_continue(struct azx *chip);
 
+#ifdef SUPPORT_VGA_SWITCHEROO
 static struct pci_dev __devinit *get_bound_vga(struct pci_dev *pci);
 
-#ifdef SUPPORT_VGA_SWITCHEROO
 static void azx_vs_set_state(struct pci_dev *pci,
 			     enum vga_switcheroo_state state)
 {
@@ -2641,6 +2641,7 @@ static int azx_dev_free(struct snd_device *device)
 /*
  * Check of disabled HDMI controller by vga-switcheroo
  */
+#ifdef SUPPORT_VGA_SWITCHEROO
 static struct pci_dev __devinit *get_bound_vga(struct pci_dev *pci)
 {
 	struct pci_dev *p;
@@ -2663,10 +2664,12 @@ static struct pci_dev __devinit *get_bound_vga(struct pci_dev *pci)
 	}
 	return NULL;
 }
+#endif /* SUPPORT_VGA_SWITCHER */
 
 static bool __devinit check_hdmi_disabled(struct pci_dev *pci)
 {
 	bool vga_inactive = false;
+#ifdef SUPPORT_VGA_SWITCHEROO
 	struct pci_dev *p = get_bound_vga(pci);
 
 	if (p) {
@@ -2674,6 +2677,7 @@ static bool __devinit check_hdmi_disabled(struct pci_dev *pci)
 			vga_inactive = true;
 		pci_dev_put(p);
 	}
+#endif
 	return vga_inactive;
 }
 

Re: [PATCH] ALSA: hda - HDMI Audio init all connectors

[Takashi Iwai]

At the next time you submit a patch, please add Cc to maintainer and
subsystem ML.  Otherwise it'd be easily overlooked.

At Thu, 07 Jun 2012 06:52:59 +0100,
Steven Newbury wrote:
> 
> When VGA_SWITCHEROO support is enabled hda_intel initialises the HDMI audio device on the current VGA device.  When it's not enabled it only initialises the HDMI device on the default VGA adaptor, this means secondary cards get no audio support which is very unhelpful for multi-seat!
> 
> With this patch, when SUPPORT_VGA_SWITCHEROO is disabled hda_intel initialises all HDMI audio devices, not just the default VGA.
> 
> Signed-off-by: Steven Newbury <steve@snewbury.org.uk>

OK, it must be fixed indeed.  The bug was introduced due to the change
in VGA-switcheroo side -- originally check_hdmi_disabled() really
checked the VGA-switcheroo state, so it returned the correct value in
the earlier version.

In anyway, your patch looks almost OK, but it'd be better to move
ifdef and simplify like below.  Could you check whether it works for
you?

Also, please give a patch that is applicable directly via git-am.
Apparently you created it via git show.  Instead, use git-format-patch
or use --pretty=email for git-show.  Or, send the patch directly via
git-send-email.


thanks,

Takashi

---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 2b6392b..2eb1b47 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2484,9 +2484,9 @@ static void azx_notifier_unregister(struct azx *chip)
 static int DELAYED_INIT_MARK azx_first_init(struct azx *chip);
 static int DELAYED_INIT_MARK azx_probe_continue(struct azx *chip);
 
+#ifdef SUPPORT_VGA_SWITCHEROO
 static struct pci_dev __devinit *get_bound_vga(struct pci_dev *pci);
 
-#ifdef SUPPORT_VGA_SWITCHEROO
 static void azx_vs_set_state(struct pci_dev *pci,
 			     enum vga_switcheroo_state state)
 {
@@ -2578,6 +2578,7 @@ static int __devinit register_vga_switcheroo(struct azx *chip)
 #else
 #define init_vga_switcheroo(chip)		/* NOP */
 #define register_vga_switcheroo(chip)		0
+#define check_hdmi_disabled(pci)	false
 #endif /* SUPPORT_VGA_SWITCHER */
 
 /*
@@ -2638,6 +2639,7 @@ static int azx_dev_free(struct snd_device *device)
 	return azx_free(device->device_data);
 }
 
+#ifdef SUPPORT_VGA_SWITCHEROO
 /*
  * Check of disabled HDMI controller by vga-switcheroo
  */
@@ -2676,6 +2678,7 @@ static bool __devinit check_hdmi_disabled(struct pci_dev *pci)
 	}
 	return vga_inactive;
 }
+#endif /* SUPPORT_VGA_SWITCHEROO */
 
 /*
  * white/black-listing for position_fix

Re: [PATCH] ALSA: hda - HDMI Audio init all connectors

[Steven Newbury]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/06/12 09:47, Takashi Iwai wrote:
> At the next time you submit a patch, please add Cc to maintainer
> and subsystem ML.  Otherwise it'd be easily overlooked.
> 
I meant to, somehow I managed to send it off before adding the CC,
probably too early in the morning!

> At Thu, 07 Jun 2012 06:52:59 +0100, Steven Newbury wrote:
>> 
>> When VGA_SWITCHEROO support is enabled hda_intel initialises the
>> HDMI audio device on the current VGA device.  When it's not
>> enabled it only initialises the HDMI device on the default VGA
>> adaptor, this means secondary cards get no audio support which is
>> very unhelpful for multi-seat!
>> 
>> With this patch, when SUPPORT_VGA_SWITCHEROO is disabled
>> hda_intel initialises all HDMI audio devices, not just the
>> default VGA.
>> 
>> Signed-off-by: Steven Newbury <steve@snewbury.org.uk>
> 
> OK, it must be fixed indeed.  The bug was introduced due to the
> change in VGA-switcheroo side -- originally check_hdmi_disabled()
> really checked the VGA-switcheroo state, so it returned the correct
> value in the earlier version.
> 
> In anyway, your patch looks almost OK, but it'd be better to move 
> ifdef and simplify like below.  Could you check whether it works
> for you?
> 
Looks better, saves the pointless function call. I'll give it spin on
my next boot.

> Also, please give a patch that is applicable directly via git-am. 
> Apparently you created it via git show.  Instead, use
> git-format-patch or use --pretty=email for git-show.  Or, send the
> patch directly via git-send-email.
> 
I think it's the first time I've attempted to send a complete patch to
LKML (in the git era at least), rather than just a one-liner.  Thanks
for the tips, hopefully I'll get it right next time! :)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/QbacACgkQGcb56gMuC6226wCffwsBXTF7M2MhmUgggleo4gHB
v1MAoIwXx4REh1lSEEVhsGRuFQBZEs0q
=R4Ys
-----END PGP SIGNATURE-----

Re: [PATCH] ALSA: hda - HDMI Audio init all connectors

[Takashi Iwai]

At Thu, 07 Jun 2012 10:00:24 +0100,
Steven Newbury wrote:
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 07/06/12 09:47, Takashi Iwai wrote:
> > At the next time you submit a patch, please add Cc to maintainer
> > and subsystem ML.  Otherwise it'd be easily overlooked.
> > 
> I meant to, somehow I managed to send it off before adding the CC,
> probably too early in the morning!
> 
> > At Thu, 07 Jun 2012 06:52:59 +0100, Steven Newbury wrote:
> >> 
> >> When VGA_SWITCHEROO support is enabled hda_intel initialises the
> >> HDMI audio device on the current VGA device.  When it's not
> >> enabled it only initialises the HDMI device on the default VGA
> >> adaptor, this means secondary cards get no audio support which is
> >> very unhelpful for multi-seat!
> >> 
> >> With this patch, when SUPPORT_VGA_SWITCHEROO is disabled
> >> hda_intel initialises all HDMI audio devices, not just the
> >> default VGA.
> >> 
> >> Signed-off-by: Steven Newbury <steve@snewbury.org.uk>
> > 
> > OK, it must be fixed indeed.  The bug was introduced due to the
> > change in VGA-switcheroo side -- originally check_hdmi_disabled()
> > really checked the VGA-switcheroo state, so it returned the correct
> > value in the earlier version.
> > 
> > In anyway, your patch looks almost OK, but it'd be better to move 
> > ifdef and simplify like below.  Could you check whether it works
> > for you?
> > 
> Looks better, saves the pointless function call. I'll give it spin on
> my next boot.

Thanks!

Meanwhile I found that the current version is also buggy for some
setups with VGA-switcheroo.  The current version won't work, for
example, when VGA-switcheroo is built-in for systems without
switcheroo.  I'll need to cook up the fix for 3.5-rc2 in addition to
your fix.


Takashi