[RFC 00/19] Add interconnect and devfreq support for i.MX8MQ

[RFC 00/19] Add interconnect and devfreq support for i.MX8MQ

[Abel Vesa]

In vendor tree there is something called busfreq but it is
non-upstreamable. This approach with interconnect+devfreq gives us the
same functionality by using already upstream and mature subsystems.
This series is more of a proof-of-concept.

Abel Vesa (19):
  dt-bindings: interconnect: imx8mq: Add missing pl301 and SAI ids
  devfreq: imx-bus: Switch governor to powersave
  devfreq: imx-bus: Decouple imx-bus from icc made
  devfreq: imx8m-ddrc: Change governor to powersave
  devfreq: imx8m-ddrc: Use the opps acquired from EL3
  devfreq: imx8m-ddrc: Add late system sleep PM ops
  interconnect: imx: Switch from imx_icc_node_adj_desc to fsl,icc-id
    node assignment
  interconnect: imx8: Remove the imx_icc_node_adj_desc
  interconnect: imx8mq: Add the pl301_per_m and pl301_wakeup nodes and
    subnodes
  interconnect: imx8mq: Add of_match_table
  interconnect: imx: Add imx_icc_get_bw and imx_icc_aggregate functions
  arm64: dts: imx8mq: Add fsl,icc-id property to ddrc node
  arm64: dts: imx8mq: Add fsl,icc-id to noc node
  arm64: dts: imx8mq: Add all pl301 nodes
  arm64: dts: imx8mq: Add the interconnect node
  arm64: dts: imx8mq: Add interconnect properties to icc consumer nodes
  net: ethernet: fec_main: Add interconnect support
  mmc: sdhci-esdhc-imx: Add interconnect support
  arm64: defconfig: Add necessary configs for icc+devfreq on i.MX8MQ

 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 222 +++++++++++++++++++++-
 arch/arm64/configs/defconfig              |   9 +-
 drivers/devfreq/imx-bus.c                 |  42 +---
 drivers/devfreq/imx8m-ddrc.c              |  78 +++-----
 drivers/interconnect/imx/imx.c            |  92 +++++----
 drivers/interconnect/imx/imx.h            |  19 +-
 drivers/interconnect/imx/imx8mm.c         |  32 +---
 drivers/interconnect/imx/imx8mn.c         |  28 +--
 drivers/interconnect/imx/imx8mq.c         |  59 +++---
 drivers/mmc/host/sdhci-esdhc-imx.c        |  27 +++
 drivers/net/ethernet/freescale/fec.h      |   3 +
 drivers/net/ethernet/freescale/fec_main.c |  19 ++
 include/dt-bindings/interconnect/imx8mq.h |   9 +
 13 files changed, 422 insertions(+), 217 deletions(-)

-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 01/19] dt-bindings: interconnect: imx8mq: Add missing pl301 and SAI ids

[Abel Vesa]

According to the bus diagram, there are two more pl301s that need to
be added here. The pl301_per_m which is an intermediary node between
pl301_main and its masters: usdhc1, usdhc2 and sdma. The pl301_wakeup
is an intermediary node between pl301_main and its masters, in this case
all the SAIs.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 include/dt-bindings/interconnect/imx8mq.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/dt-bindings/interconnect/imx8mq.h b/include/dt-bindings/interconnect/imx8mq.h
index 1a4cae7f8be2..1953de8af5cb 100644
--- a/include/dt-bindings/interconnect/imx8mq.h
+++ b/include/dt-bindings/interconnect/imx8mq.h
@@ -44,5 +44,14 @@
 #define IMX8MQ_ICM_PCIE1	26
 #define IMX8MQ_ICM_PCIE2	27
 #define IMX8MQ_ICN_MAIN		28
+#define IMX8MQ_ICN_PER_M	30
+
+#define IMX8MQ_ICN_WAKEUP	31
+#define IMX8MQ_ICM_SAI1		32
+#define IMX8MQ_ICM_SAI2		33
+#define IMX8MQ_ICM_SAI3		34
+#define IMX8MQ_ICM_SAI4		35
+#define IMX8MQ_ICM_SAI5		36
+#define IMX8MQ_ICM_SAI6		37
 
 #endif /* __DT_BINDINGS_INTERCONNECT_IMX8MQ_H */
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 01/19] dt-bindings: interconnect: imx8mq: Add missing pl301 and SAI ids

[Georgi Djakov]

Hi Abel,

On 13.09.21 20:37, Abel Vesa wrote:
> According to the bus diagram, there are two more pl301s that need to
> be added here. The pl301_per_m which is an intermediary node between
> pl301_main and its masters: usdhc1, usdhc2 and sdma. The pl301_wakeup
> is an intermediary node between pl301_main and its masters, in this case
> all the SAIs.

Thanks for working on this!

> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>   include/dt-bindings/interconnect/imx8mq.h | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/include/dt-bindings/interconnect/imx8mq.h b/include/dt-bindings/interconnect/imx8mq.h
> index 1a4cae7f8be2..1953de8af5cb 100644
> --- a/include/dt-bindings/interconnect/imx8mq.h
> +++ b/include/dt-bindings/interconnect/imx8mq.h
> @@ -44,5 +44,14 @@
>   #define IMX8MQ_ICM_PCIE1	26
>   #define IMX8MQ_ICM_PCIE2	27
>   #define IMX8MQ_ICN_MAIN		28
> +#define IMX8MQ_ICN_PER_M	30

Is there any reason to jump from 28 to 30?

Thanks,
Georgi

> +
> +#define IMX8MQ_ICN_WAKEUP	31
> +#define IMX8MQ_ICM_SAI1		32
> +#define IMX8MQ_ICM_SAI2		33
> +#define IMX8MQ_ICM_SAI3		34
> +#define IMX8MQ_ICM_SAI4		35
> +#define IMX8MQ_ICM_SAI5		36
> +#define IMX8MQ_ICM_SAI6		37
>   
>   #endif /* __DT_BINDINGS_INTERCONNECT_IMX8MQ_H */
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 02/19] devfreq: imx-bus: Switch governor to powersave

[Abel Vesa]

By switching to powersave governor, we allow the imx-bus to always run
at minimum rate needed by all the running masters.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/devfreq/imx-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
index f3f6e25053ed..9d8dc9824d21 100644
--- a/drivers/devfreq/imx-bus.c
+++ b/drivers/devfreq/imx-bus.c
@@ -87,7 +87,7 @@ static int imx_bus_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct imx_bus *priv;
-	const char *gov = DEVFREQ_GOV_USERSPACE;
+	const char *gov = DEVFREQ_GOV_POWERSAVE;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 03/19] devfreq: imx-bus: Decouple imx-bus from icc made

[Abel Vesa]

The link between an imx-bus device and its icc id will be done
through the fsl,icc-id property in each dts node. The imx
interconnect driver will pick up all the dts nodes that have that
property defined and will link them to the rightfull icc id.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/devfreq/imx-bus.c | 40 +++------------------------------------
 1 file changed, 3 insertions(+), 37 deletions(-)

diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
index 9d8dc9824d21..95339de8cccf 100644
--- a/drivers/devfreq/imx-bus.c
+++ b/drivers/devfreq/imx-bus.c
@@ -53,36 +53,6 @@ static void imx_bus_exit(struct device *dev)
 	platform_device_unregister(priv->icc_pdev);
 }
 
-/* imx_bus_init_icc() - register matching icc provider if required */
-static int imx_bus_init_icc(struct device *dev)
-{
-	struct imx_bus *priv = dev_get_drvdata(dev);
-	const char *icc_driver_name;
-
-	if (!of_get_property(dev->of_node, "#interconnect-cells", 0))
-		return 0;
-	if (!IS_ENABLED(CONFIG_INTERCONNECT_IMX)) {
-		dev_warn(dev, "imx interconnect drivers disabled\n");
-		return 0;
-	}
-
-	icc_driver_name = of_device_get_match_data(dev);
-	if (!icc_driver_name) {
-		dev_err(dev, "unknown interconnect driver\n");
-		return 0;
-	}
-
-	priv->icc_pdev = platform_device_register_data(
-			dev, icc_driver_name, -1, NULL, 0);
-	if (IS_ERR(priv->icc_pdev)) {
-		dev_err(dev, "failed to register icc provider %s: %ld\n",
-				icc_driver_name, PTR_ERR(priv->icc_pdev));
-		return PTR_ERR(priv->icc_pdev);
-	}
-
-	return 0;
-}
-
 static int imx_bus_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -130,10 +100,6 @@ static int imx_bus_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	ret = imx_bus_init_icc(dev);
-	if (ret)
-		goto err;
-
 	return 0;
 
 err:
@@ -142,9 +108,9 @@ static int imx_bus_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id imx_bus_of_match[] = {
-	{ .compatible = "fsl,imx8mq-noc", .data = "imx8mq-interconnect", },
-	{ .compatible = "fsl,imx8mm-noc", .data = "imx8mm-interconnect", },
-	{ .compatible = "fsl,imx8mn-noc", .data = "imx8mn-interconnect", },
+	{ .compatible = "fsl,imx8mq-noc",},
+	{ .compatible = "fsl,imx8mm-noc",},
+	{ .compatible = "fsl,imx8mn-noc",},
 	{ .compatible = "fsl,imx8m-noc", },
 	{ .compatible = "fsl,imx8m-nic", },
 	{ /* sentinel */ },
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 04/19] devfreq: imx8m-ddrc: Change governor to powersave

[Abel Vesa]

By switching to powersave governor, we allow the imx8m-ddrc to always
run at minimum rate needed by all the running masters.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/devfreq/imx8m-ddrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index 16636973eb10..583123bf2100 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -367,7 +367,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct imx8m_ddrc *priv;
-	const char *gov = DEVFREQ_GOV_USERSPACE;
+	const char *gov = DEVFREQ_GOV_POWERSAVE;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 05/19] devfreq: imx8m-ddrc: Use the opps acquired from EL3

[Abel Vesa]

i.MX8M platforms get their dram OPPs from the EL3.
We don't need to duplicate that in the kernel dram dts node.
We should just trust the OPPs provided by the EL3.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/devfreq/imx8m-ddrc.c | 50 +++---------------------------------
 1 file changed, 3 insertions(+), 47 deletions(-)

diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index 583123bf2100..f18a5c3c1c03 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -321,38 +321,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
 		if (freq->dram_core_parent_index == 2 &&
 				freq->dram_alt_parent_index == 0)
 			return -ENODEV;
-	}
-
-	return 0;
-}
-
-static int imx8m_ddrc_check_opps(struct device *dev)
-{
-	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
-	struct imx8m_ddrc_freq *freq_info;
-	struct dev_pm_opp *opp;
-	unsigned long freq;
-	int i, opp_count;
-
-	/* Enumerate DT OPPs and disable those not supported by firmware */
-	opp_count = dev_pm_opp_get_opp_count(dev);
-	if (opp_count < 0)
-		return opp_count;
-	for (i = 0, freq = 0; i < opp_count; ++i, ++freq) {
-		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
-		if (IS_ERR(opp)) {
-			dev_err(dev, "Failed enumerating OPPs: %ld\n",
-				PTR_ERR(opp));
-			return PTR_ERR(opp);
-		}
-		dev_pm_opp_put(opp);
 
-		freq_info = imx8m_ddrc_find_freq(priv, freq);
-		if (!freq_info) {
-			dev_info(dev, "Disable unsupported OPP %luHz %luMT/s\n",
-					freq, DIV_ROUND_CLOSEST(freq, 250000));
-			dev_pm_opp_disable(dev, freq);
-		}
+		if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
+			return -ENODEV;
 	}
 
 	return 0;
@@ -360,7 +331,6 @@ static int imx8m_ddrc_check_opps(struct device *dev)
 
 static void imx8m_ddrc_exit(struct device *dev)
 {
-	dev_pm_opp_of_remove_table(dev);
 }
 
 static int imx8m_ddrc_probe(struct platform_device *pdev)
@@ -407,16 +377,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = dev_pm_opp_of_add_table(dev);
-	if (ret < 0) {
-		dev_err(dev, "failed to get OPP table\n");
-		return ret;
-	}
-
-	ret = imx8m_ddrc_check_opps(dev);
-	if (ret < 0)
-		goto err;
-
+	priv->profile.polling_ms = 1000;
 	priv->profile.target = imx8m_ddrc_target;
 	priv->profile.exit = imx8m_ddrc_exit;
 	priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
@@ -427,13 +388,8 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->devfreq)) {
 		ret = PTR_ERR(priv->devfreq);
 		dev_err(dev, "failed to add devfreq device: %d\n", ret);
-		goto err;
 	}
 
-	return 0;
-
-err:
-	dev_pm_opp_of_remove_table(dev);
 	return ret;
 }
 
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 05/19] devfreq: imx8m-ddrc: Use the opps acquired from EL3

[Chanwoo Choi]

Hi,

OPP is mandatory for devfreq driver. Also, must need to add
the OPP levels to  devicetree file, it is better to show
the supported OPP list for the developer who don't know
the detailed background of driver. If there are no any
critical issue. I prefer the existing approach for the readability.

On 21. 9. 14. 오전 2:38, Abel Vesa wrote:
> i.MX8M platforms get their dram OPPs from the EL3.
> We don't need to duplicate that in the kernel dram dts node.
> We should just trust the OPPs provided by the EL3.
> 
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>   drivers/devfreq/imx8m-ddrc.c | 50 +++---------------------------------
>   1 file changed, 3 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
> index 583123bf2100..f18a5c3c1c03 100644
> --- a/drivers/devfreq/imx8m-ddrc.c
> +++ b/drivers/devfreq/imx8m-ddrc.c
> @@ -321,38 +321,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
>   		if (freq->dram_core_parent_index == 2 &&
>   				freq->dram_alt_parent_index == 0)
>   			return -ENODEV;
> -	}
> -
> -	return 0;
> -}
> -
> -static int imx8m_ddrc_check_opps(struct device *dev)
> -{
> -	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> -	struct imx8m_ddrc_freq *freq_info;
> -	struct dev_pm_opp *opp;
> -	unsigned long freq;
> -	int i, opp_count;
> -
> -	/* Enumerate DT OPPs and disable those not supported by firmware */
> -	opp_count = dev_pm_opp_get_opp_count(dev);
> -	if (opp_count < 0)
> -		return opp_count;
> -	for (i = 0, freq = 0; i < opp_count; ++i, ++freq) {
> -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> -		if (IS_ERR(opp)) {
> -			dev_err(dev, "Failed enumerating OPPs: %ld\n",
> -				PTR_ERR(opp));
> -			return PTR_ERR(opp);
> -		}
> -		dev_pm_opp_put(opp);
>   
> -		freq_info = imx8m_ddrc_find_freq(priv, freq);
> -		if (!freq_info) {
> -			dev_info(dev, "Disable unsupported OPP %luHz %luMT/s\n",
> -					freq, DIV_ROUND_CLOSEST(freq, 250000));
> -			dev_pm_opp_disable(dev, freq);
> -		}
> +		if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> +			return -ENODEV;
>   	}
>   
>   	return 0;
> @@ -360,7 +331,6 @@ static int imx8m_ddrc_check_opps(struct device *dev)
>   
>   static void imx8m_ddrc_exit(struct device *dev)
>   {
> -	dev_pm_opp_of_remove_table(dev);
>   }
>   
>   static int imx8m_ddrc_probe(struct platform_device *pdev)
> @@ -407,16 +377,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>   
> -	ret = dev_pm_opp_of_add_table(dev);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to get OPP table\n");
> -		return ret;
> -	}
> -
> -	ret = imx8m_ddrc_check_opps(dev);
> -	if (ret < 0)
> -		goto err;
> -
> +	priv->profile.polling_ms = 1000;

This change is not related to role of this patch.
Need to make the separate patch.

>   	priv->profile.target = imx8m_ddrc_target;
>   	priv->profile.exit = imx8m_ddrc_exit;
>   	priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
> @@ -427,13 +388,8 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
>   	if (IS_ERR(priv->devfreq)) {
>   		ret = PTR_ERR(priv->devfreq);
>   		dev_err(dev, "failed to add devfreq device: %d\n", ret);
> -		goto err;
>   	}
>   
> -	return 0;
> -
> -err:
> -	dev_pm_opp_of_remove_table(dev);
>   	return ret;
>   }
>   
> 


-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 05/19] devfreq: imx8m-ddrc: Use the opps acquired from EL3

[Chanwoo Choi]

On 21. 9. 15. 오후 12:29, Chanwoo Choi wrote:
> Hi,
> 
> OPP is mandatory for devfreq driver. Also, must need to add
> the OPP levels to  devicetree file, it is better to show
> the supported OPP list for the developer who don't know
> the detailed background of driver. If there are no any
> critical issue. I prefer the existing approach for the readability.

Also, by keeping the existing approach, the user is able to
select the their own OPP entries among the all supported frequencies
from EL3. Even if the some clock support the multiple frequencies,
the user might want to use the a few frequency instead of using
all supported frequencies.

