-
Notifications
You must be signed in to change notification settings - Fork 322
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.
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
basefw: Add handling of IPC4_DMA_CONTROL messages #9156
basefw: Add handling of IPC4_DMA_CONTROL messages #9156
Conversation
cc6a12c
to
0124d88
Compare
8b66f23
to
d03015e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the comments! Formally the PR looks good to me now, although I still don't know why exactly this is needed - "improving the flexibility" doesn't tell me much, sorry :-) Would be nice to know what specific use-cases this is addressing.
The implementation of the IPC4_DMA_CONTROL message handling is a requirement for SOF to align with the IPC4 interface specifications, which are part of the convergence effort with Windows audio infrastructure. This specific IPC message allows for dynamic configuration of DMA gateways, which is essential for use-cases where audio streams need to be routed between various components in a flexible manner, adapting to changes in audio topologies at runtime. For instance, this capability is crucial for scenarios where audio endpoints can be added or removed dynamically, or where stream parameters may change on-the-fly without restarting the system or audio pipeline. It enables SOF to handle complex audio routing scenarios that are common in modern Windows-based systems, ensuring compatibility and feature parity with the Windows audio stack. @lyakh Please also take a look at the changes on the Zephyr side that are required for this PR. |
d03015e
to
5c45e51
Compare
@tmleman any update on ETA ? Do you have the Zephyr update now merged ? |
@tmleman I guess we are blocked until after Zephyr merge Window ? |
@lgirdwood Yes, this change is blocked until the release of Zephyr LTS. |
5c45e51
to
913972e
Compare
ok, tagged for v2.11 to remind everyone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use IPC4 error codes
913972e
to
3801123
Compare
src/audio/base_fw.c
Outdated
} | ||
|
||
dma_control = (struct ipc4_dma_control *)data; | ||
if (dma_control->config_length > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my perspective this check is wrong and should be removed or modified to
if (!dma_control->config_length)
.
config_length
cannot be zero if DMA Control IPC has config data and contains the size of such data in number of dw. At least it is true for DMIC and UAOL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmleman can you comment, is the config_length used or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And from my perspective, it was correct (however, I focused only on SSP during the implementation). I looked at how it looks in the reference fw and changed the logic in the IF. Now the error is returned when config_length
is greater than the actual data size. The condition you proposed will not work for SSP because in all cases I was able to check, this parameter is equal to zero, which is consistent with the description in the documentation.
3801123
to
1d8d32f
Compare
Rebase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but lets get alignment with @iganakov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. If you can conclude the one discussion with @iganakov w.r.t. dma_control->config_length, this is good to go.
src/audio/base_fw.c
Outdated
} | ||
|
||
dma_control = (struct ipc4_dma_control *)data; | ||
if (dma_control->config_length > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmleman can you comment, is the config_length used or not?
0673918
to
8359eb5
Compare
8359eb5
to
3fa5277
Compare
This patch exposes the function to retrieve a pointer to the Zephyr device structure for a DAI of a given type and index. Previously, the function `dai_get_zephyr_device` was static and only usable within `dai.c`. By introducing `dai_get_device`, other parts of the SOF codebase can now access the Zephyr DAI device pointers, facilitating integration with Zephyr native DAI drivers. Signed-off-by: Tomasz Leman <[email protected]>
This patch introduces handling for the IPC4_DMA_CONTROL message type in the base firmware. The implementation includes a new function `basefw_vendor_dma_control` to process the DMA Control configuration for any DAI type. The `basefw_dma_control` function has been added to handle the IPC4_DMA_CONTROL message. It ensures the message is atomic and contains all necessary information before casting the data buffer to the `ipc4_dma_control` structure and processing it. The function also calls `basefw_vendor_dma_control` to apply the DMA Control configuration to the hardware. The `basefw_set_large_config` function in `src/audio/base_fw.c` has been updated to call `basefw_dma_control` when an IPC4_DMA_CONTROL message is received. If the `dai_config_update` operation is not implemented by the DAI driver, the function will return `-ENOSYS`. This change allows the base firmware to initialize or modify DMA gateway configurations dynamically, improving the flexibility of DMA management in response to IPC messages. Signed-off-by: Tomasz Leman <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Ready to go, but we need a clean pass of mandatory tests. |
SOFCI TEST |
This pull request introduces a handling of IPC4_DMA_CONTROL messages for the SSP DAI driver.
Below is a summary of the commits included in this pull request:
TODO before merge: