Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 29 Sep 2021 14:44:34 +0300
From:   Abel Vesa <abel.vesa@nxp.com>
To:     Martin Kepplinger <martin.kepplinger@puri.sm>
Cc:     a.fatoum@pengutronix.de, adrian.hunter@intel.com,
        aisheng.dong@nxp.com, catalin.marinas@arm.com,
        cw00.choi@samsung.com, devicetree@vger.kernel.org,
        djakov@kernel.org, festevam@gmail.com, kernel@pengutronix.de,
        kyungmin.park@samsung.com, linux-arm-kernel@lists.infradead.org,
        linux-imx@nxp.com, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org, linux-serial@vger.kernel.org,
        myungjoo.ham@samsung.com, robh@kernel.org, s.hauer@pengutronix.de,
        shawnguo@kernel.org, ulf.hansson@linaro.org, will.deacon@arm.com
Subject: Re: [RFC 00/19] Add interconnect and devfreq support for i.MX8MQ
Message-ID: <YVRRomS8ut6NjYlV@ryzen>
References: <1631554694-9599-1-git-send-email-abel.vesa@nxp.com>
 <20210924102026.2679952-1-martin.kepplinger@puri.sm>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210924102026.2679952-1-martin.kepplinger@puri.sm>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?RAda+RRIROAS4jSLcA9vdIkml1Zk/Csu+xBn5FITbebiQLsrscT22VW4z1iI?=
 =?us-ascii?Q?9N5bW7tYJfVsOeZk2bRAqUTNkmWGqk4jTaqhiBBWXtfZCaHh+MZykSPI5tWd?=
 =?us-ascii?Q?x8rK6yEr6NpEQNidTClSNvLMTLgwMtQaLihHImDjUN/UXkEh9Buqa2zGDFq4?=
 =?us-ascii?Q?EgLtdln/EdTVtH1Y28zdse6OOmbo+ApM4I9TEMZBYLtdbGcVYnN5DCaQVqAJ?=
 =?us-ascii?Q?cpqImSX1maGP2i6gsDau+ACZpqyeYGcSXAi4o/KZG3ptoSrf0g0DLUoI2FI/?=
 =?us-ascii?Q?ED0SEgSxOuKBBHiTbMDc2G5E+DJuXsDk/Wsp7BK18XYvnxOjZ4zv+aBuSLAz?=
 =?us-ascii?Q?gWBI/XUSl/JBmv2qpxUSSmccIF5zrBi8KHa1awEagYCVngjv1ynAN9z84uo7?=
 =?us-ascii?Q?u4ObDUaWuY8IxcIe9FoF1sq9yal5teqWPtxYtSY5LpmmCCnWOzEnq3/bvxa/?=
 =?us-ascii?Q?i46uAPNAWBY65NiN+aXt6jnPrRPlge0ptlj93oeomU62dP/IaBje6PvZ8T2M?=
 =?us-ascii?Q?uqZFZRd6CkwP5VX9OsozF8sYT2BC/Gu5LUJ1AM/8hfm5wOWjAr0h+2B3gyGk?=
 =?us-ascii?Q?RQw0uq8aFtuuCBoajpqUgae6KSiRpQww1tvyQEXTQ5Wn6PrCsq97yUGIBvv4?=
 =?us-ascii?Q?ytxNhnBCEQz5/ujAiflhYvpmrMNQXSR3AQvdOhOt4GzkVBuHPssnXox044OG?=
 =?us-ascii?Q?4FVZUAKASNHB1nIuofgstdgo8n/8FyrL1+oBJgFqTJv1wDv23nYK+YqmO/Bk?=
 =?us-ascii?Q?2355rWuxTsla+PdKzM0M1dW2Hv9rv2YqTXMZtm33fbaIQSjWfRq3Nw6jdc1b?=
 =?us-ascii?Q?tZsJBkrs3rS9Jbcfw0VBVufVJ6vDW5Mv5guJ3vlvaU6DTs3QmT+OIFST4a3N?=
 =?us-ascii?Q?ct6e4m5/uZjBwD7m9DClGQc9pE7XSEsn4mOcnas23XjbG4UyddBymhBELnSC?=
 =?us-ascii?Q?1eAeEpeZZniuxBQQCZSSwYJTke0bUQRVHjCyds2L76xLBDwVfpA1Qw6p8zVK?=
 =?us-ascii?Q?+beeLCgvHSARatDlMquqM4AK/2A+B2+GG0jubz24wTJSIcukbrsfhpzTTIkx?=
 =?us-ascii?Q?Ng4rjCz9dW4RRJ/hnm3g3bQ/5d+IlOmsLhtbI/yLt/9byZeUtYN+oVieTv3R?=
 =?us-ascii?Q?Z+e1nqVoBzd73xfjwnYF/QwpfKmFmatie1TSsnlB0wxDxKcdGMwyOp+gVY/O?=
 =?us-ascii?Q?lj6o9NY2VZ8LeJ4SJiTg9CrH+EbRyuGcv8ry6kUYkJ3DCs8y33FUjLIQdENp?=
 =?us-ascii?Q?lrD6Wevepdxwaq3qDCfKXJIJpgBZYsCuummgiy2noPXgMFKEovV0v5+0Beb0?=
 =?us-ascii?Q?ETNJDx7hUQrn/LHAIu1xeg/c?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7361a64d-33fe-43af-a933-08d9833e836a