> 
> On 21. 9. 14. 오전 2:38, Abel Vesa wrote:
>> i.MX8M platforms get their dram OPPs from the EL3.
>> We don't need to duplicate that in the kernel dram dts node.
>> We should just trust the OPPs provided by the EL3.
>>
>> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
>> ---
>>   drivers/devfreq/imx8m-ddrc.c | 50 +++---------------------------------
>>   1 file changed, 3 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
>> index 583123bf2100..f18a5c3c1c03 100644
>> --- a/drivers/devfreq/imx8m-ddrc.c
>> +++ b/drivers/devfreq/imx8m-ddrc.c
>> @@ -321,38 +321,9 @@ static int imx8m_ddrc_init_freq_info(struct 
>> device *dev)
>>           if (freq->dram_core_parent_index == 2 &&
>>                   freq->dram_alt_parent_index == 0)
>>               return -ENODEV;
>> -    }
>> -
>> -    return 0;
>> -}
>> -
>> -static int imx8m_ddrc_check_opps(struct device *dev)
>> -{
>> -    struct imx8m_ddrc *priv = dev_get_drvdata(dev);
>> -    struct imx8m_ddrc_freq *freq_info;
>> -    struct dev_pm_opp *opp;
>> -    unsigned long freq;
>> -    int i, opp_count;
>> -
>> -    /* Enumerate DT OPPs and disable those not supported by firmware */
>> -    opp_count = dev_pm_opp_get_opp_count(dev);
>> -    if (opp_count < 0)
>> -        return opp_count;
>> -    for (i = 0, freq = 0; i < opp_count; ++i, ++freq) {
>> -        opp = dev_pm_opp_find_freq_ceil(dev, &freq);
>> -        if (IS_ERR(opp)) {
>> -            dev_err(dev, "Failed enumerating OPPs: %ld\n",
>> -                PTR_ERR(opp));
>> -            return PTR_ERR(opp);
>> -        }
>> -        dev_pm_opp_put(opp);
>> -        freq_info = imx8m_ddrc_find_freq(priv, freq);
>> -        if (!freq_info) {
>> -            dev_info(dev, "Disable unsupported OPP %luHz %luMT/s\n",
>> -                    freq, DIV_ROUND_CLOSEST(freq, 250000));
>> -            dev_pm_opp_disable(dev, freq);
>> -        }
>> +        if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
>> +            return -ENODEV;
>>       }
>>       return 0;
>> @@ -360,7 +331,6 @@ static int imx8m_ddrc_check_opps(struct device *dev)
>>   static void imx8m_ddrc_exit(struct device *dev)
>>   {
>> -    dev_pm_opp_of_remove_table(dev);
>>   }
>>   static int imx8m_ddrc_probe(struct platform_device *pdev)
>> @@ -407,16 +377,7 @@ static int imx8m_ddrc_probe(struct 
>> platform_device *pdev)
>>           return ret;
>>       }
>> -    ret = dev_pm_opp_of_add_table(dev);
>> -    if (ret < 0) {
>> -        dev_err(dev, "failed to get OPP table\n");
>> -        return ret;
>> -    }
>> -
>> -    ret = imx8m_ddrc_check_opps(dev);
>> -    if (ret < 0)
>> -        goto err;
>> -
>> +    priv->profile.polling_ms = 1000;
> 
> This change is not related to role of this patch.
> Need to make the separate patch.
> 
>>       priv->profile.target = imx8m_ddrc_target;
>>       priv->profile.exit = imx8m_ddrc_exit;
>>       priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
>> @@ -427,13 +388,8 @@ static int imx8m_ddrc_probe(struct 
>> platform_device *pdev)
>>       if (IS_ERR(priv->devfreq)) {
>>           ret = PTR_ERR(priv->devfreq);
>>           dev_err(dev, "failed to add devfreq device: %d\n", ret);
>> -        goto err;
>>       }
>> -    return 0;
>> -
>> -err:
>> -    dev_pm_opp_of_remove_table(dev);
>>       return ret;
>>   }
>>
> 
> 


-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops

[Abel Vesa]

Seems that, in order to be able to resume from suspend, the dram rate
needs to be the highest one available. Therefore, add the late system
suspend/resume PM ops which set the highest rate on suspend and the
latest one used before suspending on resume.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index f18a5c3c1c03..f39741b4a0b0 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -72,6 +72,8 @@ struct imx8m_ddrc {
 	struct clk *dram_alt;
 	struct clk *dram_apb;
 
+	unsigned long suspend_rate;
+	unsigned long resume_rate;
 	int freq_count;
 	struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
 };
@@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev, unsigned long *freq, u32 flags)
 	return ret;
 }
 
+static int imx8m_ddrc_suspend(struct device *dev)
+{
+	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
+
+	priv->resume_rate = clk_get_rate(priv->dram_core);
+
+	return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
+}
+
+static int imx8m_ddrc_resume(struct device *dev)
+{
+	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
+
+	return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
+}
+
 static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
 {
 	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
@@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
 
 		if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
 			return -ENODEV;
+
+		if (index ==  0)
+			priv->suspend_rate = freq->rate * 250000;
 	}
 
 	return 0;
@@ -399,11 +420,16 @@ static const struct of_device_id imx8m_ddrc_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
 
+static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend, imx8m_ddrc_resume)
+};
+
 static struct platform_driver imx8m_ddrc_platdrv = {
 	.probe		= imx8m_ddrc_probe,
 	.driver = {
 		.name	= "imx8m-ddrc-devfreq",
-		.of_match_table = imx8m_ddrc_of_match,
+		.pm = &imx8m_ddrc_pm_ops,
+		.of_match_table = of_match_ptr(imx8m_ddrc_of_match),
 	},
 };
 module_platform_driver(imx8m_ddrc_platdrv);
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops

[Chanwoo Choi]

Hi,

As I commented on patch5, you keep the OPP list on devicetree file
and then you better to use the 'suspend_opp' property
for setting the highest frequency during suspend/resume.

On 21. 9. 14. 오전 2:38, Abel Vesa wrote:
> Seems that, in order to be able to resume from suspend, the dram rate
> needs to be the highest one available. Therefore, add the late system
> suspend/resume PM ops which set the highest rate on suspend and the
> latest one used before suspending on resume.
> 
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>   drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
> index f18a5c3c1c03..f39741b4a0b0 100644
> --- a/drivers/devfreq/imx8m-ddrc.c
> +++ b/drivers/devfreq/imx8m-ddrc.c
> @@ -72,6 +72,8 @@ struct imx8m_ddrc {
>   	struct clk *dram_alt;
>   	struct clk *dram_apb;
>   
> +	unsigned long suspend_rate;
> +	unsigned long resume_rate;
>   	int freq_count;
>   	struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
>   };
> @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev, unsigned long *freq, u32 flags)
>   	return ret;
>   }
>   
> +static int imx8m_ddrc_suspend(struct device *dev)
> +{
> +	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> +
> +	priv->resume_rate = clk_get_rate(priv->dram_core);
> +
> +	return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> +}
> +
> +static int imx8m_ddrc_resume(struct device *dev)
> +{
> +	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> +
> +	return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> +}
> +
>   static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
>   {
>   	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
>   
>   		if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
>   			return -ENODEV;
> +
> +		if (index ==  0)
> +			priv->suspend_rate = freq->rate * 250000;
>   	}
>   
>   	return 0;
> @@ -399,11 +420,16 @@ static const struct of_device_id imx8m_ddrc_of_match[] = {
>   };
>   MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
>   
> +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend, imx8m_ddrc_resume)
> +};
> +
>   static struct platform_driver imx8m_ddrc_platdrv = {
>   	.probe		= imx8m_ddrc_probe,
>   	.driver = {
>   		.name	= "imx8m-ddrc-devfreq",
> -		.of_match_table = imx8m_ddrc_of_match,
> +		.pm = &imx8m_ddrc_pm_ops,
> +		.of_match_table = of_match_ptr(imx8m_ddrc_of_match),
>   	},
>   };
>   module_platform_driver(imx8m_ddrc_platdrv);
> 


-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops

[Abel Vesa]

On 21-09-15 12:37:45, Chanwoo Choi wrote:
> Hi,
> 
> As I commented on patch5, you keep the OPP list on devicetree file
> and then you better to use the 'suspend_opp' property
> for setting the highest frequency during suspend/resume.
> 

Hi,

I think there is no mechanism in place to ensure that the suspend opp
will be set only after all the icc users have suspended. I only tested
briefly, but I can tell you that there are cases where some icc user
asks for a different opp right after the suspend opp was set. This leads
to suspending with a different rate than the one from suspend opp.
So I guess I still need the late system sleep pm opps to circumvent such
situations.

> On 21. 9. 14. 오전 2:38, Abel Vesa wrote:
> > Seems that, in order to be able to resume from suspend, the dram rate
> > needs to be the highest one available. Therefore, add the late system
> > suspend/resume PM ops which set the highest rate on suspend and the
> > latest one used before suspending on resume.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > ---
> >   drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
> >   1 file changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
> > index f18a5c3c1c03..f39741b4a0b0 100644
> > --- a/drivers/devfreq/imx8m-ddrc.c
> > +++ b/drivers/devfreq/imx8m-ddrc.c
> > @@ -72,6 +72,8 @@ struct imx8m_ddrc {
> >   	struct clk *dram_alt;
> >   	struct clk *dram_apb;
> > +	unsigned long suspend_rate;
> > +	unsigned long resume_rate;
> >   	int freq_count;
> >   	struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
> >   };
> > @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev, unsigned long *freq, u32 flags)
> >   	return ret;
> >   }
> > +static int imx8m_ddrc_suspend(struct device *dev)
> > +{
> > +	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > +
> > +	priv->resume_rate = clk_get_rate(priv->dram_core);
> > +
> > +	return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> > +}
> > +
> > +static int imx8m_ddrc_resume(struct device *dev)
> > +{
> > +	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > +
> > +	return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> > +}
> > +
> >   static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
> >   {
> >   	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
> >   		if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> >   			return -ENODEV;
> > +
> > +		if (index ==  0)
> > +			priv->suspend_rate = freq->rate * 250000;
> >   	}
> >   	return 0;
> > @@ -399,11 +420,16 @@ static const struct of_device_id imx8m_ddrc_of_match[] = {
> >   };
> >   MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
> > +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> > +	SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend, imx8m_ddrc_resume)
> > +};
> > +
> >   static struct platform_driver imx8m_ddrc_platdrv = {
> >   	.probe		= imx8m_ddrc_probe,
> >   	.driver = {
> >   		.name	= "imx8m-ddrc-devfreq",
> > -		.of_match_table = imx8m_ddrc_of_match,
> > +		.pm = &imx8m_ddrc_pm_ops,
> > +		.of_match_table = of_match_ptr(imx8m_ddrc_of_match),
> >   	},
> >   };
> >   module_platform_driver(imx8m_ddrc_platdrv);
> > 
> 
> 
> -- 
> Best Regards,
> Samsung Electronics
> Chanwoo Choi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops

[Martin Kepplinger]

Am Montag, dem 13.09.2021 um 20:38 +0300 schrieb Abel Vesa:
> Seems that, in order to be able to resume from suspend, the dram rate
> needs to be the highest one available. Therefore, add the late system
> suspend/resume PM ops which set the highest rate on suspend and the
> latest one used before suspending on resume.

Hi Abel, wouldn't this mean that s2idle / freeze would be kind of
broken by this?

Does is make sense to test the lowest rate? How would I force that
here? (just for testing)

Also, you could think about splitting this series up a bit and do this
patch seperately onto mainline (before or after the other work).

thank you
                          martin


> 
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>  drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-
> ddrc.c
> index f18a5c3c1c03..f39741b4a0b0 100644
> --- a/drivers/devfreq/imx8m-ddrc.c
> +++ b/drivers/devfreq/imx8m-ddrc.c
> @@ -72,6 +72,8 @@ struct imx8m_ddrc {
>         struct clk *dram_alt;
>         struct clk *dram_apb;
>  
> +       unsigned long suspend_rate;
> +       unsigned long resume_rate;
>         int freq_count;
>         struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
>  };
> @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev,
> unsigned long *freq, u32 flags)
>         return ret;
>  }
>  
> +static int imx8m_ddrc_suspend(struct device *dev)
> +{
> +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> +
> +       priv->resume_rate = clk_get_rate(priv->dram_core);
> +
> +       return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> +}
> +
> +static int imx8m_ddrc_resume(struct device *dev)
> +{
> +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> +
> +       return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> +}
> +
>  static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long
> *freq)
>  {
>         struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct
> device *dev)
>  
>                 if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
>                         return -ENODEV;
> +
> +               if (index ==  0)
> +                       priv->suspend_rate = freq->rate * 250000;
>         }
>  
>         return 0;
> @@ -399,11 +420,16 @@ static const struct of_device_id
> imx8m_ddrc_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
>  
> +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> +       SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend,
> imx8m_ddrc_resume)
> +};
> +
>  static struct platform_driver imx8m_ddrc_platdrv = {
>         .probe          = imx8m_ddrc_probe,
>         .driver = {
>                 .name   = "imx8m-ddrc-devfreq",
> -               .of_match_table = imx8m_ddrc_of_match,
> +               .pm = &imx8m_ddrc_pm_ops,
> +               .of_match_table = of_match_ptr(imx8m_ddrc_of_match),
>         },
>  };
>  module_platform_driver(imx8m_ddrc_platdrv);



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops

[Abel Vesa]

On 21-11-10 13:15:26, Martin Kepplinger wrote:
> Am Montag, dem 13.09.2021 um 20:38 +0300 schrieb Abel Vesa:
> > Seems that, in order to be able to resume from suspend, the dram rate
> > needs to be the highest one available. Therefore, add the late system
> > suspend/resume PM ops which set the highest rate on suspend and the
> > latest one used before suspending on resume.
> 
> Hi Abel, wouldn't this mean that s2idle / freeze would be kind of
> broken by this?
> 

Nope. Only the DDR rate needs to be raised at 800M before suspending.
Everything else stays the same.

> Does is make sense to test the lowest rate? How would I force that
> here? (just for testing)

You can try, but it will surely freeze. See [1] what you need to change
for testing.
> 
> Also, you could think about splitting this series up a bit and do this
> patch seperately onto mainline (before or after the other work).
> 

Well, I sent as RFC until now. Seems there are no big issues with the
approach. So I'll split the patches between subsystems on the next
iteration.

> thank you
>                           martin
> 
> 
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > ---
> >  drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-
> > ddrc.c
> > index f18a5c3c1c03..f39741b4a0b0 100644
> > --- a/drivers/devfreq/imx8m-ddrc.c
> > +++ b/drivers/devfreq/imx8m-ddrc.c
> > @@ -72,6 +72,8 @@ struct imx8m_ddrc {
> >         struct clk *dram_alt;
> >         struct clk *dram_apb;
> >  
> > +       unsigned long suspend_rate;
> > +       unsigned long resume_rate;
> >         int freq_count;
> >         struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
> >  };
> > @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev,
> > unsigned long *freq, u32 flags)
> >         return ret;
> >  }
> >  
> > +static int imx8m_ddrc_suspend(struct device *dev)
> > +{
> > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > +
> > +       priv->resume_rate = clk_get_rate(priv->dram_core);
> > +
> > +       return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> > +}
> > +
> > +static int imx8m_ddrc_resume(struct device *dev)
> > +{
> > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > +
> > +       return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> > +}
> > +
> >  static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long
> > *freq)
> >  {
> >         struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct
> > device *dev)
> >  
> >                 if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> >                         return -ENODEV;
> > +
> > +               if (index ==  0)

[1] Change this line to:
		    if (index == 1)

It will select the 166935483 freq for suspending.

> > +                       priv->suspend_rate = freq->rate * 250000;
> >         }
> >  
> >         return 0;
> > @@ -399,11 +420,16 @@ static const struct of_device_id
> > imx8m_ddrc_of_match[] = {
> >  };
> >  MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
> >  
> > +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> > +       SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend,
> > imx8m_ddrc_resume)
> > +};
> > +
> >  static struct platform_driver imx8m_ddrc_platdrv = {
> >         .probe          = imx8m_ddrc_probe,
> >         .driver = {
> >                 .name   = "imx8m-ddrc-devfreq",
> > -               .of_match_table = imx8m_ddrc_of_match,
> > +               .pm = &imx8m_ddrc_pm_ops,
> > +               .of_match_table = of_match_ptr(imx8m_ddrc_of_match),
> >         },
> >  };
> >  module_platform_driver(imx8m_ddrc_platdrv);
> 
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops

[Martin Kepplinger]

Am Dienstag, dem 30.11.2021 um 22:06 +0200 schrieb Abel Vesa:
> On 21-11-10 13:15:26, Martin Kepplinger wrote:
> > Am Montag, dem 13.09.2021 um 20:38 +0300 schrieb Abel Vesa:
> > > Seems that, in order to be able to resume from suspend, the dram
> > > rate
> > > needs to be the highest one available. Therefore, add the late
> > > system
> > > suspend/resume PM ops which set the highest rate on suspend and
> > > the
> > > latest one used before suspending on resume.
> > 
> > Hi Abel, wouldn't this mean that s2idle / freeze would be kind of
> > broken by this?
> > 
> 
> Nope. Only the DDR rate needs to be raised at 800M before suspending.
> Everything else stays the same.

well, for s2idle, linux stays running, so no calling out to atf (dram
retention...) is happening, so it'll stay at 800M *during* being
suspended.

I tested that by observing power consumption of the system - although
for now without the cpu-sleep state (via your workaround) due to
https://lore.kernel.org/linux-arm-kernel/6ca0bcabfa3b6643f9ab7e311cd8697df223c5cb.camel@puri.sm/


> 
> > Does is make sense to test the lowest rate? How would I force that
> > here? (just for testing)
> 
> You can try, but it will surely freeze. See [1] what you need to
> change
> for testing.

thanks, that looks nicer than forcing 50M.

> > 
> > Also, you could think about splitting this series up a bit and do
> > this
> > patch seperately onto mainline (before or after the other work).
> > 
> 
> Well, I sent as RFC until now. Seems there are no big issues with the
> approach. So I'll split the patches between subsystems on the next
> iteration.

great, looking forward to it!

