Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio
On Wed, Jun 12, 2019 at 12:42 PM Russell King - ARM Linux admin wrote: > > I think you're confusing tda998x_derive_cts_n() and tda998x_get_adiv(). > tda998x_derive_cts_n() only returns 0 or -EINVAL. > True. Apologies for the confusion. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio
On Wed, Jun 12, 2019 at 12:28 PM Russell King - ARM Linux admin wrote: > > The platform data path has never supported the HDMI codec way of doing > things, so that really isn't a concern here. The platform data way > was sufficient to allow SPDIF streams to work with a static setup of > the TDA998x, and has never been intended to support anything beyond > that. Thank you, I am not using the platform data path, so I had no idea. Wouldn't the current code always fail on the platform data path though? create() calls tda998x_set_config() in the platform data case. If the audio_params.format is used (i.e. the platform data configures audio), the function then returns the return value of tda998x_derive_cts_n(), which is a positive divider (can be 0 if /1). Then in create(): if (ret) goto fail; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio
On Tue, Jun 11, 2019 at 7:02 AM Russell King wrote:
>
> The TDA998x derives the CTS value using the supplied I2S bit clock
> (ACLK, in TDA998x parlence) rather than 128·fs. TDA998x uses two
> constants named m and k in the CTS generator such that we have this
> relationship between the I2S source ACLK and the sink fs:
>
> 128·fs_sink = ACLK·m / k
>
> Where ACLK = aclk_ratio·fs_source.
>
> When audio support was originally added, we supported a fixed ratio
> of 64·fs, intending to support the Kirkwood I2S on Dove. However,
> when hdmi-codec support was added, this was changed to scale the
> ratio with the sample width, which would've broken its use with
> Kirkwood I2S.
>
> We are now starting to see other users whose I2S blocks send at 64·fs
> for 16-bit samples, so we need to reinstate the support for the fixed
> ratio I2S bit clock.
>
> This commit takes a step towards supporting these configurations by
> selecting the CTS_N register m and k values based on the bit clock
> ratio. However, as the driver is not given the bit clock ratio from
> ALSA, continue deriving this from the sample width. This will be
> addressed in a later commit.
>
> Signed-off-by: Russell King
> ---
@@ -1657,9 +1701,17 @@ static void tda998x_set_config(struct tda998x_priv *priv,
(p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
if (p->audio_params.format != AFMT_UNUSED) {
+ unsigned int ratio;
+ bool spdif = p->audio_params.format == AFMT_SPDIF;
+
priv->audio.params = p->audio_params;
priv->audio.i2s_format = I2S_FORMAT_PHILIPS;
+
+ ratio = spdif ? 64 : p->audio_params.sample_width * 2;
+ return tda998x_derive_cts_n(priv, >audio, ratio);
Won't this make the platform_data path fail all the time?
Also, the platform_data path doesn't appear to instantiate the HDMI_CODEC,
so tda audio support would be completely missing in this case?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio
On Wed, Jun 12, 2019 at 12:37:57PM -0400, Sven Van Asbroeck wrote: > On Wed, Jun 12, 2019 at 12:28 PM Russell King - ARM Linux admin > wrote: > > > > The platform data path has never supported the HDMI codec way of doing > > things, so that really isn't a concern here. The platform data way > > was sufficient to allow SPDIF streams to work with a static setup of > > the TDA998x, and has never been intended to support anything beyond > > that. > > Thank you, I am not using the platform data path, so I had no idea. > > Wouldn't the current code always fail on the platform data path though? > > create() calls tda998x_set_config() in the platform data case. > If the audio_params.format is used (i.e. the platform data configures > audio), the function then returns the return value of tda998x_derive_cts_n(), > which is a positive divider (can be 0 if /1). I think you're confusing tda998x_derive_cts_n() and tda998x_get_adiv(). tda998x_derive_cts_n() only returns 0 or -EINVAL. > > Then in create(): if (ret) goto fail; > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio
On Wed, Jun 12, 2019 at 11:27:16AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:02 AM Russell King
> wrote:
> >
> > The TDA998x derives the CTS value using the supplied I2S bit clock
> > (ACLK, in TDA998x parlence) rather than 128·fs. TDA998x uses two
> > constants named m and k in the CTS generator such that we have this
> > relationship between the I2S source ACLK and the sink fs:
> >
> > 128·fs_sink = ACLK·m / k
> >
> > Where ACLK = aclk_ratio·fs_source.
> >
> > When audio support was originally added, we supported a fixed ratio
> > of 64·fs, intending to support the Kirkwood I2S on Dove. However,
> > when hdmi-codec support was added, this was changed to scale the
> > ratio with the sample width, which would've broken its use with
> > Kirkwood I2S.
> >
> > We are now starting to see other users whose I2S blocks send at 64·fs
> > for 16-bit samples, so we need to reinstate the support for the fixed
> > ratio I2S bit clock.
> >
> > This commit takes a step towards supporting these configurations by
> > selecting the CTS_N register m and k values based on the bit clock
> > ratio. However, as the driver is not given the bit clock ratio from
> > ALSA, continue deriving this from the sample width. This will be
> > addressed in a later commit.
> >
> > Signed-off-by: Russell King
> > ---
>
> @@ -1657,9 +1701,17 @@ static void tda998x_set_config(struct tda998x_priv
> *priv,
> (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
>
> if (p->audio_params.format != AFMT_UNUSED) {
> + unsigned int ratio;
> + bool spdif = p->audio_params.format == AFMT_SPDIF;
> +
> priv->audio.params = p->audio_params;
> priv->audio.i2s_format = I2S_FORMAT_PHILIPS;
> +
> + ratio = spdif ? 64 : p->audio_params.sample_width * 2;
> + return tda998x_derive_cts_n(priv, >audio, ratio);
>
> Won't this make the platform_data path fail all the time?
> Also, the platform_data path doesn't appear to instantiate the HDMI_CODEC,
> so tda audio support would be completely missing in this case?
The platform data path has never supported the HDMI codec way of doing
things, so that really isn't a concern here. The platform data way
was sufficient to allow SPDIF streams to work with a static setup of
the TDA998x, and has never been intended to support anything beyond
that.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio
The TDA998x derives the CTS value using the supplied I2S bit clock
(ACLK, in TDA998x parlence) rather than 128·fs. TDA998x uses two
constants named m and k in the CTS generator such that we have this
relationship between the I2S source ACLK and the sink fs:
128·fs_sink = ACLK·m / k
Where ACLK = aclk_ratio·fs_source.
When audio support was originally added, we supported a fixed ratio
of 64·fs, intending to support the Kirkwood I2S on Dove. However,
when hdmi-codec support was added, this was changed to scale the
ratio with the sample width, which would've broken its use with
Kirkwood I2S.
We are now starting to see other users whose I2S blocks send at 64·fs
for 16-bit samples, so we need to reinstate the support for the fixed
ratio I2S bit clock.
This commit takes a step towards supporting these configurations by
selecting the CTS_N register m and k values based on the bit clock
ratio. However, as the driver is not given the bit clock ratio from
ALSA, continue deriving this from the sample width. This will be
addressed in a later commit.
Signed-off-by: Russell King
---
drivers/gpu/drm/i2c/tda998x_drv.c | 94 ++-
1 file changed, 74 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 78a2b815a8de..bf75414a8c63 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -43,6 +43,7 @@ struct tda998x_audio_port {
struct tda998x_audio_settings {
struct tda998x_audio_params params;
u8 i2s_format;
+ u8 cts_n;
};
struct tda998x_priv {
@@ -891,6 +892,58 @@ static u8 tda998x_get_adiv(struct tda998x_priv *priv,
unsigned int fs)
return adiv;
}
+/*
+ * In auto-CTS mode, the TDA998x uses a "measured time stamp" counter to
+ * generate the CTS value. It appears that the "measured time stamp" is
+ * the number of TDMS clock cycles within a number of audio input clock
+ * cycles defined by the k and N parameters defined below, in a similar
+ * way to that which is set out in the CTS generation in the HDMI spec.
+ *
+ * tmdsclk > mts -> /m ---> CTS
+ * ^
+ * sclk -> /k -> /N
+ *
+ * CTS = mts / m, where m is 2^M.
+ * /k is a divider based on the K value below, K+1 for K < 4, or 8 for K >= 4
+ * /N is a divider based on the HDMI specified N value.
+ *
+ * This produces the following equation:
+ * CTS = tmds_clock * k * N / (sclk * m)
+ *
+ * When combined with the sink-side equation, and realising that sclk is
+ * bclk_ratio * fs, we end up with:
+ * k = m * bclk_ratio / 128.
+ *
+ * Note: S/PDIF always uses a bclk_ratio of 64.
+ */
+static int tda998x_derive_cts_n(struct tda998x_priv *priv,
+ struct tda998x_audio_settings *settings,
+ unsigned int ratio)
+{
+ switch (ratio) {
+ case 16:
+ settings->cts_n = CTS_N_M(3) | CTS_N_K(0);
+ break;
+ case 32:
+ settings->cts_n = CTS_N_M(3) | CTS_N_K(1);
+ break;
+ case 48:
+ settings->cts_n = CTS_N_M(3) | CTS_N_K(2);
+ break;
+ case 64:
+ settings->cts_n = CTS_N_M(3) | CTS_N_K(3);
+ break;
+ case 128:
+ settings->cts_n = CTS_N_M(0) | CTS_N_K(0);
+ break;
+ default:
+ dev_err(>hdmi->dev, "unsupported bclk ratio %ufs\n",
+ ratio);
+ return -EINVAL;
+ }
+ return 0;
+}
+
static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
{
if (on) {
@@ -905,7 +958,7 @@ static void tda998x_audio_mute(struct tda998x_priv *priv,
bool on)
static int tda998x_configure_audio(struct tda998x_priv *priv,
const struct tda998x_audio_settings *settings)
{
- u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv;
+ u8 buf[6], clksel_aip, clksel_fs, adiv;
u32 n;
adiv = tda998x_get_adiv(priv, settings->params.sample_rate);
@@ -920,7 +973,6 @@ static int tda998x_configure_audio(struct tda998x_priv
*priv,
reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF);
clksel_aip = AIP_CLKSEL_AIP_SPDIF;
clksel_fs = AIP_CLKSEL_FS_FS64SPDIF;
- cts_n = CTS_N_M(3) | CTS_N_K(3);
break;
case AFMT_I2S:
@@ -928,20 +980,6 @@ static int 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 (settings->params.sample_width) {
- case 16:
- cts_n = CTS_N_M(3) | CTS_N_K(1);
- break;
- case 18:
- case 20:
- case 24:
- cts_n = CTS_N_M(3) | CTS_N_K(2);
-
