[PATCH 09/14] ASoC: amd: add Renoir ACP PCI driver PM ops

[Mukunda, Vijendar]

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> Sent: Wednesday, May 6, 2020 3:19 AM
> To: Alex Deucher <alexdeucher at gmail.com>; alsa-devel at alsa-project.org;
> broonie at kernel.org; Mukunda, Vijendar <Vijendar.Mukunda at amd.com>;
> tiwai at suse.de
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>
> 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;
> >   }
> >