linux-kernel - Re: [PATCH] arm64: dts: rockchip: Set RK3399-Gru PCLK

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD=FV=XO++XMrdZmupY8ED97W6AaU=H=sB2PN=6EpRxOOo4Afw@mail.gmail.com>
Date:   Tue, 30 Aug 2022 12:51:22 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Brian Norris <briannorris@...omium.org>
Cc:     Heiko Stuebner <heiko@...ech.de>, zain wang <wzz@...k-chips.com>,
        Lin Huang <hl@...k-chips.com>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arm64: dts: rockchip: Set RK3399-Gru PCLK_EDP to 24 MHz

Hi,

On Tue, Aug 30, 2022 at 12:32 PM Brian Norris <briannorris@...omium.org> wrote:
>
> We've found the AUX channel to be less reliable with PCLK_EDP at a
> higher rate (typically 25 MHz). This is especially important on systems
> with PSR-enabled panels (like Gru-Kevin), since we make heavy, constant
> use of AUX.
>
> According to Rockchip, using any rate other than 24 MHz can cause
> "problems between syncing the PHY an PCLK", which leads to all sorts of
> unreliabilities around register operations.
>
> Signed-off-by: zain wang <wzz@...k-chips.com>
> Signed-off-by: Brian Norris <briannorris@...omium.org>

nit: Sinze zain wang's SoB is first then I think either the patch
needs to be "From" zain wang or you need some different tagging, like
Co-Developed-by or something.

> ---
>
>  arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> index 45796b9fd94f..ee6095baba4d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> @@ -244,6 +244,14 @@ &dmc {
>  &edp {
>         status = "okay";
>
> +       /*
> +        * eDP PHY/clk don't sync reliably at anything other than 24 MHz. Only
> +        * set this here, because rk3399-gru.dtsi ensures we can generate this
> +        * off GPLL=600MHz, whereas some other RK3399 boards may not.
> +        */
> +       assigned-clocks = <&cru PCLK_EDP>;
> +       assigned-clock-rates = <24000000>;
> +

Yeah, this looks right to me. From my memory of the problem:

* For some reason, some devices hit it more than others (maybe just
process variance?)

* The problem affected nearly any register read or write on the eDP
controller and wasn't rk3399 specific (other Rockchip models were
impacted) but the failure rate was crazy low so it wasn't discovered
until rk3399 came about and we implemented PSR and the number of
register reads/writes to the eDP controller went through the roof.

* Most other devices use GPLL at 594 so that they can use it to make
certain standard HDMI rates. Changing other devices so GPLL = 600
wouldn't be trivial.

* It's fairly hard to change GPLL to 600 without coordinating with the
bootloader. Certainly it could be possible, but you'd have to make
sure that no important devices were impacted at boot when doing the
change. gru devices bootloaders are the only known ones to set GPLL to
600 MHz.

So:

Reviewed-by: Douglas Anderson <dianders@...omium.org>