[RESEND] drm/i2c: tda998x: Reset the I2S_FORMAT (Page0, Reg 0xfc) to it's default

[Jyri Sarha]

On 22/02/2019 18:40, Russell King - ARM Linux admin wrote:
> On Fri, Feb 22, 2019 at 03:27:43PM +0000, Russell King - ARM Linux admin wrote:
>> On Fri, Feb 22, 2019 at 05:20:15PM +0200, Peter Ujfalusi wrote:
>>> Hi Russell,
>>>
>>> On 22/02/2019 16.35, Russell King - ARM Linux admin wrote:
>>>> It would also be good to know what Fs value(s) BBB uses, and what
>>>> sample sizes you have tested.
>>>
>>> On BBB McASP is the clock master and as I recall I have tested 44.1, 48
>>> KHz at least with 16 and 24 bits.
>>

Peter, I doubt you you were able to natively play 24bit 48kHz stereo, or
anything at 44.1kHz. Did you have ALSA plug -plugin configured?

BBB has 24.576MHz McASP mclk, which divides nicely by 48kHz to 512 clock
cycles / sample. 512 is divisible by 32 (= 16-bit stereo) and 64 (=
32-bit stereo), but not by 48 e.g. 24-bit stereo.

That is why I originally insisted in allowing 32-bit sample-format in
HDMI-codec (that is used in tda998x) and simply marking in struct
snd_soc_dai_driver that there is only 24 significant bits per a sample.

The other alternative would have been using set_bclk_ratio(), but it was
a quite new thing back then and had even less support in the drivers
than the currently.

>> Sorry, I wasn't clear enough... is the bus clocked at 32Fs for 16bit
>> samples and 64Fs for 24bit samples, or is it 64Fs for both?
> 

It is 32Fs for 16-bit and 64Fs for 32-bit, but 24-bit format is not
supported as such.

> To be clear, the reason I'm asking for this information is that
> Sven Van Asbroeck is trying to use TDA998x, and he has a problem with
> the current implementation that adjusts CTS_N_M and CTS_N_K parameters
> according to the _sample_ size.
> 
> This appears to be wrong, these settings should be set according to
> the BCLK ratio (Fs multiplier) and not the sample size.
> 
> If you say that the existing code works for you, but your device
> always produces a bclk at 64fs, then we have a problem - it means
> that to add support for Sven's platform, we're probably going to end
> up causing a regression for your platform.
> 
> The next issue is with snd_soc_dai_set_bclk_ratio().  Today, no one
> calls that function, so none of the DAI .set_bclk_ratio
> implementations are used (and probably completely untested.)  If your
> CPU DAI changes the bclk ratio depending on the sample size, I will
> need something to call that function at the appropriate time to set
> the clocking ratio.
> 
> I suspect most codecs don't care about it, but TDA998x does, because
> it _looks_ like it uses the BCLK to generate the CTS value to be
> passed to the HDMI sink.  Since CTS = f(tmds) * N / (128 * fs),
> using BCLK to derive the CTS value requires TDA998x to know the
> BCLK ratio.
> 
> So, knowing this information ahead of time is very advantageous.
> 

Sound to me that .set_bclk_ratio() support should be added to tda998x
(and HDMI-codec), but with the default behaviour that assumes the
bclk-ratio to be 2*sample-width if .set_bclk_ratio() is not called.
bcm2835-i2s codec driver appears to already implement .set_bclk_ratio()
like that.

Or am I missing something?

Best regards,
Jyri

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki