[PATCH] ASoC: rockchip: rockchip_sai: Set slot width for non-TDM mode

[PATCH] ASoC: rockchip: rockchip_sai: Set slot width for non-TDM mode

[Alexey Charkov]

Currently the slot width in non-TDM mode is always kept at the POR value
of 32 bits, regardless of the sample width, which doesn't work well for
some codecs such as NAU8822.

Set the slot width according to the sample width in non-TDM mode, which
is what other CPU DAI drivers do.

Tested on the following RK3576 configurations:
- SAI2 + NAU8822 (codec as the clock master), custom board
- SAI1 + ES8388 (codec as the clock master), RK3576 EVB1
- SAI2 + RT5616 (SAI as the clock master), FriendlyElec NanoPi M5

NAU8822 didn't work prior to this patch but works after the patch. Other
two configurations work both before and after the patch.

Fixes: cc78d1eaabad ("ASoC: rockchip: add Serial Audio Interface (SAI) driver")
Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 sound/soc/rockchip/rockchip_sai.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/rockchip/rockchip_sai.c b/sound/soc/rockchip/rockchip_sai.c
index 1bf614dbdf4d..ed393e5034a4 100644
--- a/sound/soc/rockchip/rockchip_sai.c
+++ b/sound/soc/rockchip/rockchip_sai.c
@@ -628,6 +628,10 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream,
 
 	regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val);
 
+	if (!sai->is_tdm)
+		regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK,
+				   SAI_XCR_SBW(params_physical_width(params)));
+
 	regmap_read(sai->regmap, reg, &val);
 
 	slot_width = SAI_XCR_SBW_V(val);

---
base-commit: 8e5a478b6d6a5bb0a3d52147862b15e4d826af19
change-id: 20260318-sai-slot-width-378eed5c22cd

Best regards,
-- 
Alexey Charkov <alchark@flipper.net>

Re: [PATCH] ASoC: rockchip: rockchip_sai: Set slot width for non-TDM mode

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

On Wed, Mar 18, 2026 at 06:50:25PM +0400, Alexey Charkov wrote:

>  	regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val);
>  
> +	if (!sai->is_tdm)
> +		regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK,
> +				   SAI_XCR_SBW(params_physical_width(params)));
> +

What happens if playback and capture have different widths?  I wonder if
forcing a TDM configuration with the affected CODECs would be easier.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: rockchip_sai: Set slot width for non-TDM mode

[Alexey Charkov]

On Wed, Mar 18, 2026 at 8:17 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Mar 18, 2026 at 06:50:25PM +0400, Alexey Charkov wrote:
>
> >       regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val);
> >
> > +     if (!sai->is_tdm)
> > +             regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK,
> > +                                SAI_XCR_SBW(params_physical_width(params)));
> > +
>
> What happens if playback and capture have different widths?  I wonder if
> forcing a TDM configuration with the affected CODECs would be easier.

Playback and capture go to different "reg" each, and each has a slot
width field. The correct "reg" for the stream is selected 50 lines up
in the same function. So they shouldn't really get in each other's
way, from what I can tell.

Best regards,
Alexey

Re: [PATCH] ASoC: rockchip: rockchip_sai: Set slot width for non-TDM mode

[Nicolas Frattaroli]

On Wednesday, 18 March 2026 15:50:25 Central European Standard Time Alexey Charkov wrote:
> Currently the slot width in non-TDM mode is always kept at the POR value
> of 32 bits, regardless of the sample width, which doesn't work well for
> some codecs such as NAU8822.
> 
> Set the slot width according to the sample width in non-TDM mode, which
> is what other CPU DAI drivers do.
> 
> Tested on the following RK3576 configurations:
> - SAI2 + NAU8822 (codec as the clock master), custom board
> - SAI1 + ES8388 (codec as the clock master), RK3576 EVB1
> - SAI2 + RT5616 (SAI as the clock master), FriendlyElec NanoPi M5
> 
> NAU8822 didn't work prior to this patch but works after the patch. Other
> two configurations work both before and after the patch.
> 
> Fixes: cc78d1eaabad ("ASoC: rockchip: add Serial Audio Interface (SAI) driver")
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> ---
>  sound/soc/rockchip/rockchip_sai.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/soc/rockchip/rockchip_sai.c b/sound/soc/rockchip/rockchip_sai.c
> index 1bf614dbdf4d..ed393e5034a4 100644
> --- a/sound/soc/rockchip/rockchip_sai.c
> +++ b/sound/soc/rockchip/rockchip_sai.c
> @@ -628,6 +628,10 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream,
>  
>  	regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val);
>  
> +	if (!sai->is_tdm)
> +		regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK,
> +				   SAI_XCR_SBW(params_physical_width(params)));
> +
>  	regmap_read(sai->regmap, reg, &val);
>  
>  	slot_width = SAI_XCR_SBW_V(val);
> 
> ---
> base-commit: 8e5a478b6d6a5bb0a3d52147862b15e4d826af19
> change-id: 20260318-sai-slot-width-378eed5c22cd
> 
> Best regards,
> 

