Rpi 4.4.y msperl i2s/clockmgr backport #1592
Conversation
|
We have no shortterm plan to move stable from 4.4. But we'll obviously the bump the stable kernel eventually. I suspect the arm side graphics driver may get harder to support the further we are away from mainline where the patches are going, which would be one reason for bumping. But we'll also keep an eye out for requested features/drivers/bug fixes that are present in newer kernels. But nothing planned in next few months. |
clivem
force-pushed
the
rpi-4.4.y-msperl
branch
2 times, most recently
from
d3c6313 to
77fd0ca
Compare
clivem
force-pushed
the
rpi-4.4.y-msperl
branch
from
8d1d312 to
cce3c06
Compare
|
@popcornmix I'm a little confused. Just looking to rebase my audio patchsets from rpi-4.4.y to rpi-4.6.x, and the patches I thought I'd need to drop, ie, the ones that there wont be any need for, because bcm2835-i2s will be using the clockmgr by default now, you've reverted clock manager in your 4.6 tree, back to pre-clockmgr code. ie. 42cb68a Is there a reason why you don't want to move forward with clockmgr? |
|
Only that reports were that I2S audio wasn't working. I believe @pelwell fixed it up by reverting back so we could carry on using the tree. If you have it working on 4.6 then we're quite happy to move forward again. BTW 4.6 is already EOL upstream, so 4.7 may be a more useful tree to work on. |
|
OK, makes sense. I'll take a look at 4.7.y tree tomorrow. We probably want to merge most of the patches in this PR anyway, converting everything downstream to use the clkmgr framework, not just I2S and the Anholt GPU driver using clockmgr.... I haven't looked yet, but I'm hoping anything from Anholt for the vc4 driver that has been backported to rpi-4.4.y, is already in the rpi-4.7.y tree.... |
|
4.7 should have most of those patches already in place (but some may have been reverted for i2s-bcm2835). The main thing that remains are;
These patches are the ones that are already upstream in 4.7:
So most of it should be there... Also note that there was some talk about a LTS release for 4.8 or 4.9 (can't remember the details and can not find the announcement at this very moment) and the foundation will probably want to move to that LTS release. |
|
Yes, LTS kernels always seem to be a bit of secret until kernel.org lists them. |
|
LTS is probably 4.9: https://plus.google.com/+gregkroahhartman/posts/DjCWwSo7kqY But Ubuntu will use 4.8: http://www.techworm.net/2016/06/linux-kernel-4-8-power-upcoming-ubuntu-16-10-yakkety-yak.html So what should the target be for now? As said: it is mostly the device-tree (+ SMI) that needs to get fixed - all the clock stuff + DMA + most of i2s is in 4.7 already. Martin |
|
I'd say 4.7 would be my preference. It's being used by LibreELEC nightly builds, so will get reasonable testing for I2S audio. Hopefully it will apply to 4.8 and 4.9 without too much grief. |
|
I'm probably about an hour away from making the PR to get all of the clockmgr stuff into rpi-4.7.y. Just testing now....... |
|
https://github.com/DigitalDreamtimeLtd/linux/commits/rpi-4.7.y-clockmgr I've run out of time to do any more testing today. PR tomorrow. |
|
Just going over the list of patches quickly - some notes (some with regards to what is upstream and what the policies there are):
|
|
@msperl With the greatest respect..... I'm OUT. The stuff you are saying "do not apply", is the very stuff that needs to be applied for there to be any chance of any sort of reliability. I would suggest that @pelwell sticks with the reverting the bcm2835-i2s upstream until this whole clock mess gets sorted out upstream and trickles downstream! |
|
Sorry to have frustrated you - i have just summarized what is likely also to go in upstream and what is in doubt and for which other solutions have been chosen by Eric upstream. Popcornmix prefers things that go upstream,so I am mentioning that... I had thought that these patches made it into 4.7. But it seems as if these have not made it into 4.7... Your work is a good basis and (after seeing those plld things not going in) understand your frustration. I can try to have a look at those patches of Eric when I find more time in September... |
|
Note that this is the patchset I have been talking about:
That should put away most of the grievances that we have had without all those out of tree patches. Unfortunately there never was any followup on those, so maybe Eric can elaborate on the status? |
|
While coming from upstream is desirable, working reliably is more important. |
I'll spin a build dropping HAND_OFF et all and I'll do some testing including Eric's 4x patches you referenced, and see if that resolves the issues that were resolved with the patches you'd rather not see applied, ie. never choosing PLLC as a parent and not disabling clocks, when reference count drops to zero, which are still needed, and causes hard-lock, something that CRITICAL was meant to resolve but never did....... I fully understand why it is always preferable to go via upstream..... Yep, I get that. But in this instance, specifically I2S HAT's, (clocks are make or break to either a bullet-proof implementation or it not working at all), I've said it before and I'll say it again...... No-one (well maybe aside from you and one other) is using the upstream kernel with any of the I2S HAT boards. 1000's of users are using the downstream kernel for this purpose. Which brings me back to, this stuff needs to be refined and perfected downstream, before the "reference" implementation is submitted back upstream and the loop is closed. That's my opinion..... |
|
I also started on working on the patchset again - I guess there are 2 things:
The point why I am advocating against hands off and such is because the hands-off approach was voted down - similarly the "clock selection policy in devicetree" (argument there: dt should describe HW not policy - configfs may be the right place to apply policies). As you were frustrated I started to work also on a new patchset that tries to first handle the first point, then adds the second point (erics patches) and then let us see what is still missing for I2S - that then we can try to upstream. Note that while going over the patchset of yours I also found some totally unrelated patchsets with regards to USB, so these should not be part of this clock switch. I should be able to come up with something tomorrow as well - we then can compare notes... |
|
Confused..... USB patches..... You are looking at my rpi-4.7.y-clockmgr branch, right? Only USB patches I can think of are the "quirk" patches for native DSD endpoint support. But unless I've done something stupid they should be in another branch. |
|
These patches show up in the history:
|
|
@msperl This is where I am at now, testing Eric x4 patch set. Just did a quick "sanity" test on a 3B.... No lock-ups, music playing at wrong rates, etc. etc. So about to start rolling this out for thorough testing to Zero, B, B+, 2B.... 32 bit samples, bclk ratio=64 32 bit samples, bclk ratio=100 for multiples of 8k |
Nothing to do with me. I pulled from rpi-4.7.y. Talk to @popcornmix. They were committed by Dom to the rpi-4.7.y tree. |
|
My error - they just showed at the same date, so I had assumed... Besides that: So what is missing in principle - compared to the old i2s driver? |
|
If the patches by Eric resolve most things then we should add an asked/tested to them so that they can get merged for 4.9 |
Yes, without your "only use OSC and PLLD patches", PLLC used to be selected for some frequencies, with the obvious results to music playback speed, when turbo engaged/disengaged. Obviously one of Erics patches specifically deals with that now.
Nothing. I think we are good to go. But as that original patch set (complete with the "bastard" patches) had several months testing, on every piece of Pi hardware known to man, with DAC boards in master and slave, at all known sample rates, and sample depth combos..... on my 20 machine I2S "farm" and I have people in the wild using it, with my "private" Fedora kernel builds, I'll reserve my thumbs-up temporarily until I've thoroughly tested where we are at now..... I've barely kicked the tires.
Sure. I'll add a Tested-By when I've finished testing. |
|
@msperl Now I really am getting confused. Those 4x Anholt patches are already in rpi-4.4.y, but not rpi-4.7.y and not upstream........ |
|
@anholt has submitted some PR's to 4.4 to get upstream graphics driver fixes into raspbian. |
Cleanup of includes so that they are ordered alphabetically. Signed-off-by: Martin Sperl <[email protected]> Signed-off-by: Mark Brown <[email protected]>
Since the move to the new clock framework with commit 94cb7f7 ("ARM: bcm2835: Switch to using the new clock driver support.") this driver was no longer functional as it was manipulating the clock registers locally without going true the framework. This patch moves to use the new clock framework and also moves away from the hardcoded address offsets for DMA getting the dma-address directly from the device tree. Note that the optimal bclk_ratio selection to avoid jitter due to the use of fractional dividers, which is in the current version has been removed, because not all devices support these non power of 2 sized transfers, which resulted in lots of (downstream) modules that use: snd_soc_dai_set_bclk_ratio(cpu_dai, sample_bits * 2); Signed-off-by: Martin Sperl <[email protected]> Signed-off-by: Mark Brown <[email protected]>
This adds 24 bit support to the I2S driver of the BCM2835 Code ported from bcm2708-i2s driver in Raspberry Pi tree. Signed-off-by: Florian Meier <[email protected]> Signed-off-by: Matthias Reichl <[email protected]> Signed-off-by: Martin Sperl <[email protected]>
We only need to enable the clock if we are a clock master. Code ported from bcm2708-i2s driver in Raspberry Pi tree. Original work by Zoltan Szenczi. Signed-off-by: Matthias Reichl <[email protected]> Signed-off-by: Martin Sperl <[email protected]>
dmaengine_pcm currently only supports setups where FIFO reads/writes correspond to exactly one sample, eg 16-bit sample data is transferred via 16-bit FIFO accesses, 32-bit data via 32-bit accesses. This patch adds support for setups with fixed width FIFOs where multiple samples are packed into a larger word. For example setups with a 32-bit wide FIFO register that expect 16-bit sample transfers to be done with the left+right sample data packed into a 32-bit word. Support for packed transfers is controlled via the SND_DMAENGINE_PCM_DAI_FLAG_PACK flag in snd_dmaengine_dai_dma_data.flags If this flag is set dmaengine_pcm doesn't put any restriction on the supported formats and sets the DMA transfer width to undefined. This means control over the constraints is now transferred to the DAI driver and it's responsible to provide proper configuration and check for possible corner cases that aren't handled by the ALSA core. Signed-off-by: Matthias Reichl <[email protected]>
The bcm2835-i2s driver already has support for the S16_LE format but that format hasn't been made available because dmaengine_pcm didn't support packed data transfers. bcm2835-i2s needs 16-bit left+right channel data to be packed into a 32-bit word, the FIFO register is 32-bit only and doesn't support 16-bit access. Now that dmaengine_pcm supports packed transfers the format can be made available by setting the SND_DMAENGINE_PCM_DAI_FLAG_PACK flag. No further configuration is necessary: - snd_dmaengine_dai_dma_data.addr_width is already set to DMA_SLAVE_BUSWIDTH_4_BYTES to force 32-bit DMA transfers - dmaengine_pcm will pick up the S16_LE format from the DAI configuration and make it available since it's no longer masked out due to the PACK flag. - there are no further corner cases to catch in hw_params, since the channel count is fixed at 2 we always have two 16-bit stereo samples that can be transferred via 32-bit DMA Signed-off-by: Matthias Reichl <[email protected]>
Upstream mandates the use of "serial" as name inside the device-tree. Note that this does not affect the use of the alias - there uart0 and uart1 are permissible. This also fixed the patch references inside the overlays for the rpi3. Signed-off-by: Martin Sperl <[email protected]>
Moved uart0 to use new clock framework. At the same time removed the fixed clock clk_uart0 and the corresponding references, as the clock is now read directly from the configured clocks. If it ever becomes necessary the following can get added to modify the base clock rate later: assigned-clocks = <&clocks BCM2835_CLOCK_UART>; assigned-clock-rates = <X>; Signed-off-by: Martin Sperl <[email protected]>
use spi0 to use new clock framwork Signed-off-by: Martin Sperl <[email protected]>
move i2c to use the new clock framework instead of the fixed clock Signed-off-by: Martin Sperl <[email protected]>
Move PWM to use the new clock framework Signed-off-by: Martin Sperl <[email protected]>
Fix the auxiliar clock provider to use the clock framework Signed-off-by: Martin Sperl <[email protected]>
Fix the register range of the spi-aux devices, as they do not control the gate. Signed-off-by: Martin Sperl <[email protected]>
The bcm2835 SOC contains an auxiliary uart, which is very close to the ns16550 with some differences. The big difference is that the uart HW is not using an internal divider of 16 but 8, which results in an effictive baud-rate being twice the requested baud-rate. This driver handles this device correctly and handles the difference in the HW divider by scaling up the clock by a factor of 2. The approach to write a separate (wrapper) driver instead of using a multiplying clock and "ns16550" as compatibility in the device-tree has been recommended by Stephen Warren. Signed-off-by: Martin Sperl <[email protected]> Acked-by: Eric Anholt <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
Move to new aux-uart driver (from upstream) and enable it correctly in the device tree Note that this may also solve the "serial not 115200 baud" issue on the rpi3... With an extension to the driver (requesting clock change notifications) the driver could change the baud divider when such a clock change occurs... It still would require some communication between the firmware and the kernel, so that this would get propagated. Signed-off-by: Martin Sperl <[email protected]>
Remove no longer necessary fixed pwm clock. Signed-off-by: Martin Sperl <[email protected]>
Use clock manager instead of self-made clockmanager. Also fix some error paths that showd up during development (especially missing release of dma resources on rmmod) Signed-off-by: Martin Sperl <[email protected]>
Move smi to use clock framework. Signed-off-by: Martin Sperl <[email protected]>
Move mmc to use clock framework. For some reason the emmc clock dirver does not claim the clock - enable count stays at 0 for the emmc clock. Signed-off-by: Martin Sperl <[email protected]>
Move the sdhost device to use the clock framework. Unfortunately I can not test it on my CM, as it seem as if there is no overlay that would enable sdhost and change the gpio-mux to the correct ALT0 (from Alt3 from mmc). Signed-off-by: Martin Sperl <[email protected]>
Remove clk_core from device tree as it is no longer used. Any "modification" to the core_clock in the bootloader is now automatically detected via the cnew clockmgr. Signed-off-by: Martin Sperl <[email protected]>
Some clocks need to be enabled to accept rate changes. This patch adds a new flag CLK_SET_RATE_UNGATE that lets clk_change_rate enable the clock before trying to change the rate and disable it again afterwards. This of course doesn't effect clocks that are already running at that point, as their refcount will only temporarily increase. Signed-off-by: Heiko Stuebner <[email protected]> Tested-by: Sjoerd Simons <[email protected]> Reviewed-by: Sjoerd Simons <[email protected]> Signed-off-by: Michael Turquette <[email protected]>
Signed-off-by: Lee Jones <[email protected]> Reviewed-by: Stephen Boyd <[email protected]> Signed-off-by: Michael Turquette <[email protected]> Link: lkml.kernel.org/r/[email protected]
Revert the i2s-mmap dt overlay (7ee829f). Signed-off-by: DigitalDreamtime <[email protected]>
Moved uart0 to use new clock framework. At the same time removed the fixed clock clk_uart0 and the corresponding references, as the clock is now read directly from the configured clocks. If it ever becomes necessary the following can get added to modify the base clock rate later: assigned-clocks = <&clocks BCM2835_CLOCK_UART>; assigned-clock-rates = <X>; Signed-off-by: DigitalDreamtime <[email protected]>
Remove clk_core from device tree as it is no longer used. Any "modification" to the core_clock in the bootloader is now automatically detected via the cnew clockmgr. Signed-off-by: DigitalDreamtime <[email protected]>
Clocks are now managed by cprman, so pwm_clk no longer exists. Signed-off-by: Phil Elwell <[email protected]>
clivem
force-pushed
the
rpi-4.4.y-msperl
branch
from
0ce0c6c to
6a76bb3
Compare
This is another of my PR's that isn't actually a PR.
@msperl Martin, I've just quickly hacked your last unmerged backport, (obviously sans the dma patchset, that is already in rpi-4.4.y)....... Is there still any interest in trying to get this back into 4.4.y?
@pelwell / @popcornmix How much longer do you expect rpi-4.4.y to remain the "stable" release? Will rpi-4.6.y be the next "stable"? How far away is that?