Re: [PATCH 1/2] drm/i2c: tda998x: adjust CTS_N audio pre-divider calculation

[Sven Van Asbroeck <thesven73@gmail.com>]

On Fri, Feb 22, 2019 at 8:21 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Thu, Feb 21, 2019 at 01:18:13PM -0500, Sven Van Asbroeck wrote:
>
> >       [SNDRV_PCM_FORMAT_S24_LE] = {
> >               .width = 24, .phys = 32, .le = 1, .signd = 1,
> >               .silence = {},
> >       },
>
> The above table describes the memory format, not the wire format.
> Look further down for SNDRV_PCM_FORMAT_S24_3LE, which is 24-bit
> packed into three bytes (see include/uapi/sound/asound.h for
> the comment specifying that.)
>
> ASoC uses DAIFMT to specify the on-wire format in connection with
> the above.
>

Interesting ! So you're saying that currently, nobody strictly defines the
layout of the on-wire format, correct? I'm not sure how this works in practice,
because codec and cpu dai should agree on the on-wire format? Except if the
formats used have enough flexibility so you don't have to care.

If so, we don't seem to have this luxury here :(

>
> This doesn't really help in terms of working out what the correct
> settings should be, and other information I have laying around does not
> provide any further enlightenment.

I have access to the NXP software library shipped with the tda19988.
The library's release notes have the following entry:

  . "I2S audio does not work, CTS value is not good"
    Check the audio I2S format <snip>
    CTS is automatically computed by the TDA accordingly to the audio input
    so accordingly to the upstream settings (like an OMAP ;)
    For example, I2S 16 bits or 32 bits do not produce the same CTS value

The config structure which you need to fill in to init the audio has a
"i2s qualifier" field, where you have the choice between 16 and 32 bits.
This then maps to a "Clock Time Stamp factor x" called CTSX, which maps to
the following CTS_N register settings:

CTSX -> CTS_N (m,k)
-----------------------------------
16 -> (3,0)
32 -> (3,1) (i2s qualifier = 16 bits)
48 -> (3,2)
64 -> (3,3) (i2s qualifier = 32 bits)
128 -> (0,0)

Does this information bring us any closer to our assumption that CTS_N needs
to be calculated off the bclk to sample rate ratio ?

>
> I think what I'd like to see is passing of the Fs value into the driver
> from hdmi-codec, but I suspect that requires a bit of work in multiple
> drivers.
>

I'd love to take a shot at this, but first I'd like to understand what you're
suggesting :)

Currently there is set_bclk_ratio() support, but no-one is actually using it.
If hdmi-codec is to retrieve the ratio, wouldn't we need to add .GET_blk_ratio
to snd_soc_dai_ops ?

I could add this to fsl_ssi in master mode, but what if somebody connects the
tda to a cpu dai for which no-one implemented .GET_bclk_ratio ? Do we guess?
Or just error out?

Also, what would a proposed snd_soc_dai_GET_bclk_ratio() return e.g. on
fsl_ssi in slave mode, where the value arguably doesn't exist because the ssi
will accept pretty much anything you throw at it?