On 03. 06. 24 10:31, Shengjiu Wang wrote:
On Tue, May 28, 2024 at 7:14 PM Shengjiu Wang <shengjiu.wang@xxxxxxxxx> wrote:
On Mon, May 27, 2024 at 8:23 PM Jaroslav Kysela <perex@xxxxxxxx> wrote:
There is a requirement to expose the audio hardware that accelerates various
tasks for user space such as sample rate converters, compressed
stream decoders, etc.
This is description for the API extension for the compress ALSA API which
is able to handle "tasks" that are not bound to real-time operations
and allows for the serialization of operations.
For details, refer to "compress-passthrough.rst" document.
Note: This code is RFC (not tested, just to clearify the API requirements).
My goal is to add a test (loopback) driver and add a support to tinycompress
library in the next step.
Cc: Mark Brown <broonie@xxxxxxxxxx>
Cc: Shengjiu Wang <shengjiu.wang@xxxxxxxxx>
Cc: Nicolas Dufresne <nicolas@xxxxxxxxxxxx>
Cc: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
Cc: Vinod Koul <vkoul@xxxxxxxxxx>
Signed-off-by: Jaroslav Kysela <perex@xxxxxxxx>
---
v1..v2:
- fix some documentation typos (thanks Amadeusz Sławiński)
- fix memdup_user() error handling (thanks Takashi)
- use one state variable instead multiple (thanks Takashi)
- handle task limit (set to 64 - mentioned in documentation, NIY)
- fix file release (free all tasks)
---
.../sound/designs/compress-passthrough.rst | 125 +++++++
include/sound/compress_driver.h | 32 ++
include/uapi/sound/compress_offload.h | 51 ++-
sound/core/Kconfig | 4 +
sound/core/compress_offload.c | 334 +++++++++++++++++-
5 files changed, 537 insertions(+), 9 deletions(-)
create mode 100644 Documentation/sound/designs/compress-passthrough.rst
diff --git a/Documentation/sound/designs/compress-passthrough.rst b/Documentation/sound/designs/compress-passthrough.rst
new file mode 100644
index 000000000000..975462500c33
--- /dev/null
+++ b/Documentation/sound/designs/compress-passthrough.rst
@@ -0,0 +1,125 @@
+=================================
+ALSA Co-processor Passthrough API
+=================================
+
+Jaroslav Kysela <perex@xxxxxxxx>
+
+
+Overview
+========
+
+There is a requirement to expose the audio hardware that accelerates various
+tasks for user space such as sample rate converters, compressed
+stream decoders, etc.
+
+This is description for the API extension for the compress ALSA API which
+is able to handle "tasks" that are not bound to real-time operations
+and allows for the serialization of operations.
+
+Requirements
+============
+
+The main requirements are:
+
+- serialization of multiple tasks for user space to allow multiple
+ operations without user space intervention
+
+- separate buffers (input + output) for each operation
+
+- expose buffers using mmap to user space
+
+- signal user space when the task is finished (standard poll mechanism)
+
+Design
+======
+
+A new direction SND_COMPRESS_PASSTHROUGH is introduced to identify
+the passthrough API.
+
+The API extension shares device enumeration and parameters handling from
+the main compressed API. All other realtime streaming ioctls are deactivated
+and a new set of task related ioctls are introduced. The standard
+read/write/mmap I/O operations are not supported in the passthrough device.
+
+Device ("stream") state handling is reduced to OPEN/SETUP. All other
+states are not available for the passthrough mode.
+
+Data I/O mechanism is using standard dma-buf interface with all advantages
+like mmap, standard I/O, buffer sharing etc. One buffer is used for the
+input data and second (separate) buffer is used for the output data. Each task
+have separate I/O buffers.
+
+For the buffering parameters, the fragments means a limit of allocated tasks
+for given device. The fragment_size limits the input buffer size for the given
+device. The output buffer size is determined by the driver (may be different
+from the input buffer size).
+
+State Machine
+=============
+
+The passthrough audio stream state machine is described below :
+
+ +----------+
+ | |
+ | OPEN |
+ | |
+ +----------+
+ |
+ |
+ | compr_set_params()
+ |
+ v
+ all passthrough task ops +----------+
+ +------------------------------------| |
+ | | SETUP |
+ | |
+ | +----------+
+ | |
+ +------------------------------------------+
+
+
+Passthrough operations (ioctls)
+===============================
+
+CREATE
+------
+Creates a set of input/output buffers. The input buffer size is
+fragment_size. Allocates unique seqno.
+
+The hardware drivers allocate internal 'struct dma_buf' for both input and
+output buffers (using 'dma_buf_export()' function). The anonymous
+file descriptors for those buffers are passed to user space.
+
Audio data is always separated into a lot of periods. so does this mean
that the call sequence is "CREATE" (user fill period data to buffer ) "START"
-> "CREATE" (user fill period data to buffer )"START"
-> "CREATE" (user fill period data to buffer )"START".... ?
Will this cause memory fragmentation?
How can the user get each output?
Or does this mean the sequence should be:
"CREATE"
(user space fill period data to buffer )
"START"
"STOP" : if the task is running, stop it there is no output generated?
"STATUS": polling the status to check the task finished for every task?
then another loop for each period?
Per my understanding, I use the below sequence:
create -> (start, stop, status) -> (start, stop, status) ...-> free.
The stop is optional. When the driver calls snd_compr_task_finished(), it's
expected that the task has finished (stopped) and the next started task in the
queue is processed.
Below are issues found in the code.
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index da048e61c574..7b6168d43d41 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -134,6 +134,7 @@ static int snd_compr_open(struct inode *inode,
struct file *f)
}
runtime->state = SNDRV_PCM_STATE_OPEN;
init_waitqueue_head(&runtime->sleep);
+ INIT_LIST_HEAD(&runtime->tasks);
Thanks.
data->stream.runtime = runtime;
f->private_data = (void *)data;
scoped_guard(mutex, &compr->lock)
@@ -1003,10 +1004,6 @@ static struct snd_compr_task_runtime *
static void snd_compr_task_free(struct snd_compr_task_runtime *task)
{
- if (task->output)
- dma_buf_put(task->output);
- if (task->input)
- dma_buf_put(task->input);
kfree(task);
}
See bellow.
@@ -1045,6 +1042,7 @@ static int snd_compr_task_new(struct
snd_compr_stream *stream, struct snd_compr_
retval = utask->output_fd;
goto cleanup;
}
+ utask->seqno = task->seqno;
Thanks.
list_add_tail(&task->list, &stream->runtime->tasks);
stream->runtime->total_tasks++;
return 0;
There is no dma_buf_get() called, so remove the dma_buf_put().
I expected that the driver will increase this reference counter in the
task_create callback. But yes, it will be probably better to do in
snd_compr_task_new() when file descriptors are used.
I will send v3 soon. Thank you for your comments.
Jaroslav
--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
[Index of Archives]
[Pulseaudio]
[Linux Audio Users]
[ALSA Devel]
[Fedora Desktop]
[Fedora SELinux]
[Big List of Linux Books]
[Yosemite News]
[KDE Users]
