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

[Hui Wang]

On 2020/4/14 下午8:41, Takashi Iwai wrote:
> On Tue, 14 Apr 2020 14:40:35 +0200,
> Takashi Iwai wrote:
>> On Tue, 14 Apr 2020 14:35:50 +0200,
>> Hui Wang wrote:
>>>
>>> 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.
>>>>>
>>>>> 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.
>> OK, I just watned to know the situation.  So it's no regression but
>> rather to align the behavior among drivers, and that's fine.
>> Let's take the patch, then.
> On the second thought, we can drop the vga_switcheroo check and always
> allow it?  We do want to enable in anyway, and calling it twice should
> be fine.
>
Yes, could drop the check, it is fine to call runtime_allow() twice.

Thanks,

Hui.

> thanks,
>
> Takashi
>
>
>>
>> thanks,
>>
>> Takashi
>>
>>> 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
>>>>>