Skip to content

audio: dai-zephyr: rework calls to DMA driver, remove channel pointer#10767

Open
kv2019i wants to merge 3 commits into
thesofproject:mainfrom
kv2019i:202605-dai-zephyr-chan-idx
Open

audio: dai-zephyr: rework calls to DMA driver, remove channel pointer#10767
kv2019i wants to merge 3 commits into
thesofproject:mainfrom
kv2019i:202605-dai-zephyr-chan-idx

Conversation

@kv2019i
Copy link
Copy Markdown
Collaborator

@kv2019i kv2019i commented May 12, 2026

For historical reasons, dai-zephyr has somewhat complicated code to manage the DMA channel instance information. When a DMA channel is allocated, a pointer to the system DMA channel table is acquired and some additional information is stored per channel. This is however redundant as the only piece of information actually needed is the channel index.

Simplify the code by not storing the channel pointer anymore, but rather just store the channel index and use that in all calls to the DMA driver.

Copilot AI review requested due to automatic review settings May 12, 2026 12:48
@kv2019i
Copy link
Copy Markdown
Collaborator Author

kv2019i commented May 12, 2026

Similar to previously done change for host-zephyr.c in #10721

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies dai-zephyr DMA channel handling by removing the stored DMA channel pointer and keeping only the DMA channel index, then updating DMA driver calls to use that index.

Changes:

  • Replaced struct dai_data::chan with chan_index and updated call sites accordingly.
  • Updated DAI DMA stop/release and position/status queries to use chan_index.
  • Removed per-channel dev_data handling associated with the removed channel pointer.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
src/ipc/ipc4/dai.c Switches IPC4 DAI DMA release/config/position code paths to use chan_index instead of a channel pointer.
src/include/sof/lib/dai-zephyr.h Replaces struct dai_data *chan with int chan_index.
src/audio/dai-zephyr.c Updates DMA config/prepare/trigger/copy/status paths to use chan_index and dd->dma directly.

Comment thread src/audio/dai-zephyr.c
Comment thread src/audio/dai-zephyr.c
Comment thread src/audio/dai-zephyr.c Outdated
Comment thread src/ipc/ipc4/dai.c
Comment thread src/ipc/ipc4/dai.c Outdated
Comment thread src/ipc/ipc4/dai.c Outdated
Comment thread src/ipc/ipc4/dai.c Outdated
@softwarecki
Copy link
Copy Markdown
Collaborator

In my opinion we should treat any negative value as "no channel". This simplifies the code compared to checking against a specific constant, makes it safer for future changes and more consistent. The channel id value comes from sof_dma_request_channel(), and code check < 0 to detect errors, so any negative value mean no assigned channel.

@lgirdwood
Copy link
Copy Markdown
Member

In my opinion we should treat any negative value as "no channel". This simplifies the code compared to checking against a specific constant, makes it safer for future changes and more consistent. The channel id value comes from sof_dma_request_channel(), and code check < 0 to detect errors, so any negative value mean no assigned channel.

Ack - and I think historically 0 is also a valid channel number used in the HW DMA specs so negative better.

Comment thread src/ipc/ipc4/dai.c Outdated
@kv2019i
Copy link
Copy Markdown
Collaborator Author

kv2019i commented May 13, 2026

@softwarecki wrote:

In my opinion we should treat any negative value as "no channel". This simplifies the code compared to checking against a specific constant, makes it safer for future changes and more consistent. The channel id value comes from sof_dma_request_channel(), and code check < 0 to detect errors, so any negative value mean no assigned channel.

Ack, this was actually @abonislawski 's idea to use Zephyr DMA API error values directly. But makes sense to continue and check for a negative value, will change in V2.

kv2019i added 3 commits May 13, 2026 17:30
Remove old code to support IPC4 use with non-Zephyr drivers. This code
was used to transition to Zephyr, but no all active users of IPC4
have moved to native Zephyr drivers, so there are no users for this code
left.

Add a Kconfig dependency to native driver support.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
For historical reasons, dai-zephyr has somewhat complicated code to
manage the DMA channel instance information. When a DMA channel is
allocated, a pointer to the system DMA channel table is acquired and
some additional information is stored per channel. This is however
redundant as the only piece of information actually needed is the
channel index.