Thanks for the patch! Looking at where else this reg mask is used, I
got curious about `rockchip_sai_set_tdm_slot`. It seems to reset
SAI_XCR_SBW back to 32 when something calls it with 0 slots. Do we
have to adjust this as well there?

I think what we're running into here is the difference between I2S
normal mode, and I2S justified mode. In justified modes, it'd be
normal to have a slot bit width of 32 but a valid data width of, say,
16. In that case, VDJ would specify whether something is left or right
justified as far as I can tell.

So basically I'm wondering if we've accidentally been sending
I2S left-justified data all along when using SND_SOC_DAIFMT_I2S,
and whether we should only be setting this in SND_SOC_DAIFMT_I2S.

Either way, I can give this a

Tested-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

for ES8388 on my ROCK 4D (with my enablement patches for it there
on top that I need to get back around to).

Kind regards,
Nicolas Frattaroli

Re: [PATCH] ASoC: rockchip: rockchip_sai: Set slot width for non-TDM mode

[Alexey Charkov]

On Wed, Mar 18, 2026 at 11:56 PM Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
>
> On Wednesday, 18 March 2026 15:50:25 Central European Standard Time Alexey Charkov wrote:
> > Currently the slot width in non-TDM mode is always kept at the POR value
> > of 32 bits, regardless of the sample width, which doesn't work well for
> > some codecs such as NAU8822.
> >
> > Set the slot width according to the sample width in non-TDM mode, which
> > is what other CPU DAI drivers do.
> >
> > Tested on the following RK3576 configurations:
> > - SAI2 + NAU8822 (codec as the clock master), custom board
> > - SAI1 + ES8388 (codec as the clock master), RK3576 EVB1
> > - SAI2 + RT5616 (SAI as the clock master), FriendlyElec NanoPi M5
> >
> > NAU8822 didn't work prior to this patch but works after the patch. Other
> > two configurations work both before and after the patch.
> >
> > Fixes: cc78d1eaabad ("ASoC: rockchip: add Serial Audio Interface (SAI) driver")
> > Signed-off-by: Alexey Charkov <alchark@flipper.net>
> > ---
> >  sound/soc/rockchip/rockchip_sai.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/sound/soc/rockchip/rockchip_sai.c b/sound/soc/rockchip/rockchip_sai.c
> > index 1bf614dbdf4d..ed393e5034a4 100644
> > --- a/sound/soc/rockchip/rockchip_sai.c
> > +++ b/sound/soc/rockchip/rockchip_sai.c
> > @@ -628,6 +628,10 @@ static int rockchip_sai_hw_params(struct snd_pcm_substream *substream,
> >
> >       regmap_update_bits(sai->regmap, reg, SAI_XCR_VDW_MASK | SAI_XCR_CSR_MASK, val);
> >
> > +     if (!sai->is_tdm)
> > +             regmap_update_bits(sai->regmap, reg, SAI_XCR_SBW_MASK,
> > +                                SAI_XCR_SBW(params_physical_width(params)));
> > +
> >       regmap_read(sai->regmap, reg, &val);
> >
> >       slot_width = SAI_XCR_SBW_V(val);
> >
> > ---
> > base-commit: 8e5a478b6d6a5bb0a3d52147862b15e4d826af19
> > change-id: 20260318-sai-slot-width-378eed5c22cd
> >
> > Best regards,
> >
>
> Thanks for the patch! Looking at where else this reg mask is used, I
> got curious about `rockchip_sai_set_tdm_slot`. It seems to reset
> SAI_XCR_SBW back to 32 when something calls it with 0 slots. Do we
> have to adjust this as well there?

Hmm, from what I'm understanding set_tdm_slot is called before
hw_params - e.g. the Freescale SAI driver doesn't even program the
slot width to the hardware from set_tdm_slot and only updates the
requested value in its internal data structure - to be then written
out when hw_params runs. So I believe it should work as-is, although I
haven't seen any TDM users of the Rockchip SAI in the wild to verify
:)

> I think what we're running into here is the difference between I2S
> normal mode, and I2S justified mode. In justified modes, it'd be
> normal to have a slot bit width of 32 but a valid data width of, say,
> 16. In that case, VDJ would specify whether something is left or right
> justified as far as I can tell.
>
> So basically I'm wondering if we've accidentally been sending
> I2S left-justified data all along when using SND_SOC_DAIFMT_I2S,
> and whether we should only be setting this in SND_SOC_DAIFMT_I2S.

Isn't the difference between SND_SOC_DAIFMT_I2S vs.
SND_SOC_DAIFMT_LEFT_J just about clock timings / edges? I thought both
can have slot width >= sample width, but I could be wrong (not an
audio guru here).

> Either way, I can give this a
>
> Tested-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>
> for ES8388 on my ROCK 4D (with my enablement patches for it there
> on top that I need to get back around to).

Thanks a lot!

Best regards,
Alexey