| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87fs86q5h4.fsf@oltmanns.dev>
Date: Mon, 08 May 2023 16:08:32 +0200
From: Frank Oltmanns <frank@...manns.dev>
To: Roman Beranek <me@...y.cz>
Cc: Maxime Ripard <mripard@...nel.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Chen-Yu Tsai <wens@...e.org>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Samuel Holland <samuel@...lland.org>,
Ondrej Jirman <megi@....cz>, dri-devel@...ts.freedesktop.org,
linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
linux-kernel@...r.kernel.org, Icenowy Zheng <uwu@...nowy.me>
Subject: Re: [PATCH v2 0/7] drm: sun4i: set proper TCON0 DCLK rate in DSI mode
Hello again,
On 2023-05-08 at 08:54:28 +0200, Frank Oltmanns <frank@...manns.dev> wrote:
> Hello Roman,
>
> On 2023-05-03 at 16:22:32 +0200, "Roman Beranek" <me@...y.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
Powered by blists - more mailing lists