diff mbox series

[v2] arm64: dts: imx8mp: Add Hantro G1, G2 DT nodes

Message ID 20221218042056.328324-1-marex@denx.de (mailing list archive)
State New, archived
Headers show
Series [v2] arm64: dts: imx8mp: Add Hantro G1, G2 DT nodes | expand

Commit Message

Marek Vasut Dec. 18, 2022, 4:20 a.m. UTC 
Add DT nodes for the Hantro VPU found in i.MX8MP SoC.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Abel Vesa <abel.vesa@nxp.com>
Cc: Adam Ford <aford173@gmail.com>
Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Jacky Bai <ping.bai@nxp.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Marek Vasut <marex@denx.de>
Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
To: linux-arm-kernel@lists.infradead.org
---
V2: Drop the VC8000E
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 25 +++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Comments

Laurent Pinchart Dec. 18, 2022, 2:54 p.m. UTC | #1 
Hi Marek,

Thank you for the patch.

On Sun, Dec 18, 2022 at 05:20:56AM +0100, Marek Vasut wrote:
> Add DT nodes for the Hantro VPU found in i.MX8MP SoC.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Abel Vesa <abel.vesa@nxp.com>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Jacky Bai <ping.bai@nxp.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> To: linux-arm-kernel@lists.infradead.org
> ---
> V2: Drop the VC8000E
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 25 +++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 34c2c80c47eb5..d472170ee9ff1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1639,6 +1639,28 @@ gpu2d: gpu@38008000 {
>  			power-domains = <&pgc_gpu2d>;
>  		};
>  
> +		vpu_g1: video-codec@38300000 {
> +			compatible = "nxp,imx8mm-vpu-g1";
> +			reg = <0x38300000 0x10000>;
> +			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 = <700000000>;
> +			power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G1>;
> +		};
> +
> +		vpu_g2: video-codec@38310000 {
> +			compatible = "nxp,imx8mq-vpu-g2";
> +			reg = <0x38310000 0x10000>;
> +			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_VPU_PLL_OUT>;
> +			assigned-clock-rates = <700000000>;
> +			power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G2>;
> +		};
> +
>  		vpumix_blk_ctrl: blk-ctrl@38330000 {
>  			compatible = "fsl,imx8mp-vpu-blk-ctrl", "syscon";
>  			reg = <0x38330000 0x100>;
> @@ -1650,6 +1672,9 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
>  				 <&clk IMX8MP_CLK_VPU_G2_ROOT>,
>  				 <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
>  			clock-names = "g1", "g2", "vc8000e";
> +			assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>;
> +			assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> +			assigned-clock-rates = <700000000>;

Those three assigned-clock-rates all configure the same IMX8MP_VPU_PLL
clock. It's a bit redundant, and maybe we could instead configure
IMX8MP_VPU_PLL explicitly. This being said, I'm fine with this patch
as-is.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  			interconnects = <&noc IMX8MP_ICM_VPU_G1 &noc IMX8MP_ICN_VIDEO>,
>  					<&noc IMX8MP_ICM_VPU_G2 &noc IMX8MP_ICN_VIDEO>,
>  					<&noc IMX8MP_ICM_VPU_H1 &noc IMX8MP_ICN_VIDEO>;
Lucas Stach Dec. 19, 2022, 5:50 p.m. UTC | #2 
Am Sonntag, dem 18.12.2022 um 05:20 +0100 schrieb Marek Vasut:
> Add DT nodes for the Hantro VPU found in i.MX8MP SoC.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Abel Vesa <abel.vesa@nxp.com>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Jacky Bai <ping.bai@nxp.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> To: linux-arm-kernel@lists.infradead.org
> ---
> V2: Drop the VC8000E
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 25 +++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 34c2c80c47eb5..d472170ee9ff1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1639,6 +1639,28 @@ gpu2d: gpu@38008000 {
>  			power-domains = <&pgc_gpu2d>;
>  		};
>  
> +		vpu_g1: video-codec@38300000 {
> +			compatible = "nxp,imx8mm-vpu-g1";
> +			reg = <0x38300000 0x10000>;
> +			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 = <700000000>;

Where does this clock rate come from? G1 nominal rate is 600MHz,
overdrive rate is 800MHz. The general convention is to put the nominal
rate into the assigned clocks. Overdrive rates need proper OPP support
in the drivers and is not generally enabled right now.

> +			power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G1>;
> +		};
> +
> +		vpu_g2: video-codec@38310000 {
> +			compatible = "nxp,imx8mq-vpu-g2";
> +			reg = <0x38310000 0x10000>;
> +			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_VPU_PLL_OUT>;
> +			assigned-clock-rates = <700000000>;

Same comment as above G2 nominal rate is 500MHz. You might want to
source this from SYS_PLL2, instead of the VPU PLL.

Regards,
Lucas
> +			power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G2>;
> +		};
> +
>  		vpumix_blk_ctrl: blk-ctrl@38330000 {
>  			compatible = "fsl,imx8mp-vpu-blk-ctrl", "syscon";
>  			reg = <0x38330000 0x100>;
> @@ -1650,6 +1672,9 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
>  				 <&clk IMX8MP_CLK_VPU_G2_ROOT>,
>  				 <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
>  			clock-names = "g1", "g2", "vc8000e";
> +			assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>;
> +			assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> +			assigned-clock-rates = <700000000>;

VPU_BUS is the parent of VPU_ROOT and should be assigned in the
pgc_vpumix node, where it is used. Again nominal clock frequency
600MHz. But then maybe it would make sense to limit this to 500MHz for
now to be able to source it from a system PLL.

Regards,
Lucas
>  			interconnects = <&noc IMX8MP_ICM_VPU_G1 &noc IMX8MP_ICN_VIDEO>,
>  					<&noc IMX8MP_ICM_VPU_G2 &noc IMX8MP_ICN_VIDEO>,
>  					<&noc IMX8MP_ICM_VPU_H1 &noc IMX8MP_ICN_VIDEO>;
Marek Vasut Dec. 19, 2022, 7:15 p.m. UTC | #3 
On 12/19/22 18:50, Lucas Stach wrote:
> Am Sonntag, dem 18.12.2022 um 05:20 +0100 schrieb Marek Vasut:
>> Add DT nodes for the Hantro VPU found in i.MX8MP SoC.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Abel Vesa <abel.vesa@nxp.com>
>> Cc: Adam Ford <aford173@gmail.com>
>> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
>> Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>> Cc: Fabio Estevam <festevam@gmail.com>
>> Cc: Jacky Bai <ping.bai@nxp.com>
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
>> Cc: NXP Linux Team <linux-imx@nxp.com>
>> Cc: Peng Fan <peng.fan@nxp.com>
>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>> Cc: Richard Cochran <richardcochran@gmail.com>
>> Cc: Richard Zhu <hongxing.zhu@nxp.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> To: linux-arm-kernel@lists.infradead.org
>> ---
>> V2: Drop the VC8000E
>> ---
>>   arch/arm64/boot/dts/freescale/imx8mp.dtsi | 25 +++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>> index 34c2c80c47eb5..d472170ee9ff1 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>> @@ -1639,6 +1639,28 @@ gpu2d: gpu@38008000 {
>>   			power-domains = <&pgc_gpu2d>;
>>   		};
>>   
>> +		vpu_g1: video-codec@38300000 {
>> +			compatible = "nxp,imx8mm-vpu-g1";
>> +			reg = <0x38300000 0x10000>;
>> +			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 = <700000000>;
> 
> Where does this clock rate come from? G1 nominal rate is 600MHz,

NXP downstream, see:

https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8mp.dtsi?h=lf-5.15.y#n2231

[...]
Lucas Stach Dec. 20, 2022, 9:07 a.m. UTC | #4 
Am Montag, dem 19.12.2022 um 20:15 +0100 schrieb Marek Vasut:
> On 12/19/22 18:50, Lucas Stach wrote:
> > Am Sonntag, dem 18.12.2022 um 05:20 +0100 schrieb Marek Vasut:
> > > Add DT nodes for the Hantro VPU found in i.MX8MP SoC.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > ---
> > > Cc: Abel Vesa <abel.vesa@nxp.com>
> > > Cc: Adam Ford <aford173@gmail.com>
> > > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> > > Cc: Fabio Estevam <festevam@gmail.com>
> > > Cc: Jacky Bai <ping.bai@nxp.com>
> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Cc: Lucas Stach <l.stach@pengutronix.de>
> > > Cc: Marek Vasut <marex@denx.de>
> > > Cc: Markus Niebel <Markus.Niebel@ew.tq-group.com>
> > > Cc: NXP Linux Team <linux-imx@nxp.com>
> > > Cc: Peng Fan <peng.fan@nxp.com>
> > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> > > Cc: Richard Cochran <richardcochran@gmail.com>
> > > Cc: Richard Zhu <hongxing.zhu@nxp.com>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Shawn Guo <shawnguo@kernel.org>
> > > To: linux-arm-kernel@lists.infradead.org
> > > ---
> > > V2: Drop the VC8000E
> > > ---
> > >   arch/arm64/boot/dts/freescale/imx8mp.dtsi | 25 +++++++++++++++++++++++
> > >   1 file changed, 25 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > index 34c2c80c47eb5..d472170ee9ff1 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > @@ -1639,6 +1639,28 @@ gpu2d: gpu@38008000 {
> > >   			power-domains = <&pgc_gpu2d>;
> > >   		};
> > >   
> > > +		vpu_g1: video-codec@38300000 {
> > > +			compatible = "nxp,imx8mm-vpu-g1";
> > > +			reg = <0x38300000 0x10000>;
> > > +			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 = <700000000>;
> > 
> > Where does this clock rate come from? G1 nominal rate is 600MHz,
> 
> NXP downstream, see:
> 
> https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8mp.dtsi?h=lf-5.15.y#n2231

Nope, as you can see, downstream is running all clocks at the overdrive
rates: G1 at 800MHz, G2 at 700MHz, bus at 800MHz. That is quite
different from all 3 clocks at 700MHz from your patch.

Regards,
Lucas
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 34c2c80c47eb5..d472170ee9ff1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1639,6 +1639,28 @@  gpu2d: gpu@38008000 {
 			power-domains = <&pgc_gpu2d>;
 		};
 
+		vpu_g1: video-codec@38300000 {
+			compatible = "nxp,imx8mm-vpu-g1";
+			reg = <0x38300000 0x10000>;
+			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 = <700000000>;
+			power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G1>;
+		};
+
+		vpu_g2: video-codec@38310000 {
+			compatible = "nxp,imx8mq-vpu-g2";
+			reg = <0x38310000 0x10000>;
+			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_VPU_PLL_OUT>;
+			assigned-clock-rates = <700000000>;
+			power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G2>;
+		};
+
 		vpumix_blk_ctrl: blk-ctrl@38330000 {
 			compatible = "fsl,imx8mp-vpu-blk-ctrl", "syscon";
 			reg = <0x38330000 0x100>;
@@ -1650,6 +1672,9 @@  vpumix_blk_ctrl: blk-ctrl@38330000 {
 				 <&clk IMX8MP_CLK_VPU_G2_ROOT>,
 				 <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
 			clock-names = "g1", "g2", "vc8000e";
+			assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>;
+			assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
+			assigned-clock-rates = <700000000>;
 			interconnects = <&noc IMX8MP_ICM_VPU_G1 &noc IMX8MP_ICN_VIDEO>,
 					<&noc IMX8MP_ICM_VPU_G2 &noc IMX8MP_ICN_VIDEO>,
 					<&noc IMX8MP_ICM_VPU_H1 &noc IMX8MP_ICN_VIDEO>;