Conversation
dbaluta
requested review from
bardliao,
kv2019i,
lbetlej,
lgirdwood,
marcinszkudlinski,
mmaka1,
pblaszko and
plbossart
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where buffer flags are being incorrectly interpreted due to flag overlap after heap API changes. The SOF_BUF_OVERRUN_PERMITTED flag was overlapping with SOF_MEM_FLAG_DMA, causing incorrect behavior when setting underrun/overrun permissions.
- Move audio_stream_set_underrun/overrun calls from buffer_alloc_struct to buffer_new
- Use original unmodified flags from Linux IPC instead of merged flags with caps
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ipc/ipc-helper.c | Added audio_stream_set_underrun/overrun calls using original desc->flags |
| src/audio/buffers/comp_buffer.c | Removed audio_stream_set_underrun/overrun calls that used merged flags |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
After commit 5821682 ("heap: simplify heap API") flags sent via IPC by the App processor are merged with caps. There is an overlap after this merged so SOF_BUF_OVERRUN_PERMITTED overlaps with SOF_MEM_FLAG_DMA for example. Also, it looks like the old `flags` are mostly unused. For now just remove the call audio_stream_set_underrun/audio_stream_set_overrun inside buffer_alloc_struct and set it in the buffer_new() function where we do have access to unmodified flags received from Linux. In the future, we might completely remove the `old` flags. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
dbaluta
force-pushed
the
fix_heap_api2
branch
from
c4c4557 to
b31388f
Compare
|
Changes since v1:
|
| audio_stream_set_underrun(&buffer->stream, | ||
| !!(desc->flags & SOF_BUF_UNDERRUN_PERMITTED)); | ||
| audio_stream_set_overrun(&buffer->stream, | ||
| !!(desc->flags & SOF_BUF_OVERRUN_PERMITTED)); |
There was a problem hiding this comment.
this doesn't seem enough to me. Line 81 above still calls buffer_alloc() with flags derived from desc->flags which now isn't correct. I think #10227 is a better fix.
|
Merged this #10227 instead so closing. |
4 participants
After commit 5821682 ("heap: simplify heap API") flags sent via IPC by the App processor are merged with caps.
There is an overlap after this merged so SOF_BUF_OVERRUN_PERMITTED overlaps with SOF_MEM_FLAG_DMA for example.
Also, it looks like the old
flagsare mostly unused. For now just remove the call audio_stream_set_underrun/audio_stream_set_overrun inside buffer_alloc_struct and set it in the buffer_new() function where we do have access to unmodified flags received from Linux.In the future, we might completely remove the
oldflags.