Re: Adding movable PCIe BARs support in snd_hda_intel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 ?

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 ?

Andrey


On 2021-03-24 4:36 p.m., Takashi Iwai wrote:
On Wed, 24 Mar 2021 16:43:02 +0100,
Andrey Grodzovsky wrote:

I merged the patches, since it looks like my branch
is older I didn't have chip->init_failed member and
also seems there was a typo on ur side as there
is no flush_work_sync, only flush_work which waits
synchronously anyway.

Ah that must be some typos I forgot to refresh.

Everything works fine when testing with maplyer running concurrently
to PCI rescan cycle.

Can you be more specific what are those get/put calls,
I am thinking about some waitqueue to wait on in rescan_prepare
after setting snd_hdac_bus_freeze,
on wakeup it checks that a counter dropped back to zero.
Not sure on which entity to hang this counter ?

It's something like the patch 6 in the v2 series below.
At this time, I dropped snd_hdac_bus_freeze() as this is basically
useless.

Maybe this is no optimal implementation and we might improve a bit,
but I guess you get an idea from that.

Also I added ifdef CONFIG_PM in the last patch as it obviously depends
on it.  It could be put in Kconfig somehow, too, but a simple ifdef
should suffice.


Takashi




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux