* [PATCH] ALSA: hda: intel: Don't free interrupt when suspending @ 2024-10-31 19:32 Heiner Kallweit 2024-11-01 8:21 ` Takashi Iwai 2024-11-04 13:23 ` Takashi Iwai 0 siblings, 2 replies; 5+ messages in thread From: Heiner Kallweit @ 2024-10-31 19:32 UTC (permalink / raw) To: Jaroslav Kysela, Takashi Iwai; +Cc: linux-sound There's no need to free/re-request the interrupt on system suspend. PCI core takes care, using functions like pci_restore_msi_state(). Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- sound/pci/hda/hda_intel.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index b4540c5cd..9fc5e6c5d 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -1040,14 +1040,6 @@ static int azx_suspend(struct device *dev) chip = card->private_data; bus = azx_bus(chip); azx_shutdown_chip(chip); - if (bus->irq >= 0) { - free_irq(bus->irq, chip); - bus->irq = -1; - chip->card->sync_irq = -1; - } - - if (chip->msi) - pci_disable_msi(chip->pci); trace_azx_suspend(chip); return 0; @@ -1062,11 +1054,6 @@ static int __maybe_unused azx_resume(struct device *dev) return 0; chip = card->private_data; - if (chip->msi) - if (pci_enable_msi(chip->pci) < 0) - chip->msi = 0; - if (azx_acquire_irq(chip, 1) < 0) - return -EIO; __azx_runtime_resume(chip); -- 2.47.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: hda: intel: Don't free interrupt when suspending 2024-10-31 19:32 [PATCH] ALSA: hda: intel: Don't free interrupt when suspending Heiner Kallweit @ 2024-11-01 8:21 ` Takashi Iwai 2024-11-01 9:49 ` Heiner Kallweit 2024-11-04 13:23 ` Takashi Iwai 1 sibling, 1 reply; 5+ messages in thread From: Takashi Iwai @ 2024-11-01 8:21 UTC (permalink / raw) To: Heiner Kallweit; +Cc: Jaroslav Kysela, Takashi Iwai, linux-sound On Thu, 31 Oct 2024 20:32:52 +0100, Heiner Kallweit wrote: > > There's no need to free/re-request the interrupt on system suspend. > PCI core takes care, using functions like pci_restore_msi_state(). Well, it wasn't always true for hibernation in the past, e.g. snd-intel8x0 driver had to deal with it. I guess it's no longer needed for modern platforms for HD-audio, so I'm going to take this for 6.13. thanks, Takashi > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > sound/pci/hda/hda_intel.c | 13 ------------- > 1 file changed, 13 deletions(-) > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > index b4540c5cd..9fc5e6c5d 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -1040,14 +1040,6 @@ static int azx_suspend(struct device *dev) > chip = card->private_data; > bus = azx_bus(chip); > azx_shutdown_chip(chip); > - if (bus->irq >= 0) { > - free_irq(bus->irq, chip); > - bus->irq = -1; > - chip->card->sync_irq = -1; > - } > - > - if (chip->msi) > - pci_disable_msi(chip->pci); > > trace_azx_suspend(chip); > return 0; > @@ -1062,11 +1054,6 @@ static int __maybe_unused azx_resume(struct device *dev) > return 0; > > chip = card->private_data; > - if (chip->msi) > - if (pci_enable_msi(chip->pci) < 0) > - chip->msi = 0; > - if (azx_acquire_irq(chip, 1) < 0) > - return -EIO; > > __azx_runtime_resume(chip); > > -- > 2.47.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: hda: intel: Don't free interrupt when suspending 2024-11-01 8:21 ` Takashi Iwai @ 2024-11-01 9:49 ` Heiner Kallweit 2024-11-02 10:08 ` Takashi Iwai 0 siblings, 1 reply; 5+ messages in thread From: Heiner Kallweit @ 2024-11-01 9:49 UTC (permalink / raw) To: Takashi Iwai; +Cc: Jaroslav Kysela, Takashi Iwai, linux-sound On 01.11.2024 09:21, Takashi Iwai wrote: > On Thu, 31 Oct 2024 20:32:52 +0100, > Heiner Kallweit wrote: >> >> There's no need to free/re-request the interrupt on system suspend. >> PCI core takes care, using functions like pci_restore_msi_state(). > > Well, it wasn't always true for hibernation in the past, > e.g. snd-intel8x0 driver had to deal with it. I guess it's no longer > needed for modern platforms for HD-audio, so I'm going to take this > for 6.13. > Interesting, do you have any link to what the issue with intel8x0 was? In the commit history I didn't find something related at a first glance. Maybe also PCI core simply is more mature meanwhile. Heiner > > thanks, > > Takashi > >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> sound/pci/hda/hda_intel.c | 13 ------------- >> 1 file changed, 13 deletions(-) >> >> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c >> index b4540c5cd..9fc5e6c5d 100644 >> --- a/sound/pci/hda/hda_intel.c >> +++ b/sound/pci/hda/hda_intel.c >> @@ -1040,14 +1040,6 @@ static int azx_suspend(struct device *dev) >> chip = card->private_data; >> bus = azx_bus(chip); >> azx_shutdown_chip(chip); >> - if (bus->irq >= 0) { >> - free_irq(bus->irq, chip); >> - bus->irq = -1; >> - chip->card->sync_irq = -1; >> - } >> - >> - if (chip->msi) >> - pci_disable_msi(chip->pci); >> >> trace_azx_suspend(chip); >> return 0; >> @@ -1062,11 +1054,6 @@ static int __maybe_unused azx_resume(struct device *dev) >> return 0; >> >> chip = card->private_data; >> - if (chip->msi) >> - if (pci_enable_msi(chip->pci) < 0) >> - chip->msi = 0; >> - if (azx_acquire_irq(chip, 1) < 0) >> - return -EIO; >> >> __azx_runtime_resume(chip); >> >> -- >> 2.47.0 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: hda: intel: Don't free interrupt when suspending 2024-11-01 9:49 ` Heiner Kallweit @ 2024-11-02 10:08 ` Takashi Iwai 0 siblings, 0 replies; 5+ messages in thread From: Takashi Iwai @ 2024-11-02 10:08 UTC (permalink / raw) To: Heiner Kallweit; +Cc: Takashi Iwai, Jaroslav Kysela, Takashi Iwai, linux-sound On Fri, 01 Nov 2024 10:49:23 +0100, Heiner Kallweit wrote: > > On 01.11.2024 09:21, Takashi Iwai wrote: > > On Thu, 31 Oct 2024 20:32:52 +0100, > > Heiner Kallweit wrote: > >> > >> There's no need to free/re-request the interrupt on system suspend. > >> PCI core takes care, using functions like pci_restore_msi_state(). > > > > Well, it wasn't always true for hibernation in the past, > > e.g. snd-intel8x0 driver had to deal with it. I guess it's no longer > > needed for modern platforms for HD-audio, so I'm going to take this > > for 6.13. > > > Interesting, do you have any link to what the issue with intel8x0 was? > In the commit history I didn't find something related at a first glance. I also don't remember exactly, sorry. It was decades ago. The old bug entry https://bugzilla.kernel.org/show_bug.cgi?id=219416 showed why the irq handler was freed at suspend on intel8x0. My vague memory says that this was the original fix, and there was another bug report of hibernation issue. I'm no longer sure whether it was intel8x0, though -- it might be another PCI sound driver, too. But the only certain thing is that it was tad old issue. > Maybe also PCI core simply is more mature meanwhile. Hopefully, and I bet for it by taking a risk to apply your cleanup :) thanks, Takashi > Heiner > > > > thanks, > > > > Takashi > > > >> > >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > >> --- > >> sound/pci/hda/hda_intel.c | 13 ------------- > >> 1 file changed, 13 deletions(-) > >> > >> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > >> index b4540c5cd..9fc5e6c5d 100644 > >> --- a/sound/pci/hda/hda_intel.c > >> +++ b/sound/pci/hda/hda_intel.c > >> @@ -1040,14 +1040,6 @@ static int azx_suspend(struct device *dev) > >> chip = card->private_data; > >> bus = azx_bus(chip); > >> azx_shutdown_chip(chip); > >> - if (bus->irq >= 0) { > >> - free_irq(bus->irq, chip); > >> - bus->irq = -1; > >> - chip->card->sync_irq = -1; > >> - } > >> - > >> - if (chip->msi) > >> - pci_disable_msi(chip->pci); > >> > >> trace_azx_suspend(chip); > >> return 0; > >> @@ -1062,11 +1054,6 @@ static int __maybe_unused azx_resume(struct device *dev) > >> return 0; > >> > >> chip = card->private_data; > >> - if (chip->msi) > >> - if (pci_enable_msi(chip->pci) < 0) > >> - chip->msi = 0; > >> - if (azx_acquire_irq(chip, 1) < 0) > >> - return -EIO; > >> > >> __azx_runtime_resume(chip); > >> > >> -- > >> 2.47.0 > >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: hda: intel: Don't free interrupt when suspending 2024-10-31 19:32 [PATCH] ALSA: hda: intel: Don't free interrupt when suspending Heiner Kallweit 2024-11-01 8:21 ` Takashi Iwai @ 2024-11-04 13:23 ` Takashi Iwai 1 sibling, 0 replies; 5+ messages in thread From: Takashi Iwai @ 2024-11-04 13:23 UTC (permalink / raw) To: Heiner Kallweit; +Cc: Jaroslav Kysela, Takashi Iwai, linux-sound On Thu, 31 Oct 2024 20:32:52 +0100, Heiner Kallweit wrote: > > There's no need to free/re-request the interrupt on system suspend. > PCI core takes care, using functions like pci_restore_msi_state(). > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to for-next branch now. Thanks. Takashi ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-11-04 13:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-31 19:32 [PATCH] ALSA: hda: intel: Don't free interrupt when suspending Heiner Kallweit 2024-11-01 8:21 ` Takashi Iwai 2024-11-01 9:49 ` Heiner Kallweit 2024-11-02 10:08 ` Takashi Iwai 2024-11-04 13:23 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox