All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Luca Ceresoli" <luca.ceresoli@bootlin.com>,
	"Detlev Casanova" <detlev.casanova@collabora.com>,
	<linux-kernel@vger.kernel.org>
Cc: "Nicolas Frattaroli" <frattaroli.nicolas@gmail.com>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Jaroslav Kysela" <perex@perex.cz>,
	"Takashi Iwai" <tiwai@suse.com>,
	"Heiko Stuebner" <heiko@sntech.de>,
	<linux-rockchip@lists.infradead.org>,
	<linux-sound@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>, <kernel@collabora.com>,
	<alexandre.belloni@bootlin.com>, <thomas.petazzoni@bootlin.com>
Subject: Re: [REGRESSION] Re: [PATCH v1] ASoC: rockchip: i2s_tdm: Re-add the set_sysclk callback
Date: Wed, 18 Feb 2026 13:19:00 +0100	[thread overview]
Message-ID: <DGI2YVYBECUM.3O0UN9VMKF7FJ@bootlin.com> (raw)
In-Reply-To: <DGB6VK7LC8N7.322SYGWZXPL5W@bootlin.com>

Hello Detlev,

On Tue Feb 10, 2026 at 10:54 AM CET, Luca Ceresoli wrote:
> Hello Detlev,
>
> On Fri Jan 17, 2025 at 5:31 PM CET, Detlev Casanova wrote:
>> In commit
>> 9e2ab4b18ebd ("ASoC: rockchip: i2s-tdm: Fix inaccurate sampling rates"),
>> the set_sysclk callback was removed as considered unused as the mclk rate
>> can be set in the hw_params callback.
>> The difference between hw_params and set_sysclk is that the former is
>> called with the audio sampling rate set in the params (e.g.: 48000 Hz)
>> while the latter is called with a clock rate already computed with
>>   sampling_rate * mclk-fs (e.g.: 48000 * 256)
>>
>> For HDMI audio using the Rockchip I2S TDM driver, the mclk-fs value must
>> be set to 128 instead of the default 256, and that value is set in the
>> device tree at the machine driver level (like a simple-audio-card
>> compatible node).
>> Therefore, the i2s_tdm driver has no idea that another mclk-fs value can
>> be configured and simply computes the mclk rate in the hw_params callback
>> with DEFAULT_MCLK_FS * params_rate(params), which is wrong for HDMI
>> audio.
>>
>> Re-add the set_sysclk callback so that the mclk rate is computed by the
>> machine driver which has the correct mclk-fs value set in its device tree
>> node.
>
> I'm afraid I just found this commit breaks audio capture on the RK3308.
>
> Using 'arecord -Vmono -d 2 -c 8 -f S16_LE -r 96000 /dev/null' I get:
>
>   rockchip-i2s-tdm ff320000.i2s: ASoC error (-22): at snd_soc_dai_hw_params() on ff320000.i2s
>    ff320000.i2s-rk3308-hifi: ASoC error (-22): at __soc_pcm_hw_params() on ff320000.i2s-rk3308-hifi
>
> Tested on:
>
>  * Radxa Rock Pi S
>  * Upstream kernel
>  * arm64 defconfig
>
> Tested kernel versions:
>
>  * v6.12 works
>  * 5323186e2e8d (this commit) fails
>  * 5323186e2e8d^ works
>  * 21cfbeae7d7c (same patch on stable/linux-6.12.y) fails
>  * 21cfbeae7d7c^ works
>  * v6.19 fails
>  * v6.19 + 'git revert 5323186e2e8d' works
>
> I don't have more information at the moment.

Let me add some more info to my previous report.

The function failing is clk_set_rate() in rockchip_i2s_tdm_hw_params().

