| Message ID | E1gxILr-0004Mm-WE@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tda998x updates for DAI formats and bclk_ratio | expand |
On 22/02/2019 23:27, Russell King wrote: > It appears that TDA998x derives the CTS value using the supplied I2S > bit clock (BCLK) rather than 128·fs. TDA998x uses two constants named > m and k in the CTS generator such that we have this relationship > between the source BCLK and the sink fs: > > 128·fs_sink = BCLK·m / k > > Where BCLK = bclk_ratio·fs_source. > > We have been lucky up to now that all users have scaled their bclk_ratio > to match the sample width - for example, on Beagle Bone Black, with a > 16-bit sample width, BCLK = 32·fs, which increases to 64·fs for 32-bit > samples. 24-bit samples are sent as 32-bit. > > We are now starting to see users whose I2S blocks send at 64·fs for > 16-bit samples, which means TDA998x now breaks. > > ASoC has a snd_soc_dai_set_bclk_ratio() call available which sets the > ratio of BCLK to the sample rate. Implement support for this. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Works at least on Beaglebone-black. Tested-by: Jyri Sarha <jsarha@ti.com> Reviewed-by: Jyri Sarha <jsarha@ti.com> > --- > drivers/gpu/drm/i2c/tda998x_drv.c | 20 +++++++++++++------- > include/drm/i2c/tda998x.h | 1 + > 2 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c > index 645d884fb9e8..f2d40235acf9 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -895,21 +895,26 @@ tda998x_configure_audio(struct tda998x_priv *priv, > reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S); > clksel_aip = AIP_CLKSEL_AIP_I2S; > clksel_fs = AIP_CLKSEL_FS_ACLK; > - switch (params->sample_width) { > + switch (params->bclk_ratio) { > case 16: > + cts_n = CTS_N_M(3) | CTS_N_K(0); > + break; > + case 32: > cts_n = CTS_N_M(3) | CTS_N_K(1); > break; > - case 18: > - case 20: > - case 24: > + case 48: > cts_n = CTS_N_M(3) | CTS_N_K(2); > break; > - default: > - case 32: > + case 64: > cts_n = CTS_N_M(3) | CTS_N_K(3); > break; > + case 128: > + cts_n = CTS_N_M(0) | CTS_N_K(0); > + break; > + default: > + dev_err(&priv->hdmi->dev, "unsupported I2S bclk ratio\n"); > + return -EINVAL; > } > - > switch (params->format & AFMT_I2S_MASK) { > case AFMT_I2S_LEFT_J: > i2s_fmt = I2S_FORMAT_LEFT_J; > @@ -997,6 +1002,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, > struct tda998x_priv *priv = dev_get_drvdata(dev); > int i, ret; > struct tda998x_audio_params audio = { > + .bclk_ratio = daifmt->bclk_ratio, > .sample_width = params->sample_width, > .sample_rate = params->sample_rate, > .cea = params->cea, > diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h > index b0864f0be017..4e0f0cd2d428 100644 > --- a/include/drm/i2c/tda998x.h > +++ b/include/drm/i2c/tda998x.h > @@ -19,6 +19,7 @@ enum { > struct tda998x_audio_params { > u8 config; > u8 format; > + u8 bclk_ratio; > unsigned sample_width; > unsigned sample_rate; > struct hdmi_audio_infoframe cea; >
On Fri, Feb 22, 2019 at 4:27 PM Russell King <rmk+kernel@armlinux.org.uk> wrote: > > It appears that TDA998x derives the CTS value using the supplied I2S > bit clock (BCLK) rather than 128·fs. TDA998x uses two constants named > m and k in the CTS generator such that we have this relationship > between the source BCLK and the sink fs: > > 128·fs_sink = BCLK·m / k > > Where BCLK = bclk_ratio·fs_source. > > We have been lucky up to now that all users have scaled their bclk_ratio > to match the sample width - for example, on Beagle Bone Black, with a > 16-bit sample width, BCLK = 32·fs, which increases to 64·fs for 32-bit > samples. 24-bit samples are sent as 32-bit. > > We are now starting to see users whose I2S blocks send at 64·fs for > 16-bit samples, which means TDA998x now breaks. > > ASoC has a snd_soc_dai_set_bclk_ratio() call available which sets the > ratio of BCLK to the sample rate. Implement support for this. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Works with an imx6q ssi, but only if the card driver calls set_bclk_ratio() with the correct value, which is 32/channel for the fsl_ssi in master mode. I will post an RFC patch shortly which adds bclk-ratio support to simple-card. Tested-by: Sven Van Asbroeck <TheSven73@gmail.com>
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 645d884fb9e8..f2d40235acf9 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -895,21 +895,26 @@ tda998x_configure_audio(struct tda998x_priv *priv, reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S); clksel_aip = AIP_CLKSEL_AIP_I2S; clksel_fs = AIP_CLKSEL_FS_ACLK; - switch (params->sample_width) { + switch (params->bclk_ratio) { case 16: + cts_n = CTS_N_M(3) | CTS_N_K(0); + break; + case 32: cts_n = CTS_N_M(3) | CTS_N_K(1); break; - case 18: - case 20: - case 24: + case 48: cts_n = CTS_N_M(3) | CTS_N_K(2); break; - default: - case 32: + case 64: cts_n = CTS_N_M(3) | CTS_N_K(3); break; + case 128: + cts_n = CTS_N_M(0) | CTS_N_K(0); + break; + default: + dev_err(&priv->hdmi->dev, "unsupported I2S bclk ratio\n"); + return -EINVAL; } - switch (params->format & AFMT_I2S_MASK) { case AFMT_I2S_LEFT_J: i2s_fmt = I2S_FORMAT_LEFT_J; @@ -997,6 +1002,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, struct tda998x_priv *priv = dev_get_drvdata(dev); int i, ret; struct tda998x_audio_params audio = { + .bclk_ratio = daifmt->bclk_ratio, .sample_width = params->sample_width, .sample_rate = params->sample_rate, .cea = params->cea, diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h index b0864f0be017..4e0f0cd2d428 100644 --- a/include/drm/i2c/tda998x.h +++ b/include/drm/i2c/tda998x.h @@ -19,6 +19,7 @@ enum { struct tda998x_audio_params { u8 config; u8 format; + u8 bclk_ratio; unsigned sample_width; unsigned sample_rate; struct hdmi_audio_infoframe cea;
It appears that TDA998x derives the CTS value using the supplied I2S bit clock (BCLK) rather than 128·fs. TDA998x uses two constants named m and k in the CTS generator such that we have this relationship between the source BCLK and the sink fs: 128·fs_sink = BCLK·m / k Where BCLK = bclk_ratio·fs_source. We have been lucky up to now that all users have scaled their bclk_ratio to match the sample width - for example, on Beagle Bone Black, with a 16-bit sample width, BCLK = 32·fs, which increases to 64·fs for 32-bit samples. 24-bit samples are sent as 32-bit. We are now starting to see users whose I2S blocks send at 64·fs for 16-bit samples, which means TDA998x now breaks. ASoC has a snd_soc_dai_set_bclk_ratio() call available which sets the ratio of BCLK to the sample rate. Implement support for this. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/gpu/drm/i2c/tda998x_drv.c | 20 +++++++++++++------- include/drm/i2c/tda998x.h | 1 + 2 files changed, 14 insertions(+), 7 deletions(-)