Skip to content

fix cyclic dma setup and i2s parameters #1233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 122 commits into from

Conversation

HiassofT
Copy link
Contributor

This is a followup to #1193

Instead of using a too large value for period_bytes_max in the I2S driver and then try to work around it in the DMA driver let's take the easy route:

Setup period_bytes_max to a value which the DMA controller can support and drop the workaround code which was incorrect anyways.

This works fine for audio drivers and currently I'm not aware of any other uses of the cyclic DMA code.

If some other driver really needs to be able to use cyclic DMA with periods larger than what the hardware can support it should be implemented correctly like eg in the img-mdc-dma driver (which splits up each requested period into chunks of supported sizes).

Steve Glendinning and others added 23 commits December 28, 2015 17:25
smsc95xx is adjusting truesize when it shouldn't, and following a recent patch from Eric this is now triggering warnings.

This patch stops smsc95xx from changing truesize.

Signed-off-by: Steve Glendinning <[email protected]>
The old arch-specific IRQ macros included a dsb to ensure the
write to clear the mailbox interrupt completed before returning
from the interrupt. The BCM2836 irqchip driver needs the same
precaution to avoid spurious interrupts.

Spurious interrupts are still possible for other reasons,
though, so trap them early.
Add a duplicate irq range with an offset on the hwirq's so the
driver can detect that enable_fiq() is used.
Tested with downstream dwc_otg USB controller driver.

Signed-off-by: Noralf Trønnes <[email protected]>
Reviewed-by: Eric Anholt <[email protected]>
Acked-by: Stephen Warren <[email protected]>
Contrary to the documentation, the BCM2835 GPIO controller actually has
four interrupt lines - one each for the three IRQ groups and one common. Rather
confusingly, the GPIO interrupt groups don't correspond directly with the GPIO
control banks. Instead, GPIOs 0-27 generate IRQ GPIO0, 28-45 GPIO1 and
46-53 GPIO2.

Awkwardly, the GPIOS for IRQ GPIO1 straddle two 32-entry GPIO banks, so it is
cleaner to split out a function to process the interrupts for a single GPIO
bank.

This bug has only just been observed because GPIOs above 27 can only be
accessed on an old Raspberry Pi with the optional P5 header fitted, where
the pins are often used for I2S instead.
Although the GPIO controller can generate three interrupts (four counting
the common one), the device tree files currently only specify two. In the
absence of the third, simply don't register that interrupt (as opposed to
registering 0), which has the effect of making it impossible to generate
interrupts for GPIOs 46-53 which, since they share pins with the SD card
interface, is unlikely to be a problem.
The spi-bcm2835 driver automatically uses GPIO chip-selects due to
some unreliability of the native ones. In doing so it chooses the
same pins as the native chip-selects would use, but the existing
code always uses pins 7 and 8, wherever the SPI function is mapped.

Search the pinctrl group assigned to the driver for pins that
correspond to native chip-selects, and use those for GPIO chip-
selects.

Signed-off-by: Phil Elwell <[email protected]>
The VideoCore bootloader passes in Serial number and
Revision number through Device Tree. Make these available to
userspace through /proc/cpuinfo.

Mainline status:

There is a commit in linux-next that standardize passing the serial
number through Device Tree (string: /serial-number):
ARM: 8355/1: arch: Show the serial number from devicetree in cpuinfo

There was an attempt to do the same with the revision number, but it
didn't get in:
[PATCH v2 1/2] arm: devtree: Set system_rev from DT revision

Signed-off-by: Noralf Trønnes <[email protected]>
Code copied from spi-bcm2835. Get physical address from devicetree
instead of using hardcoded constant.

Signed-off-by: Matthias Reichl <[email protected]>
Code ported from bcm2708-i2s driver in Raspberry Pi tree.

