nxp: Add SAI driver with the DAI interface

[LaurentiuM1234]

For now, this PR shall remain a draft until all NXP HAL PRs are ready.

An example DTS node can be found below (i.MX93, SOF usage):

sai3: dai@42660000 {
    compatible = "nxp,dai-sai";
    reg = <0x42660000 DT_SIZE_K(64)>;
    mclk-is-output;
    clocks = <&ccm IMX_CCM_SAI3_CLK 0x0 0x0>;
    clock-names = "mclk1";
    rx-fifo-watermark = <96>;
    tx-fifo-watermark = <1>;
    fifo-depth = <96>;
    interrupt-parent = <&gic>;
    interrupts = <GIC_SPI 171 IRQ_TYPE_LEVEL IRQ_DEFAULT_PRIORITY>;
 };

The additional configurations required to get the driver working are (i.MX93):

CONFIG_DAI=y
CONFIG_SAI_HAS_MCLK_CONFIG_OPTION=y
CONFIG_SAI_HAS_XCR4_CHMOD=y

Tagging @dbaluta and @iuliana-prodan for initial comments.

[zephyrbot]

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_nxp	zephyrproject-rtos/hal_nxp@2a294b5	zephyrproject-rtos/hal_nxp@ed3efff (master)	zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

[danieldegrasse]

A very broad comment- based on the dai_protocol enum, it sort of looks like these formats are just the same as those we support in I2S today: https://docs.zephyrproject.org/latest/hardware/peripherals/audio/i2s.html. Given that, is there any reason this DAI driver could not be written to use the I2S API itself, instead of the SAI peripheral? It seems like the DAI API is intended as a superset of the I2S API, but maybe I'm missing some advantage here.

[danieldegrasse]

If we'd use I2S interface here, SOF application code would require a target/hw based separate code path to talk to I2S driver interface in case of this NXP target, and this is what we wanted to avoid

Absolutely- I don't think we should change SOF application code. What I'm wondering is if it would be possible to create a DAI driver (say dai_i2s.c) that implements the DAI interface by calling the I2S APIs, essentially as a wrapper layer. This way there is no change needed to the SOF application layer, and we reuse the existing code in I2S drivers.

[dbaluta]

Absolutely- I don't think we should change SOF application code. What I'm wondering is if it would be possible to create a DAI driver (say dai_i2s.c) that implements the DAI interface by calling the I2S APIs, essentially as a wrapper layer. This way there is no change needed to the SOF application layer, and we reuse the existing code in I2S drivers.

Although an interesting idea I think that would be an overkill. Sometimes there will be functions that could be imported from i2s but sometimes not. So, I think we complicate things unnecessary. Maybe after we get this in and have it in a stable form we could reduce code duplication by looking at this.

[danieldegrasse]

Although an interesting idea I think that would be an overkill. Sometimes there will be functions that could be imported from i2s but sometimes not. So, I think we complicate things unnecessary. Maybe after we get this in and have it in a stable form we could reduce code duplication by looking at this.

Understood- you're the expert here, this was just a passing thought :) If it doesn't make sense than no worries

[LaurentiuM1234]

V2 updates

1. sai_trigger() now only allows COPY and PRE_START operations (apart from those which are already supported and translate to a trigger function). The rest of the trigger commands will be treated as errors. If there's ever such a trigger causing an error we're going to have to decide between adding support for it or marking it as success if the SAI doesn't need to implement it.
2. Fixed typo inside SAI_TX_RX_SW_RESET().
3. CONFIG_SAI_HAS_XCR4_CHMOD has been entirely removed and replaced by FSL_FEATURE_SAI_HAS_CHANNEL_MODE.
4. Removed comment regarding READY->READY state transition (a.k.a doing a config_set after a config_set). This transition should be allowed since re-configuring the SAI immediately after a configuration isn't illogical and won't break anything. As far as I can tell, the double set_config() call from SOF still happens. Not sure why (perhaps it's done for every direction?)
5. Added explanation about why we're going to keep data->cfg.rate = bespoke->fsync_rate; and where it's used.
6. sai_tx_rx_disable() now uses WAIT_FOR(). The timeout value has been determined empirically. The core problem (the fac that we shouldn't be doing busy waiting) wasn't solved.

Hopefully I didn't miss anything

[LaurentiuM1234]

V3 updates

1. Moved initialization to POST_KERNEL and removed stage check from sai_tx_rx_disable().
2. SAI-related files are now found under dai/nxp/sai.

[dbaluta]

Looks good to me. Now we need to merge dependencies and then we are good to go.

Thanks @LaurentiuM1234 !

[LaurentiuM1234]

V4 updates

1. The number of channels from struct dai_config, kept inside struct sai_data is now set to the number of TDM slots via sai_config_set(). See comment from line 374 (in sai.c) for explanation.
2. Removed the cfg->channels > bespoke->tdm_slots check from sai_config_set(). The number of channels is set via bespoke configuration's tdm_slots field. Also, SOF doesn't set cfg->channels. Instead, it expects the DAI driver to do so. This means that cfg->channels is 0 so the check is useless.

[LaurentiuM1234]

@kv2019i @lgirdwood could you please take a look at the changes to dai.h? Would like to have at least one approval from Intel's side on this. Thanks!

[dbaluta]

All good!

@lgirdwood @kv2019i can you please review commit modifying dai.h common part: 7a9401d