> 
> > thank you
> >                           martin
> > 
> > 
> > > 
> > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > > ---
> > >  drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/devfreq/imx8m-ddrc.c
> > > b/drivers/devfreq/imx8m-
> > > ddrc.c
> > > index f18a5c3c1c03..f39741b4a0b0 100644
> > > --- a/drivers/devfreq/imx8m-ddrc.c
> > > +++ b/drivers/devfreq/imx8m-ddrc.c
> > > @@ -72,6 +72,8 @@ struct imx8m_ddrc {
> > >         struct clk *dram_alt;
> > >         struct clk *dram_apb;
> > >  
> > > +       unsigned long suspend_rate;
> > > +       unsigned long resume_rate;
> > >         int freq_count;
> > >         struct imx8m_ddrc_freq
> > > freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
> > >  };
> > > @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device
> > > *dev,
> > > unsigned long *freq, u32 flags)
> > >         return ret;
> > >  }
> > >  
> > > +static int imx8m_ddrc_suspend(struct device *dev)
> > > +{
> > > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > +
> > > +       priv->resume_rate = clk_get_rate(priv->dram_core);
> > > +
> > > +       return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> > > +}
> > > +
> > > +static int imx8m_ddrc_resume(struct device *dev)
> > > +{
> > > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > +
> > > +       return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> > > +}
> > > +
> > >  static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned
> > > long
> > > *freq)
> > >  {
> > >         struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct
> > > device *dev)
> > >  
> > >                 if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> > >                         return -ENODEV;
> > > +
> > > +               if (index ==  0)
> 
> [1] Change this line to:
>                     if (index == 1)
> 
> It will select the 166935483 freq for suspending.
> 
> > > +                       priv->suspend_rate = freq->rate * 250000;
> > >         }
> > >  
> > >         return 0;
> > > @@ -399,11 +420,16 @@ static const struct of_device_id
> > > imx8m_ddrc_of_match[] = {
> > >  };
> > >  MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
> > >  
> > > +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> > > +       SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend,
> > > imx8m_ddrc_resume)
> > > +};
> > > +
> > >  static struct platform_driver imx8m_ddrc_platdrv = {
> > >         .probe          = imx8m_ddrc_probe,
> > >         .driver = {
> > >                 .name   = "imx8m-ddrc-devfreq",
> > > -               .of_match_table = imx8m_ddrc_of_match,
> > > +               .pm = &imx8m_ddrc_pm_ops,
> > > +               .of_match_table =
> > > of_match_ptr(imx8m_ddrc_of_match),
> > >         },
> > >  };
> > >  module_platform_driver(imx8m_ddrc_platdrv);
> > 
> > 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops

[Martin Kepplinger]

Am Dienstag, dem 30.11.2021 um 22:06 +0200 schrieb Abel Vesa:
> On 21-11-10 13:15:26, Martin Kepplinger wrote:
> > Am Montag, dem 13.09.2021 um 20:38 +0300 schrieb Abel Vesa:
> > > Seems that, in order to be able to resume from suspend, the dram
> > > rate
> > > needs to be the highest one available. Therefore, add the late
> > > system
> > > suspend/resume PM ops which set the highest rate on suspend and
> > > the
> > > latest one used before suspending on resume.
> > 
> > Hi Abel, wouldn't this mean that s2idle / freeze would be kind of
> > broken by this?
> > 
> 
> Nope. Only the DDR rate needs to be raised at 800M before suspending.
> Everything else stays the same.

fyi I just tested this and you're right. freezes when not at 800M. So
for this patchset, I think this is fine as it enables and fixes stuff.

It would not hurt to mention s2idle at least, where of course 800M
should not be selected, as no userspace is running at all. But I'd be
fine with looking at that later.

> 
> > Does is make sense to test the lowest rate? How would I force that
> > here? (just for testing)
> 
> You can try, but it will surely freeze. See [1] what you need to
> change
> for testing.
> > 
> > Also, you could think about splitting this series up a bit and do
> > this
> > patch seperately onto mainline (before or after the other work).
> > 
> 
> Well, I sent as RFC until now. Seems there are no big issues with the
> approach. So I'll split the patches between subsystems on the next
> iteration.
> 
> > thank you
> >                           martin
> > 
> > 
> > > 
> > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > > ---
> > >  drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/devfreq/imx8m-ddrc.c
> > > b/drivers/devfreq/imx8m-
> > > ddrc.c
> > > index f18a5c3c1c03..f39741b4a0b0 100644
> > > --- a/drivers/devfreq/imx8m-ddrc.c
> > > +++ b/drivers/devfreq/imx8m-ddrc.c
> > > @@ -72,6 +72,8 @@ struct imx8m_ddrc {
> > >         struct clk *dram_alt;
> > >         struct clk *dram_apb;
> > >  
> > > +       unsigned long suspend_rate;
> > > +       unsigned long resume_rate;
> > >         int freq_count;
> > >         struct imx8m_ddrc_freq
> > > freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
> > >  };
> > > @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device
> > > *dev,
> > > unsigned long *freq, u32 flags)
> > >         return ret;
> > >  }
> > >  
> > > +static int imx8m_ddrc_suspend(struct device *dev)
> > > +{
> > > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > +
> > > +       priv->resume_rate = clk_get_rate(priv->dram_core);
> > > +
> > > +       return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> > > +}
> > > +
> > > +static int imx8m_ddrc_resume(struct device *dev)
> > > +{
> > > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > +
> > > +       return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> > > +}
> > > +
> > >  static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned
> > > long
> > > *freq)
> > >  {
> > >         struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct
> > > device *dev)
> > >  
> > >                 if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> > >                         return -ENODEV;
> > > +
> > > +               if (index ==  0)
> 
> [1] Change this line to:
>                     if (index == 1)
> 
> It will select the 166935483 freq for suspending.
> 
> > > +                       priv->suspend_rate = freq->rate * 250000;
> > >         }
> > >  
> > >         return 0;
> > > @@ -399,11 +420,16 @@ static const struct of_device_id
> > > imx8m_ddrc_of_match[] = {
> > >  };
> > >  MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
> > >  
> > > +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> > > +       SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend,
> > > imx8m_ddrc_resume)
> > > +};
> > > +
> > >  static struct platform_driver imx8m_ddrc_platdrv = {
> > >         .probe          = imx8m_ddrc_probe,
> > >         .driver = {
> > >                 .name   = "imx8m-ddrc-devfreq",
> > > -               .of_match_table = imx8m_ddrc_of_match,
> > > +               .pm = &imx8m_ddrc_pm_ops,
> > > +               .of_match_table =
> > > of_match_ptr(imx8m_ddrc_of_match),
> > >         },
> > >  };
> > >  module_platform_driver(imx8m_ddrc_platdrv);
> > 
> > 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 07/19] interconnect: imx: Switch from imx_icc_node_adj_desc to fsl, icc-id node assignment

[Abel Vesa]

In order to be able to have more than one NoCs in the interconnect net
we need to decouple the NoC from the dram. So instead of using the
imx_icc_node_adj_desc, we use the fsl,icc-id property that is in
each NoC (or pl301) to the icc node (based on the id) to it.
Along with all the NoC and pl301 nodes in the dts we will have a
interconnect dedicated node. This node will be the actual device of the
icc provider.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/interconnect/imx/imx.c | 72 +++++++++++++++-------------------
 1 file changed, 32 insertions(+), 40 deletions(-)

diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
index c770951a909c..db048df80011 100644
--- a/drivers/interconnect/imx/imx.c
+++ b/drivers/interconnect/imx/imx.c
@@ -34,9 +34,9 @@ static int imx_icc_node_set(struct icc_node *node)
 	if (!node_data->qos_dev)
 		return 0;
 
-	freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
-	do_div(freq, node_data->desc->adj->bw_div);
-	dev_dbg(dev, "node %s device %s avg_bw %ukBps peak_bw %ukBps min_freq %llukHz\n",
+	freq = max(node->avg_bw, node->peak_bw);
+
+	dev_dbg(dev, "	DBG node %s device %s avg_bw %ukBps peak_bw %ukBps min_freq %llukHz\n",
 		node->name, dev_name(node_data->qos_dev),
 		node->avg_bw, node->peak_bw, freq);
 
@@ -79,41 +79,35 @@ static int imx_icc_node_init_qos(struct icc_provider *provider,
 				 struct icc_node *node)
 {
 	struct imx_icc_node *node_data = node->data;
-	const struct imx_icc_node_adj_desc *adj = node_data->desc->adj;
 	struct device *dev = provider->dev;
-	struct device_node *dn = NULL;
 	struct platform_device *pdev;
+	struct device_node *np = NULL, *dn = NULL;
+	int idx;
 
-	if (adj->main_noc) {
-		node_data->qos_dev = dev;
-		dev_dbg(dev, "icc node %s[%d] is main noc itself\n",
-			node->name, node->id);
-	} else {
-		dn = of_parse_phandle(dev->of_node, adj->phandle_name, 0);
-		if (!dn) {
-			dev_warn(dev, "Failed to parse %s\n",
-				 adj->phandle_name);
-			return -ENODEV;
-		}
-		/* Allow scaling to be disabled on a per-node basis */
-		if (!of_device_is_available(dn)) {
-			dev_warn(dev, "Missing property %s, skip scaling %s\n",
-				 adj->phandle_name, node->name);
-			of_node_put(dn);
-			return 0;
-		}
+	for_each_node_with_property(np, "fsl,icc-id") {
+		of_property_read_u32(np, "fsl,icc-id", &idx);
+		if (idx == node_data->desc->id)
+			dn = np;
+	}
 
-		pdev = of_find_device_by_node(dn);
-		of_node_put(dn);
-		if (!pdev) {
-			dev_warn(dev, "node %s[%d] missing device for %pOF\n",
-				 node->name, node->id, dn);
-			return -EPROBE_DEFER;
-		}
-		node_data->qos_dev = &pdev->dev;
-		dev_dbg(dev, "node %s[%d] has device node %pOF\n",
-			node->name, node->id, dn);
+	if (!dn)
+		return 0;
+
+	if (!of_device_is_available(dn)) {
+		dev_warn(dev, "%pOF is disabled\n", dn);
+		return 0;
+	}
+
+	pdev = of_find_device_by_node(dn);
+	of_node_put(dn);
+	if (!pdev) {
+		dev_warn(dev, "node %s[%d] missing device for %pOF\n",
+			 node->name, node->id, dn);
+		return -EPROBE_DEFER;
 	}
+	node_data->qos_dev = &pdev->dev;
+	dev_dbg(dev, "node %s[%d] has device node %pOF\n", node->name,
+							node->id, dn);
 
 	return dev_pm_qos_add_request(node_data->qos_dev,
 				      &node_data->qos_req,
@@ -151,12 +145,10 @@ static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
 	node_data->desc = node_desc;
 	icc_node_add(node, provider);
 
-	if (node_desc->adj) {
-		ret = imx_icc_node_init_qos(provider, node);
-		if (ret < 0) {
-			imx_icc_node_destroy(node);
-			return ERR_PTR(ret);
-		}
+	ret = imx_icc_node_init_qos(provider, node);
+	if (ret < 0) {
+		imx_icc_node_destroy(node);
+		return ERR_PTR(ret);
 	}
 
 	return node;
@@ -244,7 +236,7 @@ int imx_icc_register(struct platform_device *pdev,
 	provider->aggregate = icc_std_aggregate;
 	provider->xlate = of_icc_xlate_onecell;
 	provider->data = data;
-	provider->dev = dev->parent;
+	provider->dev = dev;
 	platform_set_drvdata(pdev, provider);
 
 	ret = icc_provider_add(provider);
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 08/19] interconnect: imx8: Remove the imx_icc_node_adj_desc

[Abel Vesa]

Now that the imx generic interconnect doesn't use the
imx_icc_node_adj_desc, we remove it from all the i.MX8M
platform drivers.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/interconnect/imx/imx.h    | 19 ++++-------------
 drivers/interconnect/imx/imx8mm.c | 32 +++++++++-------------------
 drivers/interconnect/imx/imx8mn.c | 28 +++++++------------------
 drivers/interconnect/imx/imx8mq.c | 35 ++++++++++---------------------
 4 files changed, 33 insertions(+), 81 deletions(-)

diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
index 75da51076c68..5c9f5138f6aa 100644
--- a/drivers/interconnect/imx/imx.h
+++ b/drivers/interconnect/imx/imx.h
@@ -14,15 +14,6 @@
 
 #define IMX_ICC_MAX_LINKS	4
 
-/*
- * struct imx_icc_node_adj - Describe a dynamic adjustable node
- */
-struct imx_icc_node_adj_desc {
-	unsigned int bw_mul, bw_div;
-	const char *phandle_name;
-	bool main_noc;
-};
-
 /*
  * struct imx_icc_node - Describe an interconnect node
  * @name: name of the node
@@ -35,23 +26,21 @@ struct imx_icc_node_desc {
 	u16 id;
 	u16 links[IMX_ICC_MAX_LINKS];
 	u16 num_links;
-	const struct imx_icc_node_adj_desc *adj;
 };
 
-#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, ...)			\
+#define DEFINE_BUS_INTERCONNECT(_name, _id, ...)			\
 	{								\
 		.id = _id,						\
 		.name = _name,						\
-		.adj = _adj,						\
 		.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),	\
 		.links = { __VA_ARGS__ },				\
 	}
 
 #define DEFINE_BUS_MASTER(_name, _id, _dest_id)				\
-	DEFINE_BUS_INTERCONNECT(_name, _id, NULL, _dest_id)
+	DEFINE_BUS_INTERCONNECT(_name, _id, _dest_id)
 
-#define DEFINE_BUS_SLAVE(_name, _id, _adj)				\
-	DEFINE_BUS_INTERCONNECT(_name, _id, _adj)
+#define DEFINE_BUS_SLAVE(_name, _id)					\
+	DEFINE_BUS_INTERCONNECT(_name, _id)
 
 int imx_icc_register(struct platform_device *pdev,
 		     struct imx_icc_node_desc *nodes,
diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
index 1083490bb391..0c16110bef9d 100644
--- a/drivers/interconnect/imx/imx8mm.c
+++ b/drivers/interconnect/imx/imx8mm.c
@@ -14,18 +14,6 @@
 
 #include "imx.h"
 
-static const struct imx_icc_node_adj_desc imx8mm_dram_adj = {
-	.bw_mul = 1,
-	.bw_div = 16,
-	.phandle_name = "fsl,ddrc",
-};
-
-static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
-	.bw_mul = 1,
-	.bw_div = 16,
-	.main_noc = true,
-};
-
 /*
  * Describe bus masters, slaves and connections between them
  *
@@ -33,43 +21,43 @@ static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
  * PL301 nics which are skipped/merged into PL301_MAIN
  */
 static struct imx_icc_node_desc nodes[] = {
-	DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, &imx8mm_noc_adj,
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC,
 			IMX8MM_ICS_DRAM, IMX8MM_ICN_MAIN),
 
-	DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM, &imx8mm_dram_adj),
-	DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM, NULL),
+	DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM),
 	DEFINE_BUS_MASTER("A53", IMX8MM_ICM_A53, IMX8MM_ICN_NOC),
 
 	/* VPUMIX */
 	DEFINE_BUS_MASTER("VPU H1", IMX8MM_ICM_VPU_H1, IMX8MM_ICN_VIDEO),
 	DEFINE_BUS_MASTER("VPU G1", IMX8MM_ICM_VPU_G1, IMX8MM_ICN_VIDEO),
 	DEFINE_BUS_MASTER("VPU G2", IMX8MM_ICM_VPU_G2, IMX8MM_ICN_VIDEO),
-	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, NULL, IMX8MM_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, IMX8MM_ICN_NOC),
 
 	/* GPUMIX */
 	DEFINE_BUS_MASTER("GPU 2D", IMX8MM_ICM_GPU2D, IMX8MM_ICN_GPU),
 	DEFINE_BUS_MASTER("GPU 3D", IMX8MM_ICM_GPU3D, IMX8MM_ICN_GPU),
-	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MM_ICN_GPU, NULL, IMX8MM_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MM_ICN_GPU, IMX8MM_ICN_NOC),
 
 	/* DISPLAYMIX */
 	DEFINE_BUS_MASTER("CSI", IMX8MM_ICM_CSI, IMX8MM_ICN_MIPI),
 	DEFINE_BUS_MASTER("LCDIF", IMX8MM_ICM_LCDIF, IMX8MM_ICN_MIPI),
-	DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MM_ICN_MIPI, NULL, IMX8MM_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MM_ICN_MIPI, IMX8MM_ICN_NOC),
 
 	/* HSIO */
 	DEFINE_BUS_MASTER("USB1", IMX8MM_ICM_USB1, IMX8MM_ICN_HSIO),
 	DEFINE_BUS_MASTER("USB2", IMX8MM_ICM_USB2, IMX8MM_ICN_HSIO),
 	DEFINE_BUS_MASTER("PCIE", IMX8MM_ICM_PCIE, IMX8MM_ICN_HSIO),
-	DEFINE_BUS_INTERCONNECT("PL301_HSIO", IMX8MM_ICN_HSIO, NULL, IMX8MM_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_HSIO", IMX8MM_ICN_HSIO, IMX8MM_ICN_NOC),
 
 	/* Audio */
 	DEFINE_BUS_MASTER("SDMA2", IMX8MM_ICM_SDMA2, IMX8MM_ICN_AUDIO),
 	DEFINE_BUS_MASTER("SDMA3", IMX8MM_ICM_SDMA3, IMX8MM_ICN_AUDIO),
-	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MM_ICN_AUDIO, NULL, IMX8MM_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MM_ICN_AUDIO, IMX8MM_ICN_MAIN),
 
 	/* Ethernet */
 	DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
-	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, NULL, IMX8MM_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, IMX8MM_ICN_MAIN),
 
 	/* Other */
 	DEFINE_BUS_MASTER("SDMA1", IMX8MM_ICM_SDMA1, IMX8MM_ICN_MAIN),
@@ -77,7 +65,7 @@ static struct imx_icc_node_desc nodes[] = {
 	DEFINE_BUS_MASTER("USDHC1", IMX8MM_ICM_USDHC1, IMX8MM_ICN_MAIN),
 	DEFINE_BUS_MASTER("USDHC2", IMX8MM_ICM_USDHC2, IMX8MM_ICN_MAIN),
 	DEFINE_BUS_MASTER("USDHC3", IMX8MM_ICM_USDHC3, IMX8MM_ICN_MAIN),
-	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MM_ICN_MAIN, NULL,
+	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MM_ICN_MAIN,
 			IMX8MM_ICN_NOC, IMX8MM_ICS_OCRAM),
 };
 
diff --git a/drivers/interconnect/imx/imx8mn.c b/drivers/interconnect/imx/imx8mn.c
index ad97e55fd4e5..8d16bd5cf006 100644
--- a/drivers/interconnect/imx/imx8mn.c
+++ b/drivers/interconnect/imx/imx8mn.c
@@ -11,18 +11,6 @@
 
 #include "imx.h"
 