RPi commit 62c05a0 ("ASoC: BCM2708:
Add 24 bit support")

This adds 24 bit support to the I2S driver of the BCM2708.
Besides enabling the 24 bit flags, it includes two bug fixes:

MMAP is not supported. Claiming this leads to strange issues
when the format of driver and file do not match.

The datasheet states that the width extension bit should be set
for widths greater than 24, but greater or equal would be correct.
This follows from the definition of the width field.

Signed-off-by: Florian Meier <[email protected]>

RPi commit 3e8c672 ("bcm2708-i2s:
Update bclk_ratio to more correct values")

Discussion about blck_ratio affecting sound quality:
raspberrypi#681

Signed-off-by: Matthias Reichl <[email protected]>
Code ported from bcm2708-i2s driver in Raspberry Pi tree.

RPi commit c14827e ("bcm2708: Allow
option card devices to be configured via DT")

Original work by Zoltan Szenczi, committed to RPi tree by
Phil Elwell.

Signed-off-by: Matthias Reichl <[email protected]>
Code ported from bcm2708-i2s driver in Raspberry Pi tree.

RPi commit fd7d7a3 ("bcm2708:
Eliminate i2s debugfs directory error")

Qualify the two regmap ranges uses by bcm2708-i2s ('-i2s' and '-clk')
to avoid the name clash when registering debugfs entries.

Signed-off-by: Matthias Reichl <[email protected]>
Code ported from bcm2708-i2s driver in Raspberry Pi tree.

RPi commit ba46b49 ("ASoC: Add
support for BCM2708")

This driver adds support for digital audio (I2S)
for the BCM2708 SoC that is used by the
Raspberry Pi. External audio codecs can be
connected to the Raspberry Pi via P5 header.

It relies on cyclic DMA engine support for BCM2708.

Signed-off-by: Florian Meier <[email protected]>

Signed-off-by: Matthias Reichl <[email protected]>
Code ported from bcm2708-i2s driver in Raspberry Pi tree.

RPi commit 7ee829f ("bcm2708-i2s:
Enable MMAP support via a DT property and overlay")

The i2s driver used to claim to support MMAP, but that feature was disabled
when some problems were found. Add the ability to enable this feature
through Device Tree, using the i2s-mmap overlay.

See: raspberrypi#1004

Signed-off-by: Matthias Reichl <[email protected]>
Add slave transfer capability to BCM2835 dmaengine driver.
This patch is pulled from the bcm2708-dmaengine driver in the
Raspberry Pi repo. The work was done by Gellert Weisz.

Tested using the bcm2835-mmc driver from the same repo.

Signed-off-by: Noralf Trønnes <[email protected]>
bcm2835-dma supports residue reporting at burst level but didn't report
this via the residue_granularity field.

Without this field set properly we get playback issues with I2S cards.

[by HiassofT, taken from bcm2708-dmaengine]
Signed-off-by: Noralf Trønnes <[email protected]>
Load driver early since at least bcm2708_fb doesn't support deferred
probing and even if it did, we don't want the video driver deferred.
Support the legacy DMA API which is needed by bcm2708_fb.
Don't mask out channel 2.

Signed-off-by: Noralf Trønnes <[email protected]>
Set dreq to slave_id if it is not set like in bcm2708-dmaengine.
hsteinhaus and others added 15 commits December 28, 2015 17:26
The HiFiBerry DAC+ and DAC+ Pro products both use the existing bcm sound driver with the DAC+ Pro having a special clock device driver representing the two high precision oscillators.

An addition bug fix is included for the PCM512x codec where by the physical size of the sample frame is used in the calculation of the LRCK divisor as it was found to be wrong when using 24-bit depth sample contained in a little endian 4-byte sample frame.
Add an overlay to support the Atmel AT86RF233 WPAN transceiver on spi0.0.

See: raspberrypi#1151
See commit dae803e -- the warning is
expected sometimes when using CMA.  However, that commit still spams
my kernel log with these warnings.

Signed-off-by: Eric Anholt <[email protected]>
This allows a driver to derive from the CMA object without copying all
of the code.

Signed-off-by: Eric Anholt <[email protected]>
This can be parsed with vc4-gpu-tools tools for trying to figure out
what was going on.

Signed-off-by: Eric Anholt <[email protected]>
This gets almost everything matching, except for the MSAA support and
using generic PM domains.

Signed-off-by: Eric Anholt <[email protected]>
VC4 wraps the CMA objects in its own structures, so it needs to do its
own teardown (waiting for GPU to finish, updating bo_stats tracking).
The other CMA drivers are using drm_gem_cma_free_object as their
gem_free_object, so this should be a no-op for them.

Signed-off-by: Eric Anholt <[email protected]>
For MSAA, you set a bit in the binner that halves the size of tiles in
each direction, so you can pack 4 samples per pixel in the tile
buffer.  During rendering, you can load and store raw tile buffer
contents (to save the per-sample MSAA contents), or you can load/store
resolved tile buffer contents (loads spam the pixel value to all 4
samples, and stores either average the 4 color samples, or store the
first sample for Z/S).

Signed-off-by: Eric Anholt <[email protected]>
At this point all that's left is the force-enable of HDMI connector,
and using direct firmware calls to turn on V3D instead of the generic
power domain support.

Signed-off-by: Eric Anholt <[email protected]>
bcm2835-dma supports a maximum transfer length of 64k-4 bytes on
the lite channels. period_bytes_max should reflect this limit.

Also use SZ_xxx constants for buffer/prealloc size. The usage of
PAGE_SIZE is misleading here, the buffer sizes are not related
to the page size.

Signed-off-by: Matthias Reichl <[email protected]>
The current implementation is broken in several ways:

The number of frames was calculated wrong, which results in
one frame less than requested. eg when requesting a 96000 bytes
buffer split into 4 periods of 24000 bytes the code would allocate
3 frames: the first two with 24000 bytes size and the third one
with 48000 bytes.

This not only results in the cyclic callback being called at other
times than requested but can also lead to frames larger than what
the DMA hardware can support. eg when requesting a 192000 bytes buffer
split into 4 periods of 48000 bytes each (well within the limit
of what the DMA hardware supports) it would allocate 2 48000 byte
frames and one 96000 byte frame - the latter larger than what the
hardware supports.

Frames larger than the hardware limit are the reason for clicking
playback and the previous fix of using a smaller limit for
cyclic transfers only worked around the underlying bug.

Another issue is the way the code tried to handle period sizes larger
than what the hardware supports. It results in cyclic callbacks
not at the requested period length but in callbacks at the hardware
limit boundaries.

The simple fix for these issues is to reject requests that can't
be handled by the hardware.

The maximum period length in bcm2835-i2s has been fixed (set to
64k-4 bytes, the limit of the lite DMA channels) so the cyclic
DMA setup code should never be called with period lenght larger
than the hardware limit, thus period splitting is not neccessary
at all.

Signed-off-by: Matthias Reichl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.