I added some logging to rockchip_i2s_tdm_hw_params() and here's the output:

  On v6.19-11566-g254edc893f3a (current Linux master)

  At boot:
    [    3.324622] rockchip-i2s-tdm ff320000.i2s: The target mclk_tx freq is: 50176000
    [    3.324720] rockchip-i2s-tdm ff320000.i2s: using zero-initialized flat cache, this may cause unexpected behavior
  When launching arecord:
    [   18.429389] rockchip_i2s_tdm_hw_params:653 i2s_tdm->is_master_mode = 1
    [   18.430154] rockchip_i2s_tdm_hw_params:654 i2s_tdm->mclk_tx clk_i2s2_8ch_tx
    [   18.430815] rockchip_i2s_tdm_hw_params:655 i2s_tdm->mclk_rx clk_i2s2_8ch_rx
    [   18.431465] rockchip_i2s_tdm_hw_params:656 i2s_tdm->mclk_tx_freq 50176000
    [   18.432097] rockchip_i2s_tdm_hw_params:657 i2s_tdm->mclk_rx_freq 0
    [   18.432677] rockchip_i2s_tdm_hw_params:658 i2s_tdm->clk_trcm 0
    [   18.433230] rockchip_i2s_tdm_hw_params:659 substream->stream 1
    [   18.433858] rockchip_i2s_tdm_hw_params:678 mclk clk_i2s2_8ch_rx
    [   18.434423] rockchip_i2s_tdm_hw_params:679 mclk_rate 0
    [   18.434977] rockchip_i2s_tdm_hw_params:682 clk_set_rate(clk_i2s2_8ch_rx, 0) returned -22
    [   18.435743] rockchip-i2s-tdm ff320000.i2s: ASoC error (-22): at snd_soc_dai_hw_params() on ff320000.i2s
    [   18.436699]  ff320000.i2s-rk3308-hifi: ASoC error (-22): at __soc_pcm_hw_params() on ff320000.i2s-rk3308-hifi

  On v6.19-11566-g254edc893f3a + 'git revert 5323186e2e8d'

  At boot:
    [    3.283853] rockchip-i2s-tdm ff320000.i2s: using zero-initialized flat cache, this may cause unexpected behavior
  When launching arecord:
    [   11.952290] rockchip_i2s_tdm_hw_params:631 i2s_tdm->is_master_mode = 1
    [   11.952955] rockchip_i2s_tdm_hw_params:632 i2s_tdm->mclk_tx clk_i2s2_8ch_tx
    [   11.953615] rockchip_i2s_tdm_hw_params:633 i2s_tdm->mclk_rx clk_i2s2_8ch_rx
    [   11.954355] rockchip_i2s_tdm_hw_params:636 i2s_tdm->clk_trcm 0
    [   11.954917] rockchip_i2s_tdm_hw_params:637 substream->stream 1
    [   11.955477] rockchip_i2s_tdm_hw_params:652 mclk clk_i2s2_8ch_rx
    [   11.956048] rockchip_i2s_tdm_hw_params:653 params_rate(params) 96000
    [   11.956769] rockchip_i2s_tdm_hw_params:656 clk_set_rate(clk_i2s2_8ch_rx, 24576000) returned 0

I cannot point to the right fix but the value 0 in i2s_tdm->mclk_rx_freq
appears as a good clue, perhaps it has never been set.

I hope this helps.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


WARNING: multiple messages have this Message-ID (diff)
From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Luca Ceresoli" <luca.ceresoli@bootlin.com>,
	"Detlev Casanova" <detlev.casanova@collabora.com>,
	<linux-kernel@vger.kernel.org>
Cc: "Nicolas Frattaroli" <frattaroli.nicolas@gmail.com>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Jaroslav Kysela" <perex@perex.cz>,
	"Takashi Iwai" <tiwai@suse.com>,
	"Heiko Stuebner" <heiko@sntech.de>,
	<linux-rockchip@lists.infradead.org>,
	<linux-sound@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>, <kernel@collabora.com>,
	<alexandre.belloni@bootlin.com>, <thomas.petazzoni@bootlin.com>
