bcm2708-i2s lacks mmap support

[jimbojr]

If the driver supports mmap then ALSA will do sensible automatic rate and sample size conversion.

The problem (apart from it should support mmap as most drivers seem to) is it isn't obvious to those using I2S based sound cards why a lot of things don't work properly. I've now seen quite a few posts on the R-Pi forums and elsewhere. Just two examples: https://www.hifiberry.com/forums/topic/jackd/ and https://www.raspberrypi.org/forums/viewtopic.php?f=98&t=111982&p=769781#p769781

It would be great to add this if we can.

[HiassofT]

MMAP support was present in the inital version of bcm2708-i2s but shortly afterwards removed in commit 62c05a0.

It's not quite clear to me what the strange issues actually were/are.

Ping @koalo, could you elaborate on this?

Ragnar Jensen provides a Wolfson/Cirrus card kernel build with MMAP support enabled and it seems to be working very well.
http://www.element14.com/community/message/149403/l/re-cirrus-logic-audio-card-working-on-the-raspberry-pi-2#149403

I just did a few tests with my Cirrus kernel build (based on top of rpi-4.0.y) with MMAP enabled and it worked fine, too. With MMAP aplay is able to play back 24bit WAVs which won't work without MMAP support.

This is the patch I used when testing:

--- a/sound/soc/bcm/bcm2708-i2s.c
+++ b/sound/soc/bcm/bcm2708-i2s.c
@@ -875,7 +875,9 @@ static const struct snd_soc_component_driver bcm2708_i2s_component = {
 };

 static const struct snd_pcm_hardware bcm2708_pcm_hardware = {
-       .info                   = SNDRV_PCM_INFO_INTERLEAVED |
+       .info                   = SNDRV_PCM_INFO_MMAP |
+                                 SNDRV_PCM_INFO_MMAP_VALID |
+                                 SNDRV_PCM_INFO_INTERLEAVED |
                                  SNDRV_PCM_INFO_JOINT_DUPLEX,
        .formats                = SNDRV_PCM_FMTBIT_S16_LE |
                                  SNDRV_PCM_FMTBIT_S24_LE |

I think we should do some more testing with the various cards (I only have Wolfson/Cirrus cards here), maybe MMAP works fine with the current kernel tree and it's just a matter of adding back the info flags.

[pelwell]

If that's all it takes, I can add a custom DT property to enable MMAP support and expose it via a dtparam setting.

[koalo]

I do not know how exactly MMAP is implemented, but it seems to rely on some kind of advanced memory management of the DMA engine or maybe there is still a bug in the driver itself or ALSA.
Anyway, under some constellations, playing back a file that require MMAP and afterwards recording in 24-bit sometimes result in having a loop of the previously played file in the record instead of the recorded audio signal or no signal at all.
Since I was not able to further debug this issue, I decided to disable MMAP, since don't having a feature is better than to have a feature that results in unreliable behavior.
If you decide to enable it per default, it would be very advisable to test it with a wide range of different files with different formats, recording formats and hardware that does support 24-bit and hardware that does not.

[pelwell]

I've pushed a commit that makes the MMAP support conditional on a DT property. After rebuilding and copying the kernel, modules and overlays to your SD card, add:

dtoverlay=i2s-mmap

to config.txt.

[pelwell]

The latest firmware (Hexxeh/rpi-firmware@9ad0e75) contains the updated module and overlay. Please try it and comment on your findings.

[HiassofT]

Thanks a lot for the explanation and the commit!

@pelwell could you include the commit in the 4.0 tree as well?

I did several tests with my Cirrus card and the driver rebased
onto rpi-3.18.y and so far everything went well.

First I played a testfile, tried with different samplerates
and 16/24/32 bit. The (packed) 24bit wavs required MMAP support,
for the others I tried both without MMAP and with forced MMAP
(aplay -M).

Immediately afterwards I recorded an audio file. Tried with 44.1
and 48kHz, 16/24/32 bit and also without and with MMAP (arecord -M).

After recording I played back the recorded file, also sometimes
tried with aplay -M.

I did some 15 play-record-play runs and didn't see any of the
issues koalo wrote about.

The only issue I ran into was playback not starting together with a

bcm2708-dmaengine bcm2708-dmaengine: DMA transfer could not be terminated

entry in dmesg. But I think that's unrelated to MMAP support, I saw
this before without MMAP support too (not sure if it's a bug in
the Cirrus driver or somewhere else).

[pelwell]

Pushed.

[jimbojr]

I am still not sure this is 'proper' mmap support but I'll give it a whirl for some test files I have.

I wonder if @martinezjavier (who I believe very kindly added mmap support to the bcm2835 GPU driver) would mind looking over the code to say yes or no whether it looks like it supports mmap properly, or if we need to do more work here...

[BBUK]

Thanks for this. I want to use a Hifiberry Amp+ to run two independent mono streams for a multi room solution (one mapped to the left output and one to the right output).

The easiest route to do this is via the alsa dmix plugin which (I understand) needs MMAP.

The i2s-mmap overlay has enabled proper use of dmix and is confirmed working fine for my use case on a RPi B+ (running mpd).

Let me know if you would like me to assist by undertaking any further tests.

Thanks again

[pelwell]

@BBUK Thanks for the feedback. Unless we reopen the original problem of why enabling MMAP support by default is problematic, I can't think of anything else for you to test.

[iqaudio]

@BBUK Can you post details of dmix usage so we can replicate please? I'd like to check this on our boards too.

[agherzan]

@popcornmix Any idea why this overlay (i2s-mmap) is not available anymore in 4.9?

[HiassofT]

Any idea why this overlay (i2s-mmap) is not available anymore in 4.9?

bcm2835-i2s now supports MMAP by default so this overlay is no longer needed.

[agherzan]

Sounds good. Do you have any idea where is this enabled now in the source?

[HiassofT]

Sounds good. Do you have any idea where is this enabled now in the source?

sound/soc/soc-generic-dmaengine-pcm.c All I2S devices using soc-generic-dmaengine implicitly have MMAP capabilities. see dmaengine_pcm_set_runtime_hwparams: ... hw.info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_INTERLEAVED;

[agherzan]

Thanks @HiassofT . That makes sense.