Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: dai-zephyr: copy data to all available sinks #9200

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+56 −17
Open

src: dai-zephyr: copy data to all available sinks #9200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions posix/include/sof/lib/dma.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,13 +538,13 @@ int dma_buffer_copy_to(struct comp_buffer __sparse_cache *source,
dma_process_func process, uint32_t sink_bytes);

/*
* Used when copying DMA buffer bytes into multiple sink buffers, one at a time using the provided
* Used when copying stream audio into multiple sink buffers, one at a time using the provided
* conversion function. DMA buffer consume should be performed after the data has been copied
* to all sinks.
*/
int dma_buffer_copy_from_no_consume(struct comp_buffer __sparse_cache *source,
struct comp_buffer __sparse_cache *sink,
dma_process_func process, uint32_t source_bytes);
int stream_copy_from_no_consume(struct comp_buffer __sparse_cache *source,
struct comp_buffer __sparse_cache *sink,
dma_process_func process, uint32_t source_bytes);

/* generic DMA DSP <-> Host copier */

Expand Down
51 changes: 45 additions & 6 deletions src/audio/dai-zephyr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,16 +268,55 @@ dai_dma_cb(struct dai_data *dd, struct comp_dev *dev, uint32_t bytes,
}

if (dev->direction == SOF_IPC_STREAM_PLAYBACK) {
#if CONFIG_IPC_MAJOR_4
struct list_item *sink_list;
/*
* copy from local buffer to all sinks that are not gateway buffers
lgirdwood marked this conversation as resolved.
Show resolved Hide resolved
* using the right PCM converter function.
*/
list_for_item(sink_list, &dev->bsink_list) {
struct comp_dev *sink_dev;
struct comp_buffer *sink;
int j;

sink = container_of(sink_list, struct comp_buffer, source_list);

if (sink == dd->dma_buffer)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dma_buffer is not on the list of sinks, so this condition is always false. However, maybe it is OK to leave it as is or replace with assert() "just in case" someone add dma_buffer to sink list in some future refactoring.

continue;

sink_dev = sink->sink;

j = IPC4_SRC_QUEUE_ID(buf_get_id(sink));

if (j >= IPC4_COPIER_MODULE_OUTPUT_PINS_COUNT) {
comp_err(dev, "Sink queue ID: %d >= max output pin count: %d\n",
j, IPC4_COPIER_MODULE_OUTPUT_PINS_COUNT);
ret = -EINVAL;
continue;
}

if (!converter[j]) {
comp_err(dev, "No PCM converter for sink queue %d\n", j);
ret = -EINVAL;
continue;
}

if (sink_dev && sink_dev->state == COMP_STATE_ACTIVE) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those additional sinks are (always?) connected to other pipelines. And so some other pipelines might be responsible to set buffer stream params in .params() handler of components connected to those sinks. For unknown reason checking sink_dev->state == COMP_STATE_ACTIVE is not enough, I recently observed problems with multiple-pause-resume test on my draft PR and have to fix it like this: 4ebd807.

Frame format of uninitialized buffer is 0 which is treated as SOF_IPC_FRAME_S16_LE. If test uses 24 or 32 bit formats that could lead to wrong consume/produce and invalidate/writeback values.

The problem could be fixed in a separate PR, I just comment it here as your PR adds new place there the fix should be applied.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thanks for review. I'll take a look at your comments and get back with answers

ret = stream_copy_from_no_consume(dd->local_buffer, sink,
converter[j], bytes);
}
}
#endif
ret = dma_buffer_copy_to(dd->local_buffer, dd->dma_buffer,
dd->process, bytes);
} else {
audio_stream_invalidate(&dd->dma_buffer->stream, bytes);
/*
* The PCM converter functions used during DMA buffer copy can never fail,
* so no need to check the return value of dma_buffer_copy_from_no_consume().
* so no need to check the return value of stream_copy_from_no_consume().
*/
ret = dma_buffer_copy_from_no_consume(dd->dma_buffer, dd->local_buffer,
dd->process, bytes);
ret = stream_copy_from_no_consume(dd->dma_buffer, dd->local_buffer,
dd->process, bytes);
#if CONFIG_IPC_MAJOR_4
struct list_item *sink_list;
/* Skip in case of endpoint DAI devices created by the copier */
Expand Down Expand Up @@ -315,9 +354,9 @@ dai_dma_cb(struct dai_data *dd, struct comp_dev *dev, uint32_t bytes,
}

if (sink_dev && sink_dev->state == COMP_STATE_ACTIVE)
ret = dma_buffer_copy_from_no_consume(dd->dma_buffer,
sink, converter[j],
bytes);
ret = stream_copy_from_no_consume(dd->dma_buffer,
sink, converter[j],
bytes);
}
}
#endif
Expand Down
6 changes: 3 additions & 3 deletions src/lib/dma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,9 @@ int dma_buffer_copy_to(struct comp_buffer *source,
return ret;
}

int dma_buffer_copy_from_no_consume(struct comp_buffer *source,
struct comp_buffer *sink,
dma_process_func process, uint32_t source_bytes)
int stream_copy_from_no_consume(struct comp_buffer *source,
struct comp_buffer *sink,
dma_process_func process, uint32_t source_bytes)
{
struct audio_stream *istream = &source->stream;
uint32_t samples = source_bytes /
Expand Down
8 changes: 4 additions & 4 deletions xtos/include/sof/lib/dma.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,13 +538,13 @@ int dma_buffer_copy_from(struct comp_buffer *source,
dma_process_func process, uint32_t source_bytes);

/*
* Used when copying DMA buffer bytes into multiple sink buffers, one at a time using the provided
* Used when copying stream audio into multiple sink buffers, one at a time using the provided
* conversion function. DMA buffer consume should be performed after the data has been copied
* to all sinks.
*/
int dma_buffer_copy_from_no_consume(struct comp_buffer *source,
struct comp_buffer *sink,
dma_process_func process, uint32_t source_bytes);
int stream_copy_from_no_consume(struct comp_buffer *source,
struct comp_buffer *sink,
dma_process_func process, uint32_t source_bytes);

/* copies data to DMA buffer using provided processing function */
int dma_buffer_copy_to(struct comp_buffer *source,
Expand Down