arch/arm/boot/dts/sun5i.dtsi | 2 +-
arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 +-
arch/arm/boot/dts/sun8i-a83t.dtsi | 2 +-
arch/arm/boot/dts/sun8i-v3s.dtsi | 2 +-
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 4 +-
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 6 ++-
drivers/clk/sunxi-ng/ccu-sun50i-a64.h | 4 +-
drivers/gpu/drm/sun4i/Makefile | 2 +-
drivers/gpu/drm/sun4i/sun4i_tcon.c | 46 +++++++++++--------
.../{sun4i_dotclock.c => sun4i_tcon_dclk.c} | 2 +-
.../{sun4i_dotclock.h => sun4i_tcon_dclk.h} | 0
include/dt-bindings/clock/sun50i-a64-ccu.h | 1 +
12 files changed, 43 insertions(+), 30 deletions(-)
rename drivers/gpu/drm/sun4i/{sun4i_dotclock.c => sun4i_tcon_dclk.c} (99%)
rename drivers/gpu/drm/sun4i/{sun4i_dotclock.h => sun4i_tcon_dclk.h} (100%)
According to Allwinner's BSP code, in DSI mode, TCON0 clock needs to be
running at what's effectively the per-lane datarate of the DSI link.
Given that the TCON DCLK divider is fixed to 4 (SUN6I_DSI_TCON_DIV),
DCLK can't be set equal to the dotclock. Therefore labeling TCON DCLK
as sun4i_dotclock or tcon-pixel-clock shall be avoided.
With bpp bits per pixel transmitted over n DSI lanes, the target DCLK
rate for a given pixel clock is obtained as follows:
DCLK rate = 1/4 * bpp / n * pixel clock
Effect of this change can be observed through the rate of Vblank IRQs
which should now match refresh rate implied by set display mode. It
was verified to do so on a A64 board with a 2-lane and a 4-lane panel.
v2:
1. prevent reparent of tcon0 to pll-video0-2x
2. include pll-video0 in setting TCON0 DCLK rate
3. tested the whole thing also on a PinePhone
Roman Beranek (7):
clk: sunxi-ng: a64: propagate rate change from pll-mipi
clk: sunxi-ng: a64: export PLL_MIPI
clk: sunxi-ng: a64: prevent CLK_TCON0 being reparented
arm64: dts: allwinner: a64: assign PLL_MIPI to CLK_TCON0
ARM: dts: sunxi: rename tcon's clock output
drm: sun4i: rename sun4i_dotclock to sun4i_tcon_dclk
drm: sun4i: calculate proper DCLK rate for DSI
arch/arm/boot/dts/sun5i.dtsi | 2 +-
arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 +-
arch/arm/boot/dts/sun8i-a83t.dtsi | 2 +-
arch/arm/boot/dts/sun8i-v3s.dtsi | 2 +-
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 4 +-
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 6 ++-
drivers/clk/sunxi-ng/ccu-sun50i-a64.h | 4 +-
drivers/gpu/drm/sun4i/Makefile | 2 +-
drivers/gpu/drm/sun4i/sun4i_tcon.c | 46 +++++++++++--------
.../{sun4i_dotclock.c => sun4i_tcon_dclk.c} | 2 +-
.../{sun4i_dotclock.h => sun4i_tcon_dclk.h} | 0
include/dt-bindings/clock/sun50i-a64-ccu.h | 1 +
12 files changed, 43 insertions(+), 30 deletions(-)
rename drivers/gpu/drm/sun4i/{sun4i_dotclock.c => sun4i_tcon_dclk.c} (99%)
rename drivers/gpu/drm/sun4i/{sun4i_dotclock.h => sun4i_tcon_dclk.h} (100%)
base-commit: 4aa35a0130d6b8afbefc9ef530a521fb0fb9b8e1
--
2.34.1
Hi Roman,
On 2023-04-18 at 09:40:01 +0200, Roman Beranek <me@crly.cz> wrote:
> According to Allwinner's BSP code, in DSI mode, TCON0 clock needs to be
> running at what's effectively the per-lane datarate of the DSI link.
> Given that the TCON DCLK divider is fixed to 4 (SUN6I_DSI_TCON_DIV),
> DCLK can't be set equal to the dotclock. Therefore labeling TCON DCLK
> as sun4i_dotclock or tcon-pixel-clock shall be avoided.
>
> With bpp bits per pixel transmitted over n DSI lanes, the target DCLK
> rate for a given pixel clock is obtained as follows:
>
> DCLK rate = 1/4 * bpp / n * pixel clock
>
> Effect of this change can be observed through the rate of Vblank IRQs
> which should now match refresh rate implied by set display mode. It
> was verified to do so on a A64 board with a 2-lane and a 4-lane panel.
>
> v2:
> 1. prevent reparent of tcon0 to pll-video0-2x
> 2. include pll-video0 in setting TCON0 DCLK rate
> 3. tested the whole thing also on a PinePhone
>
> Roman Beranek (7):
> clk: sunxi-ng: a64: propagate rate change from pll-mipi
> clk: sunxi-ng: a64: export PLL_MIPI
> clk: sunxi-ng: a64: prevent CLK_TCON0 being reparented
> arm64: dts: allwinner: a64: assign PLL_MIPI to CLK_TCON0
> ARM: dts: sunxi: rename tcon's clock output
> drm: sun4i: rename sun4i_dotclock to sun4i_tcon_dclk
> drm: sun4i: calculate proper DCLK rate for DSI
>
> arch/arm/boot/dts/sun5i.dtsi | 2 +-
> arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 +-
> arch/arm/boot/dts/sun8i-a83t.dtsi | 2 +-
> arch/arm/boot/dts/sun8i-v3s.dtsi | 2 +-
> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 4 +-
> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 6 ++-
> drivers/clk/sunxi-ng/ccu-sun50i-a64.h | 4 +-
> drivers/gpu/drm/sun4i/Makefile | 2 +-
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 46 +++++++++++--------
> .../{sun4i_dotclock.c => sun4i_tcon_dclk.c} | 2 +-
> .../{sun4i_dotclock.h => sun4i_tcon_dclk.h} | 0
> include/dt-bindings/clock/sun50i-a64-ccu.h | 1 +
> 12 files changed, 43 insertions(+), 30 deletions(-)
> rename drivers/gpu/drm/sun4i/{sun4i_dotclock.c => sun4i_tcon_dclk.c} (99%)
> rename drivers/gpu/drm/sun4i/{sun4i_dotclock.h => sun4i_tcon_dclk.h} (100%)
>
>
> base-commit: 4aa35a0130d6b8afbefc9ef530a521fb0fb9b8e1
I've tried your patches on my pinephone. I also set the panel's clock to
72 MHz, so at 24 bpp and 4 lanes that should result in a data clock of
108 MHz. This should be possible when pll-video0 is at 297 MHz.
Unfortunately, pll-video0 is not set and therefore the relevant part of
the clk_summary looks like this:
enable prepare protect hardware
clock count count count rate enable
------------------------------------------------------------------------
pll-video0 1 1 1 294000000 Y
hdmi 0 0 0 294000000 N
tcon1 0 0 0 294000000 N
pll-mipi 1 1 1 431200000 Y
tcon0 2 2 1 431200000 Y
tcon-data-clock 1 1 1 107800000 Y
pll-video0-2x 0 0 0 588000000 Y
Note, I've cut the columns accuracy, phase, and duty cycle, because they
show the same values for all clocks (0, 0, 50000).
My understanding was that with this patchset setting the parent clock
should be possible. Do you have any idea why it doesn't work on the
pinephone? Or maybe it does work on yours and I'm making some kind of
mistake?
On a brighter note, when I initialize pll-video0 to 297 MHz in
sunxi-ng/ccu-sun50i-a64.c:sun50i_a64_ccu_probe() I get an even 108 Mhz
for the data clock. The patch is:
writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
+ /*
+ * Initialize PLL VIDEO0 to default values (297 MHz)
+ * to clean up any changes made by bootloader
+ */
+ writel(0x03006207, reg + 0x10);
+
ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun50i_a64_ccu_desc);
if (ret)
return ret;
Best,
Frank
Maxime, Jernej, I was trying to understand why pll-video0 is not updated
and I tracked down the culprit to ccu_nkm.c.
On 2023-04-23 at 15:24:33 +0200, Frank Oltmanns <frank@oltmanns.dev> wrote:
> On 2023-04-18 at 09:40:01 +0200, Roman Beranek <me@crly.cz> wrote:
>> According to Allwinner's BSP code, in DSI mode, TCON0 clock needs to be
>> running at what's effectively the per-lane datarate of the DSI link.
>> Given that the TCON DCLK divider is fixed to 4 (SUN6I_DSI_TCON_DIV),
>> DCLK can't be set equal to the dotclock. Therefore labeling TCON DCLK
>> as sun4i_dotclock or tcon-pixel-clock shall be avoided.
>>
>> With bpp bits per pixel transmitted over n DSI lanes, the target DCLK
>> rate for a given pixel clock is obtained as follows:
>>
>> DCLK rate = 1/4 * bpp / n * pixel clock
>>
>> Effect of this change can be observed through the rate of Vblank IRQs
>> which should now match refresh rate implied by set display mode. It
>> was verified to do so on a A64 board with a 2-lane and a 4-lane panel.
[...]
> I've tried your patches on my pinephone. I also set the panel's clock to
> 72 MHz, so at 24 bpp and 4 lanes that should result in a data clock of
> 108 MHz. This should be possible when pll-video0 is at 297 MHz.
>
> Unfortunately, pll-video0 is not set and therefore the relevant part of
> the clk_summary looks like this:
>
> enable prepare protect hardware
> clock count count count rate enable
> ------------------------------------------------------------------------
> pll-video0 1 1 1 294000000 Y
> hdmi 0 0 0 294000000 N
> tcon1 0 0 0 294000000 N
> pll-mipi 1 1 1 431200000 Y
> tcon0 2 2 1 431200000 Y
> tcon-data-clock 1 1 1 107800000 Y
> pll-video0-2x 0 0 0 588000000 Y
>
> Note, I've cut the columns accuracy, phase, and duty cycle, because they
> show the same values for all clocks (0, 0, 50000).
>
> My understanding was that with this patchset setting the parent clock
> should be possible. Do you have any idea why it doesn't work on the
> pinephone? Or maybe it does work on yours and I'm making some kind of
> mistake?
To better understand what's going on I've extended the clk_rate_request
class to also output the requested rate. The relevant output is this
(leading line numbers by me for referencing the lines below):
line 1: kworker/u8:2-49 [002] ..... 1.850141: clk_rate_request_start: tcon-data-clock rate 108000000 min 0 max 18446744073709551615, parent tcon0 (588000000)
line 2: kworker/u8:2-49 [002] ..... 1.850149: clk_rate_request_start: tcon0 rate 432000000 min 0 max 18446744073709551615, parent pll-mipi (588000000)
line 3: kworker/u8:2-49 [002] ..... 1.850154: clk_rate_request_start: pll-mipi rate 432000000 min 0 max 18446744073709551615, parent pll-video0 (294000000)
line 4: kworker/u8:2-49 [002] ..... 1.850168: clk_rate_request_done: pll-mipi rate 431200000 min 0 max 18446744073709551615, parent pll-video0 (294000000)
line 5: kworker/u8:2-49 [002] ..... 1.850169: clk_rate_request_done: tcon0 rate 431200000 min 0 max 18446744073709551615, parent pll-mipi (431200000)
line 6: kworker/u8:2-49 [002] ..... 1.850171: clk_rate_request_done: tcon-data-clock rate 107800000 min 0 max 18446744073709551615, parent tcon0 (431200000)
line 7: kworker/u8:2-49 [002] ..... 1.850172: clk_rate_request_start: tcon-data-clock rate 108000000 min 0 max 18446744073709551615, parent tcon0 (588000000)
line 8: kworker/u8:2-49 [002] ..... 1.850174: clk_rate_request_start: tcon0 rate 432000000 min 0 max 18446744073709551615, parent pll-mipi (588000000)
line 9: kworker/u8:2-49 [002] ..... 1.850179: clk_rate_request_start: pll-mipi rate 432000000 min 0 max 18446744073709551615, parent pll-video0 (294000000)
line 10: kworker/u8:2-49 [002] ..... 1.850190: clk_rate_request_done: pll-mipi rate 431200000 min 0 max 18446744073709551615, parent pll-video0 (294000000)
line 11: kworker/u8:2-49 [002] ..... 1.850191: clk_rate_request_done: tcon0 rate 431200000 min 0 max 18446744073709551615, parent pll-mipi (431200000)
line 12: kworker/u8:2-49 [002] ..... 1.850192: clk_rate_request_done: tcon-data-clock rate 107800000 min 0 max 18446744073709551615, parent tcon0 (431200000)
line 13: kworker/u8:2-49 [002] ..... 1.850193: clk_rate_request_start: tcon0 rate 431200000 min 0 max 18446744073709551615, parent pll-mipi (588000000)
line 14: kworker/u8:2-49 [002] ..... 1.850195: clk_rate_request_start: pll-mipi rate 431200000 min 0 max 18446744073709551615, parent pll-video0 (294000000)
line 15: kworker/u8:2-49 [002] ..... 1.850205: clk_rate_request_done: pll-mipi rate 431200000 min 0 max 18446744073709551615, parent pll-video0 (294000000)
line 16: kworker/u8:2-49 [002] ..... 1.850206: clk_rate_request_done: tcon0 rate 431200000 min 0 max 18446744073709551615, parent pll-mipi (431200000)
line 17: kworker/u8:2-49 [002] ..... 1.850208: clk_rate_request_start: pll-mipi rate 431200000 min 0 max 18446744073709551615, parent pll-video0 (294000000)
line 18: kworker/u8:2-49 [002] ..... 1.850219: clk_rate_request_done: pll-mipi rate 431200000 min 0 max 18446744073709551615, parent pll-video0 (294000000)
line 19: kworker/u8:2-49 [002] ..... 1.850229: clk_set_rate: pll-mipi 431200000
line 20: kworker/u8:2-49 [003] ..... 1.850508: clk_set_rate_complete: pll-mipi 431200000
line 21: kworker/u8:2-49 [003] ..... 1.850513: clk_set_rate: tcon0 431200000
line 22: kworker/u8:2-49 [003] ..... 1.850515: clk_set_rate_complete: tcon0 431200000
line 23: kworker/u8:2-49 [003] ..... 1.850516: clk_set_rate: tcon-data-clock 107800000
line 24: kworker/u8:2-49 [003] ..... 1.850524: clk_set_rate_complete: tcon-data-clock 107800000
line 25: kworker/u8:2-49 [003] ..... 1.853320: clk_prepare: tcon-data-clock
line 26: kworker/u8:2-49 [003] ..... 1.853324: clk_prepare_complete: tcon-data-clock
line 27: kworker/u8:2-49 [003] d..1. 1.853328: clk_enable: tcon-data-clock
line 28: kworker/u8:2-49 [003] d..1. 1.853333: clk_enable_complete: tcon-data-clock
In line 1 we can see that a rate of 108 MHz is requested for
tcon-data-clock. In lines 2 and 3 this is forwarded to tcon0 and
pll-mipi (432 MHz). What surprised me, is that there is no request to
set the rate of pll-video0. Instead pll-mipi (and subsequently tcon0)
are set to 431.2 MHz (lines 4,5) and consequently tcon-data-clock is at
107.8 MHz (line 6) as I also reported in my previous mail (see quote
above).
When figuring out the call stack, I traced the whole thing down to
ccu_nkm_determine_rate(). The simplified call stack looks like this:
clk_set_rate(tcon-data-clock, 108MHz)
clk_core_set_rate_nolock(tcon-data-clock, 108MHz)
clk_core_req_round_rate_nolock(tcon-data-clock, 108MHz)
clk_core_round_rate_nolock(tcon-data-clock, 108MHz)
sun4i_dclk_round_rate(tcon-data-clock)
clk_hw_round_rate(tcon0, 432MHz)
clk_core_round_rate_nolock(tcon0, 432MHz)
clk_mux_determine_rate_flags(tcon0, 432MHz)
clk_core_round_rate_nolock(pll-mipi, 432MHz)
ccu_nkm_determine_rate(pll-mipi, 432MHz)
Looking at ccu_nkm_determine_rate(), we've found our culprit because it
does not try parent clock rates other than the current one. The same
applies to all other ccu_nkm_* functions.
So, I can see two options:
a. Set pll-video0 to 297 MHz on boot
b. Add functionality to ccu_nkm_* to also update the parent clock rate.
I'm actually interested in tackling b, but I can't make any promises as
to if and when I'll be able to solve it. I'm not certain about any side
effects this might have.
Until then, is option a acceptable in mainline?
Thanks,
Frank
>
> On a brighter note, when I initialize pll-video0 to 297 MHz in
> sunxi-ng/ccu-sun50i-a64.c:sun50i_a64_ccu_probe() I get an even 108 Mhz
> for the data clock. The patch is:
>
> writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
>
> + /*
> + * Initialize PLL VIDEO0 to default values (297 MHz)
> + * to clean up any changes made by bootloader
> + */
> + writel(0x03006207, reg + 0x10);
> +
> ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun50i_a64_ccu_desc);
> if (ret)
> return ret;
>
> Best,
> Frank
Hello everyone, I apologize for my absence from the discussion during past week, I got hit with tonsillitis. On Mon May 1, 2023 at 3:40 PM CEST, Frank Oltmanns wrote: > Looking at ccu_nkm_determine_rate(), we've found our culprit because it > does not try parent clock rates other than the current one. The same > applies to all other ccu_nkm_* functions. Yes, that's why I dropped CLK_SET_RATE_PARENT from pll-mipi in v3. > b. Add functionality to ccu_nkm_* to also update the parent clock rate. > > I'm actually interested in tackling b, but I can't make any promises as > to if and when I'll be able to solve it. I'm not certain about any side > effects this might have. It sounds like an interesting exercise. But what if HDMI is then added to the mix? Best regards Roman
Hello Roman,
On 2023-05-03 at 16:22:32 +0200, "Roman Beranek" <me@crly.cz> wrote:
> Hello everyone,
>
> I apologize for my absence from the discussion during past week, I got
> hit with tonsillitis.
I hope you feel better!
> On Mon May 1, 2023 at 3:40 PM CEST, Frank Oltmanns wrote:
>> Looking at ccu_nkm_determine_rate(), we've found our culprit because it
>> does not try parent clock rates other than the current one. The same
>> applies to all other ccu_nkm_* functions.
>
> Yes, that's why I dropped CLK_SET_RATE_PARENT from pll-mipi in v3.
>
>> b. Add functionality to ccu_nkm_* to also update the parent clock rate.
>>
>> I'm actually interested in tackling b, but I can't make any promises as
>> to if and when I'll be able to solve it. I'm not certain about any side
>> effects this might have.
>
> It sounds like an interesting exercise. But what if HDMI is then added
> to the mix?
Thanks for interest in this discussion! I really appreciate it!
First of all, let me admit that I'm no expert on this. But nobody else
has replied so far, and I want to keep this conversation going, so let
me share my view.
My understanding is that pll-mipi being able to set pll-video0's rate
should not have an impact on HDMI, neither positive nor negative. If I'm
not mistaken those two things are orthogonal.
The relevant part of the clk_summary with your v4 [1] patch on top of
drm-next looks like this:
enable protect hardware
clock count count rate enable
----------------------------------------------------------------------
pll-video0 1 1 294000000 Y
hdmi 0 0 294000000 N
tcon1 0 0 294000000 N
pll-mipi 1 1 431200000 Y
tcon0 2 1 431200000 Y
tcon-data-clock 1 1 107800000 Y
pll-video0-2x 0 0 588000000 Y
Note, that pll-video0 is protected.
I don't own any boards that support HDMI in mainline. For the pinephone
this support is added e.g. in megi's kernel, where connecting an HDMI
output results in pll-video0's rate being set to 297MHz, even though it
is 294MHz after boot.
So, for reference, this is the same part of the clk_summary with megi's
6.3.0 kernel, USB-C dock unplugged:
enable protect hardware
clock count count rate enable
----------------------------------------------------------------------
pll-video0 3 0 294000000 Y
hdmi-phy-clk 1 0 73500000 Y
hdmi 1 0 294000000 Y
tcon1 0 0 294000000 N
pll-mipi 1 0 431200000 Y
tcon0 2 0 431200000 Y
tcon-pixel-clock 1 0 107800000 Y
pll-video0-2x 0 0 588000000 Y
pll-video0 is not protected. When plugging in the USB-C dock with an HDMI
monitor connected, the situation looks like this:
enable protect hardware
clock count count rate enable
----------------------------------------------------------------------
pll-video0 4 1 297000000 Y
hdmi-phy-clk 1 0 148500000 Y
hdmi 1 0 148500000 Y
tcon1 1 1 148500000 Y
pll-mipi 1 0 424285714 Y
tcon0 2 0 424285714 Y
tcon-pixel-clock 1 0 106071428 Y
pll-video0-2x 0 0 594000000 Y
As you can see, pll-video0 is updated to 297 MHz. My understanding is
(again: not an expert here) this is only possible due to the missing
protection.
What I'm trying to say is that I don't see a connection between HDMI and
having the functionality in ccu_nkm_* to update the parent clock rate.
But I do think it would be preferable to have pll-video0 at 297 MHz
after boot on the pinephone. We could achieve this with my two previous
proposals:
a) Set pll-video0 to 297 MHz on boot
b) Add functionality to ccu_nkm_* to also update the parent clock rate.
If solution b is viable, the goal for the pinephone (and any other
boards supporting HDMI) would then be to select a pixel-data-clock so
that the rate for pll-video0 is set to 297 MHz (by selecting an
appropriate clock speed for the internal panel).
Maybe I'm misunderstanding something. If so, I'd appreciate any
corrections.
Thanks,
Frank
[1]: https://lore.kernel.org/all/20230505052110.67514-1-me@crly.cz/
>
> Best regards
> Roman
Hello again, On 2023-05-08 at 08:54:28 +0200, Frank Oltmanns <frank@oltmanns.dev> wrote: > Hello Roman, > > On 2023-05-03 at 16:22:32 +0200, "Roman Beranek" <me@crly.cz> wrote: >> Hello everyone, >> >> I apologize for my absence from the discussion during past week, I got >> hit with tonsillitis. > > I hope you feel better! > >> On Mon May 1, 2023 at 3:40 PM CEST, Frank Oltmanns wrote: >>> Looking at ccu_nkm_determine_rate(), we've found our culprit because it >>> does not try parent clock rates other than the current one. The same >>> applies to all other ccu_nkm_* functions. >> >> Yes, that's why I dropped CLK_SET_RATE_PARENT from pll-mipi in v3. >> >>> b. Add functionality to ccu_nkm_* to also update the parent clock rate. >>> >>> I'm actually interested in tackling b, but I can't make any promises as >>> to if and when I'll be able to solve it. I'm not certain about any side >>> effects this might have. >> >> It sounds like an interesting exercise. But what if HDMI is then added >> to the mix? > > Thanks for interest in this discussion! I really appreciate it! > > First of all, let me admit that I'm no expert on this. But nobody else > has replied so far, and I want to keep this conversation going, so let > me share my view. > > My understanding is that pll-mipi being able to set pll-video0's rate > should not have an impact on HDMI, neither positive nor negative. If I'm > not mistaken those two things are orthogonal. > > The relevant part of the clk_summary with your v4 [1] patch on top of > drm-next looks like this: > > enable protect hardware > clock count count rate enable > ---------------------------------------------------------------------- > pll-video0 1 1 294000000 Y > hdmi 0 0 294000000 N > tcon1 0 0 294000000 N > pll-mipi 1 1 431200000 Y > tcon0 2 1 431200000 Y > tcon-data-clock 1 1 107800000 Y > pll-video0-2x 0 0 588000000 Y > > Note, that pll-video0 is protected. > > I don't own any boards that support HDMI in mainline. For the pinephone > this support is added e.g. in megi's kernel, where connecting an HDMI > output results in pll-video0's rate being set to 297MHz, even though it > is 294MHz after boot. > > So, for reference, this is the same part of the clk_summary with megi's > 6.3.0 kernel, USB-C dock unplugged: > > enable protect hardware > clock count count rate enable > ---------------------------------------------------------------------- > pll-video0 3 0 294000000 Y > hdmi-phy-clk 1 0 73500000 Y > hdmi 1 0 294000000 Y > tcon1 0 0 294000000 N > pll-mipi 1 0 431200000 Y > tcon0 2 0 431200000 Y > tcon-pixel-clock 1 0 107800000 Y > pll-video0-2x 0 0 588000000 Y > > pll-video0 is not protected. When plugging in the USB-C dock with an HDMI > monitor connected, the situation looks like this: Just for reference, the protection count is disabled by this commit [1] in megi's kernel. In the commit message Icenowy Zheng refers to "the ability to keep TCON0 clock stable when HDMI changes its parent's clock." She implemented this in these two previous commits [2] [3]. None of this is in mainline. Best regards, Frank [1]: https://github.com/megous/linux/commit/039f7ee3f44adfbe4c6b7c2f1798b9a70c9fb9ee [2]: https://github.com/megous/linux/commit/a927843932f16e5a7f5ff57fbfd2d5f11c712b67 [3]: https://github.com/megous/linux/commit/0e305371eaa49128856acce9830e6af079442ad6 > > enable protect hardware > clock count count rate enable > ---------------------------------------------------------------------- > pll-video0 4 1 297000000 Y > hdmi-phy-clk 1 0 148500000 Y > hdmi 1 0 148500000 Y > tcon1 1 1 148500000 Y > pll-mipi 1 0 424285714 Y > tcon0 2 0 424285714 Y > tcon-pixel-clock 1 0 106071428 Y > pll-video0-2x 0 0 594000000 Y > > As you can see, pll-video0 is updated to 297 MHz. My understanding is > (again: not an expert here) this is only possible due to the missing > protection. > > What I'm trying to say is that I don't see a connection between HDMI and > having the functionality in ccu_nkm_* to update the parent clock rate. > > But I do think it would be preferable to have pll-video0 at 297 MHz > after boot on the pinephone. We could achieve this with my two previous > proposals: > > a) Set pll-video0 to 297 MHz on boot > b) Add functionality to ccu_nkm_* to also update the parent clock rate. > > If solution b is viable, the goal for the pinephone (and any other > boards supporting HDMI) would then be to select a pixel-data-clock so > that the rate for pll-video0 is set to 297 MHz (by selecting an > appropriate clock speed for the internal panel). > > Maybe I'm misunderstanding something. If so, I'd appreciate any > corrections. > > Thanks, > Frank > > [1]: https://lore.kernel.org/all/20230505052110.67514-1-me@crly.cz/ > >> >> Best regards >> Roman
Dne ponedeljek, 08. maj 2023 ob 16:08:32 CEST je Frank Oltmanns napisal(a): > Hello again, > > On 2023-05-08 at 08:54:28 +0200, Frank Oltmanns <frank@oltmanns.dev> wrote: > > Hello Roman, > > > > On 2023-05-03 at 16:22:32 +0200, "Roman Beranek" <me@crly.cz> wrote: > >> Hello everyone, > >> > >> I apologize for my absence from the discussion during past week, I got > >> hit with tonsillitis. > > > > I hope you feel better! > > > >> On Mon May 1, 2023 at 3:40 PM CEST, Frank Oltmanns wrote: > >>> Looking at ccu_nkm_determine_rate(), we've found our culprit because it > >>> does not try parent clock rates other than the current one. The same > >>> applies to all other ccu_nkm_* functions. > >> > >> Yes, that's why I dropped CLK_SET_RATE_PARENT from pll-mipi in v3. > >> > >>> b. Add functionality to ccu_nkm_* to also update the parent clock rate. > >>> > >>> I'm actually interested in tackling b, but I can't make any promises as > >>> to if and when I'll be able to solve it. I'm not certain about any side > >>> effects this might have. > >> > >> It sounds like an interesting exercise. But what if HDMI is then added > >> to the mix? > > > > Thanks for interest in this discussion! I really appreciate it! > > > > First of all, let me admit that I'm no expert on this. But nobody else > > has replied so far, and I want to keep this conversation going, so let > > me share my view. > > > > My understanding is that pll-mipi being able to set pll-video0's rate > > should not have an impact on HDMI, neither positive nor negative. If I'm > > not mistaken those two things are orthogonal. > > > > The relevant part of the clk_summary with your v4 [1] patch on top of > > > > drm-next looks like this: > > enable protect hardware > > > > clock count count rate enable > > > > ---------------------------------------------------------------------- > > > > pll-video0 1 1 294000000 Y > > > > hdmi 0 0 294000000 N > > tcon1 0 0 294000000 N > > pll-mipi 1 1 431200000 Y > > > > tcon0 2 1 431200000 Y > > > > tcon-data-clock 1 1 107800000 Y > > > > pll-video0-2x 0 0 588000000 Y > > > > Note, that pll-video0 is protected. > > > > I don't own any boards that support HDMI in mainline. For the pinephone > > this support is added e.g. in megi's kernel, where connecting an HDMI > > output results in pll-video0's rate being set to 297MHz, even though it > > is 294MHz after boot. > > > > So, for reference, this is the same part of the clk_summary with megi's > > > > 6.3.0 kernel, USB-C dock unplugged: > > enable protect hardware > > > > clock count count rate enable > > > > ---------------------------------------------------------------------- > > > > pll-video0 3 0 294000000 Y > > > > hdmi-phy-clk 1 0 73500000 Y > > hdmi 1 0 294000000 Y > > tcon1 0 0 294000000 N > > pll-mipi 1 0 431200000 Y > > > > tcon0 2 0 431200000 Y > > > > tcon-pixel-clock 1 0 107800000 Y > > > > pll-video0-2x 0 0 588000000 Y > > > > pll-video0 is not protected. When plugging in the USB-C dock with an HDMI > > > monitor connected, the situation looks like this: > Just for reference, the protection count is disabled by this commit [1] > in megi's kernel. > > In the commit message Icenowy Zheng refers to "the ability to keep TCON0 > clock stable when HDMI changes its parent's clock." She implemented this > in these two previous commits [2] [3]. None of this is in mainline. Those commits are good follow up series to this, if anyone wants to improve things further. Best regards, Jernej > > Best regards, > Frank > > [1]: > https://github.com/megous/linux/commit/039f7ee3f44adfbe4c6b7c2f1798b9a70c9f > b9ee [2]: > https://github.com/megous/linux/commit/a927843932f16e5a7f5ff57fbfd2d5f11c71 > 2b67 [3]: > https://github.com/megous/linux/commit/0e305371eaa49128856acce9830e6af07944 > 2ad6 > > enable protect hardware > > > > clock count count rate enable > > > > ---------------------------------------------------------------------- > > > > pll-video0 4 1 297000000 Y > > > > hdmi-phy-clk 1 0 148500000 Y > > hdmi 1 0 148500000 Y > > tcon1 1 1 148500000 Y > > pll-mipi 1 0 424285714 Y > > > > tcon0 2 0 424285714 Y > > > > tcon-pixel-clock 1 0 106071428 Y > > > > pll-video0-2x 0 0 594000000 Y > > > > As you can see, pll-video0 is updated to 297 MHz. My understanding is > > (again: not an expert here) this is only possible due to the missing > > protection. > > > > What I'm trying to say is that I don't see a connection between HDMI and > > having the functionality in ccu_nkm_* to update the parent clock rate. > > > > But I do think it would be preferable to have pll-video0 at 297 MHz > > after boot on the pinephone. We could achieve this with my two previous > > > > proposals: > > a) Set pll-video0 to 297 MHz on boot > > b) Add functionality to ccu_nkm_* to also update the parent clock rate. > > > > If solution b is viable, the goal for the pinephone (and any other > > boards supporting HDMI) would then be to select a pixel-data-clock so > > that the rate for pll-video0 is set to 297 MHz (by selecting an > > appropriate clock speed for the internal panel). > > > > Maybe I'm misunderstanding something. If so, I'd appreciate any > > corrections. > > > > Thanks, > > > > Frank > > > > [1]: https://lore.kernel.org/all/20230505052110.67514-1-me@crly.cz/ > > > >> Best regards > >> Roman
© 2016 - 2026 Red Hat, Inc.