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