Simplify the code by not storing the channel pointer anymore, but rather
just store the channel index and use that in all calls to the DMA
driver.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Align with recent changes in dai-zephyr.c and check for negative
channel index to detect whether DMA channel has been set or not,
instead of checking specifically for -EINVAL.

Suggested-by: Adrian Warecki <adrian.warecki@intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i force-pushed the 202605-dai-zephyr-chan-idx branch from 97983d0 to b271352 Compare May 13, 2026 14:33
@kv2019i
Copy link
Copy Markdown
Collaborator Author

kv2019i commented May 13, 2026

V2 pushed:

  • addressed all review comments so far
  • added commit to drop support for non-native drivers and IPC4 (no known users, pretty much everybody has moved to native drivers and those who are not, are using IPC3)
  • add a comment to align host-zephyr.c to feedback from @softwarecki

@lgirdwood
Copy link
Copy Markdown
Member

@kv2019i btw, some build errors:

modules/sof/CMakeFiles/modules_sof.dir/__w/sof/sof/sof/src/ipc/ipc3/dai.c.obj.d -o modules/sof/CMakeFiles/modules_sof.dir/__w/sof/sof/sof/src/ipc/ipc3/dai.c.obj -c /__w/sof/sof/sof/src/ipc/ipc3/dai.c
/__w/sof/sof/sof/src/ipc/ipc3/dai.c: In function 'dai_dma_release':
/__w/sof/sof/sof/src/ipc/ipc3/dai.c:314:15: error: 'struct dai_data' has no member named 'chan'
  314 |         if (dd->chan) {
      |               ^~
/__w/sof/sof/sof/src/ipc/ipc3/dai.c:316:44: error: 'struct dai_data' has no member named 'chan'
  316 |                 notifier_unregister(dev, dd->chan, NOTIFIER_ID_DMA_COPY);
      |                                            ^~
/__w/sof/sof/sof/src/ipc/ipc3/dai.c:318:39: error: 'struct dai_data' has no member named 'chan'
  318 |                 dma_release_channel(dd->chan->dma->z_dev, dd->chan->index);
      |                                       ^~
/__w/sof/sof/sof/src/ipc/ipc3/dai.c:318:61: error: 'struct dai_data' has no member named 'chan'
  318 |                 dma_release_channel(dd->chan->dma->z_dev, dd->chan->index);
      |                                                             ^~
/__w/sof/sof/sof/src/ipc/ipc3/dai.c:322:19: error: 'struct dai_data' has no member named 'chan'
  322 |                 dd->chan->dev_data = NULL;
      |                   ^~
/__w/sof/sof/sof/src/ipc/ipc3/dai.c:323:19: error: 'struct dai_data' has no member named 'chan'
  323 |                 dd->chan = NULL;
      |                   ^~
/__w/sof/sof/sof/src/ipc/ipc3/dai.c: In function 'dai_config':
/__w/sof/sof/sof/src/ipc/ipc3/dai.c:355:23: error: 'struct dai_data' has no member named 'chan'
  355 |                 if (dd->chan) {
      |                       ^~
In file included from /__w/sof/sof/zephyr/include/zephyr/logging/log.h:11,
                 from /__w/sof/sof/sof/zephyr/include/sof/trace/trace.h:10,
                 from /__w/sof/sof/sof/zephyr/include/rtos/alloc.h:12,
                 from /__w/sof/sof/sof/zephyr/include/sof/lib/dma.h:23,
                 from /__w/sof/sof/sof/src/include/sof/audio/audio_stream.h:23,
                 from /__w/sof/sof/sof/src/include/sof/audio/buffer.h:11,
                 from /__w/sof/sof/sof/src/include/sof/audio/component.h:19,
                 from /__w/sof/sof/sof/src/include/sof/audio/component_ext.h:19,
                 from /__w/sof/sof/sof/src/ipc/ipc3/dai.c:8:
/__w/sof/sof/sof/src/ipc/ipc3/dai.c:357:37: error: 'struct dai_data' has no member named 'chan'
  357 |                                   dd->chan->index);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants