linux-kernel - Re: [PATCH 1/2] drm/i2c: tda998x: adjust CTS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGngYiVU0wLVOm=tY+TZ9h1P+qNnDdv_3PybfhRreuBpCZ6D1g@mail.gmail.com>
Date:   Fri, 22 Feb 2019 10:47:35 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peter Rosin <peda@...ntia.se>
Subject: Re: [PATCH 1/2] drm/i2c: tda998x: adjust CTS_N audio pre-divider calculation

On Fri, Feb 22, 2019 at 8:21 AM Russell King - ARM Linux admin
<linux@...linux.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?