lkml.org 
[lkml]   [2021]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] clk: meson: gxbb: Add the spread spectrum bit for MPLL0 on GXBB
Date

On Mon 18 Oct 2021 at 13:44, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Jerome,
>
> On Mon, Oct 18, 2021 at 12:19 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>>
>>
>> On Sat 16 Oct 2021 at 16:59, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>>
>> > Christian reports that 48kHz audio does not work on his WeTek Play 2
>> > (which uses a GXBB SoC), while 44.1kHz audio works fine on the same
>> > board. He also reports that 48kHz audio works on GXL and GXM SoCs,
>> > which are using an (almost) identical AIU (audio controller).
>>
>> The above is a bit "personal" - it is not great fit for the commit
>> description. Please rephrase or put it in comment section bellow
> sure, I can rephrase that
>
> [...]
>> > Add the SSEN (spread spectrum enable) bit and add the
>> > CLK_MESON_MPLL_SPREAD_SPECTRUM flag to enable this bit for MPLL0. Do
>> > this for GXBB *only* since GXL doesn't seem to care if this bit is set
>> > or not, meaning that meson-clk-msr always sees (approximately) the same
>> > frequency as common clock framework.
>>
>> 1 - it is odd that we need to poke a bit in the register related to the
>> fixed PLL but ok ...
>> 2 - 3.14 does yes, 4.9 does not soooo ... no real proof there
> The fact that 4.9 doesn't do it is also no proof for anything either.
> Amlogic hasn't ported forward GXBB support to their 4.9 kernel. Even
> "mesongxbb.dtsi" is missing there [0]
> So in my opinion the code from Amlogic's patched 4.9 kernel cannot be
> used as reference for anything GXBB related. Only the 3.14 code can be
> used as reference.
>
> [...]
>> So 2 things:
>> - If this bit really enables spread spectrum on MPLL0 (or worse, the
>> Fixed PLL) - checking clk measure is not enough. It is just a mean of
>> the rate seen by the SoC itself. You would not see the effect of the
>> spread spectrum here ... you need to capture the clock output with a
>> scope for that.
> I suspect that a board with pin headers is needed to route the signal there?
> Personally I don't have any GXBB board(s).
>
>> - Or the bit is incorrectly documented (or DDS0_SSEN does not mean
>> spread spectrum). If it is not a spread spectrum function, then this
>> patch seems to indicate it is and it is misleading.
>>
>> Either way, I'm not OK with it.
>>
>> To me, the rate drop that happens when you flip this bit looks more like
>> the effect SDM_EN should have.
>>
>> Could you check the internal values (n2 and sdm) compare this to the
>> output rate you actually get ? see if this leads to anything ? does
>> SDM_EN really has an effect on this MPLL ? it is a combination of both ?
> Christian has provided a dump of the HHI registers (via userspace,
> regmap makes them accessible).
> On GXBB WP2 HHI_MPLL_CNTL7 is set to 0x0006f208
> On GXL Le Potato (which he used for comparison as it wasn't affected
> by the MPLL0 issue) HHI_MPLL_CNTL7 is set to 0x0006b208
>
> If you're interested in the full HHI register dump you can find it here: [1]
> GXBB WP2 is on the left and GXL Le Potato is on the right.
>
> The difference here is BIT(14). un-setting BIT(14) (documented as
> EN_DDS0) did not change anything according to Christian's test.
> That also means that SDM, SDM_EN and N2 have the expected values.
> I manually did the maths:
> (2000000000Hz * 16384) / ((16384 * 6) + 12808) = 294909640.7Hz
> which matches what clk_summary sees:
> 294909641Hz

... and (2000000000Hz * 16384) / ((16384 * 6) = 333MHz which is fairly close
to what you get w/o flipping the bit

>
> Just to clarify that I am not wasting Christian's time when I ask him
> to test something:
> next up we'll set SDM_EN to 0 (instead of 1) and see the reaction using clk-msr?
>

For example yes. I am asking check a bit more what this bit does and
what it does not:
- I need confirmation whether or not it does spread spectrum. Yes this
needs to be observed on a SoC pin, like MCLK with a fairly low divider
to the averaging effect which could partially mask spread spectrum.

- Get an idea what it actually does. The 2 calculations above are an
hint. (Spread spectrum does not change the rate mean value)

>
> Best regards,
> Martin
>
>
> [0] https://github.com/hardkernel/linux/tree/db88db3864c5d6903fb11f6528874887d1c473ce/arch/arm64/boot/dts/amlogic
> [1] https://pastebin.com/raw/1cjPFsfa

\
 
 \ /
  Last update: 2021-10-18 14:04    [W:0.079 / U:0.864 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site