On Mon, Jun 3, 2024 at 4:57 PM Jaroslav Kysela <perex@xxxxxxxx> wrote:
>
> 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.
Thanks, So when can we get the v3?
Best regards
Shengjiu Wang
>
> 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]
