> -----Original Message----- > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > Sent: Wednesday, May 6, 2020 3:19 AM > To: Alex Deucher <alexdeucher@xxxxxxxxx>; alsa-devel@xxxxxxxxxxxxxxxx; > broonie@xxxxxxxxxx; Mukunda, Vijendar <Vijendar.Mukunda@xxxxxxx>; > tiwai@xxxxxxx > Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx> > Subject: Re: [PATCH 09/14] ASoC: amd: add Renoir ACP PCI driver PM ops > > > > > diff --git a/sound/soc/amd/renoir/rn-pci-acp3x.c > b/sound/soc/amd/renoir/rn-pci-acp3x.c > > index 362409ef0d85..6d013a1bffa6 100644 > > --- a/sound/soc/amd/renoir/rn-pci-acp3x.c > > +++ b/sound/soc/amd/renoir/rn-pci-acp3x.c > > @@ -10,6 +10,7 @@ > > #include <linux/delay.h> > > #include <linux/platform_device.h> > > #include <linux/interrupt.h> > > +#include <linux/pm_runtime.h> > > > > #include "rn_acp3x.h" > > > > @@ -233,6 +234,12 @@ static int snd_rn_acp_probe(struct pci_dev *pci, > > ret = PTR_ERR(adata->pdev); > > goto unregister_devs; > > } > > + pm_runtime_set_autosuspend_delay(&pci->dev, > ACP_SUSPEND_DELAY_MS); > > + pm_runtime_use_autosuspend(&pci->dev); > > + pm_runtime_set_active(&pci->dev); > > is the set_active() needed? I haven't seen this in the other PCI audio > drivers? We have similar implementation in our Raven ACP PCI driver as well which got up streamed. I will give a try by modifying this sequence. Could you please point me , what's exactly wrong with this code? > > > + pm_runtime_put_noidle(&pci->dev); > > + pm_runtime_enable(&pci->dev); > > same, is the _enable() needed()? We have similar implementation in Raven ACP PCI driver as well. > > > + pm_runtime_allow(&pci->dev); > > return 0; > > > > unregister_devs: > > @@ -250,6 +257,42 @@ static int snd_rn_acp_probe(struct pci_dev *pci, > > return ret; > > } > >