Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

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 <rmk+kernel@armlinux.org.uk> 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 <rmk+kernel@armlinux.org.uk>
> > ---
> 
> @@ -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, &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