Re: [PATCH 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Takashi Iwai]

On Wed, 12 Aug 2026 14:22:06 +0200,
Andre Eikmeyer wrote:
> 
> Hello everyone,
> 
> The HDA controller normally needs different WAKEEN settings for runtime and
> system suspend, so azx_prepare() prevents direct complete. For a switcheroo
> audio client whose bound GPU is already runtime suspended and switched off,
> however, the controller is physically inaccessible and cannot signal wake
> events.
> 
> Resuming the HDA function during system suspend also acquires its runtime
> PM supplier. This powers the discrete GPU back on only to suspend it again
> and can expose failures while restoring an otherwise unused GPU.
> 
> Direct complete is now allowed when the bound VGA device is both runtime
> suspended and reported off by vga_switcheroo. Active GPUs and HDA
> controllers without a switcheroo binding retain the existing WAKEEN
> transition.
> 
> This was tested as part of the MacBookPro15,1 hybrid graphics series on
> both the 2018 and 2019 revisions. The discrete GPU remains powered off
> across system suspend when it is unused, and suspend and resume complete
> successfully.
> 
> Thank you for your time and consideration.
> 
> Signed-off-by: Andre Eikmeyer <dev@deq.rocks>

Please drop greeting and thank-you texts.  They can be put in the
cover letter, but not in each patch description.

About the code changes:

> +static bool azx_vga_is_powered_off(struct azx *chip)
> +{
> +#ifdef SUPPORT_VGA_SWITCHEROO
> +	struct pci_dev *pci;
> +	bool powered_off = false;
> +
> +	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
> +		return false;
> +
> +	pci = get_bound_vga(chip->pci);
> +	if (pci) {
> +		powered_off = pm_runtime_suspended(&pci->dev) &&
> +			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
> +		pci_dev_put(pci);
> +	}
> +
> +	return powered_off;
> +#else
> +	return false;
> +#endif
> +}

This ifdef can be moved to the caller side below...

> +
>  static int azx_prepare(struct device *dev)
>  {
>  	struct snd_card *card = dev_get_drvdata(dev);
> @@ -1025,6 +1048,10 @@ static int azx_prepare(struct device *dev)
>  		return 0;
>  
>  	chip = card->private_data;
> +	/* A powered-off dGPU cannot signal HDA wake events. */
> +	if (azx_vga_is_powered_off(chip))
> +		return 1;
> +

... to wrap the call site, instead.  Since this is the only caller,
having the conditional here would make the intention clearer.

Also, a comment about what "return 1" plays here would be helpful for
readers.  It's a small piece but has a significant meaning.


thanks,

Takashi