ASoc / PCM recording-related regression between v5.4 and v5.5

[Hin-Tak Leung]

 On Thursday, 22 April 2021, 15:01:02 BST, Péter Ujfalusi <peter.ujfalusi at gmail.com> wrote:

> If you start the DAI first and later the DMA then it works again?
> I'm still behind of the patch which introduced different sequences for
> start and stop.

I am with you on your change too - I think your change makes sense; it is the vendor's driver that needs correcting.

> From my commit the start sequence is:
> dai_link, DMA, CPU DAI then the codec

> Is it make any difference if you try:
> dai_link, DMA, codec then the CPU DAI

I'll give this a try soon. I am just the owner of one such device, and did the porting from kernel 4.19 to v.5.4, and now troubleshooting to v5.10. The vendor has been quite absent... and I feel I perhaps spend more time on this venture than I am happy with... Anyway - 

> But the codec is usually not handled in trigger, it can not be atomic
> most of the time.

I suspect the vendor did something not quite right in the area to make it work under the previous wrong trigger order too. More below about spinlock and atomic context.

> Can you point me to the out of tree driver and the related dts files?
> Can it be reproduced with rpbi w/o any hat? I have one hifiberry but
> that is playback only and if I'm not mistaken the 3.5 on the board also?

For your purpose you can see my fork (which includes v5.4 -> v5.10 migration change, unlike the vendor's), https://github.com/HinTak/seeed-voicecard . 

The dts concerned is: https://github.com/HinTak/seeed-voicecard/blob/v5.9/seeed-8mic-voicecard-overlay.dts

The card trigger code is in: https://github.com/HinTak/seeed-voicecard/blob/v5.9/seeed-voicecard.c

while the codec trigger code is in
https://github.com/HinTak/seeed-voicecard/blob/v5.9/ac108.c
and also https://github.com/HinTak/seeed-voicecard/blob/v5.9/ac101.c ! 

Note that the routine ac101_trigger() is not used, although the ac101 codec itself is involved.
There is a spinlock in ac108_trigger(), which I have long suspected to be incorrect, and has been known to cause the pi to kernel panic if built with PREEMPT ; Now I think it is there to make it work under the previous wrong sequence.

There is another device of the same family - which records okay regardless of the trigger sequence https://github.com/HinTak/seeed-voicecard/blob/v5.9/seeed-4mic-voicecard-overlay.dts .

The problematic device has 8 mics, using two ac108 for 2 x 4-ADC and the ac101 for DAC loopback (and as clock master?). The 4-mics record-okay device has just one ac108 (and without a ac101) to record 4-channels.

As I understand it, to record / playback for 8 channels on the pi, the I2S on the pi transfer data as stereo at 4x the frequency, and the codecs try to start and stop in such a way to avoid channel shifts during the 8-channels to 4x stereo change.

I can reboot between 5.4 and 5.10 quite easily (the pi does not have a boot loader per se - you just overwrite the kernel image in the first fat32 partitition...). At the moment I am thinking of inserting some pr_info() into seeed-voicecard.c:seeed_voice_card_trigger() and ac108.c:ac108_trigger() and perhaps also in general in ac101.c to see how the driver behaves differently under v5.4 and v5.10 . And while doing that, have some thoughts about getting rid of that problematic spinlock in ac108.c:ac108_trigger() when the code get re-arranged.

The problem is with trying to record, so I assume you cannot reproduce the problem if you do not have the hardware. However, I could certainly benefit from some suggestions and guidance on what to change and how to change the current code.

I am fairly okay with kernel development (in a few other different areas, file systems and wireless mostly), but audio is new to me.

Thanks a lot for any ideas / suggestions you might want to give on modfying the two trigger functions and getting rid of that spinlock!

Regards,
Hin-Tak