ipc: ipc4: fixes to DMA driver use in user-space builds#10844
Conversation
Use sof_dma_get_status() call to allow the audio pipeline to be run in user-space. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Fix a few remaining uses of direct DMA driver calls. Use the sof_dma.h wrapper instead, allowing the code to be used also from user-space. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
requested review from
dbaluta,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
This PR updates IPC4 code paths to be compatible with Zephyr user-space LL builds by avoiding privileged DMA and interrupt APIs, while keeping existing behavior for non-user-space builds.
Changes:
- Make IPC4 component driver lookup avoid
irq_local_disable()in user-space LL builds. - Switch IPC4 DAI DMA operations from direct Zephyr DMA calls to
sof_dma_*()wrappers for stop/release/status.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/ipc/ipc4/helper.c | Conditionally avoids IRQ masking during driver lookup in user-space LL builds. |
| src/ipc/ipc4/dai.c | Uses sof_dma_*() wrapper APIs for DMA stop/release/status in IPC4 DAI paths. |
| #ifndef CONFIG_SOF_USERSPACE_LL | ||
| uint32_t flags; | ||
| #endif |
| * list is immutable by this point so no lock is needed. | ||
| */ | ||
| #ifndef CONFIG_SOF_USERSPACE_LL | ||
| irq_local_disable(flags); |
| } | ||
|
|
||
| #ifndef CONFIG_SOF_USERSPACE_LL | ||
| irq_local_enable(flags); |
| struct comp_driver_info *info; | ||
| #ifndef CONFIG_SOF_USERSPACE_LL | ||
| uint32_t flags; | ||
| #endif |
There was a problem hiding this comment.
This commit seems to suggest that no locking is needed here at all... Maybe remove it completely and see if it explodes?..
There was a problem hiding this comment.
@lyakh With current limited CI on-device coverage, not very eager to make such see-if-it-explodes tests. But this really does seems to be the case. Drivers list is initiated at component init and only dynamic code path to register is lib_manager_register_module(). @lgirdwood you were adding module reloading, could this lead to need to have locking for the driver list?
|
@lyakh good now ? |
@lgirdwood @kv2019i cannot say I'm particularly happy with the last commit in the series, sorry. If anything the comment doesn't seem quite right. That driver searching is done exactly when drivers are added to the list during LLEXT module loading. So I don't think "the (driver) list is immutable by this point" description is quite right. I'm not saying that the locking is needed there, possibly it isn't, but for a different reason - because searching for drivers is serialised with adding them. So for that reason the lock might be redundant. Or indeed we could replace it with a userspace-compatible lock because driver loading isn't time-critical and we don't expect any contention there. So... The commit might well be harmless and certainly helpful for the userspace, but TBH it doesn't seem very convincing to me. |
kv2019i
force-pushed
the
202606-dai-dma-usage
branch
from
95254e7 to
73870dc
Compare
|
V3 pushed:
|
A set of small fixes to how IPC4 code uses DMA driver and driver look-up interfaces.
Patches part of feature development done in #10558