linux-kernel - Re: [PATCH 4/4] arm64: dts: imx8mp: Configure VPU clocks for overdrive

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250604065435.nl3l6h3wxrwstswl@pengutronix.de>
Date: Wed, 4 Jun 2025 08:54:35 +0200
From: Marco Felsch <m.felsch@...gutronix.de>
To: Adam Ford <aford173@...il.com>
Cc: linux-arm-kernel@...ts.infradead.org, aford@...conembedded.com,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, devicetree@...r.kernel.org,
	imx@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] arm64: dts: imx8mp: Configure VPU clocks for
 overdrive

On 25-06-03, Adam Ford wrote:
> On Mon, Jun 2, 2025 at 10:35 AM Adam Ford <aford173@...il.com> wrote:
> >
> > On Sun, Jun 1, 2025 at 1:36 PM Marco Felsch <m.felsch@...gutronix.de> wrote:
> > >
> > > Hi Adam,
> > >
> > > thanks for the patch.
> > >
> > > On 25-05-30, Adam Ford wrote:
> > > > The defaults for this SoC are configured for overdrive mode, but
> > > > the VPU clocks are currently configured for nominal mode.
> > > > Increase VPU_G1_CLK_ROOT to 800MHZ from 600MHz,
> > > > Increase VPU_G2_CLK_ROOT to 700MHZ from 500MHz, and
> > > > Increase VPU_BUS_CLK_ROOT to 800MHz from 600MHz.
> > > >
> > > > This requires adjusting the clock parents. Since there is already
> > > > 800MHz clock references, move the VPU_BUS and G1 clocks to it.
> > > > This frees up the VPU_PLL to be configured at 700MHz to run
> > > > the G2 clock at 700MHz.
> > > >
> > > > Signed-off-by: Adam Ford <aford173@...il.com>
> > > > ---
> > > >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 12 ++++++------
> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > index 909555a5da4b..848b25c9b752 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > @@ -2256,8 +2256,8 @@ vpu_g1: video-codec@...00000 {
> > > >                       interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > > >                       clocks = <&clk IMX8MP_CLK_VPU_G1_ROOT>;
> > > >                       assigned-clocks = <&clk IMX8MP_CLK_VPU_G1>;
> > > > -                     assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> > > > -                     assigned-clock-rates = <600000000>;
> > > > +                     assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>;
> > > > +                     assigned-clock-rates = <800000000>;
> > > >                       power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G1>;
> > > >               };
> > > >
> > > > @@ -2267,8 +2267,8 @@ vpu_g2: video-codec@...10000 {
> > > >                       interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> > > >                       clocks = <&clk IMX8MP_CLK_VPU_G2_ROOT>;
> > > >                       assigned-clocks = <&clk IMX8MP_CLK_VPU_G2>;
> > > > -                     assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>;
> > > > -                     assigned-clock-rates = <500000000>;
> > > > +                     assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> > > > +                     assigned-clock-rates = <700000000>;
> > > >                       power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G2>;
> > > >               };
> > > >
> > > > @@ -2284,8 +2284,8 @@ vpumix_blk_ctrl: blk-ctrl@...30000 {
> > > >                                <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
> > > >                       clock-names = "g1", "g2", "vc8000e";
> > > >                       assigned-clocks = <&clk IMX8MP_VPU_PLL>, <&clk IMX8MP_CLK_VPU_BUS>;
> > > > -                     assigned-clock-parents = <0>, <&clk IMX8MP_VPU_PLL_OUT>;
> > > > -                     assigned-clock-rates = <600000000>, <600000000>;
> > > > +                     assigned-clock-parents = <0>, <&clk IMX8MP_SYS_PLL1_800M>;
> > > > +                     assigned-clock-rates = <700000000>, <800000000>;
> > >
> > > I think we can drop the "assigned-clocks = <&clk IMX8MP_VPU_PLL>" part
> > > completely.
> > >
> > > Before your patch the IMX8MP_VPU_PLL_OUT was used as clock-parent for
> > > the IMX8MP_CLK_VPU_BUS. With yout patch IMX8MP_SYS_PLL1_800M is used.
> >
> > I think you're right.  I'll fix that up and do a V2.  I forgot to add
> > my own s-o-b tag, so I'll fix that too.  I'll try to do it this week.
> 
> Marco,
> 
> I tried removing the assigned-clock references to the IMX8MP_VPU_PLL,
> but i found the defaul speed was 800MHz for the PLL even when asking
> G2 to be 700MHz.  The only way that I found to set the G2 to 700MHz
> was to set the VPU_PLL to 700MHz.  I can do that either inside the G2
> node or the vpumix_blk_ctrl node. I generally like to have the
> blk_ctrl do it since it controls the G1_ROOT and G2_ROOT clocks, but
> it doesn't seem to hurt it being inside the G2 node either.  I can see
> the argument for having it insde the G2 node, since the G2 clock is
> what's requiring the VPU_PLL to be 700MHz.
> 
> Let me know your preferences, and I'll move forward accordingly.

Hi Adam,

thanks for testing. IMHO we should do the clock handling within the G2
node. This requires no comments because the code would be obvious else
you need to comment the setup of the IMX8MP_VPU_PLL within the
vpumix_blk_ctrl node because this is not obvious.

Regards,
  Marco