Subject: Re: [REGRESSION] Re: [PATCH v1] ASoC: rockchip: i2s_tdm: Re-add the set_sysclk callback
Date: Wed, 18 Feb 2026 13:19:00 +0100	[thread overview]
Message-ID: <DGI2YVYBECUM.3O0UN9VMKF7FJ@bootlin.com> (raw)
In-Reply-To: <DGB6VK7LC8N7.322SYGWZXPL5W@bootlin.com>

Hello Detlev,

On Tue Feb 10, 2026 at 10:54 AM CET, Luca Ceresoli wrote:
> Hello Detlev,
>
> On Fri Jan 17, 2025 at 5:31 PM CET, Detlev Casanova wrote:
>> In commit
>> 9e2ab4b18ebd ("ASoC: rockchip: i2s-tdm: Fix inaccurate sampling rates"),
>> the set_sysclk callback was removed as considered unused as the mclk rate
>> can be set in the hw_params callback.
>> The difference between hw_params and set_sysclk is that the former is
>> called with the audio sampling rate set in the params (e.g.: 48000 Hz)
>> while the latter is called with a clock rate already computed with
>>   sampling_rate * mclk-fs (e.g.: 48000 * 256)
>>
>> For HDMI audio using the Rockchip I2S TDM driver, the mclk-fs value must
>> be set to 128 instead of the default 256, and that value is set in the
>> device tree at the machine driver level (like a simple-audio-card
>> compatible node).
>> Therefore, the i2s_tdm driver has no idea that another mclk-fs value can
>> be configured and simply computes the mclk rate in the hw_params callback
>> with DEFAULT_MCLK_FS * params_rate(params), which is wrong for HDMI
>> audio.
>>
>> Re-add the set_sysclk callback so that the mclk rate is computed by the
>> machine driver which has the correct mclk-fs value set in its device tree
>> node.
>
> I'm afraid I just found this commit breaks audio capture on the RK3308.
>
> Using 'arecord -Vmono -d 2 -c 8 -f S16_LE -r 96000 /dev/null' I get:
>
>   rockchip-i2s-tdm ff320000.i2s: ASoC error (-22): at snd_soc_dai_hw_params() on ff320000.i2s
>    ff320000.i2s-rk3308-hifi: ASoC error (-22): at __soc_pcm_hw_params() on ff320000.i2s-rk3308-hifi
>
> Tested on:
>
>  * Radxa Rock Pi S
>  * Upstream kernel
>  * arm64 defconfig
>
> Tested kernel versions:
>
>  * v6.12 works
>  * 5323186e2e8d (this commit) fails
>  * 5323186e2e8d^ works
>  * 21cfbeae7d7c (same patch on stable/linux-6.12.y) fails
>  * 21cfbeae7d7c^ works
>  * v6.19 fails
>  * v6.19 + 'git revert 5323186e2e8d' works
>
> I don't have more information at the moment.

Let me add some more info to my previous report.

The function failing is clk_set_rate() in rockchip_i2s_tdm_hw_params().

