[PATCH] ALSA: hda: enable the runtime_pm for non-vgaswitcheroo hda controllers

[Hui Wang]

On 2020/4/14 下午8:42, Roy Spliet wrote:
> Dear Hui, Takashi,
>
> Op 14-04-2020 om 13:35 schreef Hui Wang:
>>
>> On 2020/4/14 下午6:27, Takashi Iwai wrote:
>>> On Tue, 14 Apr 2020 12:14:05 +0200,
>>> Hui Wang wrote:
>>>> Before the pci_driver->probe() is called, the pci subsystem calls
>>>> runtime_forbib() and runtime_get_sync() on this pci dev, so only call
>>>> runtime_put_autosuspend() is not enough to enable the runtime_pm on
>>>> this device.
>>>>
>>>> For controllers with vgaswitcheroo feature, the pci/quirks.c will call
>>>> runtime_allow() for this dev, then the controllers could enter
>>>> rt_idle/suspend/resume, but for non-vgaswitcheroo controllers like
>>>> Intel hda controllers, the runtime_pm is not enabled even it calls
>>>> put_autosuspend(). Need to call runtime_allow() for those controllers
>>>> in the hda driver.
>
> From what I can tell there are no ill effects of calling 
> runtime_allow() twice. Technically, the check against 
> use_vga_switcheroo() is thus redundant.
> Is there a good reason why runtime_allow() is called in the pci quirks 
> rather than in hda_intel? Is it a suggestion to perform this call in 
> hda_intel regardless of whether it's a switcheroo-device or not, and 
> removing calls to runtime_allow() from the PCI quirks?

I guess after the hda driver calls the _allow() unconditionally, we 
could remove the _allow() in the pci/quirks.c. But it is no harm keeping 
it since _allow() could be called many times.

Thanks,

Hui.

> Thanks. Best,
>
> Roy
>>>>
>>>> Signed-off-by: Hui Wang <hui.wang at canonical.com>
>>> Was this behavior changed from the earlier kernels?  I thought this
>>> was left untouched because it's supposed to be set via udev rules or
>>> such.
>>
>> Oh, I don't know that,  according to my test with ubuntu rootfs, the 
>> runtime pm is not enabled on Intel's hda controllers. But with the 
>> sof driver, the controller driver calls runtime_allow() 
>> (soc/sof/sof-pci-dev.c), so I sent this patch.
>>
>> Regards,
>>
>> Hui.
>>
>>> OTOH, enabling the runtime PM is almost mandatory for modern systems,
>>> and I'm fine to apply this kind of forcible enablement.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>>>
>>>
>>>> ---
>>>>   sound/pci/hda/hda_intel.c | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
>>>> index 8519051a426e..779705bef88b 100644
>>>> --- a/sound/pci/hda/hda_intel.c
>>>> +++ b/sound/pci/hda/hda_intel.c
>>>> @@ -2356,6 +2356,8 @@ static int azx_probe_continue(struct azx *chip)
>>>>       if (azx_has_pm_runtime(chip)) {
>>>>           pm_runtime_use_autosuspend(&pci->dev);
>>>> +        if (!use_vga_switcheroo(chip))
>>>> +            pm_runtime_allow(&pci->dev);
>>>>           pm_runtime_put_autosuspend(&pci->dev);
>>>>       }
>>>> -- 
>>>> 2.17.1
>>>>