At Wed, 28 Nov 2012 09:45:39 +0100,
Takashi Iwai wrote:
>
> At Wed, 28 Nov 2012 11:45:07 +0800,
> Daniel J Blueman wrote:
> >
> > Hi Seth, Dave, Takashi,
> >
> > If I power down the unused discrete GPU before lightdm starts by
> > fiddling with the sysfs file [1] in the upstart script, I see a race
> > manifesting as the discrete GPU's HDA controller timing out to
> > commands [2].
> >
> > Adding some debug, I see that the registered audio devices are put
> > into D3 before the GPU is, but it turns out that the discrete (and
> > internal) GPU's HDA controller gets registered a bit later, so the
> > list is empty. The symptom is since the HDA driver it's talking to
> > hardware which is now in D3.
> >
> > We could add a mutex to nouveau to allow us to wait for the DGPU HDA
> > controller, but perhaps this should be solved at a higher level in the
> > vgaswitcheroo code; what do you think?
>
> Maybe it's a side effect for the recent effort to fix another race in
> the probe. A part of them problem is that the registration is done at
> the very last of probing.
>
> Instead of delaying the registration, how about the patch below?
Ping. If this really works, I'd like to queue it for 3.8 merge, at
least...
thanks,
Takashi
>
>
> Takashi
>
> ---
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index 4bb52da..17fbd68 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -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;
> @@ -2790,6 +2792,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 +2854,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 +3162,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]);
> @@ -3462,6 +3469,13 @@ static int __devinit azx_probe(struct pci_dev *pci,
> }
> #endif /* CONFIG_SND_HDA_PATCH_LOADER */
>
> + err = register_vga_switcheroo(chip);
> + if (err < 0) {
> + snd_printk(KERN_ERR SFX
> + "Error registering VGA-switcheroo client\n");
> + goto out_free;
> + }
> +
> if (probe_now) {
> err = azx_probe_continue(chip);
> if (err < 0)
> @@ -3473,14 +3487,8 @@ static int __devinit azx_probe(struct pci_dev *pci,
> 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:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