-static const struct imx_icc_node_adj_desc imx8mn_dram_adj = {
-	.bw_mul = 1,
-	.bw_div = 4,
-	.phandle_name = "fsl,ddrc",
-};
-
-static const struct imx_icc_node_adj_desc imx8mn_noc_adj = {
-	.bw_mul = 1,
-	.bw_div = 4,
-	.main_noc = true,
-};
-
 /*
  * Describe bus masters, slaves and connections between them
  *
@@ -30,23 +18,23 @@ static const struct imx_icc_node_adj_desc imx8mn_noc_adj = {
  * PL301 nics which are skipped/merged into PL301_MAIN
  */
 static struct imx_icc_node_desc nodes[] = {
-	DEFINE_BUS_INTERCONNECT("NOC", IMX8MN_ICN_NOC, &imx8mn_noc_adj,
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MN_ICN_NOC,
 			IMX8MN_ICS_DRAM, IMX8MN_ICN_MAIN),
 
-	DEFINE_BUS_SLAVE("DRAM", IMX8MN_ICS_DRAM, &imx8mn_dram_adj),
-	DEFINE_BUS_SLAVE("OCRAM", IMX8MN_ICS_OCRAM, NULL),
+	DEFINE_BUS_SLAVE("DRAM", IMX8MN_ICS_DRAM),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MN_ICS_OCRAM),
 	DEFINE_BUS_MASTER("A53", IMX8MN_ICM_A53, IMX8MN_ICN_NOC),
 
 	/* GPUMIX */
 	DEFINE_BUS_MASTER("GPU", IMX8MN_ICM_GPU, IMX8MN_ICN_GPU),
-	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MN_ICN_GPU, NULL, IMX8MN_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MN_ICN_GPU, IMX8MN_ICN_NOC),
 
 	/* DISPLAYMIX */
 	DEFINE_BUS_MASTER("CSI1", IMX8MN_ICM_CSI1, IMX8MN_ICN_MIPI),
 	DEFINE_BUS_MASTER("CSI2", IMX8MN_ICM_CSI2, IMX8MN_ICN_MIPI),
 	DEFINE_BUS_MASTER("ISI", IMX8MN_ICM_ISI, IMX8MN_ICN_MIPI),
 	DEFINE_BUS_MASTER("LCDIF", IMX8MN_ICM_LCDIF, IMX8MN_ICN_MIPI),
-	DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MN_ICN_MIPI, NULL, IMX8MN_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MN_ICN_MIPI, IMX8MN_ICN_NOC),
 
 	/* USB goes straight to NOC */
 	DEFINE_BUS_MASTER("USB", IMX8MN_ICM_USB, IMX8MN_ICN_NOC),
@@ -54,11 +42,11 @@ static struct imx_icc_node_desc nodes[] = {
 	/* Audio */
 	DEFINE_BUS_MASTER("SDMA2", IMX8MN_ICM_SDMA2, IMX8MN_ICN_AUDIO),
 	DEFINE_BUS_MASTER("SDMA3", IMX8MN_ICM_SDMA3, IMX8MN_ICN_AUDIO),
-	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MN_ICN_AUDIO, NULL, IMX8MN_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MN_ICN_AUDIO, IMX8MN_ICN_MAIN),
 
 	/* Ethernet */
 	DEFINE_BUS_MASTER("ENET", IMX8MN_ICM_ENET, IMX8MN_ICN_ENET),
-	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MN_ICN_ENET, NULL, IMX8MN_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MN_ICN_ENET, IMX8MN_ICN_MAIN),
 
 	/* Other */
 	DEFINE_BUS_MASTER("SDMA1", IMX8MN_ICM_SDMA1, IMX8MN_ICN_MAIN),
@@ -66,7 +54,7 @@ static struct imx_icc_node_desc nodes[] = {
 	DEFINE_BUS_MASTER("USDHC1", IMX8MN_ICM_USDHC1, IMX8MN_ICN_MAIN),
 	DEFINE_BUS_MASTER("USDHC2", IMX8MN_ICM_USDHC2, IMX8MN_ICN_MAIN),
 	DEFINE_BUS_MASTER("USDHC3", IMX8MN_ICM_USDHC3, IMX8MN_ICN_MAIN),
-	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MN_ICN_MAIN, NULL,
+	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MN_ICN_MAIN
 			IMX8MN_ICN_NOC, IMX8MN_ICS_OCRAM),
 };
 
diff --git a/drivers/interconnect/imx/imx8mq.c b/drivers/interconnect/imx/imx8mq.c
index d7768d3c6d8a..b8c36d668946 100644
--- a/drivers/interconnect/imx/imx8mq.c
+++ b/drivers/interconnect/imx/imx8mq.c
@@ -12,18 +12,6 @@
 
 #include "imx.h"
 
-static const struct imx_icc_node_adj_desc imx8mq_dram_adj = {
-	.bw_mul = 1,
-	.bw_div = 4,
-	.phandle_name = "fsl,ddrc",
-};
-
-static const struct imx_icc_node_adj_desc imx8mq_noc_adj = {
-	.bw_mul = 1,
-	.bw_div = 4,
-	.main_noc = true,
-};
-
 /*
  * Describe bus masters, slaves and connections between them
  *
@@ -31,43 +19,42 @@ static const struct imx_icc_node_adj_desc imx8mq_noc_adj = {
  * PL301 nics which are skipped/merged into PL301_MAIN
  */
 static struct imx_icc_node_desc nodes[] = {
-	DEFINE_BUS_INTERCONNECT("NOC", IMX8MQ_ICN_NOC, &imx8mq_noc_adj,
-			IMX8MQ_ICS_DRAM, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MQ_ICN_NOC, IMX8MQ_ICS_DRAM, IMX8MQ_ICN_MAIN),
 
-	DEFINE_BUS_SLAVE("DRAM", IMX8MQ_ICS_DRAM, &imx8mq_dram_adj),
-	DEFINE_BUS_SLAVE("OCRAM", IMX8MQ_ICS_OCRAM, NULL),
+	DEFINE_BUS_SLAVE("DRAM", IMX8MQ_ICS_DRAM),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MQ_ICS_OCRAM),
 	DEFINE_BUS_MASTER("A53", IMX8MQ_ICM_A53, IMX8MQ_ICN_NOC),
 
 	/* VPUMIX */
 	DEFINE_BUS_MASTER("VPU", IMX8MQ_ICM_VPU, IMX8MQ_ICN_VIDEO),
-	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MQ_ICN_VIDEO, NULL, IMX8MQ_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MQ_ICN_VIDEO, IMX8MQ_ICN_NOC),
 
 	/* GPUMIX */
 	DEFINE_BUS_MASTER("GPU", IMX8MQ_ICM_GPU, IMX8MQ_ICN_GPU),
-	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MQ_ICN_GPU, NULL, IMX8MQ_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MQ_ICN_GPU, IMX8MQ_ICN_NOC),
 
 	/* DISPMIX (only for DCSS) */
 	DEFINE_BUS_MASTER("DC", IMX8MQ_ICM_DCSS, IMX8MQ_ICN_DCSS),
-	DEFINE_BUS_INTERCONNECT("PL301_DC", IMX8MQ_ICN_DCSS, NULL, IMX8MQ_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_DC", IMX8MQ_ICN_DCSS, IMX8MQ_ICN_NOC),
 
 	/* USBMIX */
 	DEFINE_BUS_MASTER("USB1", IMX8MQ_ICM_USB1, IMX8MQ_ICN_USB),
 	DEFINE_BUS_MASTER("USB2", IMX8MQ_ICM_USB2, IMX8MQ_ICN_USB),
-	DEFINE_BUS_INTERCONNECT("PL301_USB", IMX8MQ_ICN_USB, NULL, IMX8MQ_ICN_NOC),
+	DEFINE_BUS_INTERCONNECT("PL301_USB", IMX8MQ_ICN_USB, IMX8MQ_ICN_NOC),
 
 	/* PL301_DISPLAY (IPs other than DCSS, inside SUPERMIX) */
 	DEFINE_BUS_MASTER("CSI1", IMX8MQ_ICM_CSI1, IMX8MQ_ICN_DISPLAY),
 	DEFINE_BUS_MASTER("CSI2", IMX8MQ_ICM_CSI2, IMX8MQ_ICN_DISPLAY),
 	DEFINE_BUS_MASTER("LCDIF", IMX8MQ_ICM_LCDIF, IMX8MQ_ICN_DISPLAY),
-	DEFINE_BUS_INTERCONNECT("PL301_DISPLAY", IMX8MQ_ICN_DISPLAY, NULL, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_DISPLAY", IMX8MQ_ICN_DISPLAY, IMX8MQ_ICN_MAIN),
 
 	/* AUDIO */
 	DEFINE_BUS_MASTER("SDMA2", IMX8MQ_ICM_SDMA2, IMX8MQ_ICN_AUDIO),
-	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MQ_ICN_AUDIO, NULL, IMX8MQ_ICN_DISPLAY),
+	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MQ_ICN_AUDIO, IMX8MQ_ICN_DISPLAY),
 
 	/* ENET */
 	DEFINE_BUS_MASTER("ENET", IMX8MQ_ICM_ENET, IMX8MQ_ICN_ENET),
-	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MQ_ICN_ENET, NULL, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MQ_ICN_ENET, IMX8MQ_ICN_MAIN),
 
 	/* OTHER */
 	DEFINE_BUS_MASTER("SDMA1", IMX8MQ_ICM_SDMA1, IMX8MQ_ICN_MAIN),
@@ -76,7 +63,7 @@ static struct imx_icc_node_desc nodes[] = {
 	DEFINE_BUS_MASTER("USDHC2", IMX8MQ_ICM_USDHC2, IMX8MQ_ICN_MAIN),
 	DEFINE_BUS_MASTER("PCIE1", IMX8MQ_ICM_PCIE1, IMX8MQ_ICN_MAIN),
 	DEFINE_BUS_MASTER("PCIE2", IMX8MQ_ICM_PCIE2, IMX8MQ_ICN_MAIN),
-	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MQ_ICN_MAIN, NULL,
+	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MQ_ICN_MAIN,
 			IMX8MQ_ICN_NOC, IMX8MQ_ICS_OCRAM),
 };
 
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 08/19] interconnect: imx8: Remove the imx_icc_node_adj_desc

[Adam Ford]

On Mon, Sep 13, 2021 at 12:39 PM Abel Vesa <abel.vesa@nxp.com> wrote:
>
> Now that the imx generic interconnect doesn't use the
> imx_icc_node_adj_desc, we remove it from all the i.MX8M
> platform drivers.
>
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>  drivers/interconnect/imx/imx.h    | 19 ++++-------------
>  drivers/interconnect/imx/imx8mm.c | 32 +++++++++-------------------
>  drivers/interconnect/imx/imx8mn.c | 28 +++++++------------------
>  drivers/interconnect/imx/imx8mq.c | 35 ++++++++++---------------------
>  4 files changed, 33 insertions(+), 81 deletions(-)
>

I noticed there are no interconnect nodes in the imx8mm nor the
imx8mn, so it appears to me like the mini and nano code won't do
anything.