I added some logging to rockchip_i2s_tdm_hw_params() and here's the output:

  On v6.19-11566-g254edc893f3a (current Linux master)

  At boot:
    [    3.324622] rockchip-i2s-tdm ff320000.i2s: The target mclk_tx freq is: 50176000
    [    3.324720] rockchip-i2s-tdm ff320000.i2s: using zero-initialized flat cache, this may cause unexpected behavior
  When launching arecord:
    [   18.429389] rockchip_i2s_tdm_hw_params:653 i2s_tdm->is_master_mode = 1
    [   18.430154] rockchip_i2s_tdm_hw_params:654 i2s_tdm->mclk_tx clk_i2s2_8ch_tx
    [   18.430815] rockchip_i2s_tdm_hw_params:655 i2s_tdm->mclk_rx clk_i2s2_8ch_rx
    [   18.431465] rockchip_i2s_tdm_hw_params:656 i2s_tdm->mclk_tx_freq 50176000
    [   18.432097] rockchip_i2s_tdm_hw_params:657 i2s_tdm->mclk_rx_freq 0
    [   18.432677] rockchip_i2s_tdm_hw_params:658 i2s_tdm->clk_trcm 0
    [   18.433230] rockchip_i2s_tdm_hw_params:659 substream->stream 1
    [   18.433858] rockchip_i2s_tdm_hw_params:678 mclk clk_i2s2_8ch_rx
    [   18.434423] rockchip_i2s_tdm_hw_params:679 mclk_rate 0
    [   18.434977] rockchip_i2s_tdm_hw_params:682 clk_set_rate(clk_i2s2_8ch_rx, 0) returned -22
    [   18.435743] rockchip-i2s-tdm ff320000.i2s: ASoC error (-22): at snd_soc_dai_hw_params() on ff320000.i2s
    [   18.436699]  ff320000.i2s-rk3308-hifi: ASoC error (-22): at __soc_pcm_hw_params() on ff320000.i2s-rk3308-hifi

  On v6.19-11566-g254edc893f3a + 'git revert 5323186e2e8d'

  At boot:
    [    3.283853] rockchip-i2s-tdm ff320000.i2s: using zero-initialized flat cache, this may cause unexpected behavior
  When launching arecord:
    [   11.952290] rockchip_i2s_tdm_hw_params:631 i2s_tdm->is_master_mode = 1
    [   11.952955] rockchip_i2s_tdm_hw_params:632 i2s_tdm->mclk_tx clk_i2s2_8ch_tx
    [   11.953615] rockchip_i2s_tdm_hw_params:633 i2s_tdm->mclk_rx clk_i2s2_8ch_rx
    [   11.954355] rockchip_i2s_tdm_hw_params:636 i2s_tdm->clk_trcm 0
    [   11.954917] rockchip_i2s_tdm_hw_params:637 substream->stream 1
    [   11.955477] rockchip_i2s_tdm_hw_params:652 mclk clk_i2s2_8ch_rx
    [   11.956048] rockchip_i2s_tdm_hw_params:653 params_rate(params) 96000
    [   11.956769] rockchip_i2s_tdm_hw_params:656 clk_set_rate(clk_i2s2_8ch_rx, 24576000) returned 0

I cannot point to the right fix but the value 0 in i2s_tdm->mclk_rx_freq
appears as a good clue, perhaps it has never been set.

I hope this helps.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  parent reply	other threads:[~2026-02-18 12:19 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-17 16:31 [PATCH v1] ASoC: rockchip: i2s_tdm: Re-add the set_sysclk callback Detlev Casanova
2025-01-17 16:31 ` Detlev Casanova
2025-01-21 18:10 ` Mark Brown
2025-01-21 18:10   ` Mark Brown
2026-02-10  9:54 ` [REGRESSION] " Luca Ceresoli
2026-02-10  9:54   ` Luca Ceresoli
2026-02-17 19:22   ` Mark Brown
2026-02-17 19:22     ` Mark Brown
2026-02-18 12:14     ` Luca Ceresoli
2026-02-18 12:14       ` Luca Ceresoli
2026-02-18 12:19   ` Luca Ceresoli [this message]
2026-02-18 12:19     ` Luca Ceresoli
2026-02-18 16:01     ` Detlev Casanova
2026-02-18 16:01       ` Detlev Casanova
2026-02-18 16:51       ` Luca Ceresoli
2026-02-18 16:51         ` Luca Ceresoli
2026-02-18 17:10         ` Mark Brown
2026-02-18 17:10           ` Mark Brown
2026-02-18 17:17           ` Detlev Casanova
2026-02-18 17:17             ` Detlev Casanova
2026-02-18 17:19             ` Mark Brown
2026-02-18 17:19               ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DGI2YVYBECUM.3O0UN9VMKF7FJ@bootlin.com \
    --to=luca.ceresoli@bootlin.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=broonie@kernel.org \
    --cc=detlev.casanova@collabora.com \
    --cc=frattaroli.nicolas@gmail.com \
    --cc=heiko@sntech.de \
    --cc=kernel@collabora.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.