audio: dai-zephyr: rework calls to DMA driver, remove channel pointer#10767
audio: dai-zephyr: rework calls to DMA driver, remove channel pointer#10767kv2019i wants to merge 1 commit into
Conversation
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>
|
Similar to previously done change for host-zephyr.c in #10721 |
There was a problem hiding this comment.
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::chanwithchan_indexand updated call sites accordingly. - Updated DAI DMA stop/release and position/status queries to use
chan_index. - Removed per-channel
dev_datahandling 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. |
| @@ -527,7 +527,6 @@ __cold int dai_common_new(struct dai_data *dd, struct comp_dev *dev, | |||
|
|
|||
| dma_sg_init(&dd->config.elem_array); | |||
| /* get DMA channel */ | ||
| channel = sof_dma_request_channel(dd->dma, channel); | ||
| if (channel < 0) { | ||
| dd->chan_index = sof_dma_request_channel(dd->dma, channel); | ||
| if (dd->chan_index < 0) { | ||
| comp_err(dev, "dma_request_channel() failed"); | ||
| dd->chan = NULL; | ||
| return -EIO; | ||
| } |
| if (dd->chan_index != -EINVAL) | ||
| sof_dma_release_channel(dd->dma, dd->chan_index); |
| /* put the allocated DMA channel first */ | ||
| if (dd->chan) { | ||
| if (dd->chan_index != -EINVAL) { | ||
| struct ipc4_llp_reading_slot slot; |
| #if CONFIG_ZEPHYR_NATIVE_DRIVERS | ||
| /* if reset is after pause dma has already been stopped */ | ||
| dma_stop(dd->chan->dma->z_dev, dd->chan->index); | ||
| dma_stop(dd->dma->z_dev, dd->chan_index); | ||
|
|
||
| dma_release_channel(dd->chan->dma->z_dev, dd->chan->index); | ||
| dma_release_channel(dd->dma->z_dev, dd->chan_index); | ||
| #else |
| dma_release_channel(dd->chan->dma->z_dev, dd->chan->index); | ||
| dma_release_channel(dd->dma->z_dev, dd->chan_index); | ||
| #else | ||
| /* TODO: to remove this, no longer works! */ |
| if (dd->chan_index != -EINVAL) { | ||
| comp_info(dev, "Configured. dma channel index %d, ignore...", | ||
| dd->chan->index); | ||
| dd->chan_index); | ||
| return 0; |
|
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 |
Ack - and I think historically 0 is also a valid channel number used in the HW DMA specs so negative better. |
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.