On Thu, 26 Aug 2021 13:49:32 +0200, Vitaly Rodionov wrote: > > On 26/08/2021 11:49 am, Takashi Iwai wrote: > > On Thu, 26 Aug 2021 08:03:45 +0200, > > Takashi Iwai wrote: > >> On Wed, 25 Aug 2021 20:04:05 +0200, > >> Vitaly Rodionov wrote: > >>> Actually when codec is suspended and we do reboot from UI, then sometimes we > >>> see suspend() calls in kernel log and no pops, but sometimes > >>> > >>> we still have no suspend() on reboot and we hear pops. But when we do reboot > >>> from command line: > sudo reboot then we always have pops and no suspend() > >>> called. > >>> > >>> Then we have added extra logging and we can see that on reboot codec somehow > >>> getting resume() call and we run jack detect on resume that causing pops. > >> Hm, it's interesting who triggers the runtime resume. > >> > >>> We were thinking about possible solution for that and we would propose some > >>> changes in generic code hda_bind.c: > >>> > >>> static void hda_codec_driver_shutdown(struct device *dev) { + if (codec-> > >>> patch_ops.suspend) + codec->patch_ops.suspend(codec); > >>> snd_hda_codec_shutdown(dev_to_hda_codec(dev)); + hda_codec_driver_remove > >>> (dev_to_hda_codec(dev)); } > >> Sorry, it's no-no. The suspend can't be called unconditionally, and > >> the driver unbind must not be called in the callback itself. > >> > >> Does the patch below work instead? > > Sorry there was a typo. A bit more revised patch is below. > > > > > > Takashi > > > > --- a/sound/pci/hda/hda_intel.c > > +++ b/sound/pci/hda/hda_intel.c > > @@ -1383,14 +1383,17 @@ static void azx_free(struct azx *chip) > > hda->freed = 1; > > } > > -static int azx_dev_disconnect(struct snd_device *device) > > +static void __azx_disconnect(struct azx *chip) > > { > > - struct azx *chip = device->device_data; > > struct hdac_bus *bus = azx_bus(chip); > > chip->bus.shutdown = 1; > > cancel_work_sync(&bus->unsol_work); > > +} > > +static int azx_dev_disconnect(struct snd_device *device) > > +{ > > + __azx_disconnect(device->device_data); > > return 0; > > } > > @@ -2356,8 +2359,10 @@ static void azx_shutdown(struct pci_dev > > *pci) > > if (!card) > > return; > > chip = card->private_data; > > - if (chip && chip->running) > > + if (chip && chip->running) { > > + __azx_disconnect(chip); > > azx_shutdown_chip(chip); > > + } > > } > > /* PCI IDs */ > > Hi Takashi, > > Applied fix and tested on dolphin HW. Issue still there, here is > captured screen on reboot from command line: > > reboot capture > > Reboot from UI works differently, no resume() call in this case. Thanks for quick testing. After reconsideration, I believe we can even take a simpler path. Use pm_runtime_force_suspend(), and keep suspended by pm_runtime_disable() call afterwards. Below is another test patch. Could you check whether this works better? Takashi --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2986,13 +2986,11 @@ void snd_hda_codec_shutdown(struct hda_codec *codec) { struct hda_pcm *cpcm; - if (pm_runtime_suspended(hda_codec_dev(codec))) - return; - list_for_each_entry(cpcm, &codec->pcm_list_head, list) snd_pcm_suspend_all(cpcm->pcm); - pm_runtime_suspend(hda_codec_dev(codec)); + pm_runtime_force_suspend(hda_codec_dev(codec)); + pm_runtime_disable(hda_codec_dev(codec)); } /*