adam
> diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
> index 75da51076c68..5c9f5138f6aa 100644
> --- a/drivers/interconnect/imx/imx.h
> +++ b/drivers/interconnect/imx/imx.h
> @@ -14,15 +14,6 @@
>
>  #define IMX_ICC_MAX_LINKS      4
>
> -/*
> - * struct imx_icc_node_adj - Describe a dynamic adjustable node
> - */
> -struct imx_icc_node_adj_desc {
> -       unsigned int bw_mul, bw_div;
> -       const char *phandle_name;
> -       bool main_noc;
> -};
> -
>  /*
>   * struct imx_icc_node - Describe an interconnect node
>   * @name: name of the node
> @@ -35,23 +26,21 @@ struct imx_icc_node_desc {
>         u16 id;
>         u16 links[IMX_ICC_MAX_LINKS];
>         u16 num_links;
> -       const struct imx_icc_node_adj_desc *adj;
>  };
>
> -#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, ...)                 \
> +#define DEFINE_BUS_INTERCONNECT(_name, _id, ...)                       \
>         {                                                               \
>                 .id = _id,                                              \
>                 .name = _name,                                          \
> -               .adj = _adj,                                            \
>                 .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),      \
>                 .links = { __VA_ARGS__ },                               \
>         }
>
>  #define DEFINE_BUS_MASTER(_name, _id, _dest_id)                                \
> -       DEFINE_BUS_INTERCONNECT(_name, _id, NULL, _dest_id)
> +       DEFINE_BUS_INTERCONNECT(_name, _id, _dest_id)
>
> -#define DEFINE_BUS_SLAVE(_name, _id, _adj)                             \
> -       DEFINE_BUS_INTERCONNECT(_name, _id, _adj)
> +#define DEFINE_BUS_SLAVE(_name, _id)                                   \
> +       DEFINE_BUS_INTERCONNECT(_name, _id)
>
>  int imx_icc_register(struct platform_device *pdev,
>                      struct imx_icc_node_desc *nodes,
> diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
> index 1083490bb391..0c16110bef9d 100644
> --- a/drivers/interconnect/imx/imx8mm.c
> +++ b/drivers/interconnect/imx/imx8mm.c
> @@ -14,18 +14,6 @@
>
>  #include "imx.h"
>
> -static const struct imx_icc_node_adj_desc imx8mm_dram_adj = {
> -       .bw_mul = 1,
> -       .bw_div = 16,
> -       .phandle_name = "fsl,ddrc",
> -};
> -
> -static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
> -       .bw_mul = 1,
> -       .bw_div = 16,
> -       .main_noc = true,
> -};
> -
>  /*
>   * Describe bus masters, slaves and connections between them
>   *
> @@ -33,43 +21,43 @@ static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
>   * PL301 nics which are skipped/merged into PL301_MAIN
>   */
>  static struct imx_icc_node_desc nodes[] = {
> -       DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, &imx8mm_noc_adj,
> +       DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC,
>                         IMX8MM_ICS_DRAM, IMX8MM_ICN_MAIN),
>
> -       DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM, &imx8mm_dram_adj),
> -       DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM, NULL),
> +       DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM),
> +       DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM),
>         DEFINE_BUS_MASTER("A53", IMX8MM_ICM_A53, IMX8MM_ICN_NOC),
>
>         /* VPUMIX */
>         DEFINE_BUS_MASTER("VPU H1", IMX8MM_ICM_VPU_H1, IMX8MM_ICN_VIDEO),
>         DEFINE_BUS_MASTER("VPU G1", IMX8MM_ICM_VPU_G1, IMX8MM_ICN_VIDEO),
>         DEFINE_BUS_MASTER("VPU G2", IMX8MM_ICM_VPU_G2, IMX8MM_ICN_VIDEO),
> -       DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, NULL, IMX8MM_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, IMX8MM_ICN_NOC),
>
>         /* GPUMIX */
>         DEFINE_BUS_MASTER("GPU 2D", IMX8MM_ICM_GPU2D, IMX8MM_ICN_GPU),
>         DEFINE_BUS_MASTER("GPU 3D", IMX8MM_ICM_GPU3D, IMX8MM_ICN_GPU),
> -       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MM_ICN_GPU, NULL, IMX8MM_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MM_ICN_GPU, IMX8MM_ICN_NOC),
>
>         /* DISPLAYMIX */
>         DEFINE_BUS_MASTER("CSI", IMX8MM_ICM_CSI, IMX8MM_ICN_MIPI),
>         DEFINE_BUS_MASTER("LCDIF", IMX8MM_ICM_LCDIF, IMX8MM_ICN_MIPI),
> -       DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MM_ICN_MIPI, NULL, IMX8MM_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MM_ICN_MIPI, IMX8MM_ICN_NOC),
>
>         /* HSIO */
>         DEFINE_BUS_MASTER("USB1", IMX8MM_ICM_USB1, IMX8MM_ICN_HSIO),
>         DEFINE_BUS_MASTER("USB2", IMX8MM_ICM_USB2, IMX8MM_ICN_HSIO),
>         DEFINE_BUS_MASTER("PCIE", IMX8MM_ICM_PCIE, IMX8MM_ICN_HSIO),
> -       DEFINE_BUS_INTERCONNECT("PL301_HSIO", IMX8MM_ICN_HSIO, NULL, IMX8MM_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_HSIO", IMX8MM_ICN_HSIO, IMX8MM_ICN_NOC),
>
>         /* Audio */
>         DEFINE_BUS_MASTER("SDMA2", IMX8MM_ICM_SDMA2, IMX8MM_ICN_AUDIO),
>         DEFINE_BUS_MASTER("SDMA3", IMX8MM_ICM_SDMA3, IMX8MM_ICN_AUDIO),
> -       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MM_ICN_AUDIO, NULL, IMX8MM_ICN_MAIN),
> +       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MM_ICN_AUDIO, IMX8MM_ICN_MAIN),
>
>         /* Ethernet */
>         DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
> -       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, NULL, IMX8MM_ICN_MAIN),
> +       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, IMX8MM_ICN_MAIN),
>
>         /* Other */
>         DEFINE_BUS_MASTER("SDMA1", IMX8MM_ICM_SDMA1, IMX8MM_ICN_MAIN),
> @@ -77,7 +65,7 @@ static struct imx_icc_node_desc nodes[] = {
>         DEFINE_BUS_MASTER("USDHC1", IMX8MM_ICM_USDHC1, IMX8MM_ICN_MAIN),
>         DEFINE_BUS_MASTER("USDHC2", IMX8MM_ICM_USDHC2, IMX8MM_ICN_MAIN),
>         DEFINE_BUS_MASTER("USDHC3", IMX8MM_ICM_USDHC3, IMX8MM_ICN_MAIN),
> -       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MM_ICN_MAIN, NULL,
> +       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MM_ICN_MAIN,
>                         IMX8MM_ICN_NOC, IMX8MM_ICS_OCRAM),
>  };
>
> diff --git a/drivers/interconnect/imx/imx8mn.c b/drivers/interconnect/imx/imx8mn.c
> index ad97e55fd4e5..8d16bd5cf006 100644
> --- a/drivers/interconnect/imx/imx8mn.c
> +++ b/drivers/interconnect/imx/imx8mn.c
> @@ -11,18 +11,6 @@
>
>  #include "imx.h"
>
> -static const struct imx_icc_node_adj_desc imx8mn_dram_adj = {
> -       .bw_mul = 1,
> -       .bw_div = 4,
> -       .phandle_name = "fsl,ddrc",
> -};
> -
> -static const struct imx_icc_node_adj_desc imx8mn_noc_adj = {
> -       .bw_mul = 1,
> -       .bw_div = 4,
> -       .main_noc = true,
> -};
> -
>  /*
>   * Describe bus masters, slaves and connections between them
>   *
> @@ -30,23 +18,23 @@ static const struct imx_icc_node_adj_desc imx8mn_noc_adj = {
>   * PL301 nics which are skipped/merged into PL301_MAIN
>   */
>  static struct imx_icc_node_desc nodes[] = {
> -       DEFINE_BUS_INTERCONNECT("NOC", IMX8MN_ICN_NOC, &imx8mn_noc_adj,
> +       DEFINE_BUS_INTERCONNECT("NOC", IMX8MN_ICN_NOC,
>                         IMX8MN_ICS_DRAM, IMX8MN_ICN_MAIN),
>
> -       DEFINE_BUS_SLAVE("DRAM", IMX8MN_ICS_DRAM, &imx8mn_dram_adj),
> -       DEFINE_BUS_SLAVE("OCRAM", IMX8MN_ICS_OCRAM, NULL),
> +       DEFINE_BUS_SLAVE("DRAM", IMX8MN_ICS_DRAM),
> +       DEFINE_BUS_SLAVE("OCRAM", IMX8MN_ICS_OCRAM),
>         DEFINE_BUS_MASTER("A53", IMX8MN_ICM_A53, IMX8MN_ICN_NOC),
>
>         /* GPUMIX */
>         DEFINE_BUS_MASTER("GPU", IMX8MN_ICM_GPU, IMX8MN_ICN_GPU),
> -       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MN_ICN_GPU, NULL, IMX8MN_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MN_ICN_GPU, IMX8MN_ICN_NOC),
>
>         /* DISPLAYMIX */
>         DEFINE_BUS_MASTER("CSI1", IMX8MN_ICM_CSI1, IMX8MN_ICN_MIPI),
>         DEFINE_BUS_MASTER("CSI2", IMX8MN_ICM_CSI2, IMX8MN_ICN_MIPI),
>         DEFINE_BUS_MASTER("ISI", IMX8MN_ICM_ISI, IMX8MN_ICN_MIPI),
>         DEFINE_BUS_MASTER("LCDIF", IMX8MN_ICM_LCDIF, IMX8MN_ICN_MIPI),
> -       DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MN_ICN_MIPI, NULL, IMX8MN_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MN_ICN_MIPI, IMX8MN_ICN_NOC),
>
>         /* USB goes straight to NOC */
>         DEFINE_BUS_MASTER("USB", IMX8MN_ICM_USB, IMX8MN_ICN_NOC),
> @@ -54,11 +42,11 @@ static struct imx_icc_node_desc nodes[] = {
>         /* Audio */
>         DEFINE_BUS_MASTER("SDMA2", IMX8MN_ICM_SDMA2, IMX8MN_ICN_AUDIO),
>         DEFINE_BUS_MASTER("SDMA3", IMX8MN_ICM_SDMA3, IMX8MN_ICN_AUDIO),
> -       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MN_ICN_AUDIO, NULL, IMX8MN_ICN_MAIN),
> +       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MN_ICN_AUDIO, IMX8MN_ICN_MAIN),
>
>         /* Ethernet */
>         DEFINE_BUS_MASTER("ENET", IMX8MN_ICM_ENET, IMX8MN_ICN_ENET),
> -       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MN_ICN_ENET, NULL, IMX8MN_ICN_MAIN),
> +       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MN_ICN_ENET, IMX8MN_ICN_MAIN),
>
>         /* Other */
>         DEFINE_BUS_MASTER("SDMA1", IMX8MN_ICM_SDMA1, IMX8MN_ICN_MAIN),
> @@ -66,7 +54,7 @@ static struct imx_icc_node_desc nodes[] = {
>         DEFINE_BUS_MASTER("USDHC1", IMX8MN_ICM_USDHC1, IMX8MN_ICN_MAIN),
>         DEFINE_BUS_MASTER("USDHC2", IMX8MN_ICM_USDHC2, IMX8MN_ICN_MAIN),
>         DEFINE_BUS_MASTER("USDHC3", IMX8MN_ICM_USDHC3, IMX8MN_ICN_MAIN),
> -       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MN_ICN_MAIN, NULL,
> +       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MN_ICN_MAIN
>                         IMX8MN_ICN_NOC, IMX8MN_ICS_OCRAM),
>  };
>
> diff --git a/drivers/interconnect/imx/imx8mq.c b/drivers/interconnect/imx/imx8mq.c
> index d7768d3c6d8a..b8c36d668946 100644
> --- a/drivers/interconnect/imx/imx8mq.c
> +++ b/drivers/interconnect/imx/imx8mq.c
> @@ -12,18 +12,6 @@
>
>  #include "imx.h"
>
> -static const struct imx_icc_node_adj_desc imx8mq_dram_adj = {
> -       .bw_mul = 1,
> -       .bw_div = 4,
> -       .phandle_name = "fsl,ddrc",
> -};
> -
> -static const struct imx_icc_node_adj_desc imx8mq_noc_adj = {
> -       .bw_mul = 1,
> -       .bw_div = 4,
> -       .main_noc = true,
> -};
> -
>  /*
>   * Describe bus masters, slaves and connections between them
>   *
> @@ -31,43 +19,42 @@ static const struct imx_icc_node_adj_desc imx8mq_noc_adj = {
>   * PL301 nics which are skipped/merged into PL301_MAIN
>   */
>  static struct imx_icc_node_desc nodes[] = {
> -       DEFINE_BUS_INTERCONNECT("NOC", IMX8MQ_ICN_NOC, &imx8mq_noc_adj,
> -                       IMX8MQ_ICS_DRAM, IMX8MQ_ICN_MAIN),
> +       DEFINE_BUS_INTERCONNECT("NOC", IMX8MQ_ICN_NOC, IMX8MQ_ICS_DRAM, IMX8MQ_ICN_MAIN),
>
> -       DEFINE_BUS_SLAVE("DRAM", IMX8MQ_ICS_DRAM, &imx8mq_dram_adj),
> -       DEFINE_BUS_SLAVE("OCRAM", IMX8MQ_ICS_OCRAM, NULL),
> +       DEFINE_BUS_SLAVE("DRAM", IMX8MQ_ICS_DRAM),
> +       DEFINE_BUS_SLAVE("OCRAM", IMX8MQ_ICS_OCRAM),
>         DEFINE_BUS_MASTER("A53", IMX8MQ_ICM_A53, IMX8MQ_ICN_NOC),
>
>         /* VPUMIX */
>         DEFINE_BUS_MASTER("VPU", IMX8MQ_ICM_VPU, IMX8MQ_ICN_VIDEO),
> -       DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MQ_ICN_VIDEO, NULL, IMX8MQ_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MQ_ICN_VIDEO, IMX8MQ_ICN_NOC),
>
>         /* GPUMIX */
>         DEFINE_BUS_MASTER("GPU", IMX8MQ_ICM_GPU, IMX8MQ_ICN_GPU),
> -       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MQ_ICN_GPU, NULL, IMX8MQ_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MQ_ICN_GPU, IMX8MQ_ICN_NOC),
>
>         /* DISPMIX (only for DCSS) */
>         DEFINE_BUS_MASTER("DC", IMX8MQ_ICM_DCSS, IMX8MQ_ICN_DCSS),
> -       DEFINE_BUS_INTERCONNECT("PL301_DC", IMX8MQ_ICN_DCSS, NULL, IMX8MQ_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_DC", IMX8MQ_ICN_DCSS, IMX8MQ_ICN_NOC),
>
>         /* USBMIX */
>         DEFINE_BUS_MASTER("USB1", IMX8MQ_ICM_USB1, IMX8MQ_ICN_USB),
>         DEFINE_BUS_MASTER("USB2", IMX8MQ_ICM_USB2, IMX8MQ_ICN_USB),
> -       DEFINE_BUS_INTERCONNECT("PL301_USB", IMX8MQ_ICN_USB, NULL, IMX8MQ_ICN_NOC),
> +       DEFINE_BUS_INTERCONNECT("PL301_USB", IMX8MQ_ICN_USB, IMX8MQ_ICN_NOC),
>
>         /* PL301_DISPLAY (IPs other than DCSS, inside SUPERMIX) */
>         DEFINE_BUS_MASTER("CSI1", IMX8MQ_ICM_CSI1, IMX8MQ_ICN_DISPLAY),
>         DEFINE_BUS_MASTER("CSI2", IMX8MQ_ICM_CSI2, IMX8MQ_ICN_DISPLAY),
>         DEFINE_BUS_MASTER("LCDIF", IMX8MQ_ICM_LCDIF, IMX8MQ_ICN_DISPLAY),
> -       DEFINE_BUS_INTERCONNECT("PL301_DISPLAY", IMX8MQ_ICN_DISPLAY, NULL, IMX8MQ_ICN_MAIN),
> +       DEFINE_BUS_INTERCONNECT("PL301_DISPLAY", IMX8MQ_ICN_DISPLAY, IMX8MQ_ICN_MAIN),
>
>         /* AUDIO */
>         DEFINE_BUS_MASTER("SDMA2", IMX8MQ_ICM_SDMA2, IMX8MQ_ICN_AUDIO),
> -       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MQ_ICN_AUDIO, NULL, IMX8MQ_ICN_DISPLAY),
> +       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MQ_ICN_AUDIO, IMX8MQ_ICN_DISPLAY),
>
>         /* ENET */
>         DEFINE_BUS_MASTER("ENET", IMX8MQ_ICM_ENET, IMX8MQ_ICN_ENET),
> -       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MQ_ICN_ENET, NULL, IMX8MQ_ICN_MAIN),
> +       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MQ_ICN_ENET, IMX8MQ_ICN_MAIN),
>
>         /* OTHER */
>         DEFINE_BUS_MASTER("SDMA1", IMX8MQ_ICM_SDMA1, IMX8MQ_ICN_MAIN),
> @@ -76,7 +63,7 @@ static struct imx_icc_node_desc nodes[] = {
>         DEFINE_BUS_MASTER("USDHC2", IMX8MQ_ICM_USDHC2, IMX8MQ_ICN_MAIN),
>         DEFINE_BUS_MASTER("PCIE1", IMX8MQ_ICM_PCIE1, IMX8MQ_ICN_MAIN),
>         DEFINE_BUS_MASTER("PCIE2", IMX8MQ_ICM_PCIE2, IMX8MQ_ICN_MAIN),
> -       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MQ_ICN_MAIN, NULL,
> +       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MQ_ICN_MAIN,
>                         IMX8MQ_ICN_NOC, IMX8MQ_ICS_OCRAM),
>  };
>
> --
> 2.31.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 08/19] interconnect: imx8: Remove the imx_icc_node_adj_desc

[Abel Vesa]

On 21-10-18 07:41:31, Adam Ford wrote:
> On Mon, Sep 13, 2021 at 12:39 PM Abel Vesa <abel.vesa@nxp.com> wrote:
> >
> > Now that the imx generic interconnect doesn't use the
> > imx_icc_node_adj_desc, we remove it from all the i.MX8M
> > platform drivers.
> >
> > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > ---
> >  drivers/interconnect/imx/imx.h    | 19 ++++-------------
> >  drivers/interconnect/imx/imx8mm.c | 32 +++++++++-------------------
> >  drivers/interconnect/imx/imx8mn.c | 28 +++++++------------------
> >  drivers/interconnect/imx/imx8mq.c | 35 ++++++++++---------------------
> >  4 files changed, 33 insertions(+), 81 deletions(-)
> >
> 
> I noticed there are no interconnect nodes in the imx8mm nor the
> imx8mn, so it appears to me like the mini and nano code won't do
> anything.
> 

icc + imx-bus + imx8m-ddrc is an approach to have support for
dropping the bus and ddr clock rates when there is no user needing the
higher rates. This used to be done in our NXP tree via something called
busfreq. The problem with busfreq is that it cannot be upstreamed and
it's quite limited. As of now, there is no support for dropping bus and
ddr clock rates for any of the i.MX platforms in upstream. Parts of the
implementation of this new approach made it upstream a couple of
years ago. Therefore, even if you compile the interconnect driver in,
you won't have the full functionality. With this patchset we'll get
support for i.MX8MQ first. The rest of 8M platforms would get support
once we agree on i.MX8MQ since the generic part would not change at all.
I'll send a subsequent updated version soon, with all the comments taken
care of.

