Re: [PATCH] ALSA: hda: intel: reset link for HP ENVY 27-p014

[Takashi Iwai]

On Fri, 28 Aug 2026 22:29:28 +0200,
Casey Tunturi via B4 Relay wrote:
> 
> From: Casey Tunturi <casey@xxxxxxxxxxxxxxxxxxxxxx>
> 
> While bringing up audio on an HP ENVY 27-p014, I found that the normal
> link reset only discovers the HDMI codec. The CX20753/4 at address 0
> appears after a second CRST pulse, with STATESTS left alone and no fixed
> delays.
> 
> I added that pulse when address 0 is missing, and again before the codecs
> resume. Initial setup uses immediate commands long enough to add the
> codec, then returns the controller to CORB/RIRB.
> 
> That brought analog audio up at boot, and capture kept working through
> four deep suspend and resume cycles.
> 
> Signed-off-by: Casey Tunturi <casey@xxxxxxxxxxxxxxxxxxxxxx>

Thanks for the patch.  But I must say that it's very hackish, and
we should reconsider to make the changes a bit more generic or
cleaner.

But, first off, could you give more details about your hardware?
At best, give alsa-info.sh output (run with --no-upload option and
attach the output).

Now more about the code change:

> +static u16 hp_envy_27_reset_link(struct azx *chip)
> +{
> +	struct hdac_bus *bus = azx_bus(chip);
> +	u32 gctl;
> +	u16 statests;
> +	int timeout;
> +
> +	snd_hdac_bus_stop_chip(bus);
> +
> +	gctl = snd_hdac_chip_readl(bus, GCTL);
> +	snd_hdac_chip_writel(bus, GCTL, gctl & ~AZX_GCTL_RESET);
> +	for (timeout = 250; timeout; timeout--) {
> +		if (!(snd_hdac_chip_readl(bus, GCTL) & AZX_GCTL_RESET))
> +			break;
> +		udelay(1);
> +	}
> +
> +	gctl = snd_hdac_chip_readl(bus, GCTL);
> +	snd_hdac_chip_writel(bus, GCTL, gctl | AZX_GCTL_RESET);
> +	for (timeout = 250; timeout; timeout--) {
> +		if (snd_hdac_chip_readl(bus, GCTL) & AZX_GCTL_RESET)
> +			break;
> +		udelay(1);
> +	}
> +
> +	statests = snd_hdac_chip_readw(bus, STATESTS);
> +	bus->codec_mask |= statests;
> +	snd_hdac_bus_init_chip(bus, false);
> +
> +	return statests;

Can't the standard snd_hdac_bus_reset_link() work?  If not, we may
tweak there in the standard code instead of open-code here.

>  static void __azx_runtime_resume(struct azx *chip)
>  {
>  	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
> @@ -1003,6 +1043,8 @@ static void __azx_runtime_resume(struct azx *chip)
>  
>  	azx_init_pci(chip);
>  	hda_intel_init_chip(chip, true);
> +	if (chip->pm_prepared && hp_envy_27_needs_reset(chip))
> +		hp_envy_27_reset_link(chip);

So you need a bus reset always at each S3/S4 resume?
Or could this be conditional?

> +static int hp_envy_27_probe_codec(struct azx *chip)
> +{
> +	struct hdac_bus *bus = azx_bus(chip);
> +	struct hda_codec *codec = NULL;
> +	bool use_pio;
> +	int probe_mask;
> +	int err;
> +
> +	if (!hp_envy_27_needs_reset(chip) || (bus->codec_mask & BIT(0)))
> +		return 0;
> +	if (!(hp_envy_27_reset_link(chip) & BIT(0)))
> +		return -ENODEV;

Trying the bus reset at probe is understandable, but...

> +	probe_mask = chip->codec_probe_mask;
> +	use_pio = bus->use_pio_for_commands;
> +	chip->codec_probe_mask = BIT(0);
> +	bus->use_pio_for_commands = true;
> +	err = snd_hda_codec_new(&chip->bus, chip->card, 0, &codec);
> +	chip->codec_probe_mask = probe_mask;
> +	if (!err && codec)
> +		err = snd_hda_codec_configure(codec);
> +	else if (!err)
> +		err = -ENODEV;
> +	bus->use_pio_for_commands = use_pio;

... those look too ugly.  The use of PIO mode isn't mentioned in the
description, either.  Maybe PIO mode should be used always for your
board?

> @@ -2444,6 +2519,8 @@ static int azx_probe_continue(struct azx *chip)
>  		pm_runtime_allow(&pci->dev);
>  		pm_runtime_put_autosuspend(&pci->dev);
>  	}
> +	if (hp_envy_27_needs_reset(chip))
> +		pm_runtime_forbid(&pci->dev);

Add to power_save_denylist, instead.


thanks,

Takashi