ALSA: hda - Fix yet another race of vga_switcheroo registration
authorTakashi Iwai <tiwai@suse.de>
Tue, 4 Dec 2012 14:09:23 +0000 (15:09 +0100)
committerTakashi Iwai <tiwai@suse.de>
Tue, 4 Dec 2012 15:00:40 +0000 (16:00 +0100)
The recent fix for vga switcheroo race in commit 128960a9 opened yet
another race.  At the time the audio driver starts probing, user may
turn off D-GPU off.  But at this moment, the audio driver still
doesn't register the vga switcheroo client, thus the switching isn't
notified.  Then the hardware gets off out of sudden, resulting in
invalid reads and lots of "spurious response" error messages.

For solving this situation, the following changes have been done in
this patch:
- Move again vga switcheroo registration to the very early stage of
  the probing; this also requires to set pci drvdata properly before
  registration
- Introduce the completion to synchronize the driver probe at vga
  switcheroo callbacks; this assures that the whole probing finished
  before executing the callbacks

Reported-by: Daniel J Blueman <daniel@quora.org>
Tested-by: Daniel J Blueman <daniel@quora.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
sound/pci/hda/hda_intel.c

index 4bb52da..22ecadc 100644 (file)
@@ -49,6 +49,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/clocksource.h>
 #include <linux/time.h>
+#include <linux/completion.h>
 
 #ifdef CONFIG_X86
 /* for snoop control */
@@ -469,6 +470,7 @@ struct azx {
        /* locks */
        spinlock_t reg_lock;
        struct mutex open_mutex;
+       struct completion probe_wait;
 
        /* streams (x num_streams) */
        struct azx_dev *azx_dev;
@@ -2745,6 +2747,7 @@ static void azx_vs_set_state(struct pci_dev *pci,
        struct azx *chip = card->private_data;
        bool disabled;
 
+       wait_for_completion(&chip->probe_wait);
        if (chip->init_failed)
                return;
 
@@ -2790,6 +2793,7 @@ static bool azx_vs_can_switch(struct pci_dev *pci)
        struct snd_card *card = pci_get_drvdata(pci);
        struct azx *chip = card->private_data;
 
+       wait_for_completion(&chip->probe_wait);
        if (chip->init_failed)
                return false;
        if (chip->disabled || !chip->bus)
@@ -2851,6 +2855,9 @@ static int azx_free(struct azx *chip)
 
        azx_notifier_unregister(chip);
 
+       chip->init_failed = 1; /* to be sure */
+       complete(&chip->probe_wait);
+
        if (use_vga_switcheroo(chip)) {
                if (chip->disabled && chip->bus)
                        snd_hda_unlock_devices(chip->bus);
@@ -3156,6 +3163,7 @@ static int __devinit azx_create(struct snd_card *card, struct pci_dev *pci,
        INIT_LIST_HEAD(&chip->pcm_list);
        INIT_LIST_HEAD(&chip->list);
        init_vga_switcheroo(chip);
+       init_completion(&chip->probe_wait);
 
        chip->position_fix[0] = chip->position_fix[1] =
                check_position_fix(chip, position_fix[dev]);
@@ -3183,26 +3191,6 @@ static int __devinit azx_create(struct snd_card *card, struct pci_dev *pci,
                }
        }
 
-       if (check_hdmi_disabled(pci)) {
-               snd_printk(KERN_INFO SFX "VGA controller for %s is disabled\n",
-                          pci_name(pci));
-               if (use_vga_switcheroo(chip)) {
-                       snd_printk(KERN_INFO SFX "Delaying initialization\n");
-                       chip->disabled = true;
-                       goto ok;
-               }
-               kfree(chip);
-               pci_disable_device(pci);
-               return -ENXIO;
-       }
-
-       err = azx_first_init(chip);
-       if (err < 0) {
-               azx_free(chip);
-               return err;
-       }
-
- ok:
        err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
        if (err < 0) {
                snd_printk(KERN_ERR SFX "Error creating device [card]!\n");
@@ -3447,7 +3435,29 @@ static int __devinit azx_probe(struct pci_dev *pci,
        if (err < 0)
                goto out_free;
        card->private_data = chip;
+
+       pci_set_drvdata(pci, card);
+
+       err = register_vga_switcheroo(chip);
+       if (err < 0) {
+               snd_printk(KERN_ERR SFX
+                          "Error registering VGA-switcheroo client\n");
+               goto out_free;
+       }
+
+       if (check_hdmi_disabled(pci)) {
+               snd_printk(KERN_INFO SFX "VGA controller for %s is disabled\n",
+                          pci_name(pci));
+               snd_printk(KERN_INFO SFX "Delaying initialization\n");
+               chip->disabled = true;
+       }
+
        probe_now = !chip->disabled;
+       if (probe_now) {
+               err = azx_first_init(chip);
+               if (err < 0)
+                       goto out_free;
+       }
 
 #ifdef CONFIG_SND_HDA_PATCH_LOADER
        if (patch[dev] && *patch[dev]) {
@@ -3468,23 +3478,16 @@ static int __devinit azx_probe(struct pci_dev *pci,
                        goto out_free;
        }
 
-       pci_set_drvdata(pci, card);
-
        if (pci_dev_run_wake(pci))
                pm_runtime_put_noidle(&pci->dev);
 
-       err = register_vga_switcheroo(chip);
-       if (err < 0) {
-               snd_printk(KERN_ERR SFX
-                          "Error registering VGA-switcheroo client\n");
-               goto out_free;
-       }
-
        dev++;
+       complete(&chip->probe_wait);
        return 0;
 
 out_free:
        snd_card_free(card);
+       pci_set_drvdata(pci, NULL);
        return err;
 }