[PATCH 0/8] Add interconnect for i.MX8MP blk ctrl

[PATCH 0/8] Add interconnect for i.MX8MP blk ctrl

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX8MP NoC settings is invalid after related power domain up. So
need to set valid values after power domain up.

This patchset is to bind interconnect for each entry in blk ctrl.

This patchset is not include DVFS DDRC feature.

A repo created here: https://github.com/MrVan/linux/tree/imx8mp-interconnect

Peng Fan (8):
  dt-bindings: soc: imx: add interconnect property for i.MX8MP media blk
    ctrl
  dt-bindings: soc: imx: add interconnect property for i.MX8MP hdmi blk
    ctrl
  dt-bindings: soc: imx: add interconnect property for i.MX8MP hsio blk
    ctrl
  soc: imx: add icc paths for i.MX8MP media blk ctrl
  soc: imx: add icc paths for i.MX8MP hsio/hdmi blk ctrl
  arm64: dts: imx8mp: add NoC node
  arm64: dts: imx8mp: add interconnects for media blk ctrl
  arm64: dts: imx8mp: add interconnect for hsio blk ctrl

 .../soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml     |  9 +++++
 .../soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml     | 10 +++++
 .../soc/imx/fsl,imx8mp-media-blk-ctrl.yaml    | 14 +++++++
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     | 39 +++++++++++++++++++
 drivers/soc/imx/imx8m-blk-ctrl.c              | 31 +++++++++++++++
 drivers/soc/imx/imx8mp-blk-ctrl.c             | 32 +++++++++++++++
 6 files changed, 135 insertions(+)

-- 
2.25.1

[PATCH 1/8] dt-bindings: soc: imx: add interconnect property for i.MX8MP media blk ctrl

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add interconnect property for i.MX8MP mediamix blk ctrl

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../soc/imx/fsl,imx8mp-media-blk-ctrl.yaml         | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
index 21d3ee486295..706bef39b87e 100644
--- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
@@ -64,6 +64,20 @@ properties:
       - const: isp
       - const: phy
 
+  interconnects:
+    maxItems: 8
+
+  interconnect-names:
+    items:
+      - const: lcdif-rd
+      - const: lcdif-wr
+      - const: isi0
+      - const: isi1
+      - const: isi2
+      - const: isp0
+      - const: isp1
+      - const: dwe
+
 required:
   - compatible
   - reg
-- 
2.25.1

Re: [PATCH 1/8] dt-bindings: soc: imx: add interconnect property for i.MX8MP media blk ctrl

[Krzysztof Kozlowski]

On 01/06/2022 11:45, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add interconnect property for i.MX8MP mediamix blk ctrl
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 1/8] dt-bindings: soc: imx: add interconnect property for i.MX8MP media blk ctrl

[Laurent Pinchart]

Hi Peng,

Thank you for the patch.

On Wed, Jun 01, 2022 at 05:45:30PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add interconnect property for i.MX8MP mediamix blk ctrl
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../soc/imx/fsl,imx8mp-media-blk-ctrl.yaml         | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> index 21d3ee486295..706bef39b87e 100644
> --- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> @@ -64,6 +64,20 @@ properties:
>        - const: isp
>        - const: phy
>  
> +  interconnects:
> +    maxItems: 8
> +
> +  interconnect-names:
> +    items:
> +      - const: lcdif-rd
> +      - const: lcdif-wr
> +      - const: isi0
> +      - const: isi1
> +      - const: isi2

If I understand correctly, these are for the 1x RD and 2x WR channels of
the ISI. Would it make sense to name thim accordingly, maybe isi-rd,
isi-wr0 and isi-wr1 ? I'm not sure about the order though.

> +      - const: isp0
> +      - const: isp1
> +      - const: dwe
> +
>  required:
>    - compatible
>    - reg

-- 
Regards,

Laurent Pinchart

RE: [PATCH 1/8] dt-bindings: soc: imx: add interconnect property for i.MX8MP media blk ctrl

[Peng Fan]

> Subject: Re: [PATCH 1/8] dt-bindings: soc: imx: add interconnect property for
> i.MX8MP media blk ctrl
> 
> Hi Peng,
> 
> Thank you for the patch.
> 
> On Wed, Jun 01, 2022 at 05:45:30PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add interconnect property for i.MX8MP mediamix blk ctrl
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  .../soc/imx/fsl,imx8mp-media-blk-ctrl.yaml         | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.
> > yaml
> > b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.
> > yaml index 21d3ee486295..706bef39b87e 100644
> > ---
> > a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.
> > yaml
> > +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-c
> > +++ trl.yaml
> > @@ -64,6 +64,20 @@ properties:
> >        - const: isp
> >        - const: phy
> >
> > +  interconnects:
> > +    maxItems: 8
> > +
> > +  interconnect-names:
> > +    items:
> > +      - const: lcdif-rd
> > +      - const: lcdif-wr
> > +      - const: isi0
> > +      - const: isi1
> > +      - const: isi2
> 
> If I understand correctly, these are for the 1x RD and 2x WR channels of the ISI.
> Would it make sense to name thim accordingly, maybe isi-rd,
> isi-wr0 and isi-wr1 ? I'm not sure about the order though.

From the doc I get, they are isi0/1/2, so I would keep them as it is.

Thanks,
Peng.

> 
> > +      - const: isp0
> > +      - const: isp1
> > +      - const: dwe
> > +
> >  required:
> >    - compatible
> >    - reg
> 
> --
> Regards,
> 
> Laurent Pinchart