> 
> adam
> > diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
> > index 75da51076c68..5c9f5138f6aa 100644
> > --- a/drivers/interconnect/imx/imx.h
> > +++ b/drivers/interconnect/imx/imx.h
> > @@ -14,15 +14,6 @@
> >
> >  #define IMX_ICC_MAX_LINKS      4
> >
> > -/*
> > - * struct imx_icc_node_adj - Describe a dynamic adjustable node
> > - */
> > -struct imx_icc_node_adj_desc {
> > -       unsigned int bw_mul, bw_div;
> > -       const char *phandle_name;
> > -       bool main_noc;
> > -};
> > -
> >  /*
> >   * struct imx_icc_node - Describe an interconnect node
> >   * @name: name of the node
> > @@ -35,23 +26,21 @@ struct imx_icc_node_desc {
> >         u16 id;
> >         u16 links[IMX_ICC_MAX_LINKS];
> >         u16 num_links;
> > -       const struct imx_icc_node_adj_desc *adj;
> >  };
> >
> > -#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, ...)                 \
> > +#define DEFINE_BUS_INTERCONNECT(_name, _id, ...)                       \
> >         {                                                               \
> >                 .id = _id,                                              \
> >                 .name = _name,                                          \
> > -               .adj = _adj,                                            \
> >                 .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),      \
> >                 .links = { __VA_ARGS__ },                               \
> >         }
> >
> >  #define DEFINE_BUS_MASTER(_name, _id, _dest_id)                                \
> > -       DEFINE_BUS_INTERCONNECT(_name, _id, NULL, _dest_id)
> > +       DEFINE_BUS_INTERCONNECT(_name, _id, _dest_id)
> >
> > -#define DEFINE_BUS_SLAVE(_name, _id, _adj)                             \
> > -       DEFINE_BUS_INTERCONNECT(_name, _id, _adj)
> > +#define DEFINE_BUS_SLAVE(_name, _id)                                   \
> > +       DEFINE_BUS_INTERCONNECT(_name, _id)
> >
> >  int imx_icc_register(struct platform_device *pdev,
> >                      struct imx_icc_node_desc *nodes,
> > diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
> > index 1083490bb391..0c16110bef9d 100644
> > --- a/drivers/interconnect/imx/imx8mm.c
> > +++ b/drivers/interconnect/imx/imx8mm.c
> > @@ -14,18 +14,6 @@
> >
> >  #include "imx.h"
> >
> > -static const struct imx_icc_node_adj_desc imx8mm_dram_adj = {
> > -       .bw_mul = 1,
> > -       .bw_div = 16,
> > -       .phandle_name = "fsl,ddrc",
> > -};
> > -
> > -static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
> > -       .bw_mul = 1,
> > -       .bw_div = 16,
> > -       .main_noc = true,
> > -};
> > -
> >  /*
> >   * Describe bus masters, slaves and connections between them
> >   *
> > @@ -33,43 +21,43 @@ static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
> >   * PL301 nics which are skipped/merged into PL301_MAIN
> >   */
> >  static struct imx_icc_node_desc nodes[] = {
> > -       DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, &imx8mm_noc_adj,
> > +       DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC,
> >                         IMX8MM_ICS_DRAM, IMX8MM_ICN_MAIN),
> >
> > -       DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM, &imx8mm_dram_adj),
> > -       DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM, NULL),
> > +       DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM),
> > +       DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM),
> >         DEFINE_BUS_MASTER("A53", IMX8MM_ICM_A53, IMX8MM_ICN_NOC),
> >
> >         /* VPUMIX */
> >         DEFINE_BUS_MASTER("VPU H1", IMX8MM_ICM_VPU_H1, IMX8MM_ICN_VIDEO),
> >         DEFINE_BUS_MASTER("VPU G1", IMX8MM_ICM_VPU_G1, IMX8MM_ICN_VIDEO),
> >         DEFINE_BUS_MASTER("VPU G2", IMX8MM_ICM_VPU_G2, IMX8MM_ICN_VIDEO),
> > -       DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, NULL, IMX8MM_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, IMX8MM_ICN_NOC),
> >
> >         /* GPUMIX */
> >         DEFINE_BUS_MASTER("GPU 2D", IMX8MM_ICM_GPU2D, IMX8MM_ICN_GPU),
> >         DEFINE_BUS_MASTER("GPU 3D", IMX8MM_ICM_GPU3D, IMX8MM_ICN_GPU),
> > -       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MM_ICN_GPU, NULL, IMX8MM_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MM_ICN_GPU, IMX8MM_ICN_NOC),
> >
> >         /* DISPLAYMIX */
> >         DEFINE_BUS_MASTER("CSI", IMX8MM_ICM_CSI, IMX8MM_ICN_MIPI),
> >         DEFINE_BUS_MASTER("LCDIF", IMX8MM_ICM_LCDIF, IMX8MM_ICN_MIPI),
> > -       DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MM_ICN_MIPI, NULL, IMX8MM_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MM_ICN_MIPI, IMX8MM_ICN_NOC),
> >
> >         /* HSIO */
> >         DEFINE_BUS_MASTER("USB1", IMX8MM_ICM_USB1, IMX8MM_ICN_HSIO),
> >         DEFINE_BUS_MASTER("USB2", IMX8MM_ICM_USB2, IMX8MM_ICN_HSIO),
> >         DEFINE_BUS_MASTER("PCIE", IMX8MM_ICM_PCIE, IMX8MM_ICN_HSIO),
> > -       DEFINE_BUS_INTERCONNECT("PL301_HSIO", IMX8MM_ICN_HSIO, NULL, IMX8MM_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_HSIO", IMX8MM_ICN_HSIO, IMX8MM_ICN_NOC),
> >
> >         /* Audio */
> >         DEFINE_BUS_MASTER("SDMA2", IMX8MM_ICM_SDMA2, IMX8MM_ICN_AUDIO),
> >         DEFINE_BUS_MASTER("SDMA3", IMX8MM_ICM_SDMA3, IMX8MM_ICN_AUDIO),
> > -       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MM_ICN_AUDIO, NULL, IMX8MM_ICN_MAIN),
> > +       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MM_ICN_AUDIO, IMX8MM_ICN_MAIN),
> >
> >         /* Ethernet */
> >         DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
> > -       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, NULL, IMX8MM_ICN_MAIN),
> > +       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, IMX8MM_ICN_MAIN),
> >
> >         /* Other */
> >         DEFINE_BUS_MASTER("SDMA1", IMX8MM_ICM_SDMA1, IMX8MM_ICN_MAIN),
> > @@ -77,7 +65,7 @@ static struct imx_icc_node_desc nodes[] = {
> >         DEFINE_BUS_MASTER("USDHC1", IMX8MM_ICM_USDHC1, IMX8MM_ICN_MAIN),
> >         DEFINE_BUS_MASTER("USDHC2", IMX8MM_ICM_USDHC2, IMX8MM_ICN_MAIN),
> >         DEFINE_BUS_MASTER("USDHC3", IMX8MM_ICM_USDHC3, IMX8MM_ICN_MAIN),
> > -       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MM_ICN_MAIN, NULL,
> > +       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MM_ICN_MAIN,
> >                         IMX8MM_ICN_NOC, IMX8MM_ICS_OCRAM),
> >  };
> >
> > diff --git a/drivers/interconnect/imx/imx8mn.c b/drivers/interconnect/imx/imx8mn.c
> > index ad97e55fd4e5..8d16bd5cf006 100644
> > --- a/drivers/interconnect/imx/imx8mn.c
> > +++ b/drivers/interconnect/imx/imx8mn.c
> > @@ -11,18 +11,6 @@
> >
> >  #include "imx.h"
> >
> > -static const struct imx_icc_node_adj_desc imx8mn_dram_adj = {
> > -       .bw_mul = 1,
> > -       .bw_div = 4,
> > -       .phandle_name = "fsl,ddrc",
> > -};
> > -
> > -static const struct imx_icc_node_adj_desc imx8mn_noc_adj = {
> > -       .bw_mul = 1,
> > -       .bw_div = 4,
> > -       .main_noc = true,
> > -};
> > -
> >  /*
> >   * Describe bus masters, slaves and connections between them
> >   *
> > @@ -30,23 +18,23 @@ static const struct imx_icc_node_adj_desc imx8mn_noc_adj = {
> >   * PL301 nics which are skipped/merged into PL301_MAIN
> >   */
> >  static struct imx_icc_node_desc nodes[] = {
> > -       DEFINE_BUS_INTERCONNECT("NOC", IMX8MN_ICN_NOC, &imx8mn_noc_adj,
> > +       DEFINE_BUS_INTERCONNECT("NOC", IMX8MN_ICN_NOC,
> >                         IMX8MN_ICS_DRAM, IMX8MN_ICN_MAIN),
> >
> > -       DEFINE_BUS_SLAVE("DRAM", IMX8MN_ICS_DRAM, &imx8mn_dram_adj),
> > -       DEFINE_BUS_SLAVE("OCRAM", IMX8MN_ICS_OCRAM, NULL),
> > +       DEFINE_BUS_SLAVE("DRAM", IMX8MN_ICS_DRAM),
> > +       DEFINE_BUS_SLAVE("OCRAM", IMX8MN_ICS_OCRAM),
> >         DEFINE_BUS_MASTER("A53", IMX8MN_ICM_A53, IMX8MN_ICN_NOC),
> >
> >         /* GPUMIX */
> >         DEFINE_BUS_MASTER("GPU", IMX8MN_ICM_GPU, IMX8MN_ICN_GPU),
> > -       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MN_ICN_GPU, NULL, IMX8MN_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MN_ICN_GPU, IMX8MN_ICN_NOC),
> >
> >         /* DISPLAYMIX */
> >         DEFINE_BUS_MASTER("CSI1", IMX8MN_ICM_CSI1, IMX8MN_ICN_MIPI),
> >         DEFINE_BUS_MASTER("CSI2", IMX8MN_ICM_CSI2, IMX8MN_ICN_MIPI),
> >         DEFINE_BUS_MASTER("ISI", IMX8MN_ICM_ISI, IMX8MN_ICN_MIPI),
> >         DEFINE_BUS_MASTER("LCDIF", IMX8MN_ICM_LCDIF, IMX8MN_ICN_MIPI),
> > -       DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MN_ICN_MIPI, NULL, IMX8MN_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MN_ICN_MIPI, IMX8MN_ICN_NOC),
> >
> >         /* USB goes straight to NOC */
> >         DEFINE_BUS_MASTER("USB", IMX8MN_ICM_USB, IMX8MN_ICN_NOC),
> > @@ -54,11 +42,11 @@ static struct imx_icc_node_desc nodes[] = {
> >         /* Audio */
> >         DEFINE_BUS_MASTER("SDMA2", IMX8MN_ICM_SDMA2, IMX8MN_ICN_AUDIO),
> >         DEFINE_BUS_MASTER("SDMA3", IMX8MN_ICM_SDMA3, IMX8MN_ICN_AUDIO),
> > -       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MN_ICN_AUDIO, NULL, IMX8MN_ICN_MAIN),
> > +       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MN_ICN_AUDIO, IMX8MN_ICN_MAIN),
> >
> >         /* Ethernet */
> >         DEFINE_BUS_MASTER("ENET", IMX8MN_ICM_ENET, IMX8MN_ICN_ENET),
> > -       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MN_ICN_ENET, NULL, IMX8MN_ICN_MAIN),
> > +       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MN_ICN_ENET, IMX8MN_ICN_MAIN),
> >
> >         /* Other */
> >         DEFINE_BUS_MASTER("SDMA1", IMX8MN_ICM_SDMA1, IMX8MN_ICN_MAIN),
> > @@ -66,7 +54,7 @@ static struct imx_icc_node_desc nodes[] = {
> >         DEFINE_BUS_MASTER("USDHC1", IMX8MN_ICM_USDHC1, IMX8MN_ICN_MAIN),
> >         DEFINE_BUS_MASTER("USDHC2", IMX8MN_ICM_USDHC2, IMX8MN_ICN_MAIN),
> >         DEFINE_BUS_MASTER("USDHC3", IMX8MN_ICM_USDHC3, IMX8MN_ICN_MAIN),
> > -       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MN_ICN_MAIN, NULL,
> > +       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MN_ICN_MAIN
> >                         IMX8MN_ICN_NOC, IMX8MN_ICS_OCRAM),
> >  };
> >
> > diff --git a/drivers/interconnect/imx/imx8mq.c b/drivers/interconnect/imx/imx8mq.c
> > index d7768d3c6d8a..b8c36d668946 100644
> > --- a/drivers/interconnect/imx/imx8mq.c
> > +++ b/drivers/interconnect/imx/imx8mq.c
> > @@ -12,18 +12,6 @@
> >
> >  #include "imx.h"
> >
> > -static const struct imx_icc_node_adj_desc imx8mq_dram_adj = {
> > -       .bw_mul = 1,
> > -       .bw_div = 4,
> > -       .phandle_name = "fsl,ddrc",
> > -};
> > -
> > -static const struct imx_icc_node_adj_desc imx8mq_noc_adj = {
> > -       .bw_mul = 1,
> > -       .bw_div = 4,
> > -       .main_noc = true,
> > -};
> > -
> >  /*
> >   * Describe bus masters, slaves and connections between them
> >   *
> > @@ -31,43 +19,42 @@ static const struct imx_icc_node_adj_desc imx8mq_noc_adj = {
> >   * PL301 nics which are skipped/merged into PL301_MAIN
> >   */
> >  static struct imx_icc_node_desc nodes[] = {
> > -       DEFINE_BUS_INTERCONNECT("NOC", IMX8MQ_ICN_NOC, &imx8mq_noc_adj,
> > -                       IMX8MQ_ICS_DRAM, IMX8MQ_ICN_MAIN),
> > +       DEFINE_BUS_INTERCONNECT("NOC", IMX8MQ_ICN_NOC, IMX8MQ_ICS_DRAM, IMX8MQ_ICN_MAIN),
> >
> > -       DEFINE_BUS_SLAVE("DRAM", IMX8MQ_ICS_DRAM, &imx8mq_dram_adj),
> > -       DEFINE_BUS_SLAVE("OCRAM", IMX8MQ_ICS_OCRAM, NULL),
> > +       DEFINE_BUS_SLAVE("DRAM", IMX8MQ_ICS_DRAM),
> > +       DEFINE_BUS_SLAVE("OCRAM", IMX8MQ_ICS_OCRAM),
> >         DEFINE_BUS_MASTER("A53", IMX8MQ_ICM_A53, IMX8MQ_ICN_NOC),
> >
> >         /* VPUMIX */
> >         DEFINE_BUS_MASTER("VPU", IMX8MQ_ICM_VPU, IMX8MQ_ICN_VIDEO),
> > -       DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MQ_ICN_VIDEO, NULL, IMX8MQ_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MQ_ICN_VIDEO, IMX8MQ_ICN_NOC),
> >
> >         /* GPUMIX */
> >         DEFINE_BUS_MASTER("GPU", IMX8MQ_ICM_GPU, IMX8MQ_ICN_GPU),
> > -       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MQ_ICN_GPU, NULL, IMX8MQ_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MQ_ICN_GPU, IMX8MQ_ICN_NOC),
> >
> >         /* DISPMIX (only for DCSS) */
> >         DEFINE_BUS_MASTER("DC", IMX8MQ_ICM_DCSS, IMX8MQ_ICN_DCSS),
> > -       DEFINE_BUS_INTERCONNECT("PL301_DC", IMX8MQ_ICN_DCSS, NULL, IMX8MQ_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_DC", IMX8MQ_ICN_DCSS, IMX8MQ_ICN_NOC),
> >
> >         /* USBMIX */
> >         DEFINE_BUS_MASTER("USB1", IMX8MQ_ICM_USB1, IMX8MQ_ICN_USB),
> >         DEFINE_BUS_MASTER("USB2", IMX8MQ_ICM_USB2, IMX8MQ_ICN_USB),
> > -       DEFINE_BUS_INTERCONNECT("PL301_USB", IMX8MQ_ICN_USB, NULL, IMX8MQ_ICN_NOC),
> > +       DEFINE_BUS_INTERCONNECT("PL301_USB", IMX8MQ_ICN_USB, IMX8MQ_ICN_NOC),
> >
> >         /* PL301_DISPLAY (IPs other than DCSS, inside SUPERMIX) */
> >         DEFINE_BUS_MASTER("CSI1", IMX8MQ_ICM_CSI1, IMX8MQ_ICN_DISPLAY),
> >         DEFINE_BUS_MASTER("CSI2", IMX8MQ_ICM_CSI2, IMX8MQ_ICN_DISPLAY),
> >         DEFINE_BUS_MASTER("LCDIF", IMX8MQ_ICM_LCDIF, IMX8MQ_ICN_DISPLAY),
> > -       DEFINE_BUS_INTERCONNECT("PL301_DISPLAY", IMX8MQ_ICN_DISPLAY, NULL, IMX8MQ_ICN_MAIN),
> > +       DEFINE_BUS_INTERCONNECT("PL301_DISPLAY", IMX8MQ_ICN_DISPLAY, IMX8MQ_ICN_MAIN),
> >
> >         /* AUDIO */
> >         DEFINE_BUS_MASTER("SDMA2", IMX8MQ_ICM_SDMA2, IMX8MQ_ICN_AUDIO),
> > -       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MQ_ICN_AUDIO, NULL, IMX8MQ_ICN_DISPLAY),
> > +       DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MQ_ICN_AUDIO, IMX8MQ_ICN_DISPLAY),
> >
> >         /* ENET */
> >         DEFINE_BUS_MASTER("ENET", IMX8MQ_ICM_ENET, IMX8MQ_ICN_ENET),
> > -       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MQ_ICN_ENET, NULL, IMX8MQ_ICN_MAIN),
> > +       DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MQ_ICN_ENET, IMX8MQ_ICN_MAIN),
> >
> >         /* OTHER */
> >         DEFINE_BUS_MASTER("SDMA1", IMX8MQ_ICM_SDMA1, IMX8MQ_ICN_MAIN),
> > @@ -76,7 +63,7 @@ static struct imx_icc_node_desc nodes[] = {
> >         DEFINE_BUS_MASTER("USDHC2", IMX8MQ_ICM_USDHC2, IMX8MQ_ICN_MAIN),
> >         DEFINE_BUS_MASTER("PCIE1", IMX8MQ_ICM_PCIE1, IMX8MQ_ICN_MAIN),
> >         DEFINE_BUS_MASTER("PCIE2", IMX8MQ_ICM_PCIE2, IMX8MQ_ICN_MAIN),
> > -       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MQ_ICN_MAIN, NULL,
> > +       DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MQ_ICN_MAIN,
> >                         IMX8MQ_ICN_NOC, IMX8MQ_ICS_OCRAM),
> >  };
> >
> > --
> > 2.31.1
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 09/19] interconnect: imx8mq: Add the pl301_per_m and pl301_wakeup nodes and subnodes

[Abel Vesa]

According to the bus diagram, there are two more pl301s that need to
be added here. The pl301_per_m which is an intermediary node between
pl301_main and its masters: usdhc1, usdhc2 and sdma. The pl301_wakeup
is an intermediary node between pl301_main and its masters, in this case
all the SAIs.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/interconnect/imx/imx8mq.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/interconnect/imx/imx8mq.c b/drivers/interconnect/imx/imx8mq.c
index b8c36d668946..010ad3d76286 100644
--- a/drivers/interconnect/imx/imx8mq.c
+++ b/drivers/interconnect/imx/imx8mq.c
@@ -57,14 +57,25 @@ static struct imx_icc_node_desc nodes[] = {
 	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MQ_ICN_ENET, IMX8MQ_ICN_MAIN),
 
 	/* OTHER */
-	DEFINE_BUS_MASTER("SDMA1", IMX8MQ_ICM_SDMA1, IMX8MQ_ICN_MAIN),
 	DEFINE_BUS_MASTER("NAND", IMX8MQ_ICM_NAND, IMX8MQ_ICN_MAIN),
-	DEFINE_BUS_MASTER("USDHC1", IMX8MQ_ICM_USDHC1, IMX8MQ_ICN_MAIN),
-	DEFINE_BUS_MASTER("USDHC2", IMX8MQ_ICM_USDHC2, IMX8MQ_ICN_MAIN),
 	DEFINE_BUS_MASTER("PCIE1", IMX8MQ_ICM_PCIE1, IMX8MQ_ICN_MAIN),
 	DEFINE_BUS_MASTER("PCIE2", IMX8MQ_ICM_PCIE2, IMX8MQ_ICN_MAIN),
 	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MQ_ICN_MAIN,
 			IMX8MQ_ICN_NOC, IMX8MQ_ICS_OCRAM),
+	DEFINE_BUS_MASTER("SDMA1", IMX8MQ_ICM_SDMA1, IMX8MQ_ICN_PER_M),
+	DEFINE_BUS_MASTER("USDHC1", IMX8MQ_ICM_USDHC1, IMX8MQ_ICN_PER_M),
+	DEFINE_BUS_MASTER("USDHC2", IMX8MQ_ICM_USDHC2, IMX8MQ_ICN_PER_M),
+	DEFINE_BUS_INTERCONNECT("PL301_PER_M", IMX8MQ_ICN_PER_M,
+			IMX8MQ_ICN_MAIN),
+
+	DEFINE_BUS_MASTER("SAI1", IMX8MQ_ICM_SAI1, IMX8MQ_ICN_WAKEUP),
+	DEFINE_BUS_MASTER("SAI2", IMX8MQ_ICM_SAI2, IMX8MQ_ICN_WAKEUP),
+	DEFINE_BUS_MASTER("SAI3", IMX8MQ_ICM_SAI3, IMX8MQ_ICN_WAKEUP),
+	DEFINE_BUS_MASTER("SAI4", IMX8MQ_ICM_SAI4, IMX8MQ_ICN_WAKEUP),
+	DEFINE_BUS_MASTER("SAI5", IMX8MQ_ICM_SAI5, IMX8MQ_ICN_WAKEUP),
+	DEFINE_BUS_MASTER("SAI6", IMX8MQ_ICM_SAI6, IMX8MQ_ICN_WAKEUP),
+	DEFINE_BUS_INTERCONNECT("PL301_WAKEUP", IMX8MQ_ICN_WAKEUP,
+			IMX8MQ_ICN_MAIN),
 };
 
 static int imx8mq_icc_probe(struct platform_device *pdev)
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 10/19] interconnect: imx8mq: Add of_match_table

[Abel Vesa]

The i.MX8MQ driver will probe based on the compatible string
instead of using device data in imx-bus devfreq driver.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/interconnect/imx/imx8mq.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/interconnect/imx/imx8mq.c b/drivers/interconnect/imx/imx8mq.c
index 010ad3d76286..64321f1d323b 100644
--- a/drivers/interconnect/imx/imx8mq.c
+++ b/drivers/interconnect/imx/imx8mq.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/interconnect-provider.h>
 #include <dt-bindings/interconnect/imx8mq.h>
@@ -88,12 +89,18 @@ static int imx8mq_icc_remove(struct platform_device *pdev)
 	return imx_icc_unregister(pdev);
 }
 
+static const struct of_device_id imx8mq_icc_of_match[] = {
+	{ .compatible = "fsl,imx8mq-icc" },
+	{ /* sentinel */ },
+};
+
 static struct platform_driver imx8mq_icc_driver = {
 	.probe = imx8mq_icc_probe,
 	.remove = imx8mq_icc_remove,
 	.driver = {
 		.name = "imx8mq-interconnect",
 		.sync_state = icc_sync_state,
+		.of_match_table = of_match_ptr(imx8mq_icc_of_match),
 	},
 };
 
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 11/19] interconnect: imx: Add imx_icc_get_bw and imx_icc_aggregate functions

[Abel Vesa]

The aggregate function will return whatever is the highest
rate for that specific node. The imx_icc_get_bw sets the
initial avg and peak to 0 in order to avoid setting them to
INT_MAX by the interconnect core.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/interconnect/imx/imx.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
index db048df80011..5cc1ce55406c 100644
--- a/drivers/interconnect/imx/imx.c
+++ b/drivers/interconnect/imx/imx.c
@@ -25,6 +25,23 @@ struct imx_icc_node {
 	struct dev_pm_qos_request qos_req;
 };
 
