Adding movable PCIe BARs support in snd_hda_intel

[Andrey Grodzovsky]

Attached.

Andrey

On 2021-03-25 12:38 p.m., Takashi Iwai wrote:
> On Thu, 25 Mar 2021 17:13:41 +0100,
> Andrey Grodzovsky wrote:
>>
>>
>>
>> On 2021-03-25 2:58 a.m., Takashi Iwai wrote:
>>> On Wed, 24 Mar 2021 22:43:24 +0100,
>>> Andrey Grodzovsky wrote:
>>>>
>>>> Few comments -
>>>>
>>>> 1) Why we don't use snd_power_wait_and_ref in patch 3 in the common
>>>> handler ?
>>>> Don't we want the PCI rescan sequence to 'wait for' any in flight
>>>> taks that might be accessing registers and not only read/write/tlv
>>>> accesses ?
>>>
>>> Right, we don't need to block all ioctls, but only the ones that may
>>> access the hardware.  So basically the patches 3-5 can be dropped if
>>> we take the patch 6.  The current patch was written on top of the
>>> previous series, that's why it has both.
>>>
>>>> 2) Possible deadlock -
>>>> In azx_rescan_prepare - you put the card into SNDRV_CTL_POWER_D3hot
>>>> first and then 'wait for' all in flight tasks with the refcount.
>>>> The in flight tasks on the other hand, using snd_power_wait_and_ref,
>>>> may have already bumped up the refcount and now 'wait for' the card
>>>> to go into SNDRV_CTL_POWER_D0 which can't happen since PCI rescan
>>>> waits for the refocunt to drop to 0 before proceeding.
>>>>
>>>> Instead of snd_power_wait_and_ref can't we just call snd_power_ref
>>>> in common IOCTL before checking for power_state != SNDRV_CTL_POWER_D0 ?
>>>> Or is it because you don't want to fail IOCTLs ?
>>>
>>> No, this is the intended behavior and should work as-is because
>>> snd_power_wait_and_ref() drops the refcount in the loop before
>>> sleeping.  The inc before the state check is a must for covering the
>>> possible race, and ditto for changing the power_state to D3hot before
>>> syncing.
>>
>> Ohh, missed the refcount dec in the wait loop... My bad.
>>
>> On boot the latest patch-set was throwing refcount warnings since
>> u are not supposed to inc from zerro count and so I fixed
>> with the attached patch. That way seems to work fine.
> 
> Ah, I completely forgot about the oddness of refcount_t.
> Then I guess it's simpler to replace with atomic_*() instead.
> 
> 
> thanks,
> 
> Takashi
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ALSA-Fix-refcount-splat.patch
Type: text/x-patch
Size: 6430 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20210326/415534d2/attachment-0001.bin>