At Tue, 9 Oct 2012 00:34:09 +0800, Daniel J Blueman wrote:
On 8 October 2012 20:58, Takashi Iwai tiwai@suse.de wrote:
At Tue, 25 Sep 2012 13:20:05 +0800, Daniel J Blueman wrote:
On my Macbook with a discrete Nvidia GPU, there is a race between selecting the integrated GPU and putting the discrete GPU into D3 [1], reliably causing a kernel oops [2].
Introducing a delay of ~1s between the calls prevents this. When the second 'OFF' write path executes, it looks like struct azx at card->private_data hasn't yet been allocated yet [3], so there is likely some locking missing.
It's rather pci_get_drvdata() returning NULL (i.e. card is NULL, thus card->private_data causes Oops). Could you check the patch like below and see whether you get a kernel warning (but no Oops) or the problem gets fixed by shifting the assignment of pci drvdata?
[...]
Good patching. Calling pci_set_drvdata later prevents the oops in HDA, though we see unexpected 0x0 responses in the response ring buffer [1], which we don't see when there's a >~1.5s delay between IGD and OFF.
If the previous patch fixed, it means that the switching occurred during the device was being probed. Maybe a better approach to register the VGA switcheroo after the proper initialization.
The patch below is a revised one. Please give it a try.
Takashi
--- diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index f09ff6c..dcbfd29 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -501,6 +501,7 @@ struct azx {
/* VGA-switcheroo setup */ unsigned int use_vga_switcheroo:1; + unsigned int vga_switcheroo_registered:1; unsigned int init_failed:1; /* delayed init failed */ unsigned int disabled:1; /* disabled by VGA-switcher */
@@ -2684,14 +2685,20 @@ static const struct vga_switcheroo_client_ops azx_vs_ops = {
static int __devinit register_vga_switcheroo(struct azx *chip) { + int err; + if (!chip->use_vga_switcheroo) return 0; /* FIXME: currently only handling DIS controller * is there any machine with two switchable HDMI audio controllers? */ - return vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops, + err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops, VGA_SWITCHEROO_DIS, chip->bus != NULL); + if (err < 0) + return err; + chip->vga_switcheroo_registered = 1; + return 0; } #else #define init_vga_switcheroo(chip) /* NOP */ @@ -2713,7 +2720,8 @@ static int azx_free(struct azx *chip) if (use_vga_switcheroo(chip)) { if (chip->disabled && chip->bus) snd_hda_unlock_devices(chip->bus); - vga_switcheroo_unregister_client(chip->pci); + if (chip->vga_switcheroo_registered) + vga_switcheroo_unregister_client(chip->pci); }
if (chip->initialized) { @@ -3067,14 +3075,6 @@ static int __devinit azx_create(struct snd_card *card, struct pci_dev *pci, }
ok: - err = register_vga_switcheroo(chip); - if (err < 0) { - snd_printk(KERN_ERR SFX - "Error registering VGA-switcheroo client\n"); - azx_free(chip); - return err; - } - err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops); if (err < 0) { snd_printk(KERN_ERR SFX "Error creating device [card]!\n"); @@ -3345,6 +3345,13 @@ 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++; return 0;