X-MS-Exchange-CrossTenant-AuthSource: VI1PR0401MB2559.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Sep 2021 11:44:36.9880
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gaPN9rKY/gywWKk1YiSYxp2+QCYyAS1TJxglF24cB0n11F7Sevn/8Yjk1sc54UMusUmVK7t4tQjg2q4uqgFacA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB5263
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 21-09-24 12:20:26, Martin Kepplinger wrote:
> hi Abel,
> 
> thank you for the update (this is actually v2 of this RFC right?)!
> 
> all in all this runs fine on the imx8mq (Librem 5 and devkit) I use. For all
> the pl301 nodes I'm not yet sure what I can actually test / switch frequencies.
> 

You can start by looking into each of the following:

 $ ls -1d /sys/devices/platform/soc@0/*/devfreq/*/trans_stat

and look if the transitions happen when a specific driver that is a icc user suspends.

You can also look at:

 /sys/kernel/debug/interconnect/interconnect_summary 

and:

 /sys/kernel/debug/interconnect/interconnect_graph

> But I still have one problem: lcdif/mxfb already has the interconnect dram
> DT property and I use the following call to request bandwidth:
> https://source.puri.sm/martin.kepplinger/linux-next/-/commit/d690e4c021293f938eb2253607f92f5a64f15688
> (mainlining this is on our todo list).
> 
> With your patchset, I get:
> 
> [    0.792960] genirq: Flags mismatch irq 30. 00000004 (mxsfb-drm) vs. 00000004 (mxsfb-drm)
> [    0.801143] mxsfb 30320000.lcd-controller: Failed to install IRQ handler
> [    0.808058] mxsfb: probe of 30320000.lcd-controller failed with error -16
> 
> so the main devfreq user (mxsfb) is not there :) why?
> 

OK, I admit, this patchset doesn't provide support for all the icc consumer drivers.
But that should come at a later stage. I only provided example like fec and usdhc, to show
how it all fits together.

> and when I remove the interconnect property from the lcdif DT node, mxsfb
> probes again, but of course it doesn't lower dram freq as needed.
> 
> Do I do the icc calls wrong in mxsfb despite it working without your
> patchset, or may there be something wrong on your side that breaks
> the mxsfb IRQ?
> 

Do you have the following changes into your tree?

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi               
index 00dd8e39a595..c43a84622af5 100644                                                                          
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi                                                                  
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi                                                                  
@@ -524,7 +524,7 @@ lcdif: lcd-controller@30320000 {                                                             
                                                  <&clk IMX8MQ_VIDEO_PLL1>,                                      
                                                  <&clk IMX8MQ_VIDEO_PLL1_OUT>;                                  
                                assigned-clock-rates = <0>, <0>, <0>, <594000000>;                               
-                               interconnects = <&noc IMX8MQ_ICM_LCDIF &noc IMX8MQ_ICS_DRAM>;                    
+                               interconnects = <&icc IMX8MQ_ICM_LCDIF &icc IMX8MQ_ICS_DRAM>;                    
                                interconnect-names = "dram";                                                     
                                status = "disabled";                                                             
                                                                                                                 
@@ -1117,7 +1117,7 @@ mipi_csi1: csi@30a70000 {                                                                  
                                         <&src IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET>,                            
                                         <&src IMX8MQ_RESET_MIPI_CSI1_ESC_RESET>;                                
                                fsl,mipi-phy-gpr = <&iomuxc_gpr 0x88>;                                           
-                               interconnects = <&noc IMX8MQ_ICM_CSI1 &noc IMX8MQ_ICS_DRAM>;                     
+                               interconnects = <&icc IMX8MQ_ICM_CSI1 &icc IMX8MQ_ICS_DRAM>;                     
                                interconnect-names = "dram";                                                     
                                status = "disabled";                                                             
                                                                                                                 
@@ -1169,7 +1169,7 @@ mipi_csi2: csi@30b60000 {                                                                  
                                         <&src IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET>,                            
                                         <&src IMX8MQ_RESET_MIPI_CSI2_ESC_RESET>;                                
                                fsl,mipi-phy-gpr = <&iomuxc_gpr 0xa4>;                                           
-                               interconnects = <&noc IMX8MQ_ICM_CSI2 &noc IMX8MQ_ICS_DRAM>;                     
+                               interconnects = <&icc IMX8MQ_ICM_CSI2 &icc IMX8MQ_ICS_DRAM>;                     
                                interconnect-names = "dram";                                                     
                                status = "disabled";                                                             

I forgot to update these in the current version of the patchset. Will do in the next version.

Also, would help a lot if you could give me a link to a tree you're testing with.
That way I can look exactly at what's going on.

> again thanks a lot for working on this! I'm always happy to test.
> 
>                           martin
> 
> 
> 
> ---
>  .../boot/dts/freescale/imx8mq-librem5.dtsi    | 20 -------------------
>  1 file changed, 20 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> index 6fac6676f412..8496a90f23bf 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> @@ -381,26 +381,6 @@ &A53_3 {
>  	cpu-supply = <&buck2_reg>;
>  };
>  
> -&ddrc {
> -	operating-points-v2 = <&ddrc_opp_table>;
> -
> -	ddrc_opp_table: ddrc-opp-table {
> -		compatible = "operating-points-v2";
> -
> -		opp-25M {
> -			opp-hz = /bits/ 64 <25000000>;
> -		};
> -
> -		opp-100M {
> -			opp-hz = /bits/ 64 <100000000>;
> -		};
> -
> -		opp-800M {
> -			opp-hz = /bits/ 64 <800000000>;
> -		};
> -	};
> -};
> -
>  &dphy {
>  	status = "okay";
>  };
> -- 
> 2.30.2
>