+static int imx_icc_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
+{
+	*avg = 0;
+	*peak = 0;
+
+	return 0;
+}
+
+static int imx_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
+		      u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+	*agg_avg = max(*agg_avg, avg_bw);
+	*agg_peak = max(*agg_peak, peak_bw);
+
+	return 0;
+}
+
 static int imx_icc_node_set(struct icc_node *node)
 {
 	struct device *dev = node->provider->dev;
@@ -233,7 +250,8 @@ int imx_icc_register(struct platform_device *pdev,
 	if (!provider)
 		return -ENOMEM;
 	provider->set = imx_icc_set;
-	provider->aggregate = icc_std_aggregate;
+	provider->get_bw = imx_icc_get_bw;
+	provider->aggregate = imx_icc_aggregate;
 	provider->xlate = of_icc_xlate_onecell;
 	provider->data = data;
 	provider->dev = dev;
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 12/19] arm64: dts: imx8mq: Add fsl,icc-id property to ddrc node

[Abel Vesa]

The fsl,icc-id property here is used to link the icc node
registered by the imx8mq interconnect driver with the ddrc
device.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 4066b1612655..8ddbeaddf55a 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -1552,10 +1552,12 @@ ddrc: memory-controller@3d400000 {
 			compatible = "fsl,imx8mq-ddrc", "fsl,imx8m-ddrc";
 			reg = <0x3d400000 0x400000>;
 			clock-names = "core", "pll", "alt", "apb";
+			fsl,icc-id = <IMX8MQ_ICS_DRAM>;
 			clocks = <&clk IMX8MQ_CLK_DRAM_CORE>,
 				 <&clk IMX8MQ_DRAM_PLL_OUT>,
 				 <&clk IMX8MQ_CLK_DRAM_ALT>,
 				 <&clk IMX8MQ_CLK_DRAM_APB>;
+			#interconnect-cells = <0>;
 		};
 
 		ddr-pmu@3d800000 {
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 13/19] arm64: dts: imx8mq: Add fsl,icc-id to noc node

[Abel Vesa]

The fsl,icc-id property here is used to link the icc node
registered by the imx8mq interconnect driver with the noc
device. Remove the fsl,ddrc property since it will not be used
anymore.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 8ddbeaddf55a..ab528665a217 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -1298,11 +1298,11 @@ fec1: ethernet@30be0000 {
 			};
 		};
 
-		noc: interconnect@32700000 {
+		noc: noc@32700000 {
 			compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
 			reg = <0x32700000 0x100000>;
 			clocks = <&clk IMX8MQ_CLK_NOC>;
-			fsl,ddrc = <&ddrc>;
+			fsl,icc-id = <IMX8MQ_ICN_NOC>;
 			#interconnect-cells = <1>;
 			operating-points-v2 = <&noc_opp_table>;
 
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 14/19] arm64: dts: imx8mq: Add all pl301 nodes

[Abel Vesa]

Add all the pl301s found on i.MX8MQ, according to the bus diagram.
Each pl301 has its own clock, icc id and opp table. They are probed
by the imx-bus driver.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 202 ++++++++++++++++++++++
 1 file changed, 202 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index ab528665a217..a05bccbb1342 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -1566,5 +1566,207 @@ ddr-pmu@3d800000 {
 			interrupt-parent = <&gic>;
 			interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
 		};
+
+		pl301_main: pl301@0 {
+			compatible = "fsl,imx8m-nic";
+			clocks = <&clk IMX8MQ_CLK_MAIN_AXI>;
+			operating-points-v2 = <&pl301_main_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_MAIN>;
+
+			pl301_main_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-133M {
+					opp-hz = /bits/ 64 <133333333>;
+				};
+				opp-333M {
+					opp-hz = /bits/ 64 <333333333>;
+				};
+			};
+		};
+
+		pl301_enet: pl301@1 {
+			compatible = "fsl,imx8m-nic";
+			clocks = <&clk IMX8MQ_CLK_ENET_AXI>;
+			operating-points-v2 = <&pl301_enet_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_ENET>;
+
+			pl301_enet_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-266M {
+					opp-hz = /bits/ 64 <266666666>;
+				};
+			};
+		};
+
+		pl301_gpu: pl301@2 {
+			compatible = "fsl,imx8m-nic";
+			clocks = <&clk IMX8MQ_CLK_GPU_AXI>;
+			operating-points-v2 = <&pl301_gpu_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_GPU>;
+
+			pl301_gpu_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-800M {
+					opp-hz = /bits/ 64 <800000000>;
+				};
+			};
+		};
+
+		pl301_dc: pl301@3 {
+			compatible = "fsl,imx8m-nic";
+			clocks = <&clk IMX8MQ_CLK_DISP_AXI>;
+			operating-points-v2 = <&pl301_dc_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_DCSS>;
+
+			pl301_dc_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-800M {
+					opp-hz = /bits/ 64 <800000000>;
+				};
+			};
+		};
+
+		/* PL301_DISPLAY (IPs other than DCSS, inside SUPERMIX) */
+		pl301_display: pl301@4 {
+			compatible = "fsl,imx8m-nic";
+			/* FIXME: don't know which clock yet */
+			clocks = <&clk IMX8MQ_CLK_DUMMY>;
+			operating-points-v2 = <&pl301_display_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_DISPLAY>;
+
+			pl301_display_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-333M {
+					opp-hz = /bits/ 64 <333333333>;
+				};
+			};
+		};
+
+		pl301_audio: pl301@5 {
+			compatible = "fsl,imx8m-nic";
+			clocks = <&clk IMX8MQ_CLK_AUDIO_AHB>;
+			operating-points-v2 = <&pl301_audio_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_AUDIO>;
+
+			pl301_audio_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-500M {
+					opp-hz = /bits/ 64 <500000000>;
+				};
+			};
+		};
+
+		pl301_video: pl301@6 {
+			compatible = "fsl,imx8m-nic";
+			clocks = <&clk IMX8MQ_CLK_VPU_BUS>;
+			operating-points-v2 = <&pl301_video_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_VIDEO>;
+
+			pl301_video_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-500M {
+					opp-hz = /bits/ 64 <500000000>;
+				};
+			};
+		};
+
+		pl301_usb: pl301@7 {
+			compatible = "fsl,imx8m-nic";
+			clocks = <&clk IMX8MQ_CLK_USB_BUS>;
+			operating-points-v2 = <&pl301_usb_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_USB>;
+
+			pl301_usb_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-128M {
+					opp-hz = /bits/ 64 <128000000>;
+				};
+				opp-500M {
+					opp-hz = /bits/ 64 <500000000>;
+				};
+			};
+		};
+
+		pl301_wakeup: pl301@8 {
+			compatible = "fsl,imx8m-nic";
+			/* FIXME: don't know which clock yet */
+			clocks = <&clk IMX8MQ_CLK_DUMMY>;
+			operating-points-v2 = <&pl301_wakeup_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_WAKEUP>;
+
+			pl301_wakeup_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-133M {
+					opp-hz = /bits/ 64 <133333333>;
+				};
+			};
+		};
+
+		pl301_per_m: pl301@9 {
+			compatible = "fsl,imx8m-nic";
+			clocks = <&clk IMX8MQ_CLK_NAND_USDHC_BUS>;
+			operating-points-v2 = <&pl301_per_m_opp_table>;
+			#interconnect-cells = <0>;
+			fsl,icc-id = <IMX8MQ_ICN_PER_M>;
+
+			pl301_per_m_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-25M {
+					opp-hz = /bits/ 64 <25000000>;
+				};
+				opp-133M {
+					opp-hz = /bits/ 64 <133333333>;
+				};
+				opp-266M {
+					opp-hz = /bits/ 64 <266666666>;
+				};
+			};
+		};
 	};
 };
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 15/19] arm64: dts: imx8mq: Add the interconnect node

[Abel Vesa]

The icc node will be probed by the imx8mq interconnect driver.
Will look-up the NoC and all the pl301s (identified by fsl,icc-id property)
and will assign the corresponding icc node to each one of them.
Then, it will register the icc provider.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index a05bccbb1342..5a73ebda6bb3 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -1768,5 +1768,10 @@ opp-266M {
 				};
 			};
 		};
+
+		icc: interconnect@0 {
+			compatible = "fsl,imx8mq-icc", "fsl,imx8m-icc";
+			#interconnect-cells = <1>;
+		};
 	};
 };
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 16/19] arm64: dts: imx8mq: Add interconnect properties to icc consumer nodes

[Abel Vesa]

We add all the properties necessary to control the interconnect
based on the required rates all the way from consumers to the dram.
The fsl,icc-rate specifies the minimum required rate the consumer needs
in order to operate.
For now, only the fec, usdhc1 and usdhc2 are added as consumers.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 5a73ebda6bb3..0489f7416993 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -1215,6 +1215,9 @@ usdhc1: mmc@30b40000 {
 				             "fsl,imx7d-usdhc";
 				reg = <0x30b40000 0x10000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
+				interconnects = <&icc IMX8MQ_ICM_USDHC1 &icc IMX8MQ_ICS_DRAM>;
+				interconnect-names = "path";
+				fsl,icc-rate = <266666>;
 				clocks = <&clk IMX8MQ_CLK_IPG_ROOT>,
 				         <&clk IMX8MQ_CLK_NAND_USDHC_BUS>,
 				         <&clk IMX8MQ_CLK_USDHC1_ROOT>;
@@ -1230,6 +1233,9 @@ usdhc2: mmc@30b50000 {
 				             "fsl,imx7d-usdhc";
 				reg = <0x30b50000 0x10000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
+				interconnects = <&icc IMX8MQ_ICM_USDHC2 &icc IMX8MQ_ICS_DRAM>;
+				interconnect-names = "path";
+				fsl,icc-rate = <266666>;
 				clocks = <&clk IMX8MQ_CLK_IPG_ROOT>,
 				         <&clk IMX8MQ_CLK_NAND_USDHC_BUS>,
 				         <&clk IMX8MQ_CLK_USDHC2_ROOT>;
@@ -1272,6 +1278,9 @@ fec1: ethernet@30be0000 {
 				             <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+				interconnects = <&icc IMX8MQ_ICM_ENET &icc IMX8MQ_ICS_DRAM>;
+				interconnect-names = "path";
+				fsl,icc-rate = <800000>;
 				clocks = <&clk IMX8MQ_CLK_ENET1_ROOT>,
 				         <&clk IMX8MQ_CLK_ENET1_ROOT>,
 				         <&clk IMX8MQ_CLK_ENET_TIMER>,
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 17/19] net: ethernet: fec_main: Add interconnect support

[Abel Vesa]

On probe, if the dts node contains a valid icc path, then look for the
fsl,icc-rate property and get the rate. Also set the icc bandwidth
for that path to the nominal rate needed for fec to function right.
Then enable and disable the path every time the fec is used or not.
This will result in reducing the clock speeds along the icc path
for each pl301 and NoC, but still meet the requirements for all the
other icc consumers.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/net/ethernet/freescale/fec.h      |  3 +++
 drivers/net/ethernet/freescale/fec_main.c | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 7b4961daa254..2c5784febbeb 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -558,6 +558,9 @@ struct fec_enet_private {
 	unsigned int num_tx_queues;
 	unsigned int num_rx_queues;
 
+	struct icc_path *bus_path;
+	unsigned int bus_rate;
+
 	/* The saved address of a sent-in-place packet/buffer, for skfree(). */
 	struct fec_enet_priv_tx_q *tx_queue[FEC_ENET_MAX_TX_QS];
 	struct fec_enet_priv_rx_q *rx_queue[FEC_ENET_MAX_RX_QS];
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 80bd5c629fa0..67374d436e73 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -48,6 +48,7 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/interconnect.h>
 #include <linux/clk.h>
 #include <linux/crc32.h>
 #include <linux/platform_device.h>
@@ -3805,6 +3806,18 @@ fec_probe(struct platform_device *pdev)
 	fep->pdev = pdev;
 	fep->dev_id = dev_id++;
 
+	fep->bus_path = devm_of_icc_get(&pdev->dev, "path");
+	if (IS_ERR(fep->bus_path)) {
+		return PTR_ERR(fep->bus_path);
+	} else if (fep->bus_path) {
+		if (of_property_read_u32(np, "fsl,icc-rate", &fep->bus_rate)) {
+			dev_err(&pdev->dev, "icc-rate missing\n");
+			return -EINVAL;
+		}
+
+		icc_set_bw(fep->bus_path, 0, fep->bus_rate);
+	}
+
 	platform_set_drvdata(pdev, ndev);
 
 	if ((of_machine_is_compatible("fsl,imx6q") ||
@@ -4075,6 +4088,8 @@ static int __maybe_unused fec_suspend(struct device *dev)
 	if (fep->clk_enet_out || fep->reg_phy)
 		fep->link = 0;
 
+	icc_disable(fep->bus_path);
+
 	return 0;
 }
 
@@ -4085,6 +4100,8 @@ static int __maybe_unused fec_resume(struct device *dev)
 	int ret;
 	int val;
 
+	icc_enable(fep->bus_path);
+
 	if (fep->reg_phy && !(fep->wol_flag & FEC_WOL_FLAG_ENABLE)) {
 		ret = regulator_enable(fep->reg_phy);
 		if (ret)
@@ -4134,6 +4151,7 @@ static int __maybe_unused fec_runtime_suspend(struct device *dev)
 	clk_disable_unprepare(fep->clk_ahb);
 	clk_disable_unprepare(fep->clk_ipg);
 
+	icc_disable(fep->bus_path);
 	return 0;
 }
 
@@ -4143,6 +4161,7 @@ static int __maybe_unused fec_runtime_resume(struct device *dev)
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	int ret;
 
+	icc_enable(fep->bus_path);
 	ret = clk_prepare_enable(fep->clk_ahb);
 	if (ret)
 		return ret;
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 18/19] mmc: sdhci-esdhc-imx: Add interconnect support

[Abel Vesa]

On probe, if the dts node contains a valid icc path, then look for the
fsl,icc-rate property and get the rate. Also set the icc bandwidth
for that path to the nominal rate needed for sdhc to function right.
Then enable and disable the path every time the sdhc is used or not.
This will result in reducing the clock speeds along the icc path
for each pl301 and NoC, but still meet the requirements for all the
other icc consumers.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index f18d169bc8ff..9773a9efaae1 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/interconnect.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/delay.h>
@@ -324,6 +325,9 @@ struct pltfm_imx_data {
 	struct clk *clk_ahb;
 	struct clk *clk_per;
 	unsigned int actual_clock;
+	struct icc_path *bus_path;
+	unsigned int bus_rate;
+
 	enum {
 		NO_CMD_PENDING,      /* no multiblock command pending */
 		MULTIBLK_IN_PROCESS, /* exact multiblock cmd in process */
@@ -1573,6 +1577,19 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (imx_data->socdata->flags & ESDHC_FLAG_PMQOS)
 		cpu_latency_qos_add_request(&imx_data->pm_qos_req, 0);
 
+	imx_data->bus_path = devm_of_icc_get(&pdev->dev, "path");
+	if (IS_ERR(imx_data->bus_path)) {
+		err = PTR_ERR(imx_data->bus_path);
+		goto free_sdhci;
+	} else if (imx_data->bus_path) {
+		if (of_property_read_u32(pdev->dev.of_node, "fsl,icc-rate", &imx_data->bus_rate)) {
+			dev_err(&pdev->dev, "icc-rate missing\n");
+			return -EINVAL;
+		}
+
+		err = icc_set_bw(imx_data->bus_path, 0, imx_data->bus_rate);
+	}
+
 	imx_data->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
 	if (IS_ERR(imx_data->clk_ipg)) {
 		err = PTR_ERR(imx_data->clk_ipg);
@@ -1762,14 +1779,20 @@ static int sdhci_esdhc_suspend(struct device *dev)
 
 	ret = mmc_gpio_set_cd_wake(host->mmc, true);
 
+	icc_disable(imx_data->bus_path);
+
 	return ret;
 }
 
 static int sdhci_esdhc_resume(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	int ret;
 
+	icc_enable(imx_data->bus_path);
+
 	ret = pinctrl_pm_select_default_state(dev);
 	if (ret)
 		return ret;
@@ -1821,6 +1844,8 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
 	if (imx_data->socdata->flags & ESDHC_FLAG_PMQOS)
 		cpu_latency_qos_remove_request(&imx_data->pm_qos_req);
 
+	icc_disable(imx_data->bus_path);
+
 	return ret;
 }
 
@@ -1831,6 +1856,8 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	int err;
 
+	icc_enable(imx_data->bus_path);
+
 	if (imx_data->socdata->flags & ESDHC_FLAG_PMQOS)
 		cpu_latency_qos_add_request(&imx_data->pm_qos_req, 0);
 
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 19/19] arm64: defconfig: Add necessary configs for icc+devfreq on i.MX8MQ

[Abel Vesa]

Compile in the interconnect and devfreq drivers for the i.MX8MQ.
Also compile in the powersave devfreq governor.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 arch/arm64/configs/defconfig | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 3bf6f8a86aae..b06f1b01714d 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1063,8 +1063,9 @@ CONFIG_ARCH_TEGRA_186_SOC=y
 CONFIG_ARCH_TEGRA_194_SOC=y
 CONFIG_ARCH_TEGRA_234_SOC=y
 CONFIG_TI_SCI_PM_DOMAINS=y
-CONFIG_ARM_IMX_BUS_DEVFREQ=m
-CONFIG_ARM_IMX8M_DDRC_DEVFREQ=m
+CONFIG_DEVFREQ_GOV_POWERSAVE=y
+CONFIG_ARM_IMX_BUS_DEVFREQ=y
+CONFIG_ARM_IMX8M_DDRC_DEVFREQ=y
 CONFIG_EXTCON_PTN5150=m
 CONFIG_EXTCON_USB_GPIO=y
 CONFIG_EXTCON_USBC_CROS_EC=y
@@ -1156,8 +1157,8 @@ CONFIG_SLIM_QCOM_CTRL=m
 CONFIG_SLIM_QCOM_NGD_CTRL=m
 CONFIG_MUX_MMIO=y
 CONFIG_INTERCONNECT=y
-CONFIG_INTERCONNECT_IMX=m
-CONFIG_INTERCONNECT_IMX8MQ=m
+CONFIG_INTERCONNECT_IMX=y
+CONFIG_INTERCONNECT_IMX8MQ=y
 CONFIG_INTERCONNECT_QCOM=y
 CONFIG_INTERCONNECT_QCOM_MSM8916=m
 CONFIG_INTERCONNECT_QCOM_OSM_L3=m
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 00/19] Add interconnect and devfreq support for i.MX8MQ

[Martin Kepplinger]

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.

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?

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?

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 00/19] Add interconnect and devfreq support for i.MX8MQ

[Abel Vesa]

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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 00/19] Add interconnect and devfreq support for i.MX8MQ

[Martin Kepplinger]

Am Mittwoch, dem 29.09.2021 um 14:44 +0300 schrieb Abel Vesa:
> 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.
> 
> 


thanks Abel, with the above fix of existing interconnects properties my
system runs as expected and here's the output of

for each in `ls -1d /sys/devices/platform/soc@0/*/devfreq/*`; do echo
$each; cat $each/trans_stat; done

for mxsfb requesting (max) bandwith (display on):

/sys/devices/platform/soc@0/32700000.noc/devfreq/32700000.noc
     From  :   To
           : 133333333 400000000 800000000   time(ms)
  133333333:         0         1         0       624
  400000000:         0         0         1        28
* 800000000:         1         0         0     30624
Total transition : 3
/sys/devices/platform/soc@0/3d400000.memory-
controller/devfreq/3d400000.memory-controller
     From  :   To
           :  25000000 100000000 800000000   time(ms)
   25000000:         0         0         1       620
  100000000:         0         0         0         0
* 800000000:         1         0         0     30652
Total transition : 2
/sys/devices/platform/soc@0/soc@0:pl301@0/devfreq/soc@0:pl301@0
     From  :   To
           :  25000000 133333333 333333333   time(ms)
   25000000:         0         0         1       616
  133333333:         0         0         0         0
* 333333333:         1         0         0     30668
Total transition : 2
/sys/devices/platform/soc@0/soc@0:pl301@1/devfreq/soc@0:pl301@1
     From  :   To
           :  25000000 266666666   time(ms)
*  25000000:         0         0     31284
  266666666:         0         0         0
Total transition : 0
/sys/devices/platform/soc@0/soc@0:pl301@2/devfreq/soc@0:pl301@2
     From  :   To
           :  25000000 800000000   time(ms)
*  25000000:         0         0     31288
  800000000:         1         0         0
Total transition : 1
/sys/devices/platform/soc@0/soc@0:pl301@3/devfreq/soc@0:pl301@3
     From  :   To
           :  25000000 800000000   time(ms)
*  25000000:         0         0     31292
  800000000:         1         0         0
Total transition : 1
/sys/devices/platform/soc@0/soc@0:pl301@4/devfreq/soc@0:pl301@4
     From  :   To
           :  25000000 333333333   time(ms)
   25000000:         0         1       648
* 333333333:         0         0     30652
Total transition : 1
/sys/devices/platform/soc@0/soc@0:pl301@5/devfreq/soc@0:pl301@5
     From  :   To
           :  25000000 500000000   time(ms)
*  25000000:         0         0     31304
  500000000:         1         0         0
Total transition : 1
/sys/devices/platform/soc@0/soc@0:pl301@6/devfreq/soc@0:pl301@6
     From  :   To
           :  25000000 500000000   time(ms)
*  25000000:         0         0     31308
  500000000:         0         0         0
Total transition : 0
/sys/devices/platform/soc@0/soc@0:pl301@7/devfreq/soc@0:pl301@7
     From  :   To
           :  25000000 128000000 500000000   time(ms)
*  25000000:         0         0         0     31312
  128000000:         0         0         0         0
  500000000:         1         0         0         0
Total transition : 1
/sys/devices/platform/soc@0/soc@0:pl301@8/devfreq/soc@0:pl301@8
     From  :   To
           :  25000000 133333333   time(ms)
*  25000000:         0         0     31316
  133333333:         0         0         0
Total transition : 0
/sys/devices/platform/soc@0/soc@0:pl301@9/devfreq/soc@0:pl301@9
     From  :   To
           :  25000000 133333333 266666666   time(ms)
   25000000:         0         0         5      1052
  133333333:         0         0         0         0
* 266666666:         5         0         0     30268
Total transition : 10


but with display off (mxsfb not requesting anything), I get the same
fast freqs for noc and memory-controller. They should use the lowest
freqs. Only pl301@4 switches to 25mhz in that case. That's odd.

said (still) out-of-tree mxsfb request is
https://source.puri.sm/martin.kepplinger/linux-next/-/commit/ee7b1453295932da1e292b734afa7a03651ad9ba

and the exact tree I'm running for the above is
https://source.puri.sm/martin.kepplinger/linux-next/-/commits/5.15-rc3/librem5__integration_byzantium_test_new_devfreq_interconnect

thanks,

                              martin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 00/19] Add interconnect and devfreq support for i.MX8MQ

