[DRAFT] userspace LL/audio test PR#10558
[DRAFT] userspace LL/audio test PR#10558kv2019i wants to merge 89 commits intothesofproject:mainfrom
Conversation
|
Running the pipeline_two_components_user test added in this PR, on a Intel PTL, looks like this: |
9cdb1be to
37b0412
Compare
|
V2 snapshot pushed:
|
37b0412 to
7317b18
Compare
|
V3 snapshot pushed:
|
|
Test output for the new test added in V3: |
src/ipc/ipc-common.c
Outdated
| /* works? yes */ | ||
| //return 0; | ||
|
|
||
| printk("ipc %p\n", ipc); |
There was a problem hiding this comment.
Oops, these shouldn't be here. :)
|
|
||
| /* create the pipeline */ | ||
| pipe = pipeline_new(NULL, pipe_desc->primary.r.instance_id, | ||
| pipe = pipeline_new(heap, pipe_desc->primary.r.instance_id, |
There was a problem hiding this comment.
@lyakh @jsarha @lgirdwood This is where you'd plug in the vregions stuff to pass a separate heap to each pipe (based on topology description of its needs). In this series, as a placeholder, I use the zephyr_ll_user_heap() instead.
There was a problem hiding this comment.
right, maybe put a comment there for now to make re-discovery easier
lyakh
left a comment
There was a problem hiding this comment.
last reviewed commit so far "schedule: zephyr_ll: implement thread_init/free domain ops"
| #ifdef CONFIG_SOF_USERSPACE_LL | ||
| void comp_grant_access_to_thread(const struct comp_dev *dev, struct k_thread *th) | ||
| { | ||
| assert(dev->list_mutex); |
There was a problem hiding this comment.
description a bit confusing - this is only granting access to a mutex. Also list_mutex is only added to comp_dev in the next commit.
| } | ||
|
|
||
| stream_addr = rballoc_align(flags, size, align); | ||
| stream_addr = sof_heap_alloc(heap, flags, size, align); |
There was a problem hiding this comment.
the commit, that is mentioned in the commit message, only moved buffer context objects to particular heaps. This commit moves actual data buffers to them too, which is different and (arguably) more risky
| #define HDA_DMA_BUFFER_PERIOD_COUNT 4 | ||
|
|
||
| SHARED_DATA struct sof_dma dma[] = { | ||
| APP_TASK_DATA SHARED_DATA struct sof_dma dma[] = { |
There was a problem hiding this comment.
do I understand correctly, that this kind of userspace access makes that data writable to userspace?
| comp_err(dev, "requested channel %d is busy", hda_chan); | ||
| return -ENODEV; | ||
| } | ||
| hd->chan = &hd->dma->chan[channel]; |
There was a problem hiding this comment.
is this also not needed for the legacy mode? Also below
| uint64_t next_sync; | ||
| uint64_t period_in_cycles; | ||
| #endif | ||
| struct k_heap *heap; |
There was a problem hiding this comment.
wasn't this already referenced in the previous commit?
|
|
||
| k_spinlock_init(&dd->dai->lock); | ||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||
| dd->dai->lock = k_object_alloc(K_OBJ_MUTEX); |
There was a problem hiding this comment.
check for NULL? Possibly in other locations too
| k_mutex_lock(dai->lock, K_FOREVER); | ||
| props = dai_get_properties(dai->dev, direction, stream_id); | ||
| hs_id = props->dma_hs_id; | ||
| ret = dai_get_properties_copy(dai->dev, direction, stream_id, &props); |
There was a problem hiding this comment.
I'm guessing this is made a syscall in one of the commits
| mod_heap = &mod_heap_user->heap; | ||
| } else { | ||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||
| mod_heap = zephyr_ll_user_heap(); |
There was a problem hiding this comment.
looks good, but this else is entered under multiple conditions, might need to double-check
| .schedule_task_before = zephyr_ll_task_schedule_before, | ||
| .schedule_task_after = zephyr_ll_task_schedule_after, | ||
| .schedule_task_free = zephyr_ll_task_free, | ||
| .schedule_task_free = zephyr_ll_task_sched_free, |
There was a problem hiding this comment.
let's "spend" 3 more characters and make it ..._schedule_free()
| return -ENOMEM; | ||
| } | ||
| tr_err(&ll_tr, "Failed to allocate thread object for core %d", core); | ||
| dt->handler = NULL; |
| { | ||
| const struct sof_man_fw_desc *desc = basefw_vendor_get_manifest(); | ||
| const struct sof_man_module *mod; | ||
| uint32_t i; |
| uint32_t i; | ||
|
|
||
| if (!desc) | ||
| return -1; |
| return (int)i; | ||
| } | ||
|
|
||
| return -1; |
| union ipc4_connector_node_id node_id; | ||
| uint32_t dma_buffer_size; | ||
| uint32_t config_length; | ||
| } __packed __aligned(4); |
There was a problem hiding this comment.
does __aligned actually make sense in a type definition?
src/ipc/ipc-common.c
Outdated
| pipe_msg.extension.dat = ipc_user->ipc_msg_ext; | ||
|
|
||
| /* Execute pipeline creation in user context */ | ||
| ipc_user->result = ipc_pipeline_new(ipc_user->ipc, (ipc_pipe_new *)&pipe_msg); |
There was a problem hiding this comment.
that's brave! ;-) I'd put a huge "TODO" here to make sure not to ship this by chance :-)
|
|
||
| /* create the pipeline */ | ||
| pipe = pipeline_new(NULL, pipe_desc->primary.r.instance_id, | ||
| pipe = pipeline_new(heap, pipe_desc->primary.r.instance_id, |
There was a problem hiding this comment.
right, maybe put a comment there for now to make re-discovery easier
7317b18 to
c029d49
Compare
|
V4 snapshot pushed:
|
|
Example output with V4 patchset: |
c029d49 to
9c778e4
Compare
|
V5 pushed:
|
9c778e4 to
c4b127f
Compare
|
V6 pushed:
|
c4b127f to
ad359d6
Compare
|
V7 submitted:
|
ad359d6 to
c7e9a37
Compare
|
V8 submitted:
|
To enable use of Zephyr sys/mutex.h interface in SOF audio pipeline code, add a rtos/mutex.h wrapper for this. In Zephyr build, the native Zephyr interface is used and in posix builds (e.g. testbench), a no-op implementation is provided (similarly that already exists for k_mutex). Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Replace the spinlock-based synchronization mechanism for firmware register access with a mutex-based approach. This change removes the fw_reg_lock spinlock field from the sof structure (both POSIX and Zephyr variants), eliminates the spinlock initialization in primary_core_init(), and updates the dai_dma_release(), dai_release_llp_slot(), and dai_get_unused_llp_slot() functions to use sys_mutex_lock/unlock instead of k_spin_lock/unlock. All SOF IPC and audio pipeline code is run in threads, so using spinlocks is no longer necessary. Using mutex is a best fit and allows the audio pipeline code to be used also in user-space threads. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
If CONFIG_SOF_BOOT_TEST_STANDALONE is set, ipc_init() is terminated early. This ensures SOF will not start to generate or respond to IPC messages that could potentially interfere with standalone test cases (some of which send and receive IPCs). The current implementation leaves the component list uninitialized and this can cause trouble to standalone tests that want to utilzie common IPC code to build messages. Fix this problem by executing more of ipc_init() also in the standalone mode. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a method to add access to a component object to a particular thread. This is required as component object state includes kernel objects and to use these from user-space, access needs to be granted. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
In CONFIG_SOF_USERSPACE_LL builds, irq_local_disable() is not available. Replace the no-op irq guard with the LL scheduler's own k_mutex to prevent the scheduler from processing tasks while pipeline state is being updated. The k_mutex is re-entrant so schedule_task() calls inside the critical section are safe. Add zephyr_ll_lock_sched() and zephyr_ll_unlock_sched() helpers that acquire and release the scheduler mutex, and call them from pipeline_schedule_triggered() under CONFIG_SOF_USERSPACE_LL. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The mod_heap_user (dp_heap_user) reference count management in comp_buffer_free() does not belong there - a buffer's free method should not be responsible for freeing a module's private heap. Additionally, in CONFIG_SOF_USERSPACE_LL builds, comp_buffer_free() may run in user-space context where rfree() of kernel-allocated memory is not ok. - Add dp_heap_put() helper to dp_schedule.h for reference-counted DP heap release - Move buffer-side client_count decrements to IPC layer (ipc_comp_disconnect, ipc_pipeline_module_free, ipc_comp_connect error paths) - Move raw data buffer client_count management to module_adapter_free/prepare with DP domain guard - Fix module_adapter_mem_free() where CONFIG_SOF_USERSPACE_LL incorrectly reassigned mod_heap for DP modules - Fix client_count increment guard in ipc_comp_connect() to check dp pointer rather than dp_heap Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a built option HOST_DMA_IPC_POSITION_UPDATES to control whether functionality to send IPC stream position updates is enabled or not. Most platforms provide more efficient means for host to monitor DMA state, so this code is in most cases unncessary. The current IPC sending code (from audio context) also assume kernel context, so making this functionality user-space compatible will require extra work. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The FPI sync functionality relies on static global objects. Make these available to the SOF application even when it is run in user-space. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Copier set_chmap() blocks IRQs to atomically update the converters. This code is not safe to be moved to user-space, so replace the locks with calls to block LL scheduler execution. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The module_adapter prepare() blocks IRQs to atomically to connect the sink/source buffers. This code is not safe to be moved to user-space, so replace the locks with calls to block LL scheduler execution, when compiled for user-space. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
This is a mostly mechanical split of initial ipc logic into kernel and user files. This is the 1st stage in supporting both privileged kernel and non privileged userspace IPC commands and security surfaces. At a high level library loading and PM will reside as kernel IPC and pipeline and module will become user IPCs. There will be no impact for devices without MMU. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> (cherry picked from commit e65c6d1)
The ll_block()/ll_unblock() macros use irq_local_disable() and irq_local_enable() for the same-core binding path, or when CONFIG_CROSS_CORE_STREAM is not enabled. These are not available in user-space. Use zephyr_ll_lock_sched()/zephyr_ll_unlock_sched() when building with CONFIG_SOF_USERSPACE_LL to acquire the LL scheduler's own k_mutex instead. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a dedicated user-space thread for handling IPC commands that operate on audio pipelines when CONFIG_SOF_USERSPACE_LL is enabled. The kernel IPC handler forwards CREATE_PIPELINE messages to the user thread via k_event signaling and collects the result through a k_sem handshake. The IPC task_mask IPC_TASK_IN_THREAD bit prevents host completion until the user thread finishes. Key changes: - add ipc_user struct and user-space IPC thread with event loop - allocate IPC context statically in a dedicated memory partition - replace sof_get()->ipc with ipc_get() returning static context - forward CREATE_PIPELINE to user thread via ipc_user_forward_cmd() - allocate pipeline and IPC container from user-space heap This is a in-progress implementation that only handles one IPC message type and is thus not a full implementation. This does allow to proceed to test IPC user thread creation and the basic mechanism to handle messages. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Incremental change to also handle PIPELINE_DELETE IPC in the user-space IPC thread. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Make IPC user thread stack size configurable with SOF_IPC_USER_THREAD_STACK_SIZE (default 4096). Also pin the IPC user thread on primar CPU. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Use sof_heap_alloc() and the new sof_sys_user_heap_get() to route the allocs to correct heap based on SOF build options. If the audio pipeline code is run completely in user-space, a compatible heap should be used. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Make ipc_msg_reply() a system call, so it can be called from audio thread even when audio thread is running in a user-space thread. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
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>
Replace rfree() calls with sof_heap_free() using the system user heap for IPC component and pipeline objects. In ipc4_create_pipeline(), replace the conditional zephyr_ll_user_heap() with the generic sof_sys_user_heap_get() to unify the allocation path across configurations. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Mark the fw_reg mutex with APP_SYSUSER_BSS, allowing the lock to be used when DAI module is run in user-space. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Use the component mutex when code is run in user-space instead of directly blocking interrupts. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Do not use IRQ disable/enable when built for user-space. The driver list is immutable by IPC processing time so no lock is needed. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a warning and a comment to all places where k_object_free() is called in code that may be run in user threads. This will result in resource leaks, but there is currently no immediate solution to how to free these objects from user threads. We may need to revisit the partition of functionality between system and user. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a new test to userspace_ll set that takes a step towards running full audio pipelines in user-space. The test creates a pipeline with two components (IPC4 host and DAI copiers), does pipeline prepare, one copy cycle in prepared state and tears down the pipeline. One user-space thread is created to manage the pipelines. This would be equivalent to user IPC handler thread. Another user-space thread is created for the LL scheduler. This test has some limitation, but is a useful test point to test resource allocations in pipeline, component and module adapter code. The code adds a reference test case where the same flow is fully run in kernel space. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a test that sends SOF_IPC4_GLB_CREATE_PIPELINE message via ipc_cmd() interface. This test can be used to test SOF when built with CONFIG_SOF_USERSPACE_LL. Handling of audio pipeline IPC messages (like CREATE_PIPELINE) will be routed to a user-space IPC thread. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Place the pipeline position lookup table in the sysuser memory partition and replace k_spinlock with a dynamically allocated k_mutex when CONFIG_SOF_USERSPACE_LL is enabled. Spinlocks disable interrupts which is a privileged operation unavailable from user-mode threads. The mutex pointer is stored in a separate APP_SYSUSER_BSS variable outside the SHARED_DATA struct so Zephyr's kernel object tracking can recognize it for syscall verification. Move pipeline_posn_init() from task_main_start() to primary_core_init() before platform_init(), so the mutex is allocated before ipc_user_init() grants thread access to it. In pipeline_posn_get(), bypass the sof_get() kernel singleton and access the shared structure directly when running in user-space. Grant the ipc_user_init thread access to the pipeline position mutex via new pipeline_posn_grant_access() helper. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
This is a set of temporary changes to audio code to remove calls to privileged interfaces that are not mandatory to run simple audio tests. These need proper solutions to be able to run all use-cases in user LL version. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
c7e9a37 to
8af1be4
Compare
|
V9 submitted:
|
SOF has recently gained ability to run DP (=preemptable audio tasks) in Zephyr user-space.
This PR is an early stage pull-request for changes to extend this capability to all of the audio pipeline code, and specifically the LL (low-latency) tasks.
This early stage as the design is not set in stone and the PR uses a number of short cuts in order to move (and tests) incrementally larger sets of audio functionality.