mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] ALSA: hda/intel: Power down GPU at error on azx_probe_continue()
@ 2026-10-13 13:31 Takashi Iwai
  2026-10-13 13:31 ` [PATCH 2/2] ALSA: hda/intel: Clean up error handling in azx_probe_continue() Takashi Iwai
  2026-10-13 16:44 ` [PATCH 1/2] ALSA: hda/intel: Power down GPU at error on azx_probe_continue() Takashi Iwai
  0 siblings, 2 replies; 3+ messages in thread
From: Takashi Iwai @ 2026-10-13 13:31 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

Since the recent fix commit fbaccb1b19c7 ("ALSA: hda: intel: Don't
free card in the probe work"), we don't release the card but just
toggles the init_failed flag to stop the operation in the case of
error at probing in azx_probe_continue().  This leaves, however,
display_power() true, which may block powering down the GPU.

Put display_power(false) at the error path for avoiding the potential
blockage.

Fixes: fbaccb1b19c7 ("ALSA: hda: intel: Don't free card in the probe work")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/hda/controllers/intel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index 6b80b59b8743..b67d569fc859 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -2453,6 +2453,7 @@ static int azx_probe_continue(struct azx *chip, bool free_on_error)
 	if (err < 0) {
 		if (hda->vga_switcheroo_registered)
 			dev_warn(&pci->dev, "GPU sound probed, but not operational: please add a quirk to driver_denylist\n");
+		display_power(chip, false);
 		if (free_on_error) {
 			pci_set_drvdata(pci, NULL);
 			snd_card_free(chip->card);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] ALSA: hda/intel: Clean up error handling in azx_probe_continue()
  2026-10-13 13:31 [PATCH 1/2] ALSA: hda/intel: Power down GPU at error on azx_probe_continue() Takashi Iwai
@ 2026-10-13 13:31 ` Takashi Iwai
  2026-10-13 16:44 ` [PATCH 1/2] ALSA: hda/intel: Power down GPU at error on azx_probe_continue() Takashi Iwai
  1 sibling, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2026-10-13 13:31 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

As all callers of azx_probe_continue() pass false to free_on_error
argument after the recent fix fbaccb1b19c7 ("ALSA: hda: intel: Don't
free card in the probe work"), just drop the argument and the
superfluous condition.

While we're at it, move the error handling around out_free label to
the tail of the function, in order to make it clearer as an error path
by renaming the goto label to "out_err".  Since the error check
becomes redundant there (as it's only for errors), drop the err check,
too.

Only code cleanup and no functional change is expected.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/hda/controllers/intel.c | 36 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index b67d569fc859..2d9c2c3bc028 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -1184,7 +1184,7 @@ static const struct dev_pm_ops azx_pm = {
 };
 
 
-static int azx_probe_continue(struct azx *chip, bool free_on_error);
+static int azx_probe_continue(struct azx *chip);
 
 #ifdef SUPPORT_VGA_SWITCHEROO
 static struct pci_dev *get_bound_vga(struct pci_dev *pci);
@@ -1211,7 +1211,7 @@ static void azx_vs_set_state(struct pci_dev *pci,
 		if (!disabled) {
 			dev_info(chip->card->dev,
 				 "Start delayed initialization\n");
-			if (azx_probe_continue(chip, false) < 0) {
+			if (azx_probe_continue(chip) < 0) {
 				hda->init_failed = 1;
 				dev_err(chip->card->dev, "initialization error\n");
 			}
@@ -1738,7 +1738,7 @@ static void azx_probe_work(struct work_struct *work)
 {
 	struct hda_intel *hda = container_of(work, struct hda_intel, probe_work.work);
 
-	if (azx_probe_continue(&hda->chip, false) < 0) {
+	if (azx_probe_continue(&hda->chip) < 0) {
 		hda->init_failed = 1;
 		complete_all(&hda->probe_wait);
 		snd_card_disconnect(hda->chip.card);
@@ -2360,7 +2360,7 @@ static const unsigned int azx_max_codecs[AZX_NUM_DRIVERS] = {
 	[AZX_DRIVER_TERA] = 1,
 };
 
-static int azx_probe_continue(struct azx *chip, bool free_on_error)
+static int azx_probe_continue(struct azx *chip)
 {
 	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
 	struct hdac_bus *bus = azx_bus(chip);
@@ -2385,7 +2385,7 @@ static int azx_probe_continue(struct azx *chip, bool free_on_error)
 
 	err = azx_first_init(chip);
 	if (err < 0)
-		goto out_free;
+		goto out_err;
 
 #ifdef CONFIG_SND_HDA_INPUT_BEEP
 	chip->beep_mode = beep_mode[dev];
@@ -2397,7 +2397,7 @@ static int azx_probe_continue(struct azx *chip, bool free_on_error)
 	if (bus->codec_mask) {
 		err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
 		if (err < 0)
-			goto out_free;
+			goto out_err;
 	}
 
 #ifdef CONFIG_SND_HDA_PATCH_LOADER
@@ -2412,7 +2412,7 @@ static int azx_probe_continue(struct azx *chip, bool free_on_error)
 		} else {
 			err = snd_hda_load_patch(&chip->bus, fw->size, fw->data);
 			if (err < 0)
-				goto out_free;
+				goto out_err;
 		}
 	}
 #endif
@@ -2428,13 +2428,13 @@ static int azx_probe_continue(struct azx *chip, bool free_on_error)
 				return 0; /* keep things up */
 			}
 			dev_err(chip->card->dev, "Cannot probe codecs, giving up\n");
-			goto out_free;
+			goto out_err;
 		}
 	}
 
 	err = snd_card_register(chip->card);
 	if (err < 0)
-		goto out_free;
+		goto out_err;
 
 	setup_vga_switcheroo_runtime_pm(chip);
 
@@ -2449,24 +2449,18 @@ static int azx_probe_continue(struct azx *chip, bool free_on_error)
 		pm_runtime_put_autosuspend(&pci->dev);
 	}
 
-out_free:
-	if (err < 0) {
-		if (hda->vga_switcheroo_registered)
-			dev_warn(&pci->dev, "GPU sound probed, but not operational: please add a quirk to driver_denylist\n");
-		display_power(chip, false);
-		if (free_on_error) {
-			pci_set_drvdata(pci, NULL);
-			snd_card_free(chip->card);
-		}
-		return err;
-	}
-
 	if (!hda->need_i915_power)
 		display_power(chip, false);
 	complete_all(&hda->probe_wait);
 	to_hda_bus(bus)->bus_probing = 0;
 	hda->probe_retry = 0;
 	return 0;
+
+out_err:
+	if (hda->vga_switcheroo_registered)
+		dev_warn(&pci->dev, "GPU sound probed, but not operational: please add a quirk to driver_denylist\n");
+	display_power(chip, false);
+	return err;
 }
 
 static void azx_remove(struct pci_dev *pci)
-- 
2.55.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] ALSA: hda/intel: Power down GPU at error on azx_probe_continue()
  2026-10-13 13:31 [PATCH 1/2] ALSA: hda/intel: Power down GPU at error on azx_probe_continue() Takashi Iwai
  2026-10-13 13:31 ` [PATCH 2/2] ALSA: hda/intel: Clean up error handling in azx_probe_continue() Takashi Iwai
@ 2026-10-13 16:44 ` Takashi Iwai
  1 sibling, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2026-10-13 16:44 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

On Tue, 13 Oct 2026 15:31:50 +0200,
Takashi Iwai wrote:
> 
> Since the recent fix commit fbaccb1b19c7 ("ALSA: hda: intel: Don't
> free card in the probe work"), we don't release the card but just
> toggles the init_failed flag to stop the operation in the case of
> error at probing in azx_probe_continue().  This leaves, however,
> display_power() true, which may block powering down the GPU.
> 
> Put display_power(false) at the error path for avoiding the potential
> blockage.
> 
> Fixes: fbaccb1b19c7 ("ALSA: hda: intel: Don't free card in the probe work")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/hda/controllers/intel.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
> index 6b80b59b8743..b67d569fc859 100644
> --- a/sound/hda/controllers/intel.c
> +++ b/sound/hda/controllers/intel.c
> @@ -2453,6 +2453,7 @@ static int azx_probe_continue(struct azx *chip, bool free_on_error)
>  	if (err < 0) {
>  		if (hda->vga_switcheroo_registered)
>  			dev_warn(&pci->dev, "GPU sound probed, but not operational: please add a quirk to driver_denylist\n");
> +		display_power(chip, false);
>  		if (free_on_error) {
>  			pci_set_drvdata(pci, NULL);
>  			snd_card_free(chip->card);

On the second thought, turning off the display power here alone would
be problematic; the rest hardware cleanup procedure will run anyway in
azx_free() and with the display_power() off, it may lead to something
unexpected.

We can power up again temporarily in azx_free(), too.  But I'm not
sure whether it's a good idea.  Going back to calling azx_free() can
still work, but then we'll hit the old issue again about the already
bound codecs (that aren't detached by azx_free() itself).

Hmm.  Maybe better to leave in the current state for now until someone
sees a real problem on an actual hardware.


Takashi

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-13 16:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-13 13:31 [PATCH 1/2] ALSA: hda/intel: Power down GPU at error on azx_probe_continue() Takashi Iwai
2026-10-13 13:31 ` [PATCH 2/2] ALSA: hda/intel: Clean up error handling in azx_probe_continue() Takashi Iwai
2026-10-13 16:44 ` [PATCH 1/2] ALSA: hda/intel: Power down GPU at error on azx_probe_continue() Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®