[Abel Vesa]

On 21-09-30 10:03:46, Martin Kepplinger wrote:
> Am Mittwoch, dem 29.09.2021 um 14:44 +0300 schrieb Abel Vesa:
> > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.puri.sm%2Fmartin.kepplinger%2Flinux-next%2F-%2Fcommit%2Fd690e4c021293f938eb2253607f92f5a64f15688&data=04%7C01%7Cabel.vesa%40nxp.com%7C7fab8aca3a5f43d56f5608d983e8da67%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637685858400552603%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=%2FzyEQdOLU8jQuUpqJ74GTWyfrDvavz%2BxZAgv1tcIu9Y%3D&reserved=0
> > > (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.
> > 
> > 
> 
> 
> thanks Abel, with the above fix of existing interconnects properties my
> system runs as expected and here's the output of
> 
> for each in `ls -1d /sys/devices/platform/soc@0/*/devfreq/*`; do echo
> $each; cat $each/trans_stat; done
> 
> for mxsfb requesting (max) bandwith (display on):
> 
> /sys/devices/platform/soc@0/32700000.noc/devfreq/32700000.noc
>      From  :   To
>            : 133333333 400000000 800000000   time(ms)
>   133333333:         0         1         0       624
>   400000000:         0         0         1        28
> * 800000000:         1         0         0     30624
> Total transition : 3
> /sys/devices/platform/soc@0/3d400000.memory-
> controller/devfreq/3d400000.memory-controller
>      From  :   To
>            :  25000000 100000000 800000000   time(ms)
>    25000000:         0         0         1       620
>   100000000:         0         0         0         0
> * 800000000:         1         0         0     30652
> Total transition : 2
> /sys/devices/platform/soc@0/soc@0:pl301@0/devfreq/soc@0:pl301@0
>      From  :   To
>            :  25000000 133333333 333333333   time(ms)
>    25000000:         0         0         1       616
>   133333333:         0         0         0         0
> * 333333333:         1         0         0     30668
> Total transition : 2
> /sys/devices/platform/soc@0/soc@0:pl301@1/devfreq/soc@0:pl301@1
>      From  :   To
>            :  25000000 266666666   time(ms)
> *  25000000:         0         0     31284
>   266666666:         0         0         0
> Total transition : 0
> /sys/devices/platform/soc@0/soc@0:pl301@2/devfreq/soc@0:pl301@2
>      From  :   To
>            :  25000000 800000000   time(ms)
> *  25000000:         0         0     31288
>   800000000:         1         0         0
> Total transition : 1
> /sys/devices/platform/soc@0/soc@0:pl301@3/devfreq/soc@0:pl301@3
>      From  :   To
>            :  25000000 800000000   time(ms)
> *  25000000:         0         0     31292
>   800000000:         1         0         0
> Total transition : 1
> /sys/devices/platform/soc@0/soc@0:pl301@4/devfreq/soc@0:pl301@4
>      From  :   To
>            :  25000000 333333333   time(ms)
>    25000000:         0         1       648
> * 333333333:         0         0     30652
> Total transition : 1
> /sys/devices/platform/soc@0/soc@0:pl301@5/devfreq/soc@0:pl301@5
>      From  :   To
>            :  25000000 500000000   time(ms)
> *  25000000:         0         0     31304
>   500000000:         1         0         0
> Total transition : 1
> /sys/devices/platform/soc@0/soc@0:pl301@6/devfreq/soc@0:pl301@6
>      From  :   To
>            :  25000000 500000000   time(ms)
> *  25000000:         0         0     31308
>   500000000:         0         0         0
> Total transition : 0
> /sys/devices/platform/soc@0/soc@0:pl301@7/devfreq/soc@0:pl301@7
>      From  :   To
>            :  25000000 128000000 500000000   time(ms)
> *  25000000:         0         0         0     31312
>   128000000:         0         0         0         0
>   500000000:         1         0         0         0
> Total transition : 1
> /sys/devices/platform/soc@0/soc@0:pl301@8/devfreq/soc@0:pl301@8
>      From  :   To
>            :  25000000 133333333   time(ms)
> *  25000000:         0         0     31316
>   133333333:         0         0         0
> Total transition : 0
> /sys/devices/platform/soc@0/soc@0:pl301@9/devfreq/soc@0:pl301@9
>      From  :   To
>            :  25000000 133333333 266666666   time(ms)
>    25000000:         0         0         5      1052
>   133333333:         0         0         0         0
> * 266666666:         5         0         0     30268
> Total transition : 10
> 
> 
> but with display off (mxsfb not requesting anything), I get the same
> fast freqs for noc and memory-controller. They should use the lowest
> freqs. Only pl301@4 switches to 25mhz in that case. That's odd.
> 

Well, have a look at: 

/sys/devices/platform/soc@0/soc@0:pl301@9/devfreq/soc@0:pl301@9

even in the output you gave here, you can see that there are 5
transisions between 25MHz and 266MHz. BTW, that is the USDHC pl301.

I'm assuming you're booting with rootfs from usdhc not through nfs,
right? Anyway, the noc and dram clocks rate only drop when there is
no user enabling its own icc path to the dram.

Keep in mind that the benefit of this approach is not only to drop the
dram clock rate, but also to drop the rates of all the bus clocks on
whenever possible. 

Yes, the perfect scenario would be, from power consumption point of view at least,
have dram clock rate as low as possible and as long as possible, which
implicitly means there is no one requesting the higher rate.

If you want to observe the transitions number change for the dram
devfreq node as well, you can run a simple sync from userspace and that will 
trigger a "high rate" request for the usdhc. Note, this will only happen
if there are no other users asking for the higher rate.

> said (still) out-of-tree mxsfb request is
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.puri.sm%2Fmartin.kepplinger%2Flinux-next%2F-%2Fcommit%2Fee7b1453295932da1e292b734afa7a03651ad9ba&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C7fab8aca3a5f43d56f5608d983e8da67%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637685858400552603%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=tY9IN8nAgYrPRD0BRLW%2FZbWEps9DTVIQi8G5jY5aw3Q%3D&amp;reserved=0
> 
> and the exact tree I'm running for the above is
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.puri.sm%2Fmartin.kepplinger%2Flinux-next%2F-%2Fcommits%2F5.15-rc3%2Flibrem5__integration_byzantium_test_new_devfreq_interconnect&amp;data=04%7C01%7Cabel.vesa%40nxp.com%7C7fab8aca3a5f43d56f5608d983e8da67%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637685858400552603%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=Kpo7sVLdgzwMv8MPX7X%2FNUxoLcvWjFVIuerlh7Cr2D4%3D&amp;reserved=0
> 
> thanks,
> 
>                               martin
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 00/19] Add interconnect and devfreq support for i.MX8MQ

[Martin Kepplinger]

Am Donnerstag, dem 30.09.2021 um 17:22 +0300 schrieb Abel Vesa:
> On 21-09-30 10:03:46, Martin Kepplinger wrote:
> > Am Mittwoch, dem 29.09.2021 um 14:44 +0300 schrieb Abel Vesa:
> > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.puri.sm%2Fmartin.kepplinger%2Flinux-next%2F-%2Fcommit%2Fd690e4c021293f938eb2253607f92f5a64f15688&data=04%7C01%7Cabel.vesa%40nxp.com%7C7fab8aca3a5f43d56f5608d983e8da67%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637685858400552603%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=%2FzyEQdOLU8jQuUpqJ74GTWyfrDvavz%2BxZAgv1tcIu9Y%3D&reserved=0
> > > > (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.
> > > 
> > > 
> > 
> > 
> > thanks Abel, with the above fix of existing interconnects
> > properties my
> > system runs as expected and here's the output of
> > 
> > for each in `ls -1d /sys/devices/platform/soc@0/*/devfreq/*`; do
> > echo
> > $each; cat $each/trans_stat; done
> > 
> > for mxsfb requesting (max) bandwith (display on):
> > 
> > /sys/devices/platform/soc@0/32700000.noc/devfreq/32700000.noc
> >      From  :   To
> >            : 133333333 400000000 800000000   time(ms)
> >   133333333:         0         1         0       624
> >   400000000:         0         0         1        28
> > * 800000000:         1         0         0     30624
> > Total transition : 3
> > /sys/devices/platform/soc@0/3d400000.memory-
> > controller/devfreq/3d400000.memory-controller
> >      From  :   To
> >            :  25000000 100000000 800000000   time(ms)
> >    25000000:         0         0         1       620
> >   100000000:         0         0         0         0
> > * 800000000:         1         0         0     30652
> > Total transition : 2
> > /sys/devices/platform/soc@0/soc@0:pl301@0/devfreq/soc@0:pl301@0
> >      From  :   To
> >            :  25000000 133333333 333333333   time(ms)
> >    25000000:         0         0         1       616
> >   133333333:         0         0         0         0
> > * 333333333:         1         0         0     30668
> > Total transition : 2
> > /sys/devices/platform/soc@0/soc@0:pl301@1/devfreq/soc@0:pl301@1
> >      From  :   To
> >            :  25000000 266666666   time(ms)
> > *  25000000:         0         0     31284
> >   266666666:         0         0         0
> > Total transition : 0
> > /sys/devices/platform/soc@0/soc@0:pl301@2/devfreq/soc@0:pl301@2
> >      From  :   To
> >            :  25000000 800000000   time(ms)
> > *  25000000:         0         0     31288
> >   800000000:         1         0         0
> > Total transition : 1
> > /sys/devices/platform/soc@0/soc@0:pl301@3/devfreq/soc@0:pl301@3
> >      From  :   To
> >            :  25000000 800000000   time(ms)
> > *  25000000:         0         0     31292
> >   800000000:         1         0         0
> > Total transition : 1
> > /sys/devices/platform/soc@0/soc@0:pl301@4/devfreq/soc@0:pl301@4
> >      From  :   To
> >            :  25000000 333333333   time(ms)
> >    25000000:         0         1       648
> > * 333333333:         0         0     30652
> > Total transition : 1
> > /sys/devices/platform/soc@0/soc@0:pl301@5/devfreq/soc@0:pl301@5
> >      From  :   To
> >            :  25000000 500000000   time(ms)
> > *  25000000:         0         0     31304
> >   500000000:         1         0         0
> > Total transition : 1
> > /sys/devices/platform/soc@0/soc@0:pl301@6/devfreq/soc@0:pl301@6
> >      From  :   To
> >            :  25000000 500000000   time(ms)
> > *  25000000:         0         0     31308
> >   500000000:         0         0         0
> > Total transition : 0
> > /sys/devices/platform/soc@0/soc@0:pl301@7/devfreq/soc@0:pl301@7
> >      From  :   To
> >            :  25000000 128000000 500000000   time(ms)
> > *  25000000:         0         0         0     31312
> >   128000000:         0         0         0         0
> >   500000000:         1         0         0         0
> > Total transition : 1
> > /sys/devices/platform/soc@0/soc@0:pl301@8/devfreq/soc@0:pl301@8
> >      From  :   To
> >            :  25000000 133333333   time(ms)
> > *  25000000:         0         0     31316
> >   133333333:         0         0         0
> > Total transition : 0
> > /sys/devices/platform/soc@0/soc@0:pl301@9/devfreq/soc@0:pl301@9
> >      From  :   To
> >            :  25000000 133333333 266666666   time(ms)
> >    25000000:         0         0         5      1052
> >   133333333:         0         0         0         0
> > * 266666666:         5         0         0     30268
> > Total transition : 10
> > 
> > 
> > but with display off (mxsfb not requesting anything), I get the
> > same
> > fast freqs for noc and memory-controller. They should use the
> > lowest
> > freqs. Only pl301@4 switches to 25mhz in that case. That's odd.
> > 
> 
> Well, have a look at: 
> 
> /sys/devices/platform/soc@0/soc@0:pl301@9/devfreq/soc@0:pl301@9
> 
> even in the output you gave here, you can see that there are 5
> transisions between 25MHz and 266MHz. BTW, that is the USDHC pl301.
> 
> I'm assuming you're booting with rootfs from usdhc not through nfs,
> right? Anyway, the noc and dram clocks rate only drop when there is
> no user enabling its own icc path to the dram.
> 
> Keep in mind that the benefit of this approach is not only to drop
> the
> dram clock rate, but also to drop the rates of all the bus clocks on
> whenever possible. 
> 
> Yes, the perfect scenario would be, from power consumption point of
> view at least,
> have dram clock rate as low as possible and as long as possible,
> which
> implicitly means there is no one requesting the higher rate.
> 
> If you want to observe the transitions number change for the dram
> devfreq node as well, you can run a simple sync from userspace and
> that will 
> trigger a "high rate" request for the usdhc. Note, this will only
> happen
> if there are no other users asking for the higher rate.


correct, I boot from usdhc. and when I disable interconnect in usdhc
the behaviour actually makes sense. tree:
https://source.puri.sm/martin.kepplinger/linux-next/-/commits/5.15-rc4/librem5__integration_byzantium_new_devfreq_interconnect

And I can see the system using a bit less power in the "display off"
case now (and various freqs switching to the lowest).

I didn't yet test whether the new "consumers" (for example usb)
correctly request more bandwidth now.

The only thing I see is with the "display on" case, that
"32700000.interconnect" is switched to 800mhz now, where it used 400mhz
before this patchset. I should be able to find out why though :)

so, for a proof of concept (and after what you mentioned to change for
a next revision) this looks good to me.

thanks a lot!
                            martin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel