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

[Pierre-Louis Bossart]

On 5/12/20 8:46 AM, Alex Deucher wrote:
> On Mon, May 11, 2020 at 6:37 PM Pierre-Louis Bossart
> <pierre-louis.bossart at linux.intel.com> wrote:
>>
>>
>>> @@ -233,6 +234,11 @@ 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_allow(&pci->dev);
>>> +     pm_runtime_mark_last_busy(&pci->dev);
>>> +     pm_runtime_put_autosuspend(&pci->dev);
>>
>> usually there is a pm_runtime_put_noidle() here?
> 
> I'm not sure.
> 
>>
>> [...]
>>
>>>    static void snd_rn_acp_remove(struct pci_dev *pci)
>>>    {
>>>        struct acp_dev_data *adata;
>>> @@ -260,6 +302,9 @@ static void snd_rn_acp_remove(struct pci_dev *pci)
>>>        ret = rn_acp_deinit(adata->acp_base);
>>>        if (ret)
>>>                dev_err(&pci->dev, "ACP de-init failed\n");
>>> +     pm_runtime_put_noidle(&pci->dev);
>>> +     pm_runtime_get_sync(&pci->dev);
>>> +     pm_runtime_forbid(&pci->dev);
>>
>> doing a put_noidle() followed by a get_sync() and immediately forbid()
>> is very odd at best.
>> Isn't the recommendation to call get_noresume() here?
>>
> 
> I'm not sure here either.  Is there some definitive documentation on
> what exact sequences are supposed to be used in drivers?  A quick
> browse through drivers that implement runtime pm seems to show a lot
> of variation.  This sequence works.  I'm not sure if it's optimal or
> not.

We based our sequence on the comments in drivers/pci/pci-driver.c

/*
  * Unbound PCI devices are always put in D0, regardless of
  * runtime PM status.  During probe, the device is set to
  * active and the usage count is incremented.  If the driver
  * supports runtime PM, it should call pm_runtime_put_noidle(),
  * or any other runtime PM helper function decrementing the usage
  * count, in its probe routine and pm_runtime_get_noresume() in
  * its remove routine.
  */