On Wed, 12 Aug 2026 15:45:27 +0200,
Takashi Iwai wrote:
>
> On Wed, 12 Aug 2026 14:22:06 +0200,
> Andre Eikmeyer wrote:
> >
> > +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.
Looking at the code again, this won't work well.
But, keeping vga_switcheroo-specific code in a single place would be
still meaningful. So,
#ifdef SUPPORT_VGA_SWITCHEROO
#define use_vga_switcheroo(chip) ((chip)->use_vga_switcheroo)
#define needs_eld_notify_link(chip) ((chip)->bus.keep_power)
+static bool azx_vga_is_powered_off(struct azx *chip);
#else
#define use_vga_switcheroo(chip) 0
#define needs_eld_notify_link(chip) false
+#define azx_vga_is_powered_off(chip) false
#endif
and then define azx_vga_is_powered_off() in the section for
vga_switcheroo code.
In anyway, it's a matter of taste, and if the resultant patch doesn't
look better, you can keep the original way, too.
thanks,
Takashi