[PATCH 2/8] dt-bindings: soc: imx: add interconnect property for i.MX8MP hdmi blk ctrl

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add interconnect property for i.MX8MP hdmi blk ctrl

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml       | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml
index 563e1d0e327f..1be4ce2a45e8 100644
--- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml
@@ -52,6 +52,15 @@ properties:
       - const: ref_266m
       - const: ref_24m
 
+  interconnects:
+    maxItems: 3
+
+  interconnect-names:
+    items:
+      - const: hrv
+      - const: lcdif-hdmi
+      - const: hdcp
+
 required:
   - compatible
   - reg
-- 
2.25.1

Re: [PATCH 2/8] dt-bindings: soc: imx: add interconnect property for i.MX8MP hdmi blk ctrl

[Krzysztof Kozlowski]

On 01/06/2022 11:45, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add interconnect property for i.MX8MP hdmi blk ctrl
> 

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

[PATCH 3/8] dt-bindings: soc: imx: add interconnect property for i.MX8MP hsio blk ctrl

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add interconnect property for i.MX8MP hsio blk ctrl

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml     | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml
index c1e29d94f40e..a776dd386ae3 100644
--- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml
@@ -48,6 +48,16 @@ properties:
       - const: usb
       - const: pcie
 
+  interconnects:
+    maxItems: 4
+
+  interconnect-names:
+    items:
+      - const: noc-pcie
+      - const: usb1
+      - const: usb2
+      - const: pcie
+
 required:
   - compatible
   - reg
-- 
2.25.1

Re: [PATCH 3/8] dt-bindings: soc: imx: add interconnect property for i.MX8MP hsio blk ctrl

[Krzysztof Kozlowski]

On 01/06/2022 11:45, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add interconnect property for i.MX8MP hsio blk ctrl
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

[PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add interconnect paths for i.MX8MP media blk ctrl

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi |  1 +
 drivers/soc/imx/imx8m-blk-ctrl.c          | 31 +++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index d9542dfff83f..2a1c6ff37e03 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -4,6 +4,7 @@
  */
 
 #include <dt-bindings/clock/imx8mp-clock.h>
+#include <dt-bindings/interconnect/fsl,imx8mp.h>
 #include <dt-bindings/power/imx8mp-power.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
index 7f49385ed2f8..423cac0c9cb6 100644
--- a/drivers/soc/imx/imx8m-blk-ctrl.c
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/device.h>
+#include <linux/interconnect.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
@@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data {
 	const char *name;
 	const char * const *clk_names;
 	int num_clks;
+	const char * const *path_names;
+	int num_paths;
 	const char *gpc_name;
 	u32 rst_mask;
 	u32 clk_mask;
@@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data {
 };
 
 #define DOMAIN_MAX_CLKS 4
+#define DOMAIN_MAX_PATHS 4
 
 struct imx8m_blk_ctrl_domain {
 	struct generic_pm_domain genpd;
 	const struct imx8m_blk_ctrl_domain_data *data;
 	struct clk_bulk_data clks[DOMAIN_MAX_CLKS];
+	struct icc_bulk_data paths[DOMAIN_MAX_PATHS];
 	struct device *power_dev;
 	struct imx8m_blk_ctrl *bc;
 };
@@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
 	if (data->mipi_phy_rst_mask)
 		regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask);
 
+	ret = icc_bulk_set_bw(data->num_paths, domain->paths);
+	if (ret)
+		dev_err(bc->dev, "failed to set icc bw\n");
+
 	/* disable upstream clocks */
 	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
 
@@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
 		for (j = 0; j < data->num_clks; j++)
 			domain->clks[j].id = data->clk_names[j];
 
+		for (j = 0; j < data->num_paths; j++) {
+			domain->paths[j].name = data->path_names[j];
+			domain->paths[j].avg_bw = INT_MAX;
+			domain->paths[j].peak_bw = INT_MAX;
+		}
+
+		ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to get noc entries\n");
+			goto cleanup_pds;
+		}
+
 		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
 		if (ret) {
 			dev_err_probe(dev, ret, "failed to get clock\n");
@@ -647,6 +668,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
 		.gpc_name = "lcdif1",
 		.rst_mask = BIT(4) | BIT(5) | BIT(23),
 		.clk_mask = BIT(4) | BIT(5) | BIT(23),
+		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
+		.num_paths = 2,
 	},
 	[IMX8MP_MEDIABLK_PD_ISI] = {
 		.name = "mediablk-isi",
@@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
 		.gpc_name = "isi",
 		.rst_mask = BIT(6) | BIT(7),
 		.clk_mask = BIT(6) | BIT(7),
+		.path_names = (const char *[]){"isi0", "isi1", "isi2"},
+		.num_paths = 3,
 	},
 	[IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = {
 		.name = "mediablk-mipi-csi2-2",
@@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
 		.gpc_name = "lcdif2",
 		.rst_mask = BIT(11) | BIT(12) | BIT(24),
 		.clk_mask = BIT(11) | BIT(12) | BIT(24),
+		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
+		.num_paths = 2,
 	},
 	[IMX8MP_MEDIABLK_PD_ISP] = {
 		.name = "mediablk-isp",
@@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
 		.gpc_name = "isp",
 		.rst_mask = BIT(16) | BIT(17) | BIT(18),
 		.clk_mask = BIT(16) | BIT(17) | BIT(18),
+		.path_names = (const char *[]){"isp0", "isp1"},
+		.num_paths = 2,
 	},
 	[IMX8MP_MEDIABLK_PD_DWE] = {
 		.name = "mediablk-dwe",
@@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
 		.gpc_name = "dwe",
 		.rst_mask = BIT(19) | BIT(20) | BIT(21),
 		.clk_mask = BIT(19) | BIT(20) | BIT(21),
+		.path_names = (const char *[]){"dwe"},
+		.num_paths = 1,
 	},
 	[IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = {
 		.name = "mediablk-mipi-dsi-2",
-- 
2.25.1

Re: [PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl

[Laurent Pinchart]

Hi Peng,

Thank you for the patch.

On Wed, Jun 01, 2022 at 05:45:33PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add interconnect paths for i.MX8MP media blk ctrl
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi |  1 +
>  drivers/soc/imx/imx8m-blk-ctrl.c          | 31 +++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index d9542dfff83f..2a1c6ff37e03 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <dt-bindings/clock/imx8mp-clock.h>
> +#include <dt-bindings/interconnect/fsl,imx8mp.h>
>  #include <dt-bindings/power/imx8mp-power.h>
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> index 7f49385ed2f8..423cac0c9cb6 100644
> --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/device.h>
> +#include <linux/interconnect.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data {
>  	const char *name;
>  	const char * const *clk_names;
>  	int num_clks;
> +	const char * const *path_names;
> +	int num_paths;
>  	const char *gpc_name;
>  	u32 rst_mask;
>  	u32 clk_mask;
> @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data {
>  };
>  
>  #define DOMAIN_MAX_CLKS 4
> +#define DOMAIN_MAX_PATHS 4
>  
>  struct imx8m_blk_ctrl_domain {
>  	struct generic_pm_domain genpd;
>  	const struct imx8m_blk_ctrl_domain_data *data;
>  	struct clk_bulk_data clks[DOMAIN_MAX_CLKS];
> +	struct icc_bulk_data paths[DOMAIN_MAX_PATHS];
>  	struct device *power_dev;
>  	struct imx8m_blk_ctrl *bc;
>  };
> @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
>  	if (data->mipi_phy_rst_mask)
>  		regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask);
>  
> +	ret = icc_bulk_set_bw(data->num_paths, domain->paths);
> +	if (ret)
> +		dev_err(bc->dev, "failed to set icc bw\n");

Don't we need to "release" the bandwidth on power off ?

> +
>  	/* disable upstream clocks */
>  	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
>  
> @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
>  		for (j = 0; j < data->num_clks; j++)
>  			domain->clks[j].id = data->clk_names[j];
>  
> +		for (j = 0; j < data->num_paths; j++) {
> +			domain->paths[j].name = data->path_names[j];
> +			domain->paths[j].avg_bw = INT_MAX;
> +			domain->paths[j].peak_bw = INT_MAX;

That's harsh :-) I suppose it's good enough to start with, but I wonder
if we'll need more control later.

> +		}
> +
> +		ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths);
> +		if (ret) {
> +			dev_err_probe(dev, ret, "failed to get noc entries\n");
> +			goto cleanup_pds;
> +		}
> +
>  		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
>  		if (ret) {
>  			dev_err_probe(dev, ret, "failed to get clock\n");
> @@ -647,6 +668,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "lcdif1",
>  		.rst_mask = BIT(4) | BIT(5) | BIT(23),
>  		.clk_mask = BIT(4) | BIT(5) | BIT(23),
> +		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> +		.num_paths = 2,
>  	},
>  	[IMX8MP_MEDIABLK_PD_ISI] = {
>  		.name = "mediablk-isi",
> @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "isi",
>  		.rst_mask = BIT(6) | BIT(7),
>  		.clk_mask = BIT(6) | BIT(7),
> +		.path_names = (const char *[]){"isi0", "isi1", "isi2"},
> +		.num_paths = 3,
>  	},
>  	[IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = {
>  		.name = "mediablk-mipi-csi2-2",
> @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "lcdif2",
>  		.rst_mask = BIT(11) | BIT(12) | BIT(24),
>  		.clk_mask = BIT(11) | BIT(12) | BIT(24),
> +		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> +		.num_paths = 2,
>  	},
>  	[IMX8MP_MEDIABLK_PD_ISP] = {
>  		.name = "mediablk-isp",
> @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "isp",
>  		.rst_mask = BIT(16) | BIT(17) | BIT(18),
>  		.clk_mask = BIT(16) | BIT(17) | BIT(18),
> +		.path_names = (const char *[]){"isp0", "isp1"},
> +		.num_paths = 2,

As far as I understand, there's a single power domain for both ISP
instances (I'm not entirely sure this is on purpose or a design mistake,
but that's another story), but one interconnect path for each ISP
instance. Would there be any use case for controlling them separately ?
I can't really think of any myself.

>  	},
>  	[IMX8MP_MEDIABLK_PD_DWE] = {
>  		.name = "mediablk-dwe",
> @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "dwe",
>  		.rst_mask = BIT(19) | BIT(20) | BIT(21),
>  		.clk_mask = BIT(19) | BIT(20) | BIT(21),
> +		.path_names = (const char *[]){"dwe"},
> +		.num_paths = 1,
>  	},
>  	[IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = {
>  		.name = "mediablk-mipi-dsi-2",

-- 
Regards,

Laurent Pinchart

RE: [PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl

[Peng Fan]

> Subject: Re: [PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl
> 
> Hi Peng,
> 
> Thank you for the patch.
> 
> On Wed, Jun 01, 2022 at 05:45:33PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add interconnect paths for i.MX8MP media blk ctrl
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi |  1 +
> >  drivers/soc/imx/imx8m-blk-ctrl.c          | 31
> +++++++++++++++++++++++
> >  2 files changed, 32 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index d9542dfff83f..2a1c6ff37e03 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -4,6 +4,7 @@
> >   */
> >
> >  #include <dt-bindings/clock/imx8mp-clock.h>
> > +#include <dt-bindings/interconnect/fsl,imx8mp.h>
> >  #include <dt-bindings/power/imx8mp-power.h>
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/input/input.h>
> > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c
> > b/drivers/soc/imx/imx8m-blk-ctrl.c
> > index 7f49385ed2f8..423cac0c9cb6 100644
> > --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> > @@ -5,6 +5,7 @@
> >   */
> >
> >  #include <linux/device.h>
> > +#include <linux/interconnect.h>
> >  #include <linux/module.h>
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> > @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data {
> >  	const char *name;
> >  	const char * const *clk_names;
> >  	int num_clks;
> > +	const char * const *path_names;
> > +	int num_paths;
> >  	const char *gpc_name;
> >  	u32 rst_mask;
> >  	u32 clk_mask;
> > @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data {  };
> >
> >  #define DOMAIN_MAX_CLKS 4
> > +#define DOMAIN_MAX_PATHS 4
> >
> >  struct imx8m_blk_ctrl_domain {
> >  	struct generic_pm_domain genpd;
> >  	const struct imx8m_blk_ctrl_domain_data *data;
> >  	struct clk_bulk_data clks[DOMAIN_MAX_CLKS];
> > +	struct icc_bulk_data paths[DOMAIN_MAX_PATHS];
> >  	struct device *power_dev;
> >  	struct imx8m_blk_ctrl *bc;
> >  };
> > @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct
> generic_pm_domain *genpd)
> >  	if (data->mipi_phy_rst_mask)
> >  		regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV,
> > data->mipi_phy_rst_mask);
> >
> > +	ret = icc_bulk_set_bw(data->num_paths, domain->paths);
> > +	if (ret)
> > +		dev_err(bc->dev, "failed to set icc bw\n");
> 
> Don't we need to "release" the bandwidth on power off ?

In the NXP ATF, the NoC settings are only configured during power on.
So I just follow ATF implementation.

Per IC design team, SW only get a value when power up. So I would
not diverge here.

If bandwidth in NoC registers are taken into control, release is needed.

> 
> > +
> >  	/* disable upstream clocks */
> >  	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> >
> > @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct
> platform_device *pdev)
> >  		for (j = 0; j < data->num_clks; j++)
> >  			domain->clks[j].id = data->clk_names[j];
> >
> > +		for (j = 0; j < data->num_paths; j++) {
> > +			domain->paths[j].name = data->path_names[j];
> > +			domain->paths[j].avg_bw = INT_MAX;
> > +			domain->paths[j].peak_bw = INT_MAX;
> 
> That's harsh :-) I suppose it's good enough to start with, but I wonder if we'll
> need more control later.

The current settings are only for priority, ext_control, mode settings. Actually
bandwidth settings are not provided. The value here is just to make
icc_bulk_set_bw could configure HW.

Yes, the values are not very good, we could step by step to move on
to make better.

> 
> > +		}
> > +
> > +		ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths);
> > +		if (ret) {
> > +			dev_err_probe(dev, ret, "failed to get noc entries\n");
> > +			goto cleanup_pds;
> > +		}
> > +
> >  		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
> >  		if (ret) {
> >  			dev_err_probe(dev, ret, "failed to get clock\n"); @@ -647,6
> +668,8
> > @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "lcdif1",
> >  		.rst_mask = BIT(4) | BIT(5) | BIT(23),
> >  		.clk_mask = BIT(4) | BIT(5) | BIT(23),
> > +		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> > +		.num_paths = 2,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_ISI] = {
> >  		.name = "mediablk-isi",
> > @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "isi",
> >  		.rst_mask = BIT(6) | BIT(7),
> >  		.clk_mask = BIT(6) | BIT(7),
> > +		.path_names = (const char *[]){"isi0", "isi1", "isi2"},
> > +		.num_paths = 3,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = {
> >  		.name = "mediablk-mipi-csi2-2",
> > @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "lcdif2",
> >  		.rst_mask = BIT(11) | BIT(12) | BIT(24),
> >  		.clk_mask = BIT(11) | BIT(12) | BIT(24),
> > +		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> > +		.num_paths = 2,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_ISP] = {
> >  		.name = "mediablk-isp",
> > @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "isp",
> >  		.rst_mask = BIT(16) | BIT(17) | BIT(18),
> >  		.clk_mask = BIT(16) | BIT(17) | BIT(18),
> > +		.path_names = (const char *[]){"isp0", "isp1"},
> > +		.num_paths = 2,
> 
> As far as I understand, there's a single power domain for both ISP instances (I'm
> not entirely sure this is on purpose or a design mistake, but that's another
> story), but one interconnect path for each ISP instance. Would there be any use
> case for controlling them separately ?
> I can't really think of any myself.

I have no idea. I just follow the DoC to list them.

Thanks,
Peng.

> 
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_DWE] = {
> >  		.name = "mediablk-dwe",
> > @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "dwe",
> >  		.rst_mask = BIT(19) | BIT(20) | BIT(21),
> >  		.clk_mask = BIT(19) | BIT(20) | BIT(21),
> > +		.path_names = (const char *[]){"dwe"},
> > +		.num_paths = 1,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = {
> >  		.name = "mediablk-mipi-dsi-2",
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl

[Lucas Stach]

Am Mittwoch, dem 01.06.2022 um 17:45 +0800 schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add interconnect paths for i.MX8MP media blk ctrl
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi |  1 +
>  drivers/soc/imx/imx8m-blk-ctrl.c          | 31 +++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index d9542dfff83f..2a1c6ff37e03 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <dt-bindings/clock/imx8mp-clock.h>
> +#include <dt-bindings/interconnect/fsl,imx8mp.h>
>  #include <dt-bindings/power/imx8mp-power.h>
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> index 7f49385ed2f8..423cac0c9cb6 100644
> --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/device.h>
> +#include <linux/interconnect.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data {
>  	const char *name;
>  	const char * const *clk_names;
>  	int num_clks;
> +	const char * const *path_names;
> +	int num_paths;
>  	const char *gpc_name;
>  	u32 rst_mask;
>  	u32 clk_mask;
> @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data {
>  };
>  
>  #define DOMAIN_MAX_CLKS 4
> +#define DOMAIN_MAX_PATHS 4
>  
>  struct imx8m_blk_ctrl_domain {
>  	struct generic_pm_domain genpd;
>  	const struct imx8m_blk_ctrl_domain_data *data;
>  	struct clk_bulk_data clks[DOMAIN_MAX_CLKS];
> +	struct icc_bulk_data paths[DOMAIN_MAX_PATHS];
>  	struct device *power_dev;
>  	struct imx8m_blk_ctrl *bc;
>  };
> @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
>  	if (data->mipi_phy_rst_mask)
>  		regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask);
>  
> +	ret = icc_bulk_set_bw(data->num_paths, domain->paths);
> +	if (ret)
> +		dev_err(bc->dev, "failed to set icc bw\n");
> +
>  	/* disable upstream clocks */
>  	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
>  
> @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
>  		for (j = 0; j < data->num_clks; j++)
>  			domain->clks[j].id = data->clk_names[j];
>  
> +		for (j = 0; j < data->num_paths; j++) {
> +			domain->paths[j].name = data->path_names[j];
> +			domain->paths[j].avg_bw = INT_MAX;
> +			domain->paths[j].peak_bw = INT_MAX;
> +		}
> +
> +		ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths);
> +		if (ret) {
> +			dev_err_probe(dev, ret, "failed to get noc entries\n");

I don't like that this introduces a new requirement to the kernel
config and DT, which is a backwards compat breaking change. Now one
could argue that the NoC configuration is pretty critical and should
not be skipped, but it has the potential to break currently working
systems.

I think it would be better do a
if (ret != -EPROBE_DEFER)
	dev_warn_once(dev, "Could not get interconnect paths, NoC will stay unconfigured!\n");

here and ignore the error to allow the blk-ctrl to probe even if the
interconnect paths couldn't be found due to lacking kernel config or
DT.

Regards,
Lucas

> +			goto cleanup_pds;
> +		}
> +
>  		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
>  		if (ret) {
>  			dev_err_probe(dev, ret, "failed to get clock\n");
> @@ -647,6 +668,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "lcdif1",
>  		.rst_mask = BIT(4) | BIT(5) | BIT(23),
>  		.clk_mask = BIT(4) | BIT(5) | BIT(23),
> +		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> +		.num_paths = 2,
>  	},
>  	[IMX8MP_MEDIABLK_PD_ISI] = {
>  		.name = "mediablk-isi",
> @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "isi",
>  		.rst_mask = BIT(6) | BIT(7),
>  		.clk_mask = BIT(6) | BIT(7),
> +		.path_names = (const char *[]){"isi0", "isi1", "isi2"},
> +		.num_paths = 3,
>  	},
>  	[IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = {
>  		.name = "mediablk-mipi-csi2-2",
> @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "lcdif2",
>  		.rst_mask = BIT(11) | BIT(12) | BIT(24),
>  		.clk_mask = BIT(11) | BIT(12) | BIT(24),
> +		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> +		.num_paths = 2,
>  	},
>  	[IMX8MP_MEDIABLK_PD_ISP] = {
>  		.name = "mediablk-isp",
> @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "isp",
>  		.rst_mask = BIT(16) | BIT(17) | BIT(18),
>  		.clk_mask = BIT(16) | BIT(17) | BIT(18),
> +		.path_names = (const char *[]){"isp0", "isp1"},
> +		.num_paths = 2,
>  	},
>  	[IMX8MP_MEDIABLK_PD_DWE] = {
>  		.name = "mediablk-dwe",
> @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
>  		.gpc_name = "dwe",
>  		.rst_mask = BIT(19) | BIT(20) | BIT(21),
>  		.clk_mask = BIT(19) | BIT(20) | BIT(21),
> +		.path_names = (const char *[]){"dwe"},
> +		.num_paths = 1,
>  	},
>  	[IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = {
>  		.name = "mediablk-mipi-dsi-2",

RE: [PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl

[Peng Fan]

> Subject: Re: [PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl
> 
> Am Mittwoch, dem 01.06.2022 um 17:45 +0800 schrieb Peng Fan (OSS):
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add interconnect paths for i.MX8MP media blk ctrl
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi |  1 +
> >  drivers/soc/imx/imx8m-blk-ctrl.c          | 31 +++++++++++++++++++++++
> >  2 files changed, 32 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index d9542dfff83f..2a1c6ff37e03 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -4,6 +4,7 @@
> >   */
> >
> >  #include <dt-bindings/clock/imx8mp-clock.h>
> > +#include <dt-bindings/interconnect/fsl,imx8mp.h>
> >  #include <dt-bindings/power/imx8mp-power.h>
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/input/input.h>
> > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c
> > b/drivers/soc/imx/imx8m-blk-ctrl.c
> > index 7f49385ed2f8..423cac0c9cb6 100644
> > --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> > @@ -5,6 +5,7 @@
> >   */
> >
> >  #include <linux/device.h>
> > +#include <linux/interconnect.h>
> >  #include <linux/module.h>
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> > @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data {
> >  	const char *name;
> >  	const char * const *clk_names;
> >  	int num_clks;
> > +	const char * const *path_names;
> > +	int num_paths;
> >  	const char *gpc_name;
> >  	u32 rst_mask;
> >  	u32 clk_mask;
> > @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data {  };
> >
> >  #define DOMAIN_MAX_CLKS 4
> > +#define DOMAIN_MAX_PATHS 4
> >
> >  struct imx8m_blk_ctrl_domain {
> >  	struct generic_pm_domain genpd;
> >  	const struct imx8m_blk_ctrl_domain_data *data;
> >  	struct clk_bulk_data clks[DOMAIN_MAX_CLKS];
> > +	struct icc_bulk_data paths[DOMAIN_MAX_PATHS];
> >  	struct device *power_dev;
> >  	struct imx8m_blk_ctrl *bc;
> >  };
> > @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct
> generic_pm_domain *genpd)
> >  	if (data->mipi_phy_rst_mask)
> >  		regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV,
> > data->mipi_phy_rst_mask);
> >
> > +	ret = icc_bulk_set_bw(data->num_paths, domain->paths);
> > +	if (ret)
> > +		dev_err(bc->dev, "failed to set icc bw\n");
> > +
> >  	/* disable upstream clocks */
> >  	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> >
> > @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct
> platform_device *pdev)
> >  		for (j = 0; j < data->num_clks; j++)
> >  			domain->clks[j].id = data->clk_names[j];
> >
> > +		for (j = 0; j < data->num_paths; j++) {
> > +			domain->paths[j].name = data->path_names[j];
> > +			domain->paths[j].avg_bw = INT_MAX;
> > +			domain->paths[j].peak_bw = INT_MAX;
> > +		}
> > +
> > +		ret = devm_of_icc_bulk_get(dev, data->num_paths,
> domain->paths);
> > +		if (ret) {
> > +			dev_err_probe(dev, ret, "failed to get noc
> entries\n");
> 
> I don't like that this introduces a new requirement to the kernel config and
> DT, which is a backwards compat breaking change. Now one could argue
> that the NoC configuration is pretty critical and should not be skipped, but it
> has the potential to break currently working systems.
> 
> I think it would be better do a
> if (ret != -EPROBE_DEFER)
> 	dev_warn_once(dev, "Could not get interconnect paths, NoC will
> stay unconfigured!\n");
> 
> here and ignore the error to allow the blk-ctrl to probe even if the
> interconnect paths couldn't be found due to lacking kernel config or DT.
[Peng Fan] 

Good point. So

If (ret && ret != -EPROBE_DEFER)
	dev_warn_once(dev, "Could not get interconnect paths, NoC will stay
unconfigured!\n");

Thanks,
Peng.

> 
> Regards,
> Lucas
> 
> > +			goto cleanup_pds;
> > +		}
> > +
> >  		ret = devm_clk_bulk_get(dev, data->num_clks, domain-
> >clks);
> >  		if (ret) {
> >  			dev_err_probe(dev, ret, "failed to get clock\n"); @@
> -647,6 +668,8
> > @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "lcdif1",
> >  		.rst_mask = BIT(4) | BIT(5) | BIT(23),
> >  		.clk_mask = BIT(4) | BIT(5) | BIT(23),
> > +		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> > +		.num_paths = 2,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_ISI] = {
> >  		.name = "mediablk-isi",
> > @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "isi",
> >  		.rst_mask = BIT(6) | BIT(7),
> >  		.clk_mask = BIT(6) | BIT(7),
> > +		.path_names = (const char *[]){"isi0", "isi1", "isi2"},
> > +		.num_paths = 3,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = {
> >  		.name = "mediablk-mipi-csi2-2",
> > @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "lcdif2",
> >  		.rst_mask = BIT(11) | BIT(12) | BIT(24),
> >  		.clk_mask = BIT(11) | BIT(12) | BIT(24),
> > +		.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> > +		.num_paths = 2,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_ISP] = {
> >  		.name = "mediablk-isp",
> > @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "isp",
> >  		.rst_mask = BIT(16) | BIT(17) | BIT(18),
> >  		.clk_mask = BIT(16) | BIT(17) | BIT(18),
> > +		.path_names = (const char *[]){"isp0", "isp1"},
> > +		.num_paths = 2,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_DWE] = {
> >  		.name = "mediablk-dwe",
> > @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data
> imx8mp_media_blk_ctl_domain_data[
> >  		.gpc_name = "dwe",
> >  		.rst_mask = BIT(19) | BIT(20) | BIT(21),
> >  		.clk_mask = BIT(19) | BIT(20) | BIT(21),
> > +		.path_names = (const char *[]){"dwe"},
> > +		.num_paths = 1,
> >  	},
> >  	[IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = {
> >  		.name = "mediablk-mipi-dsi-2",
> 

[PATCH 5/8] soc: imx: add icc paths for i.MX8MP hsio/hdmi blk ctrl

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add interconnect paths for i.MX8MP hsio/hdmi blk ctrl

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/imx8mp-blk-ctrl.c | 32 +++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
index 69191d7a2f54..9d1040f47b0c 100644
--- a/drivers/soc/imx/imx8mp-blk-ctrl.c
+++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
@@ -6,6 +6,7 @@
 
 #include <linux/clk.h>
 #include <linux/device.h>
+#include <linux/interconnect.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
@@ -36,15 +37,19 @@ struct imx8mp_blk_ctrl_domain_data {
 	const char *name;
 	const char * const *clk_names;
 	int num_clks;
+	const char * const *path_names;
+	int num_paths;
 	const char *gpc_name;
 };
 
 #define DOMAIN_MAX_CLKS 2
+#define DOMAIN_MAX_PATHS 3
 
 struct imx8mp_blk_ctrl_domain {
 	struct generic_pm_domain genpd;
 	const struct imx8mp_blk_ctrl_domain_data *data;
 	struct clk_bulk_data clks[DOMAIN_MAX_CLKS];
+	struct icc_bulk_data paths[DOMAIN_MAX_PATHS];
 	struct device *power_dev;
 	struct imx8mp_blk_ctrl *bc;
 	int id;
@@ -144,6 +149,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
 		.clk_names = (const char *[]){ "usb" },
 		.num_clks = 1,
 		.gpc_name = "usb",
+		.path_names = (const char *[]){"usb1", "usb2"},
+		.num_paths = 2,
 	},
 	[IMX8MP_HSIOBLK_PD_USB_PHY1] = {
 		.name = "hsioblk-usb-phy1",
@@ -158,6 +165,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
 		.clk_names = (const char *[]){ "pcie" },
 		.num_clks = 1,
 		.gpc_name = "pcie",
+		.path_names = (const char *[]){"noc-pcie", "pcie"},
+		.num_paths = 2,
 	},
 	[IMX8MP_HSIOBLK_PD_PCIE_PHY] = {
 		.name = "hsioblk-pcie-phy",
@@ -336,6 +345,8 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
 		.clk_names = (const char *[]){ "axi", "apb" },
 		.num_clks = 2,
 		.gpc_name = "lcdif",
+		.path_names = (const char *[]){"lcdif-hdmi"},
+		.num_paths = 1,
 	},
 	[IMX8MP_HDMIBLK_PD_PAI] = {
 		.name = "hdmiblk-pai",
@@ -372,12 +383,16 @@ static const struct imx8mp_blk_ctrl_domain_data imx8mp_hdmi_domain_data[] = {
 		.clk_names = (const char *[]){ "axi", "apb" },
 		.num_clks = 2,
 		.gpc_name = "hrv",
+		.path_names = (const char *[]){"hrv"},
+		.num_paths = 1,
 	},
 	[IMX8MP_HDMIBLK_PD_HDCP] = {
 		.name = "hdmiblk-hdcp",
 		.clk_names = (const char *[]){ "axi", "apb" },
 		.num_clks = 2,
 		.gpc_name = "hdcp",
+		.path_names = (const char *[]){"hdcp"},
+		.num_paths = 1,
 	},
 };
 
@@ -421,6 +436,10 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
 		goto clk_disable;
 	}
 
+	ret = icc_bulk_set_bw(data->num_paths, domain->paths);
+	if (ret)
+		dev_err(bc->dev, "failed to set icc bw\n");
+
 	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
 
 	return 0;
@@ -540,6 +559,19 @@ static int imx8mp_blk_ctrl_probe(struct platform_device *pdev)
 		for (j = 0; j < data->num_clks; j++)
 			domain->clks[j].id = data->clk_names[j];
 
+		for (j = 0; j < data->num_paths; j++) {
+			domain->paths[j].name = data->path_names[j];
+			domain->paths[j].avg_bw = INT_MAX;
+			domain->paths[j].peak_bw = INT_MAX;
+		}
+
+		ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to get noc entries\n");
+			goto cleanup_pds;
+		}
+
+
 		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
 		if (ret) {
 			dev_err_probe(dev, ret, "failed to get clock\n");
-- 
2.25.1

[PATCH 6/8] arm64: dts: imx8mp: add NoC node

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add i.MX8MP NoC node

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 2a1c6ff37e03..9e9e941a8906 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1019,6 +1019,27 @@ eqos: ethernet@30bf0000 {
 			};
 		};
 
+		noc: interconnect@32700000 {
+			compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc", "syscon";
+			reg = <0x32700000 0x100000>;
+			clocks = <&clk IMX8MP_CLK_NOC>;
+			#interconnect-cells = <1>;
+
+			operating-points-v2 = <&noc_opp_table>;
+
+			noc_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-200M {
+					opp-hz = /bits/ 64 <200000000>;
+				};
+
+				opp-1000M {
+					opp-hz = /bits/ 64 <1000000000>;
+				};
+			};
+		};
+
 		aips4: bus@32c00000 {
 			compatible = "fsl,aips-bus", "simple-bus";
 			reg = <0x32c00000 0x400000>;
-- 
2.25.1

Re: [PATCH 6/8] arm64: dts: imx8mp: add NoC node

[Lucas Stach]

Am Mittwoch, dem 01.06.2022 um 17:45 +0800 schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add i.MX8MP NoC node

Please move this patch into the "interconnect: support i.MX8MP" series
and add a bit more info to the commit message.

Regards,
Lucas

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 2a1c6ff37e03..9e9e941a8906 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1019,6 +1019,27 @@ eqos: ethernet@30bf0000 {
>  			};
>  		};
>  
> +		noc: interconnect@32700000 {
> +			compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc", "syscon";
> +			reg = <0x32700000 0x100000>;
> +			clocks = <&clk IMX8MP_CLK_NOC>;
> +			#interconnect-cells = <1>;
> +
> +			operating-points-v2 = <&noc_opp_table>;
> +
> +			noc_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				opp-200M {
> +					opp-hz = /bits/ 64 <200000000>;
> +				};
> +
> +				opp-1000M {
> +					opp-hz = /bits/ 64 <1000000000>;
> +				};
> +			};
> +		};
> +
>  		aips4: bus@32c00000 {
>  			compatible = "fsl,aips-bus", "simple-bus";
>  			reg = <0x32c00000 0x400000>;

RE: [PATCH 6/8] arm64: dts: imx8mp: add NoC node

[Peng Fan]

> Subject: Re: [PATCH 6/8] arm64: dts: imx8mp: add NoC node
> 
> Am Mittwoch, dem 01.06.2022 um 17:45 +0800 schrieb Peng Fan (OSS):
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add i.MX8MP NoC node
> 
> Please move this patch into the "interconnect: support i.MX8MP" series and
> add a bit more info to the commit message.

Yes. Sure.

Thanks,
Peng.

> 
> Regards,
> Lucas
> 
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 21
> +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 2a1c6ff37e03..9e9e941a8906 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -1019,6 +1019,27 @@ eqos: ethernet@30bf0000 {
> >  			};
> >  		};
> >
> > +		noc: interconnect@32700000 {
> > +			compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc",
> "syscon";
> > +			reg = <0x32700000 0x100000>;
> > +			clocks = <&clk IMX8MP_CLK_NOC>;
> > +			#interconnect-cells = <1>;
> > +
> > +			operating-points-v2 = <&noc_opp_table>;
> > +
> > +			noc_opp_table: opp-table {
> > +				compatible = "operating-points-v2";
> > +
> > +				opp-200M {
> > +					opp-hz = /bits/ 64 <200000000>;
> > +				};
> > +
> > +				opp-1000M {
> > +					opp-hz = /bits/ 64 <1000000000>;
> > +				};
> > +			};
> > +		};
> > +
> >  		aips4: bus@32c00000 {
> >  			compatible = "fsl,aips-bus", "simple-bus";
> >  			reg = <0x32c00000 0x400000>;
> 

[PATCH 7/8] arm64: dts: imx8mp: add interconnects for media blk ctrl

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add interconnect property for media blk ctrl

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 9e9e941a8906..53813f6766f6 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1065,6 +1065,18 @@ media_blk_ctrl: blk-ctrl@32ec0000 {
 						     "lcdif1", "isi", "mipi-csi2",
 						     "lcdif2", "isp", "dwe",
 						     "mipi-dsi2";
+				interconnects =
+					<&noc IMX8MP_ICM_LCDIF_RD &noc IMX8MP_ICN_MEDIA>,
+					<&noc IMX8MP_ICM_LCDIF_WR &noc IMX8MP_ICN_MEDIA>,
+					<&noc IMX8MP_ICM_ISI0 &noc IMX8MP_ICN_MEDIA>,
+					<&noc IMX8MP_ICM_ISI1 &noc IMX8MP_ICN_MEDIA>,
+					<&noc IMX8MP_ICM_ISI2 &noc IMX8MP_ICN_MEDIA>,
+					<&noc IMX8MP_ICM_ISP0 &noc IMX8MP_ICN_MEDIA>,
+					<&noc IMX8MP_ICM_ISP1 &noc IMX8MP_ICN_MEDIA>,
+					<&noc IMX8MP_ICM_DWE &noc IMX8MP_ICN_MEDIA>;
+				interconnect-names = "lcdif-rd", "lcdif-wr", "isi0",
+						     "isi1", "isi2", "isp0", "isp1",
+						     "dwe";
 				clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
 					 <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
 					 <&clk IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>,
-- 
2.25.1

[PATCH 8/8] arm64: dts: imx8mp: add interconnect for hsio blk ctrl

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add interconnect property for hsio blk ctrl

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 53813f6766f6..95a666050906 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1108,6 +1108,11 @@ hsio_blk_ctrl: blk-ctrl@32f10000 {
 						<&pgc_hsiomix>, <&pgc_pcie_phy>;
 				power-domain-names = "bus", "usb", "usb-phy1",
 						     "usb-phy2", "pcie", "pcie-phy";
+				interconnects = <&noc IMX8MP_ICM_NOC_PCIE &noc IMX8MP_ICN_HSIO>,
+						<&noc IMX8MP_ICM_USB1 &noc IMX8MP_ICN_HSIO>,
+						<&noc IMX8MP_ICM_USB2 &noc IMX8MP_ICN_HSIO>,
+						<&noc IMX8MP_ICM_PCIE &noc IMX8MP_ICN_HSIO>;
+				interconnect-names = "noc-pcie", "usb1", "usb2", "pcie";
 				#power-domain-cells = <1>;
 			};
 		};
-- 
2.25.1