All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/18] i.MX8MM GPC improvements and BLK_CTRL driver
@ 2021-09-06 18:43 ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Hi all,

third revision of the GPC improvements and BLK_CTRL driver to enable
all power domains on the i.MX8MM. Frieder is still hunting some
sporadic and hard to reproduce issues and I currently don't have much
time to help with that. Meanwhile here's a new revision with the
obvious issues (DT binding validation fail) fixed to allow people to
review the code, so we hopefully don't miss the next merge window again.

I guess it would even be possible to merge the series with the
sporadic issues still present, as it seems a lot of people are waiting
for this to land, as it's blocking lots of other work.

Regards,
Lucas

Frieder Schrempf (1):
  arm64: dts: imx8mm: Add GPU nodes for 2D and 3D core

Lucas Stach (15):
  Revert "soc: imx: gpcv2: move reset assert after requesting domain
    power up"
  soc: imx: gpcv2: add lockdep annotation
  soc: imx: gpcv2: add domain option to keep domain clocks enabled
  soc: imx: gpcv2: keep i.MX8M* bus clocks enabled
  soc: imx: gpcv2: support system suspend/resume
  dt-bindings: soc: add binding for i.MX8MM VPU blk-ctrl
  dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains
  soc: imx: add i.MX8M blk-ctrl driver
  dt-bindings: soc: add binding for i.MX8MM DISP blk-ctrl
  dt-bindings: power: imx8mm: add defines for DISP blk-ctrl domains
  soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl
  arm64: dts: imx8mm: add GPC node
  arm64: dts: imx8mm: put USB controllers into power-domains
  arm64: dts: imx8mm: add VPU blk-ctrl
  arm64: dts: imx8mm: add DISP blk-ctrl

Marek Vasut (2):
  soc: imx: gpcv2: Turn domain->pgc into bitfield
  soc: imx: gpcv2: Set both GPC_PGC_nCTRL(GPU_2D|GPU_3D) for MX8MM GPU
    domain

 .../soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml     |  94 ++++
 .../soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml      |  76 +++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 180 ++++++
 drivers/soc/imx/Makefile                      |   1 +
 drivers/soc/imx/gpcv2.c                       | 130 +++--
 drivers/soc/imx/imx8m-blk-ctrl.c              | 525 ++++++++++++++++++
 include/dt-bindings/power/imx8mm-power.h      |   9 +
 7 files changed, 974 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml
 create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c

-- 
2.30.2


^ permalink raw reply	[flat|nested] 50+ messages in thread
* [PATCH v3 00/18] i.MX8MM GPC improvements and BLK_CTRL driver
@ 2021-09-06 18:43 ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Hi all,

third revision of the GPC improvements and BLK_CTRL driver to enable
all power domains on the i.MX8MM. Frieder is still hunting some
sporadic and hard to reproduce issues and I currently don't have much
time to help with that. Meanwhile here's a new revision with the
obvious issues (DT binding validation fail) fixed to allow people to
review the code, so we hopefully don't miss the next merge window again.

I guess it would even be possible to merge the series with the
sporadic issues still present, as it seems a lot of people are waiting
for this to land, as it's blocking lots of other work.

Regards,
Lucas

Frieder Schrempf (1):
  arm64: dts: imx8mm: Add GPU nodes for 2D and 3D core

Lucas Stach (15):
  Revert "soc: imx: gpcv2: move reset assert after requesting domain
    power up"
  soc: imx: gpcv2: add lockdep annotation
  soc: imx: gpcv2: add domain option to keep domain clocks enabled
  soc: imx: gpcv2: keep i.MX8M* bus clocks enabled
  soc: imx: gpcv2: support system suspend/resume
  dt-bindings: soc: add binding for i.MX8MM VPU blk-ctrl
  dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains
  soc: imx: add i.MX8M blk-ctrl driver
  dt-bindings: soc: add binding for i.MX8MM DISP blk-ctrl
  dt-bindings: power: imx8mm: add defines for DISP blk-ctrl domains
  soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl
  arm64: dts: imx8mm: add GPC node
  arm64: dts: imx8mm: put USB controllers into power-domains
  arm64: dts: imx8mm: add VPU blk-ctrl
  arm64: dts: imx8mm: add DISP blk-ctrl

Marek Vasut (2):
  soc: imx: gpcv2: Turn domain->pgc into bitfield
  soc: imx: gpcv2: Set both GPC_PGC_nCTRL(GPU_2D|GPU_3D) for MX8MM GPU
    domain

 .../soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml     |  94 ++++
 .../soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml      |  76 +++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 180 ++++++
 drivers/soc/imx/Makefile                      |   1 +
 drivers/soc/imx/gpcv2.c                       | 130 +++--
 drivers/soc/imx/imx8m-blk-ctrl.c              | 525 ++++++++++++++++++
 include/dt-bindings/power/imx8mm-power.h      |   9 +
 7 files changed, 974 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml
 create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c

-- 
2.30.2


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

^ permalink raw reply	[flat|nested] 50+ messages in thread
* [PATCH v3 01/18] Revert "soc: imx: gpcv2: move reset assert after requesting domain power up"
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This reverts commit a77ebdd9f553. It turns out that the VPU domain has no
different requirements, even though the downstream ATF implementation seems
to suggest otherwise. Powering on the domain with the reset asserted works
fine. As the changed sequence has caused sporadic issues with the GPU
domains, just revert the change to go back to the working sequence.

Cc: <stable@vger.kernel.org> # 5.14
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Acked-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 34a9ac1f2b9b..8b7a01773aec 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -244,6 +244,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		goto out_regulator_disable;
 	}
 
+	reset_control_assert(domain->reset);
+
 	if (domain->bits.pxx) {
 		/* request the domain to power up */
 		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
@@ -266,8 +268,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 				  GPC_PGC_CTRL_PCR);
 	}
 
-	reset_control_assert(domain->reset);
-
 	/* delay for reset to propagate */
 	udelay(5);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 01/18] Revert "soc: imx: gpcv2: move reset assert after requesting domain power up"
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This reverts commit a77ebdd9f553. It turns out that the VPU domain has no
different requirements, even though the downstream ATF implementation seems
to suggest otherwise. Powering on the domain with the reset asserted works
fine. As the changed sequence has caused sporadic issues with the GPU
domains, just revert the change to go back to the working sequence.

Cc: <stable@vger.kernel.org> # 5.14
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Acked-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 34a9ac1f2b9b..8b7a01773aec 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -244,6 +244,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		goto out_regulator_disable;
 	}
 
+	reset_control_assert(domain->reset);
+
 	if (domain->bits.pxx) {
 		/* request the domain to power up */
 		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
@@ -266,8 +268,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 				  GPC_PGC_CTRL_PCR);
 	}
 
-	reset_control_assert(domain->reset);
-
 	/* delay for reset to propagate */
 	udelay(5);
 
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 02/18] soc: imx: gpcv2: Turn domain->pgc into bitfield
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

From: Marek Vasut <marex@denx.de>

There is currently the MX8MM GPU domain, which is in fact a composite domain
for both GPU2D and GPU3D. To correctly configure this domain, it is necessary
to control both GPC_PGC_nCTRL(GPU_2D) and GPC_PGC_nCTRL(GPU_3D) at the same
time. This is currently not possible.

Turn the domain->pgc from value into bitfield and use for_each_set_bit() to
iterate over all bits set in domain->pgc when configuring GPC_PGC_nCTRL
register array. This way it is possible to configure all GPC_PGC_nCTRL
registers required in a particular domain.

This is a preparatory patch, no functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
v2 (Lucas Stach):
- rebase on top of reverted reset sequence change
- also convert i.MX8MN domains
---
 drivers/soc/imx/gpcv2.c | 72 ++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 34 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 8b7a01773aec..c7826ce92f0d 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -192,7 +192,7 @@ struct imx_pgc_domain {
 	struct clk_bulk_data *clks;
 	int num_clks;
 
-	unsigned int pgc;
+	unsigned long pgc;
 
 	const struct {
 		u32 pxx;
@@ -220,7 +220,7 @@ to_imx_pgc_domain(struct generic_pm_domain *genpd)
 static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 {
 	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
-	u32 reg_val;
+	u32 reg_val, pgc;
 	int ret;
 
 	ret = pm_runtime_get_sync(domain->dev);
@@ -264,8 +264,10 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		}
 
 		/* disable power control */
-		regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-				  GPC_PGC_CTRL_PCR);
+		for_each_set_bit(pgc, &domain->pgc, 32) {
+			regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(pgc),
+					  GPC_PGC_CTRL_PCR);
+		}
 	}
 
 	/* delay for reset to propagate */
@@ -311,7 +313,7 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 {
 	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
-	u32 reg_val;
+	u32 reg_val, pgc;
 	int ret;
 
 	/* Enable reset clocks for all devices in the domain */
@@ -338,8 +340,10 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 
 	if (domain->bits.pxx) {
 		/* enable power control */
-		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-				   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
+		for_each_set_bit(pgc, &domain->pgc, 32) {
+			regmap_update_bits(domain->regmap, GPC_PGC_CTRL(pgc),
+					   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
+		}
 
 		/* request the domain to power down */
 		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
@@ -389,7 +393,7 @@ static const struct imx_pgc_domain imx7_pgc_domains[] = {
 			.map = IMX7_MIPI_PHY_A_CORE_DOMAIN,
 		},
 		.voltage   = 1000000,
-		.pgc	   = IMX7_PGC_MIPI,
+		.pgc	   = BIT(IMX7_PGC_MIPI),
 	},
 
 	[IMX7_POWER_DOMAIN_PCIE_PHY] = {
@@ -401,7 +405,7 @@ static const struct imx_pgc_domain imx7_pgc_domains[] = {
 			.map = IMX7_PCIE_PHY_A_CORE_DOMAIN,
 		},
 		.voltage   = 1000000,
-		.pgc	   = IMX7_PGC_PCIE,
+		.pgc	   = BIT(IMX7_PGC_PCIE),
 	},
 
 	[IMX7_POWER_DOMAIN_USB_HSIC_PHY] = {
@@ -413,7 +417,7 @@ static const struct imx_pgc_domain imx7_pgc_domains[] = {
 			.map = IMX7_USB_HSIC_PHY_A_CORE_DOMAIN,
 		},
 		.voltage   = 1200000,
-		.pgc	   = IMX7_PGC_USB_HSIC,
+		.pgc	   = BIT(IMX7_PGC_USB_HSIC),
 	},
 };
 
@@ -448,7 +452,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_MIPI_SW_Pxx_REQ,
 			.map = IMX8M_MIPI_A53_DOMAIN,
 		},
-		.pgc	   = IMX8M_PGC_MIPI,
+		.pgc	   = BIT(IMX8M_PGC_MIPI),
 	},
 
 	[IMX8M_POWER_DOMAIN_PCIE1] = {
@@ -459,7 +463,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_PCIE1_SW_Pxx_REQ,
 			.map = IMX8M_PCIE1_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_PCIE1,
+		.pgc   = BIT(IMX8M_PGC_PCIE1),
 	},
 
 	[IMX8M_POWER_DOMAIN_USB_OTG1] = {
@@ -470,7 +474,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_OTG1_SW_Pxx_REQ,
 			.map = IMX8M_OTG1_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_OTG1,
+		.pgc   = BIT(IMX8M_PGC_OTG1),
 	},
 
 	[IMX8M_POWER_DOMAIN_USB_OTG2] = {
@@ -481,7 +485,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_OTG2_SW_Pxx_REQ,
 			.map = IMX8M_OTG2_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_OTG2,
+		.pgc   = BIT(IMX8M_PGC_OTG2),
 	},
 
 	[IMX8M_POWER_DOMAIN_DDR1] = {
@@ -492,7 +496,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_DDR1_SW_Pxx_REQ,
 			.map = IMX8M_DDR2_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_DDR1,
+		.pgc   = BIT(IMX8M_PGC_DDR1),
 	},
 
 	[IMX8M_POWER_DOMAIN_GPU] = {
@@ -505,7 +509,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.hskreq = IMX8M_GPU_HSK_PWRDNREQN,
 			.hskack = IMX8M_GPU_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8M_PGC_GPU,
+		.pgc   = BIT(IMX8M_PGC_GPU),
 	},
 
 	[IMX8M_POWER_DOMAIN_VPU] = {
@@ -518,7 +522,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.hskreq = IMX8M_VPU_HSK_PWRDNREQN,
 			.hskack = IMX8M_VPU_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8M_PGC_VPU,
+		.pgc   = BIT(IMX8M_PGC_VPU),
 	},
 
 	[IMX8M_POWER_DOMAIN_DISP] = {
@@ -531,7 +535,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.hskreq = IMX8M_DISP_HSK_PWRDNREQN,
 			.hskack = IMX8M_DISP_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8M_PGC_DISP,
+		.pgc   = BIT(IMX8M_PGC_DISP),
 	},
 
 	[IMX8M_POWER_DOMAIN_MIPI_CSI1] = {
@@ -542,7 +546,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_MIPI_CSI1_SW_Pxx_REQ,
 			.map = IMX8M_MIPI_CSI1_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_MIPI_CSI1,
+		.pgc   = BIT(IMX8M_PGC_MIPI_CSI1),
 	},
 
 	[IMX8M_POWER_DOMAIN_MIPI_CSI2] = {
@@ -553,7 +557,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_MIPI_CSI2_SW_Pxx_REQ,
 			.map = IMX8M_MIPI_CSI2_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_MIPI_CSI2,
+		.pgc   = BIT(IMX8M_PGC_MIPI_CSI2),
 	},
 
 	[IMX8M_POWER_DOMAIN_PCIE2] = {
@@ -564,7 +568,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_PCIE2_SW_Pxx_REQ,
 			.map = IMX8M_PCIE2_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_PCIE2,
+		.pgc   = BIT(IMX8M_PGC_PCIE2),
 	},
 };
 
@@ -627,7 +631,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_PCIE_SW_Pxx_REQ,
 			.map = IMX8MM_PCIE_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_PCIE,
+		.pgc   = BIT(IMX8MM_PGC_PCIE),
 	},
 
 	[IMX8MM_POWER_DOMAIN_OTG1] = {
@@ -638,7 +642,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_OTG1_SW_Pxx_REQ,
 			.map = IMX8MM_OTG1_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_OTG1,
+		.pgc   = BIT(IMX8MM_PGC_OTG1),
 	},
 
 	[IMX8MM_POWER_DOMAIN_OTG2] = {
@@ -649,7 +653,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_OTG2_SW_Pxx_REQ,
 			.map = IMX8MM_OTG2_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_OTG2,
+		.pgc   = BIT(IMX8MM_PGC_OTG2),
 	},
 
 	[IMX8MM_POWER_DOMAIN_GPUMIX] = {
@@ -662,7 +666,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_GPUMIX_HSK_PWRDNREQN,
 			.hskack = IMX8MM_GPUMIX_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MM_PGC_GPUMIX,
+		.pgc   = BIT(IMX8MM_PGC_GPUMIX),
 	},
 
 	[IMX8MM_POWER_DOMAIN_GPU] = {
@@ -675,7 +679,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_GPU_HSK_PWRDNREQN,
 			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MM_PGC_GPU2D,
+		.pgc   = BIT(IMX8MM_PGC_GPU2D),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUMIX] = {
@@ -688,7 +692,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_VPUMIX_HSK_PWRDNREQN,
 			.hskack = IMX8MM_VPUMIX_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MM_PGC_VPUMIX,
+		.pgc   = BIT(IMX8MM_PGC_VPUMIX),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUG1] = {
@@ -699,7 +703,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_VPUG1_SW_Pxx_REQ,
 			.map = IMX8MM_VPUG1_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_VPUG1,
+		.pgc   = BIT(IMX8MM_PGC_VPUG1),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUG2] = {
@@ -710,7 +714,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_VPUG2_SW_Pxx_REQ,
 			.map = IMX8MM_VPUG2_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_VPUG2,
+		.pgc   = BIT(IMX8MM_PGC_VPUG2),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUH1] = {
@@ -721,7 +725,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_VPUH1_SW_Pxx_REQ,
 			.map = IMX8MM_VPUH1_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_VPUH1,
+		.pgc   = BIT(IMX8MM_PGC_VPUH1),
 	},
 
 	[IMX8MM_POWER_DOMAIN_DISPMIX] = {
@@ -734,7 +738,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_DISPMIX_HSK_PWRDNREQN,
 			.hskack = IMX8MM_DISPMIX_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MM_PGC_DISPMIX,
+		.pgc   = BIT(IMX8MM_PGC_DISPMIX),
 	},
 
 	[IMX8MM_POWER_DOMAIN_MIPI] = {
@@ -745,7 +749,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_MIPI_SW_Pxx_REQ,
 			.map = IMX8MM_MIPI_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_MIPI,
+		.pgc   = BIT(IMX8MM_PGC_MIPI),
 	},
 };
 
@@ -812,7 +816,7 @@ static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
 			.pxx = IMX8MN_OTG1_SW_Pxx_REQ,
 			.map = IMX8MN_OTG1_A53_DOMAIN,
 		},
-		.pgc   = IMX8MN_PGC_OTG1,
+		.pgc   = BIT(IMX8MN_PGC_OTG1),
 	},
 
 	[IMX8MN_POWER_DOMAIN_GPUMIX] = {
@@ -825,7 +829,7 @@ static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
 			.hskreq = IMX8MN_GPUMIX_HSK_PWRDNREQN,
 			.hskack = IMX8MN_GPUMIX_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MN_PGC_GPUMIX,
+		.pgc   = BIT(IMX8MN_PGC_GPUMIX),
 	},
 };
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 02/18] soc: imx: gpcv2: Turn domain->pgc into bitfield
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

From: Marek Vasut <marex@denx.de>

There is currently the MX8MM GPU domain, which is in fact a composite domain
for both GPU2D and GPU3D. To correctly configure this domain, it is necessary
to control both GPC_PGC_nCTRL(GPU_2D) and GPC_PGC_nCTRL(GPU_3D) at the same
time. This is currently not possible.

Turn the domain->pgc from value into bitfield and use for_each_set_bit() to
iterate over all bits set in domain->pgc when configuring GPC_PGC_nCTRL
register array. This way it is possible to configure all GPC_PGC_nCTRL
registers required in a particular domain.

This is a preparatory patch, no functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
v2 (Lucas Stach):
- rebase on top of reverted reset sequence change
- also convert i.MX8MN domains
---
 drivers/soc/imx/gpcv2.c | 72 ++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 34 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 8b7a01773aec..c7826ce92f0d 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -192,7 +192,7 @@ struct imx_pgc_domain {
 	struct clk_bulk_data *clks;
 	int num_clks;
 
-	unsigned int pgc;
+	unsigned long pgc;
 
 	const struct {
 		u32 pxx;
@@ -220,7 +220,7 @@ to_imx_pgc_domain(struct generic_pm_domain *genpd)
 static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 {
 	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
-	u32 reg_val;
+	u32 reg_val, pgc;
 	int ret;
 
 	ret = pm_runtime_get_sync(domain->dev);
@@ -264,8 +264,10 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		}
 
 		/* disable power control */
-		regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-				  GPC_PGC_CTRL_PCR);
+		for_each_set_bit(pgc, &domain->pgc, 32) {
+			regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(pgc),
+					  GPC_PGC_CTRL_PCR);
+		}
 	}
 
 	/* delay for reset to propagate */
@@ -311,7 +313,7 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 {
 	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
-	u32 reg_val;
+	u32 reg_val, pgc;
 	int ret;
 
 	/* Enable reset clocks for all devices in the domain */
@@ -338,8 +340,10 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 
 	if (domain->bits.pxx) {
 		/* enable power control */
-		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-				   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
+		for_each_set_bit(pgc, &domain->pgc, 32) {
+			regmap_update_bits(domain->regmap, GPC_PGC_CTRL(pgc),
+					   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
+		}
 
 		/* request the domain to power down */
 		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
@@ -389,7 +393,7 @@ static const struct imx_pgc_domain imx7_pgc_domains[] = {
 			.map = IMX7_MIPI_PHY_A_CORE_DOMAIN,
 		},
 		.voltage   = 1000000,
-		.pgc	   = IMX7_PGC_MIPI,
+		.pgc	   = BIT(IMX7_PGC_MIPI),
 	},
 
 	[IMX7_POWER_DOMAIN_PCIE_PHY] = {
@@ -401,7 +405,7 @@ static const struct imx_pgc_domain imx7_pgc_domains[] = {
 			.map = IMX7_PCIE_PHY_A_CORE_DOMAIN,
 		},
 		.voltage   = 1000000,
-		.pgc	   = IMX7_PGC_PCIE,
+		.pgc	   = BIT(IMX7_PGC_PCIE),
 	},
 
 	[IMX7_POWER_DOMAIN_USB_HSIC_PHY] = {
@@ -413,7 +417,7 @@ static const struct imx_pgc_domain imx7_pgc_domains[] = {
 			.map = IMX7_USB_HSIC_PHY_A_CORE_DOMAIN,
 		},
 		.voltage   = 1200000,
-		.pgc	   = IMX7_PGC_USB_HSIC,
+		.pgc	   = BIT(IMX7_PGC_USB_HSIC),
 	},
 };
 
@@ -448,7 +452,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_MIPI_SW_Pxx_REQ,
 			.map = IMX8M_MIPI_A53_DOMAIN,
 		},
-		.pgc	   = IMX8M_PGC_MIPI,
+		.pgc	   = BIT(IMX8M_PGC_MIPI),
 	},
 
 	[IMX8M_POWER_DOMAIN_PCIE1] = {
@@ -459,7 +463,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_PCIE1_SW_Pxx_REQ,
 			.map = IMX8M_PCIE1_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_PCIE1,
+		.pgc   = BIT(IMX8M_PGC_PCIE1),
 	},
 
 	[IMX8M_POWER_DOMAIN_USB_OTG1] = {
@@ -470,7 +474,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_OTG1_SW_Pxx_REQ,
 			.map = IMX8M_OTG1_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_OTG1,
+		.pgc   = BIT(IMX8M_PGC_OTG1),
 	},
 
 	[IMX8M_POWER_DOMAIN_USB_OTG2] = {
@@ -481,7 +485,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_OTG2_SW_Pxx_REQ,
 			.map = IMX8M_OTG2_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_OTG2,
+		.pgc   = BIT(IMX8M_PGC_OTG2),
 	},
 
 	[IMX8M_POWER_DOMAIN_DDR1] = {
@@ -492,7 +496,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_DDR1_SW_Pxx_REQ,
 			.map = IMX8M_DDR2_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_DDR1,
+		.pgc   = BIT(IMX8M_PGC_DDR1),
 	},
 
 	[IMX8M_POWER_DOMAIN_GPU] = {
@@ -505,7 +509,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.hskreq = IMX8M_GPU_HSK_PWRDNREQN,
 			.hskack = IMX8M_GPU_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8M_PGC_GPU,
+		.pgc   = BIT(IMX8M_PGC_GPU),
 	},
 
 	[IMX8M_POWER_DOMAIN_VPU] = {
@@ -518,7 +522,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.hskreq = IMX8M_VPU_HSK_PWRDNREQN,
 			.hskack = IMX8M_VPU_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8M_PGC_VPU,
+		.pgc   = BIT(IMX8M_PGC_VPU),
 	},
 
 	[IMX8M_POWER_DOMAIN_DISP] = {
@@ -531,7 +535,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.hskreq = IMX8M_DISP_HSK_PWRDNREQN,
 			.hskack = IMX8M_DISP_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8M_PGC_DISP,
+		.pgc   = BIT(IMX8M_PGC_DISP),
 	},
 
 	[IMX8M_POWER_DOMAIN_MIPI_CSI1] = {
@@ -542,7 +546,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_MIPI_CSI1_SW_Pxx_REQ,
 			.map = IMX8M_MIPI_CSI1_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_MIPI_CSI1,
+		.pgc   = BIT(IMX8M_PGC_MIPI_CSI1),
 	},
 
 	[IMX8M_POWER_DOMAIN_MIPI_CSI2] = {
@@ -553,7 +557,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_MIPI_CSI2_SW_Pxx_REQ,
 			.map = IMX8M_MIPI_CSI2_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_MIPI_CSI2,
+		.pgc   = BIT(IMX8M_PGC_MIPI_CSI2),
 	},
 
 	[IMX8M_POWER_DOMAIN_PCIE2] = {
@@ -564,7 +568,7 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 			.pxx = IMX8M_PCIE2_SW_Pxx_REQ,
 			.map = IMX8M_PCIE2_A53_DOMAIN,
 		},
-		.pgc   = IMX8M_PGC_PCIE2,
+		.pgc   = BIT(IMX8M_PGC_PCIE2),
 	},
 };
 
@@ -627,7 +631,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_PCIE_SW_Pxx_REQ,
 			.map = IMX8MM_PCIE_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_PCIE,
+		.pgc   = BIT(IMX8MM_PGC_PCIE),
 	},
 
 	[IMX8MM_POWER_DOMAIN_OTG1] = {
@@ -638,7 +642,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_OTG1_SW_Pxx_REQ,
 			.map = IMX8MM_OTG1_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_OTG1,
+		.pgc   = BIT(IMX8MM_PGC_OTG1),
 	},
 
 	[IMX8MM_POWER_DOMAIN_OTG2] = {
@@ -649,7 +653,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_OTG2_SW_Pxx_REQ,
 			.map = IMX8MM_OTG2_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_OTG2,
+		.pgc   = BIT(IMX8MM_PGC_OTG2),
 	},
 
 	[IMX8MM_POWER_DOMAIN_GPUMIX] = {
@@ -662,7 +666,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_GPUMIX_HSK_PWRDNREQN,
 			.hskack = IMX8MM_GPUMIX_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MM_PGC_GPUMIX,
+		.pgc   = BIT(IMX8MM_PGC_GPUMIX),
 	},
 
 	[IMX8MM_POWER_DOMAIN_GPU] = {
@@ -675,7 +679,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_GPU_HSK_PWRDNREQN,
 			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MM_PGC_GPU2D,
+		.pgc   = BIT(IMX8MM_PGC_GPU2D),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUMIX] = {
@@ -688,7 +692,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_VPUMIX_HSK_PWRDNREQN,
 			.hskack = IMX8MM_VPUMIX_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MM_PGC_VPUMIX,
+		.pgc   = BIT(IMX8MM_PGC_VPUMIX),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUG1] = {
@@ -699,7 +703,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_VPUG1_SW_Pxx_REQ,
 			.map = IMX8MM_VPUG1_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_VPUG1,
+		.pgc   = BIT(IMX8MM_PGC_VPUG1),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUG2] = {
@@ -710,7 +714,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_VPUG2_SW_Pxx_REQ,
 			.map = IMX8MM_VPUG2_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_VPUG2,
+		.pgc   = BIT(IMX8MM_PGC_VPUG2),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUH1] = {
@@ -721,7 +725,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_VPUH1_SW_Pxx_REQ,
 			.map = IMX8MM_VPUH1_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_VPUH1,
+		.pgc   = BIT(IMX8MM_PGC_VPUH1),
 	},
 
 	[IMX8MM_POWER_DOMAIN_DISPMIX] = {
@@ -734,7 +738,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_DISPMIX_HSK_PWRDNREQN,
 			.hskack = IMX8MM_DISPMIX_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MM_PGC_DISPMIX,
+		.pgc   = BIT(IMX8MM_PGC_DISPMIX),
 	},
 
 	[IMX8MM_POWER_DOMAIN_MIPI] = {
@@ -745,7 +749,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.pxx = IMX8MM_MIPI_SW_Pxx_REQ,
 			.map = IMX8MM_MIPI_A53_DOMAIN,
 		},
-		.pgc   = IMX8MM_PGC_MIPI,
+		.pgc   = BIT(IMX8MM_PGC_MIPI),
 	},
 };
 
@@ -812,7 +816,7 @@ static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
 			.pxx = IMX8MN_OTG1_SW_Pxx_REQ,
 			.map = IMX8MN_OTG1_A53_DOMAIN,
 		},
-		.pgc   = IMX8MN_PGC_OTG1,
+		.pgc   = BIT(IMX8MN_PGC_OTG1),
 	},
 
 	[IMX8MN_POWER_DOMAIN_GPUMIX] = {
@@ -825,7 +829,7 @@ static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
 			.hskreq = IMX8MN_GPUMIX_HSK_PWRDNREQN,
 			.hskack = IMX8MN_GPUMIX_HSK_PWRDNACKN,
 		},
-		.pgc   = IMX8MN_PGC_GPUMIX,
+		.pgc   = BIT(IMX8MN_PGC_GPUMIX),
 	},
 };
 
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 03/18] soc: imx: gpcv2: Set both GPC_PGC_nCTRL(GPU_2D|GPU_3D) for MX8MM GPU domain
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

From: Marek Vasut <marex@denx.de>

To bring up the MX8MM GPU domain, it is necessary to configure both
GPC_PGC_nCTRL(GPU_2D) and GPC_PGC_nCTRL(GPU_3D) registers. Without
this configuration, the system might hang on boot when bringing up
the GPU power domain. This is sporadically observed on multiple
disparate systems.

Add the GPU3D bit into MX8MM GPU domain pgc bitfield, so that both
GPC_PGC_nCTRL(GPU_2D) and GPC_PGC_nCTRL(GPU_3D) registers are
configured when bringing up the GPU domain. This fixes the sporadic
hang.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index c7826ce92f0d..2c43e74db0be 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -679,7 +679,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_GPU_HSK_PWRDNREQN,
 			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
 		},
-		.pgc   = BIT(IMX8MM_PGC_GPU2D),
+		.pgc   = BIT(IMX8MM_PGC_GPU2D) | BIT(IMX8MM_PGC_GPU3D),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUMIX] = {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 03/18] soc: imx: gpcv2: Set both GPC_PGC_nCTRL(GPU_2D|GPU_3D) for MX8MM GPU domain
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

From: Marek Vasut <marex@denx.de>

To bring up the MX8MM GPU domain, it is necessary to configure both
GPC_PGC_nCTRL(GPU_2D) and GPC_PGC_nCTRL(GPU_3D) registers. Without
this configuration, the system might hang on boot when bringing up
the GPU power domain. This is sporadically observed on multiple
disparate systems.

Add the GPU3D bit into MX8MM GPU domain pgc bitfield, so that both
GPC_PGC_nCTRL(GPU_2D) and GPC_PGC_nCTRL(GPU_3D) registers are
configured when bringing up the GPU domain. This fixes the sporadic
hang.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index c7826ce92f0d..2c43e74db0be 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -679,7 +679,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_GPU_HSK_PWRDNREQN,
 			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
 		},
-		.pgc   = BIT(IMX8MM_PGC_GPU2D),
+		.pgc   = BIT(IMX8MM_PGC_GPU2D) | BIT(IMX8MM_PGC_GPU3D),
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUMIX] = {
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 04/18] soc: imx: gpcv2: add lockdep annotation
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Some of the GPCv2 power domains are nested inside each other without
visibility to lockdep at the genpd level, as they are in separate
driver instances and don't have a parent/child power-domain relationship.

Add a subclass annotation to the nested domains to let lockdep know that
it is okay to take the genpd lock in a nested fashion.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 2c43e74db0be..35f26f57d1ac 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -898,6 +898,10 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 		goto out_domain_unmap;
 	}
 
+	if (IS_ENABLED(CONFIG_LOCKDEP) &&
+	    of_property_read_bool(domain->dev->of_node, "power-domains"))
+		lockdep_set_subclass(&domain->genpd.mlock, 1);
+
 	ret = of_genpd_add_provider_simple(domain->dev->of_node,
 					   &domain->genpd);
 	if (ret) {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 04/18] soc: imx: gpcv2: add lockdep annotation
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Some of the GPCv2 power domains are nested inside each other without
visibility to lockdep at the genpd level, as they are in separate
driver instances and don't have a parent/child power-domain relationship.

Add a subclass annotation to the nested domains to let lockdep know that
it is okay to take the genpd lock in a nested fashion.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 2c43e74db0be..35f26f57d1ac 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -898,6 +898,10 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 		goto out_domain_unmap;
 	}
 
+	if (IS_ENABLED(CONFIG_LOCKDEP) &&
+	    of_property_read_bool(domain->dev->of_node, "power-domains"))
+		lockdep_set_subclass(&domain->genpd.mlock, 1);
+
 	ret = of_genpd_add_provider_simple(domain->dev->of_node,
 					   &domain->genpd);
 	if (ret) {
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 05/18] soc: imx: gpcv2: add domain option to keep domain clocks enabled
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Some of the MIX domains are using clocks to drive the bus bridges. Those
must be enabled at all times, as long as the domain is powered up and
they don't have any other consumer than the power domain. Add an option
to keep the clocks attached to a domain enabled as long as the domain
is power up and only disable them after the domain is powered down.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 35f26f57d1ac..c3b1d2580963 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -202,6 +202,7 @@ struct imx_pgc_domain {
 	} bits;
 
 	const int voltage;
+	const bool keep_clocks;
 	struct device *dev;
 };
 
@@ -295,7 +296,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 	}
 
 	/* Disable reset clocks for all devices in the domain */
-	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
+	if (!domain->keep_clocks)
+		clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
 
 	return 0;
 
@@ -317,10 +319,12 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 	int ret;
 
 	/* Enable reset clocks for all devices in the domain */
-	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
-	if (ret) {
-		dev_err(domain->dev, "failed to enable reset clocks\n");
-		return ret;
+	if (!domain->keep_clocks) {
+		ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
+		if (ret) {
+			dev_err(domain->dev, "failed to enable reset clocks\n");
+			return ret;
+		}
 	}
 
 	/* request the ADB400 to power down */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 05/18] soc: imx: gpcv2: add domain option to keep domain clocks enabled
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Some of the MIX domains are using clocks to drive the bus bridges. Those
must be enabled at all times, as long as the domain is powered up and
they don't have any other consumer than the power domain. Add an option
to keep the clocks attached to a domain enabled as long as the domain
is power up and only disable them after the domain is powered down.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 35f26f57d1ac..c3b1d2580963 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -202,6 +202,7 @@ struct imx_pgc_domain {
 	} bits;
 
 	const int voltage;
+	const bool keep_clocks;
 	struct device *dev;
 };
 
@@ -295,7 +296,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 	}
 
 	/* Disable reset clocks for all devices in the domain */
-	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
+	if (!domain->keep_clocks)
+		clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
 
 	return 0;
 
@@ -317,10 +319,12 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 	int ret;
 
 	/* Enable reset clocks for all devices in the domain */
-	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
-	if (ret) {
-		dev_err(domain->dev, "failed to enable reset clocks\n");
-		return ret;
+	if (!domain->keep_clocks) {
+		ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
+		if (ret) {
+			dev_err(domain->dev, "failed to enable reset clocks\n");
+			return ret;
+		}
 	}
 
 	/* request the ADB400 to power down */
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 06/18] soc: imx: gpcv2: keep i.MX8M* bus clocks enabled
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Annotate the domains with bus clocks to keep those clocks enabled
as long as the domain is active.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index c3b1d2580963..c48f37f203ab 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -625,6 +625,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_HSIO_HSK_PWRDNREQN,
 			.hskack = IMX8MM_HSIO_HSK_PWRDNACKN,
 		},
+		.keep_clocks = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_PCIE] = {
@@ -671,6 +672,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskack = IMX8MM_GPUMIX_HSK_PWRDNACKN,
 		},
 		.pgc   = BIT(IMX8MM_PGC_GPUMIX),
+		.keep_clocks = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_GPU] = {
@@ -697,6 +699,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskack = IMX8MM_VPUMIX_HSK_PWRDNACKN,
 		},
 		.pgc   = BIT(IMX8MM_PGC_VPUMIX),
+		.keep_clocks = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUG1] = {
@@ -743,6 +746,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskack = IMX8MM_DISPMIX_HSK_PWRDNACKN,
 		},
 		.pgc   = BIT(IMX8MM_PGC_DISPMIX),
+		.keep_clocks = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_MIPI] = {
@@ -810,6 +814,7 @@ static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
 			.hskreq = IMX8MN_HSIO_HSK_PWRDNREQN,
 			.hskack = IMX8MN_HSIO_HSK_PWRDNACKN,
 		},
+		.keep_clocks = true,
 	},
 
 	[IMX8MN_POWER_DOMAIN_OTG1] = {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 06/18] soc: imx: gpcv2: keep i.MX8M* bus clocks enabled
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Annotate the domains with bus clocks to keep those clocks enabled
as long as the domain is active.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index c3b1d2580963..c48f37f203ab 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -625,6 +625,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskreq = IMX8MM_HSIO_HSK_PWRDNREQN,
 			.hskack = IMX8MM_HSIO_HSK_PWRDNACKN,
 		},
+		.keep_clocks = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_PCIE] = {
@@ -671,6 +672,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskack = IMX8MM_GPUMIX_HSK_PWRDNACKN,
 		},
 		.pgc   = BIT(IMX8MM_PGC_GPUMIX),
+		.keep_clocks = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_GPU] = {
@@ -697,6 +699,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskack = IMX8MM_VPUMIX_HSK_PWRDNACKN,
 		},
 		.pgc   = BIT(IMX8MM_PGC_VPUMIX),
+		.keep_clocks = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUG1] = {
@@ -743,6 +746,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskack = IMX8MM_DISPMIX_HSK_PWRDNACKN,
 		},
 		.pgc   = BIT(IMX8MM_PGC_DISPMIX),
+		.keep_clocks = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_MIPI] = {
@@ -810,6 +814,7 @@ static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
 			.hskreq = IMX8MN_HSIO_HSK_PWRDNREQN,
 			.hskack = IMX8MN_HSIO_HSK_PWRDNACKN,
 		},
+		.keep_clocks = true,
 	},
 
 	[IMX8MN_POWER_DOMAIN_OTG1] = {
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 07/18] soc: imx: gpcv2: support system suspend/resume
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Our usage of runtime PM to control the hierarchy of power domains is
slightly unusual and means that powering up a domain may fail in early
system resume, as runtime PM is still disallowed at this stage.

However the system suspend/resume path takes care of powering down/up
the power domains in the order defined by the device parent/child and
power-domain provider/consumer hierarachy. So we can just runtime
resume all our power-domain devices to allow the power-up to work
properly in the resume path. System suspend will still disable all
domains as intended.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Acked-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index c48f37f203ab..57ed0a6bfb13 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -947,6 +947,36 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int imx_pgc_domain_suspend(struct device *dev)
+{
+	int ret;
+
+	/*
+	 * This may look strange, but is done so the generic PM_SLEEP code
+	 * can power down our domain and more importantly power it up again
+	 * after resume, without tripping over our usage of runtime PM to
+	 * power up/down the nested domains.
+	 */
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int imx_pgc_domain_resume(struct device *dev)
+{
+	return pm_runtime_put(dev);
+}
+#endif
+
+static const struct dev_pm_ops imx_pgc_domain_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(imx_pgc_domain_suspend, imx_pgc_domain_resume)
+};
+
 static const struct platform_device_id imx_pgc_domain_id[] = {
 	{ "imx-pgc-domain", },
 	{ },
@@ -955,6 +985,7 @@ static const struct platform_device_id imx_pgc_domain_id[] = {
 static struct platform_driver imx_pgc_domain_driver = {
 	.driver = {
 		.name = "imx-pgc",
+		.pm = &imx_pgc_domain_pm_ops,
 	},
 	.probe    = imx_pgc_domain_probe,
 	.remove   = imx_pgc_domain_remove,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 07/18] soc: imx: gpcv2: support system suspend/resume
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Our usage of runtime PM to control the hierarchy of power domains is
slightly unusual and means that powering up a domain may fail in early
system resume, as runtime PM is still disallowed at this stage.

However the system suspend/resume path takes care of powering down/up
the power domains in the order defined by the device parent/child and
power-domain provider/consumer hierarachy. So we can just runtime
resume all our power-domain devices to allow the power-up to work
properly in the resume path. System suspend will still disable all
domains as intended.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Acked-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index c48f37f203ab..57ed0a6bfb13 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -947,6 +947,36 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int imx_pgc_domain_suspend(struct device *dev)
+{
+	int ret;
+
+	/*
+	 * This may look strange, but is done so the generic PM_SLEEP code
+	 * can power down our domain and more importantly power it up again
+	 * after resume, without tripping over our usage of runtime PM to
+	 * power up/down the nested domains.
+	 */
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int imx_pgc_domain_resume(struct device *dev)
+{
+	return pm_runtime_put(dev);
+}
+#endif
+
+static const struct dev_pm_ops imx_pgc_domain_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(imx_pgc_domain_suspend, imx_pgc_domain_resume)
+};
+
 static const struct platform_device_id imx_pgc_domain_id[] = {
 	{ "imx-pgc-domain", },
 	{ },
@@ -955,6 +985,7 @@ static const struct platform_device_id imx_pgc_domain_id[] = {
 static struct platform_driver imx_pgc_domain_driver = {
 	.driver = {
 		.name = "imx-pgc",
+		.pm = &imx_pgc_domain_pm_ops,
 	},
 	.probe    = imx_pgc_domain_probe,
 	.remove   = imx_pgc_domain_remove,
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 08/18] dt-bindings: soc: add binding for i.MX8MM VPU blk-ctrl
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the DT binding for the i.MX8MM VPU blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml      | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml
new file mode 100644
index 000000000000..26487daa64d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8MM VPU blk-ctrl
+
+maintainers:
+  - Lucas Stach <l.stach@pengutronix.de>
+
+description:
+  The i.MX8MM VPU blk-ctrl is a top-level peripheral providing access to
+  the NoC and ensuring proper power sequencing of the VPU peripherals
+  located in the VPU domain of the SoC.
+
+properties:
+  compatible:
+    items:
+      - const: fsl,imx8mm-vpu-blk-ctrl
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  power-domains:
+    minItems: 4
+    maxItems: 4
+
+  power-domain-names:
+    items:
+      - const: bus
+      - const: g1
+      - const: g2
+      - const: h1
+
+  clocks:
+    minItems: 3
+    maxItems: 3
+
+  clock-names:
+    items:
+      - const: g1
+      - const: g2
+      - const: h1
+
+required:
+  - compatible
+  - reg
+  - power-domains
+  - power-domain-names
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8mm-clock.h>
+    #include <dt-bindings/power/imx8mm-power.h>
+
+    vpu_blk_ctrl: blk-ctrl@38330000 {
+      compatible = "fsl,imx8mm-vpu-blk-ctrl", "syscon";
+      reg = <0x38330000 0x100>;
+      power-domains = <&pgc_vpumix>, <&pgc_vpu_g1>,
+                      <&pgc_vpu_g2>, <&pgc_vpu_h1>;
+      power-domain-names = "bus", "g1", "g2", "h1";
+      clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>,
+               <&clk IMX8MM_CLK_VPU_G2_ROOT>,
+               <&clk IMX8MM_CLK_VPU_H1_ROOT>;
+      clock-names = "g1", "g2", "h1";
+      #power-domain-cells = <1>;
+    };
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 08/18] dt-bindings: soc: add binding for i.MX8MM VPU blk-ctrl
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the DT binding for the i.MX8MM VPU blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml      | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml
new file mode 100644
index 000000000000..26487daa64d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mm-vpu-blk-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8MM VPU blk-ctrl
+
+maintainers:
+  - Lucas Stach <l.stach@pengutronix.de>
+
+description:
+  The i.MX8MM VPU blk-ctrl is a top-level peripheral providing access to
+  the NoC and ensuring proper power sequencing of the VPU peripherals
+  located in the VPU domain of the SoC.
+
+properties:
+  compatible:
+    items:
+      - const: fsl,imx8mm-vpu-blk-ctrl
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  power-domains:
+    minItems: 4
+    maxItems: 4
+
+  power-domain-names:
+    items:
+      - const: bus
+      - const: g1
+      - const: g2
+      - const: h1
+
+  clocks:
+    minItems: 3
+    maxItems: 3
+
+  clock-names:
+    items:
+      - const: g1
+      - const: g2
+      - const: h1
+
+required:
+  - compatible
+  - reg
+  - power-domains
+  - power-domain-names
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8mm-clock.h>
+    #include <dt-bindings/power/imx8mm-power.h>
+
+    vpu_blk_ctrl: blk-ctrl@38330000 {
+      compatible = "fsl,imx8mm-vpu-blk-ctrl", "syscon";
+      reg = <0x38330000 0x100>;
+      power-domains = <&pgc_vpumix>, <&pgc_vpu_g1>,
+                      <&pgc_vpu_g2>, <&pgc_vpu_h1>;
+      power-domain-names = "bus", "g1", "g2", "h1";
+      clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>,
+               <&clk IMX8MM_CLK_VPU_G2_ROOT>,
+               <&clk IMX8MM_CLK_VPU_H1_ROOT>;
+      clock-names = "g1", "g2", "h1";
+      #power-domain-cells = <1>;
+    };
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 09/18] dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the defines for the power domains provided by the VPU
blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 include/dt-bindings/power/imx8mm-power.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
index fc9c2e16aadc..38b0a56fd7d0 100644
--- a/include/dt-bindings/power/imx8mm-power.h
+++ b/include/dt-bindings/power/imx8mm-power.h
@@ -19,4 +19,8 @@
 #define IMX8MM_POWER_DOMAIN_DISPMIX	10
 #define IMX8MM_POWER_DOMAIN_MIPI	11
 
+#define IMX8MM_VPUBLK_PD_G1		0
+#define IMX8MM_VPUBLK_PD_G2		1
+#define IMX8MM_VPUBLK_PD_H1		2
+
 #endif
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 09/18] dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the defines for the power domains provided by the VPU
blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 include/dt-bindings/power/imx8mm-power.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
index fc9c2e16aadc..38b0a56fd7d0 100644
--- a/include/dt-bindings/power/imx8mm-power.h
+++ b/include/dt-bindings/power/imx8mm-power.h
@@ -19,4 +19,8 @@
 #define IMX8MM_POWER_DOMAIN_DISPMIX	10
 #define IMX8MM_POWER_DOMAIN_MIPI	11
 
+#define IMX8MM_VPUBLK_PD_G1		0
+#define IMX8MM_VPUBLK_PD_G2		1
+#define IMX8MM_VPUBLK_PD_H1		2
+
 #endif
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 10/18] soc: imx: add i.MX8M blk-ctrl driver
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds a driver for the blk-ctrl blocks found in the i.MX8M* line of
SoCs. The blk-ctrl is a top-level peripheral located in the various *MIX
power domains and interacts with the GPC power controller to provide the
peripherals in the power domain access to the NoC and ensures that those
peripherals are properly reset when their respective power domain is
brought back to life.

Software needs to do different things to make the bus handshake happen
after the the GPC *MIX domain is power up and before it is powered down.
As the requirements are quite different between the various blk-ctrls
there is a callback function provided to hook in the proper sequence.

The peripheral domains are quite uniform, they handle the soft clock
enables and resets in the blk-ctrl address space and sequencing with the
upstream GPC power domains.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
This commit includes the full code to drive the VPUMIX domain on the
i.MX8MM, as the skeleton driver would probably be harder to review
without the context provided by one blk-ctrl implementation. Other
blk-ctrl implementations will follow, based on this overall structure.
---
 drivers/soc/imx/Makefile         |   1 +
 drivers/soc/imx/imx8m-blk-ctrl.c | 455 +++++++++++++++++++++++++++++++
 2 files changed, 456 insertions(+)
 create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c

diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
index 078dc918f4f3..8a707077914c 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -5,3 +5,4 @@ endif
 obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
 obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
 obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
+obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
new file mode 100644
index 000000000000..3dd17b903636
--- /dev/null
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -0,0 +1,455 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/clk.h>
+
+#include <dt-bindings/power/imx8mm-power.h>
+
+#define BLK_SFT_RSTN	0x0
+#define BLK_CLK_EN	0x4
+
+struct imx8m_blk_ctrl_domain;
+
+struct imx8m_blk_ctrl {
+	struct device *dev;
+	struct notifier_block power_nb;
+	struct device *bus_power_dev;
+	struct regmap *regmap;
+	struct imx8m_blk_ctrl_domain *domains;
+	struct genpd_onecell_data onecell_data;
+};
+
+struct imx8m_blk_ctrl_domain_data {
+	const char *name;
+	const char **clk_names;
+	int num_clks;
+	const char *gpc_name;
+	u32 rst_mask;
+	u32 clk_mask;
+};
+
+#define DOMAIN_MAX_CLKS 3
+
+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 device *power_dev;
+	struct imx8m_blk_ctrl *bc;
+};
+
+struct imx8m_blk_ctrl_data {
+	int max_reg;
+	notifier_fn_t power_notifier_fn;
+	const struct imx8m_blk_ctrl_domain_data *domains;
+	int num_domains;
+};
+
+static inline struct imx8m_blk_ctrl_domain *
+to_imx8m_blk_ctrl_domain(struct generic_pm_domain *genpd)
+{
+	return container_of(genpd, struct imx8m_blk_ctrl_domain, genpd);
+}
+
+static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
+{
+	struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
+	const struct imx8m_blk_ctrl_domain_data *data = domain->data;
+	struct imx8m_blk_ctrl *bc = domain->bc;
+	int ret;
+
+	/* make sure bus domain is awake */
+	ret = pm_runtime_get_sync(bc->bus_power_dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(bc->bus_power_dev);
+		dev_err(bc->dev, "failed to power up bus domain\n");
+		return ret;
+	}
+
+	/* put devices into reset */
+	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
+
+	/* enable upstream and blk-ctrl clocks to allow reset to propagate */
+	ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
+	if (ret) {
+		dev_err(bc->dev, "failed to enable clocks\n");
+		goto bus_put;
+	}
+	regmap_set_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
+
+	/* power up upstream GPC domain */
+	ret = pm_runtime_get_sync(domain->power_dev);
+	if (ret < 0) {
+		dev_err(bc->dev, "failed to power up peripheral domain\n");
+		goto clk_disable;
+	}
+
+	/* wait for reset to propagate */
+	udelay(5);
+
+	/* release reset */
+	regmap_set_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
+
+	/* disable upstream clocks */
+	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
+
+	return 0;
+
+clk_disable:
+	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
+bus_put:
+	pm_runtime_put(bc->bus_power_dev);
+
+	return ret;
+}
+
+static int imx8m_blk_ctrl_power_off(struct generic_pm_domain *genpd)
+{
+	struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
+	const struct imx8m_blk_ctrl_domain_data *data = domain->data;
+	struct imx8m_blk_ctrl *bc = domain->bc;
+
+	/* put devices into reset and disable clocks */
+	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
+	regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
+
+	/* power down upstream GPC domain */
+	pm_runtime_put(domain->power_dev);
+
+	/* allow bus domain to suspend */
+	pm_runtime_put(bc->bus_power_dev);
+
+	return 0;
+}
+
+static struct generic_pm_domain *
+imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
+{
+	struct genpd_onecell_data *onecell_data = data;
+	unsigned int index = args->args[0];
+
+	if (args->args_count != 1 ||
+	    index > onecell_data->num_domains)
+		return ERR_PTR(-EINVAL);
+
+	return onecell_data->domains[index];
+}
+
+static struct lock_class_key blk_ctrl_genpd_lock_class;
+
+static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
+{
+	const struct imx8m_blk_ctrl_data *bc_data;
+	struct device *dev = &pdev->dev;
+	struct imx8m_blk_ctrl *bc;
+	void __iomem *base;
+	int i, ret;
+
+	struct regmap_config regmap_config = {
+		.reg_bits	= 32,
+		.val_bits	= 32,
+		.reg_stride	= 4,
+	};
+
+	bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
+	if (!bc)
+		return -ENOMEM;
+
+	bc->dev = dev;
+
+	bc_data = of_device_get_match_data(dev);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap_config.max_register = bc_data->max_reg;
+	bc->regmap = devm_regmap_init_mmio(dev, base, &regmap_config);
+	if (IS_ERR(bc->regmap))
+		return dev_err_probe(dev, PTR_ERR(bc->regmap),
+				     "failed to init regmap \n");
+
+	bc->domains = devm_kcalloc(dev, bc_data->num_domains,
+				    sizeof(struct imx8m_blk_ctrl_domain),
+				    GFP_KERNEL);
+	if (!bc->domains)
+		return -ENOMEM;
+
+	bc->onecell_data.num_domains = bc_data->num_domains;
+	bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
+	bc->onecell_data.domains =
+		devm_kcalloc(dev, bc_data->num_domains,
+			     sizeof(struct generic_pm_domain *), GFP_KERNEL);
+	if (!bc->onecell_data.domains)
+		return -ENOMEM;
+
+	bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
+	if (IS_ERR(bc->bus_power_dev))
+		return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
+				     "failed to attach power domain\n");
+
+	for (i = 0; i < bc_data->num_domains; i++) {
+		const struct imx8m_blk_ctrl_domain_data *data = &bc_data->domains[i];
+		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
+		int j;
+
+		domain->data = data;
+
+		for (j = 0; j < data->num_clks; j++)
+			domain->clks[j].id = data->clk_names[j];
+
+		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to get clock\n");
+			goto cleanup_pds;
+		}
+
+		domain->power_dev =
+			dev_pm_domain_attach_by_name(dev, data->gpc_name);
+		if (IS_ERR(domain->power_dev)) {
+			dev_err_probe(dev, PTR_ERR(domain->power_dev),
+				      "failed to attach power domain\n");
+			ret = PTR_ERR(domain->power_dev);
+			goto cleanup_pds;
+		}
+
+		domain->genpd.name = data->name;
+		domain->genpd.power_on = imx8m_blk_ctrl_power_on;
+		domain->genpd.power_off = imx8m_blk_ctrl_power_off;
+		domain->bc = bc;
+
+		ret = pm_genpd_init(&domain->genpd, NULL, true);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to init power domain\n");
+			dev_pm_domain_detach(domain->power_dev, true);
+			goto cleanup_pds;
+		}
+
+		/*
+		 * We use runtime PM to trigger power on/off of the upstream GPC
+		 * domain, as a strict hierarchical parent/child power domain
+		 * setup doesn't allow us to meet the sequencing requirements.
+		 * This means we have nested locking of genpd locks, without the
+		 * nesting being visible at the genpd level, so we need a
+		 * separate lock class to make lockdep aware of the fact that
+		 * this are separate domain locks that can be nested without a
+		 * self-deadlock.
+		 */
+		lockdep_set_class(&domain->genpd.mlock,
+				  &blk_ctrl_genpd_lock_class);
+
+		bc->onecell_data.domains[i] = &domain->genpd;
+	}
+
+	ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to add power domain provider\n");
+		goto cleanup_pds;
+	}
+
+	bc->power_nb.notifier_call = bc_data->power_notifier_fn;
+	ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to add power notifier\n");
+		goto cleanup_provider;
+	}
+
+	dev_set_drvdata(dev, bc);
+
+	return 0;
+
+cleanup_provider:
+	of_genpd_del_provider(dev->of_node);
+cleanup_pds:
+	for (i--; i >= 0; i--) {
+		pm_genpd_remove(&bc->domains[i].genpd);
+		dev_pm_domain_detach(bc->domains[i].power_dev, true);
+	}
+
+	dev_pm_domain_detach(bc->bus_power_dev, true);
+
+	return ret;
+}
+
+static int imx8m_blk_ctrl_remove(struct platform_device *pdev)
+{
+	struct imx8m_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
+	int i;
+
+	of_genpd_del_provider(pdev->dev.of_node);
+
+	for (i = 0; bc->onecell_data.num_domains; i++) {
+		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
+
+		pm_genpd_remove(&domain->genpd);
+		dev_pm_domain_detach(domain->power_dev, true);
+	}
+
+	dev_pm_genpd_remove_notifier(bc->bus_power_dev);
+
+	dev_pm_domain_detach(bc->bus_power_dev, true);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int imx8m_blk_ctrl_suspend(struct device *dev)
+{
+	struct imx8m_blk_ctrl *bc = dev_get_drvdata(dev);
+	int ret, i;
+
+	/*
+	 * This may look strange, but is done so the generic PM_SLEEP code
+	 * can power down our domains and more importantly power them up again
+	 * after resume, without tripping over our usage of runtime PM to
+	 * control the upstream GPC domains. Things happen in the right order
+	 * in the system suspend/resume paths due to the device parent/child
+	 * hierarchy.
+	 */
+	ret = pm_runtime_get_sync(bc->bus_power_dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(bc->bus_power_dev);
+		return ret;
+	}
+
+	for (i = 0; i < bc->onecell_data.num_domains; i++) {
+		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
+
+		ret = pm_runtime_get_sync(domain->power_dev);
+		if (ret < 0) {
+			pm_runtime_put_noidle(domain->power_dev);
+			goto out_fail;
+		}
+	}
+
+	return 0;
+
+out_fail:
+	for (i--; i >= 0; i--)
+		pm_runtime_put(bc->domains[i].power_dev);
+
+	pm_runtime_put(bc->bus_power_dev);
+
+	return ret;
+}
+
+static int imx8m_blk_ctrl_resume(struct device *dev)
+{
+	struct imx8m_blk_ctrl *bc = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < bc->onecell_data.num_domains; i++)
+		pm_runtime_put(bc->domains[i].power_dev);
+
+	pm_runtime_put(bc->bus_power_dev);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops imx8m_blk_ctrl_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(imx8m_blk_ctrl_suspend, imx8m_blk_ctrl_resume)
+};
+
+static int imx8mm_vpu_power_notifier(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
+						 power_nb);
+
+	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
+		return NOTIFY_OK;
+
+	/*
+	 * The ADB in the VPUMIX domain has no separate reset and clock
+	 * enable bits, but is ungated together with the VPU clocks. To
+	 * allow the handshake with the GPC to progress we put the VPUs
+	 * in reset and ungate the clocks.
+	 */
+	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN,
+			  BIT(0) | BIT(1) | BIT(2));
+	regmap_set_bits(bc->regmap, BLK_CLK_EN,
+			BIT(0) | BIT(1) | BIT(2));
+
+	if (action == GENPD_NOTIFY_ON) {
+		/*
+		 * On power up we have no software backchannel to the GPC to
+		 * wait for the ADB handshake to happen, so we just delay for a
+		 * bit. On power down the GPC driver waits for the handshake.
+		 */
+		udelay(5);
+
+		/* set "fuse" bits to enable the VPUs */
+		regmap_set_bits(bc->regmap, 0x8, 0xffffffff);
+		regmap_set_bits(bc->regmap, 0xc, 0xffffffff);
+		regmap_set_bits(bc->regmap, 0x10, 0xffffffff);
+		regmap_set_bits(bc->regmap, 0x14, 0xffffffff);
+	}
+
+	return NOTIFY_OK;
+}
+
+static const struct imx8m_blk_ctrl_domain_data imx8m_vpu_blk_ctl_domain_data[] = {
+	[IMX8MM_VPUBLK_PD_G1] = {
+		.name = "vpublk-g1",
+		.clk_names = (const char *[]){ "g1", },
+		.num_clks = 1,
+		.gpc_name = "g1",
+		.rst_mask = BIT(1),
+		.clk_mask = BIT(1),
+	},
+	[IMX8MM_VPUBLK_PD_G2] = {
+		.name = "vpublk-g2",
+		.clk_names = (const char *[]){ "g2", },
+		.num_clks = 1,
+		.gpc_name = "g2",
+		.rst_mask = BIT(0),
+		.clk_mask = BIT(0),
+	},
+	[IMX8MM_VPUBLK_PD_H1] = {
+		.name = "vpublk-h1",
+		.clk_names = (const char *[]){ "h1", },
+		.num_clks = 1,
+		.gpc_name = "h1",
+		.rst_mask = BIT(2),
+		.clk_mask = BIT(2),
+	},
+};
+
+static const struct imx8m_blk_ctrl_data imx8m_vpu_blk_ctl_dev_data = {
+	.max_reg = 0x18,
+	.power_notifier_fn = imx8mm_vpu_power_notifier,
+	.domains = imx8m_vpu_blk_ctl_domain_data,
+	.num_domains = ARRAY_SIZE(imx8m_vpu_blk_ctl_domain_data),
+};
+
+static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
+	{
+		.compatible = "fsl,imx8mm-vpu-blk-ctrl",
+		.data = &imx8m_vpu_blk_ctl_dev_data
+	}, {
+		/* Sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
+
+static struct platform_driver imx8m_blk_ctrl_driver = {
+	.probe = imx8m_blk_ctrl_probe,
+	.remove = imx8m_blk_ctrl_remove,
+	.driver = {
+		.name = "imx8m-blk-ctrl",
+		.pm = &imx8m_blk_ctrl_pm_ops,
+		.of_match_table = imx8m_blk_ctrl_of_match,
+	},
+};
+module_platform_driver(imx8m_blk_ctrl_driver);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 10/18] soc: imx: add i.MX8M blk-ctrl driver
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds a driver for the blk-ctrl blocks found in the i.MX8M* line of
SoCs. The blk-ctrl is a top-level peripheral located in the various *MIX
power domains and interacts with the GPC power controller to provide the
peripherals in the power domain access to the NoC and ensures that those
peripherals are properly reset when their respective power domain is
brought back to life.

Software needs to do different things to make the bus handshake happen
after the the GPC *MIX domain is power up and before it is powered down.
As the requirements are quite different between the various blk-ctrls
there is a callback function provided to hook in the proper sequence.

The peripheral domains are quite uniform, they handle the soft clock
enables and resets in the blk-ctrl address space and sequencing with the
upstream GPC power domains.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
This commit includes the full code to drive the VPUMIX domain on the
i.MX8MM, as the skeleton driver would probably be harder to review
without the context provided by one blk-ctrl implementation. Other
blk-ctrl implementations will follow, based on this overall structure.
---
 drivers/soc/imx/Makefile         |   1 +
 drivers/soc/imx/imx8m-blk-ctrl.c | 455 +++++++++++++++++++++++++++++++
 2 files changed, 456 insertions(+)
 create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c

diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
index 078dc918f4f3..8a707077914c 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -5,3 +5,4 @@ endif
 obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
 obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
 obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
+obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
new file mode 100644
index 000000000000..3dd17b903636
--- /dev/null
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -0,0 +1,455 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/clk.h>
+
+#include <dt-bindings/power/imx8mm-power.h>
+
+#define BLK_SFT_RSTN	0x0
+#define BLK_CLK_EN	0x4
+
+struct imx8m_blk_ctrl_domain;
+
+struct imx8m_blk_ctrl {
+	struct device *dev;
+	struct notifier_block power_nb;
+	struct device *bus_power_dev;
+	struct regmap *regmap;
+	struct imx8m_blk_ctrl_domain *domains;
+	struct genpd_onecell_data onecell_data;
+};
+
+struct imx8m_blk_ctrl_domain_data {
+	const char *name;
+	const char **clk_names;
+	int num_clks;
+	const char *gpc_name;
+	u32 rst_mask;
+	u32 clk_mask;
+};
+
+#define DOMAIN_MAX_CLKS 3
+
+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 device *power_dev;
+	struct imx8m_blk_ctrl *bc;
+};
+
+struct imx8m_blk_ctrl_data {
+	int max_reg;
+	notifier_fn_t power_notifier_fn;
+	const struct imx8m_blk_ctrl_domain_data *domains;
+	int num_domains;
+};
+
+static inline struct imx8m_blk_ctrl_domain *
+to_imx8m_blk_ctrl_domain(struct generic_pm_domain *genpd)
+{
+	return container_of(genpd, struct imx8m_blk_ctrl_domain, genpd);
+}
+
+static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
+{
+	struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
+	const struct imx8m_blk_ctrl_domain_data *data = domain->data;
+	struct imx8m_blk_ctrl *bc = domain->bc;
+	int ret;
+
+	/* make sure bus domain is awake */
+	ret = pm_runtime_get_sync(bc->bus_power_dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(bc->bus_power_dev);
+		dev_err(bc->dev, "failed to power up bus domain\n");
+		return ret;
+	}
+
+	/* put devices into reset */
+	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
+
+	/* enable upstream and blk-ctrl clocks to allow reset to propagate */
+	ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
+	if (ret) {
+		dev_err(bc->dev, "failed to enable clocks\n");
+		goto bus_put;
+	}
+	regmap_set_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
+
+	/* power up upstream GPC domain */
+	ret = pm_runtime_get_sync(domain->power_dev);
+	if (ret < 0) {
+		dev_err(bc->dev, "failed to power up peripheral domain\n");
+		goto clk_disable;
+	}
+
+	/* wait for reset to propagate */
+	udelay(5);
+
+	/* release reset */
+	regmap_set_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
+
+	/* disable upstream clocks */
+	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
+
+	return 0;
+
+clk_disable:
+	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
+bus_put:
+	pm_runtime_put(bc->bus_power_dev);
+
+	return ret;
+}
+
+static int imx8m_blk_ctrl_power_off(struct generic_pm_domain *genpd)
+{
+	struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
+	const struct imx8m_blk_ctrl_domain_data *data = domain->data;
+	struct imx8m_blk_ctrl *bc = domain->bc;
+
+	/* put devices into reset and disable clocks */
+	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
+	regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
+
+	/* power down upstream GPC domain */
+	pm_runtime_put(domain->power_dev);
+
+	/* allow bus domain to suspend */
+	pm_runtime_put(bc->bus_power_dev);
+
+	return 0;
+}
+
+static struct generic_pm_domain *
+imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
+{
+	struct genpd_onecell_data *onecell_data = data;
+	unsigned int index = args->args[0];
+
+	if (args->args_count != 1 ||
+	    index > onecell_data->num_domains)
+		return ERR_PTR(-EINVAL);
+
+	return onecell_data->domains[index];
+}
+
+static struct lock_class_key blk_ctrl_genpd_lock_class;
+
+static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
+{
+	const struct imx8m_blk_ctrl_data *bc_data;
+	struct device *dev = &pdev->dev;
+	struct imx8m_blk_ctrl *bc;
+	void __iomem *base;
+	int i, ret;
+
+	struct regmap_config regmap_config = {
+		.reg_bits	= 32,
+		.val_bits	= 32,
+		.reg_stride	= 4,
+	};
+
+	bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
+	if (!bc)
+		return -ENOMEM;
+
+	bc->dev = dev;
+
+	bc_data = of_device_get_match_data(dev);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap_config.max_register = bc_data->max_reg;
+	bc->regmap = devm_regmap_init_mmio(dev, base, &regmap_config);
+	if (IS_ERR(bc->regmap))
+		return dev_err_probe(dev, PTR_ERR(bc->regmap),
+				     "failed to init regmap \n");
+
+	bc->domains = devm_kcalloc(dev, bc_data->num_domains,
+				    sizeof(struct imx8m_blk_ctrl_domain),
+				    GFP_KERNEL);
+	if (!bc->domains)
+		return -ENOMEM;
+
+	bc->onecell_data.num_domains = bc_data->num_domains;
+	bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
+	bc->onecell_data.domains =
+		devm_kcalloc(dev, bc_data->num_domains,
+			     sizeof(struct generic_pm_domain *), GFP_KERNEL);
+	if (!bc->onecell_data.domains)
+		return -ENOMEM;
+
+	bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
+	if (IS_ERR(bc->bus_power_dev))
+		return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
+				     "failed to attach power domain\n");
+
+	for (i = 0; i < bc_data->num_domains; i++) {
+		const struct imx8m_blk_ctrl_domain_data *data = &bc_data->domains[i];
+		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
+		int j;
+
+		domain->data = data;
+
+		for (j = 0; j < data->num_clks; j++)
+			domain->clks[j].id = data->clk_names[j];
+
+		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to get clock\n");
+			goto cleanup_pds;
+		}
+
+		domain->power_dev =
+			dev_pm_domain_attach_by_name(dev, data->gpc_name);
+		if (IS_ERR(domain->power_dev)) {
+			dev_err_probe(dev, PTR_ERR(domain->power_dev),
+				      "failed to attach power domain\n");
+			ret = PTR_ERR(domain->power_dev);
+			goto cleanup_pds;
+		}
+
+		domain->genpd.name = data->name;
+		domain->genpd.power_on = imx8m_blk_ctrl_power_on;
+		domain->genpd.power_off = imx8m_blk_ctrl_power_off;
+		domain->bc = bc;
+
+		ret = pm_genpd_init(&domain->genpd, NULL, true);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to init power domain\n");
+			dev_pm_domain_detach(domain->power_dev, true);
+			goto cleanup_pds;
+		}
+
+		/*
+		 * We use runtime PM to trigger power on/off of the upstream GPC
+		 * domain, as a strict hierarchical parent/child power domain
+		 * setup doesn't allow us to meet the sequencing requirements.
+		 * This means we have nested locking of genpd locks, without the
+		 * nesting being visible at the genpd level, so we need a
+		 * separate lock class to make lockdep aware of the fact that
+		 * this are separate domain locks that can be nested without a
+		 * self-deadlock.
+		 */
+		lockdep_set_class(&domain->genpd.mlock,
+				  &blk_ctrl_genpd_lock_class);
+
+		bc->onecell_data.domains[i] = &domain->genpd;
+	}
+
+	ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to add power domain provider\n");
+		goto cleanup_pds;
+	}
+
+	bc->power_nb.notifier_call = bc_data->power_notifier_fn;
+	ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to add power notifier\n");
+		goto cleanup_provider;
+	}
+
+	dev_set_drvdata(dev, bc);
+
+	return 0;
+
+cleanup_provider:
+	of_genpd_del_provider(dev->of_node);
+cleanup_pds:
+	for (i--; i >= 0; i--) {
+		pm_genpd_remove(&bc->domains[i].genpd);
+		dev_pm_domain_detach(bc->domains[i].power_dev, true);
+	}
+
+	dev_pm_domain_detach(bc->bus_power_dev, true);
+
+	return ret;
+}
+
+static int imx8m_blk_ctrl_remove(struct platform_device *pdev)
+{
+	struct imx8m_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
+	int i;
+
+	of_genpd_del_provider(pdev->dev.of_node);
+
+	for (i = 0; bc->onecell_data.num_domains; i++) {
+		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
+
+		pm_genpd_remove(&domain->genpd);
+		dev_pm_domain_detach(domain->power_dev, true);
+	}
+
+	dev_pm_genpd_remove_notifier(bc->bus_power_dev);
+
+	dev_pm_domain_detach(bc->bus_power_dev, true);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int imx8m_blk_ctrl_suspend(struct device *dev)
+{
+	struct imx8m_blk_ctrl *bc = dev_get_drvdata(dev);
+	int ret, i;
+
+	/*
+	 * This may look strange, but is done so the generic PM_SLEEP code
+	 * can power down our domains and more importantly power them up again
+	 * after resume, without tripping over our usage of runtime PM to
+	 * control the upstream GPC domains. Things happen in the right order
+	 * in the system suspend/resume paths due to the device parent/child
+	 * hierarchy.
+	 */
+	ret = pm_runtime_get_sync(bc->bus_power_dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(bc->bus_power_dev);
+		return ret;
+	}
+
+	for (i = 0; i < bc->onecell_data.num_domains; i++) {
+		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
+
+		ret = pm_runtime_get_sync(domain->power_dev);
+		if (ret < 0) {
+			pm_runtime_put_noidle(domain->power_dev);
+			goto out_fail;
+		}
+	}
+
+	return 0;
+
+out_fail:
+	for (i--; i >= 0; i--)
+		pm_runtime_put(bc->domains[i].power_dev);
+
+	pm_runtime_put(bc->bus_power_dev);
+
+	return ret;
+}
+
+static int imx8m_blk_ctrl_resume(struct device *dev)
+{
+	struct imx8m_blk_ctrl *bc = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < bc->onecell_data.num_domains; i++)
+		pm_runtime_put(bc->domains[i].power_dev);
+
+	pm_runtime_put(bc->bus_power_dev);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops imx8m_blk_ctrl_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(imx8m_blk_ctrl_suspend, imx8m_blk_ctrl_resume)
+};
+
+static int imx8mm_vpu_power_notifier(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
+						 power_nb);
+
+	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
+		return NOTIFY_OK;
+
+	/*
+	 * The ADB in the VPUMIX domain has no separate reset and clock
+	 * enable bits, but is ungated together with the VPU clocks. To
+	 * allow the handshake with the GPC to progress we put the VPUs
+	 * in reset and ungate the clocks.
+	 */
+	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN,
+			  BIT(0) | BIT(1) | BIT(2));
+	regmap_set_bits(bc->regmap, BLK_CLK_EN,
+			BIT(0) | BIT(1) | BIT(2));
+
+	if (action == GENPD_NOTIFY_ON) {
+		/*
+		 * On power up we have no software backchannel to the GPC to
+		 * wait for the ADB handshake to happen, so we just delay for a
+		 * bit. On power down the GPC driver waits for the handshake.
+		 */
+		udelay(5);
+
+		/* set "fuse" bits to enable the VPUs */
+		regmap_set_bits(bc->regmap, 0x8, 0xffffffff);
+		regmap_set_bits(bc->regmap, 0xc, 0xffffffff);
+		regmap_set_bits(bc->regmap, 0x10, 0xffffffff);
+		regmap_set_bits(bc->regmap, 0x14, 0xffffffff);
+	}
+
+	return NOTIFY_OK;
+}
+
+static const struct imx8m_blk_ctrl_domain_data imx8m_vpu_blk_ctl_domain_data[] = {
+	[IMX8MM_VPUBLK_PD_G1] = {
+		.name = "vpublk-g1",
+		.clk_names = (const char *[]){ "g1", },
+		.num_clks = 1,
+		.gpc_name = "g1",
+		.rst_mask = BIT(1),
+		.clk_mask = BIT(1),
+	},
+	[IMX8MM_VPUBLK_PD_G2] = {
+		.name = "vpublk-g2",
+		.clk_names = (const char *[]){ "g2", },
+		.num_clks = 1,
+		.gpc_name = "g2",
+		.rst_mask = BIT(0),
+		.clk_mask = BIT(0),
+	},
+	[IMX8MM_VPUBLK_PD_H1] = {
+		.name = "vpublk-h1",
+		.clk_names = (const char *[]){ "h1", },
+		.num_clks = 1,
+		.gpc_name = "h1",
+		.rst_mask = BIT(2),
+		.clk_mask = BIT(2),
+	},
+};
+
+static const struct imx8m_blk_ctrl_data imx8m_vpu_blk_ctl_dev_data = {
+	.max_reg = 0x18,
+	.power_notifier_fn = imx8mm_vpu_power_notifier,
+	.domains = imx8m_vpu_blk_ctl_domain_data,
+	.num_domains = ARRAY_SIZE(imx8m_vpu_blk_ctl_domain_data),
+};
+
+static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
+	{
+		.compatible = "fsl,imx8mm-vpu-blk-ctrl",
+		.data = &imx8m_vpu_blk_ctl_dev_data
+	}, {
+		/* Sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
+
+static struct platform_driver imx8m_blk_ctrl_driver = {
+	.probe = imx8m_blk_ctrl_probe,
+	.remove = imx8m_blk_ctrl_remove,
+	.driver = {
+		.name = "imx8m-blk-ctrl",
+		.pm = &imx8m_blk_ctrl_pm_ops,
+		.of_match_table = imx8m_blk_ctrl_of_match,
+	},
+};
+module_platform_driver(imx8m_blk_ctrl_driver);
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 11/18] dt-bindings: soc: add binding for i.MX8MM DISP blk-ctrl
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the DT binding for the i.MX8MM VPU blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml     | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml
new file mode 100644
index 000000000000..ecd86cfb3da4
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8MM DISP blk-ctrl
+
+maintainers:
+  - Lucas Stach <l.stach@pengutronix.de>
+
+description:
+  The i.MX8MM DISP blk-ctrl is a top-level peripheral providing access to
+  the NoC and ensuring proper power sequencing of the display and MIPI CSI
+  peripherals located in the DISP domain of the SoC.
+
+properties:
+  compatible:
+    items:
+      - const: fsl,imx8mm-disp-blk-ctrl
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  power-domains:
+    minItems: 5
+    maxItems: 5
+
+  power-domain-names:
+    items:
+      - const: bus
+      - const: csi-bridge
+      - const: lcdif
+      - const: mipi-dsi
+      - const: mipi-csi
+
+  clocks:
+    minItems: 10
+    maxItems: 10
+
+  clock-names:
+    items:
+      - const: csi-bridge-axi
+      - const: csi-bridge-apb
+      - const: csi-bridge-core
+      - const: lcdif-axi
+      - const: lcdif-apb
+      - const: lcdif-pix
+      - const: dsi-pclk
+      - const: dsi-ref
+      - const: csi-aclk
+      - const: csi-pclk
+
+required:
+  - compatible
+  - reg
+  - power-domains
+  - power-domain-names
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8mm-clock.h>
+    #include <dt-bindings/power/imx8mm-power.h>
+
+    disp_blk_ctl: blk_ctrl@32e28000 {
+      compatible = "fsl,imx8mm-disp-blk-ctrl", "syscon";
+      reg = <0x32e28000 0x100>;
+      power-domains = <&pgc_dispmix>, <&pgc_dispmix>, <&pgc_dispmix>,
+                      <&pgc_mipi>, <&pgc_mipi>;
+      power-domain-names = "bus", "csi-bridge", "lcdif",
+                           "mipi-dsi", "mipi-csi";
+      clocks = <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
+               <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+               <&clk IMX8MM_CLK_CSI1_ROOT>,
+               <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
+               <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+               <&clk IMX8MM_CLK_DISP_ROOT>,
+               <&clk IMX8MM_CLK_DSI_CORE>,
+               <&clk IMX8MM_CLK_DSI_PHY_REF>,
+               <&clk IMX8MM_CLK_CSI1_CORE>,
+               <&clk IMX8MM_CLK_CSI1_PHY_REF>;
+       clock-names = "csi-bridge-axi", "csi-bridge-apb", "csi-bridge-core",
+                     "lcdif-axi", "lcdif-apb", "lcdif-pix", "dsi-pclk",
+                     "dsi-ref", "csi-aclk", "csi-pclk";
+       #power-domain-cells = <1>;
+    };
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 11/18] dt-bindings: soc: add binding for i.MX8MM DISP blk-ctrl
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the DT binding for the i.MX8MM VPU blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml     | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml
new file mode 100644
index 000000000000..ecd86cfb3da4
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mm-disp-blk-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8MM DISP blk-ctrl
+
+maintainers:
+  - Lucas Stach <l.stach@pengutronix.de>
+
+description:
+  The i.MX8MM DISP blk-ctrl is a top-level peripheral providing access to
+  the NoC and ensuring proper power sequencing of the display and MIPI CSI
+  peripherals located in the DISP domain of the SoC.
+
+properties:
+  compatible:
+    items:
+      - const: fsl,imx8mm-disp-blk-ctrl
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  power-domains:
+    minItems: 5
+    maxItems: 5
+
+  power-domain-names:
+    items:
+      - const: bus
+      - const: csi-bridge
+      - const: lcdif
+      - const: mipi-dsi
+      - const: mipi-csi
+
+  clocks:
+    minItems: 10
+    maxItems: 10
+
+  clock-names:
+    items:
+      - const: csi-bridge-axi
+      - const: csi-bridge-apb
+      - const: csi-bridge-core
+      - const: lcdif-axi
+      - const: lcdif-apb
+      - const: lcdif-pix
+      - const: dsi-pclk
+      - const: dsi-ref
+      - const: csi-aclk
+      - const: csi-pclk
+
+required:
+  - compatible
+  - reg
+  - power-domains
+  - power-domain-names
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8mm-clock.h>
+    #include <dt-bindings/power/imx8mm-power.h>
+
+    disp_blk_ctl: blk_ctrl@32e28000 {
+      compatible = "fsl,imx8mm-disp-blk-ctrl", "syscon";
+      reg = <0x32e28000 0x100>;
+      power-domains = <&pgc_dispmix>, <&pgc_dispmix>, <&pgc_dispmix>,
+                      <&pgc_mipi>, <&pgc_mipi>;
+      power-domain-names = "bus", "csi-bridge", "lcdif",
+                           "mipi-dsi", "mipi-csi";
+      clocks = <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
+               <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+               <&clk IMX8MM_CLK_CSI1_ROOT>,
+               <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
+               <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+               <&clk IMX8MM_CLK_DISP_ROOT>,
+               <&clk IMX8MM_CLK_DSI_CORE>,
+               <&clk IMX8MM_CLK_DSI_PHY_REF>,
+               <&clk IMX8MM_CLK_CSI1_CORE>,
+               <&clk IMX8MM_CLK_CSI1_PHY_REF>;
+       clock-names = "csi-bridge-axi", "csi-bridge-apb", "csi-bridge-core",
+                     "lcdif-axi", "lcdif-apb", "lcdif-pix", "dsi-pclk",
+                     "dsi-ref", "csi-aclk", "csi-pclk";
+       #power-domain-cells = <1>;
+    };
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 12/18] dt-bindings: power: imx8mm: add defines for DISP blk-ctrl domains
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the defines for the power domains provided by the DISP
blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 include/dt-bindings/power/imx8mm-power.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
index 38b0a56fd7d0..d7f7cdb5200f 100644
--- a/include/dt-bindings/power/imx8mm-power.h
+++ b/include/dt-bindings/power/imx8mm-power.h
@@ -23,4 +23,9 @@
 #define IMX8MM_VPUBLK_PD_G2		1
 #define IMX8MM_VPUBLK_PD_H1		2
 
+#define IMX8MM_DISPBLK_CSI_BRIDGE	0
+#define IMX8MM_DISPBLK_LCDIF		1
+#define IMX8MM_DISPBLK_MIPI_DSI		2
+#define IMX8MM_DISPBLK_MIPI_CSI		3
+
 #endif
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 12/18] dt-bindings: power: imx8mm: add defines for DISP blk-ctrl domains
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the defines for the power domains provided by the DISP
blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 include/dt-bindings/power/imx8mm-power.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
index 38b0a56fd7d0..d7f7cdb5200f 100644
--- a/include/dt-bindings/power/imx8mm-power.h
+++ b/include/dt-bindings/power/imx8mm-power.h
@@ -23,4 +23,9 @@
 #define IMX8MM_VPUBLK_PD_G2		1
 #define IMX8MM_VPUBLK_PD_H1		2
 
+#define IMX8MM_DISPBLK_CSI_BRIDGE	0
+#define IMX8MM_DISPBLK_LCDIF		1
+#define IMX8MM_DISPBLK_MIPI_DSI		2
+#define IMX8MM_DISPBLK_MIPI_CSI		3
+
 #endif
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 13/18] soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the description for the i.MX8MM disp blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/imx8m-blk-ctrl.c | 70 ++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
index 3dd17b903636..32eab800a3c6 100644
--- a/drivers/soc/imx/imx8m-blk-ctrl.c
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -433,11 +433,81 @@ static const struct imx8m_blk_ctrl_data imx8m_vpu_blk_ctl_dev_data = {
 	.num_domains = ARRAY_SIZE(imx8m_vpu_blk_ctl_domain_data),
 };
 
+static int imx8mm_disp_power_notifier(struct notifier_block *nb,
+				      unsigned long action, void *data)
+{
+	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
+						 power_nb);
+
+	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
+		return NOTIFY_OK;
+
+	/* Enable bus clock and deassert bus reset */
+	regmap_set_bits(bc->regmap, BLK_CLK_EN, BIT(12));
+	regmap_set_bits(bc->regmap, BLK_SFT_RSTN, BIT(6));
+
+	/*
+	 * On power up we have no software backchannel to the GPC to
+	 * wait for the ADB handshake to happen, so we just delay for a
+	 * bit. On power down the GPC driver waits for the handshake.
+	 */
+	if (action == GENPD_NOTIFY_ON)
+		udelay(5);
+
+
+	return NOTIFY_OK;
+}
+
+static const struct imx8m_blk_ctrl_domain_data imx8m_disp_blk_ctl_domain_data[] = {
+	[IMX8MM_DISPBLK_CSI_BRIDGE] = {
+		.name = "dispblk-csi-bridge",
+		.clk_names = (const char *[]){ "csi-bridge-axi", "csi-bridge-apb",
+					       "csi-bridge-core", },
+		.num_clks = 3,
+		.gpc_name = "csi-bridge",
+		.rst_mask = BIT(0) | BIT(1) | BIT(2),
+		.clk_mask = BIT(0) | BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5),
+	},
+	[IMX8MM_DISPBLK_LCDIF] = {
+		.name = "dispblk-lcdif",
+		.clk_names = (const char *[]){ "lcdif-axi", "lcdif-apb", "lcdif-pix", },
+		.num_clks = 3,
+		.gpc_name = "lcdif",
+		.clk_mask = BIT(6) | BIT(7),
+	},
+	[IMX8MM_DISPBLK_MIPI_DSI] = {
+		.name = "dispblk-mipi-dsi",
+		.clk_names = (const char *[]){ "dsi-pclk", "dsi-ref", },
+		.num_clks = 2,
+		.gpc_name = "mipi-dsi",
+		.rst_mask = BIT(5),
+		.clk_mask = BIT(8) | BIT(9),
+	},
+	[IMX8MM_DISPBLK_MIPI_CSI] = {
+		.name = "dispblk-mipi-csi",
+		.clk_names = (const char *[]){ "csi-aclk", "csi-pclk" },
+		.num_clks = 2,
+		.gpc_name = "mipi-csi",
+		.rst_mask = BIT(3) | BIT(4),
+		.clk_mask = BIT(10) | BIT(11),
+	},
+};
+
+static const struct imx8m_blk_ctrl_data imx8m_disp_blk_ctl_dev_data = {
+	.max_reg = 0x2c,
+	.power_notifier_fn = imx8mm_disp_power_notifier,
+	.domains = imx8m_disp_blk_ctl_domain_data,
+	.num_domains = ARRAY_SIZE(imx8m_disp_blk_ctl_domain_data),
+};
+
 static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
 	{
 		.compatible = "fsl,imx8mm-vpu-blk-ctrl",
 		.data = &imx8m_vpu_blk_ctl_dev_data
 	}, {
+		.compatible = "fsl,imx8mm-disp-blk-ctrl",
+		.data = &imx8m_disp_blk_ctl_dev_data
+	} ,{
 		/* Sentinel */
 	}
 };
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 13/18] soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

This adds the description for the i.MX8MM disp blk-ctrl.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/imx8m-blk-ctrl.c | 70 ++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
index 3dd17b903636..32eab800a3c6 100644
--- a/drivers/soc/imx/imx8m-blk-ctrl.c
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -433,11 +433,81 @@ static const struct imx8m_blk_ctrl_data imx8m_vpu_blk_ctl_dev_data = {
 	.num_domains = ARRAY_SIZE(imx8m_vpu_blk_ctl_domain_data),
 };
 
+static int imx8mm_disp_power_notifier(struct notifier_block *nb,
+				      unsigned long action, void *data)
+{
+	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
+						 power_nb);
+
+	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
+		return NOTIFY_OK;
+
+	/* Enable bus clock and deassert bus reset */
+	regmap_set_bits(bc->regmap, BLK_CLK_EN, BIT(12));
+	regmap_set_bits(bc->regmap, BLK_SFT_RSTN, BIT(6));
+
+	/*
+	 * On power up we have no software backchannel to the GPC to
+	 * wait for the ADB handshake to happen, so we just delay for a
+	 * bit. On power down the GPC driver waits for the handshake.
+	 */
+	if (action == GENPD_NOTIFY_ON)
+		udelay(5);
+
+
+	return NOTIFY_OK;
+}
+
+static const struct imx8m_blk_ctrl_domain_data imx8m_disp_blk_ctl_domain_data[] = {
+	[IMX8MM_DISPBLK_CSI_BRIDGE] = {
+		.name = "dispblk-csi-bridge",
+		.clk_names = (const char *[]){ "csi-bridge-axi", "csi-bridge-apb",
+					       "csi-bridge-core", },
+		.num_clks = 3,
+		.gpc_name = "csi-bridge",
+		.rst_mask = BIT(0) | BIT(1) | BIT(2),
+		.clk_mask = BIT(0) | BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5),
+	},
+	[IMX8MM_DISPBLK_LCDIF] = {
+		.name = "dispblk-lcdif",
+		.clk_names = (const char *[]){ "lcdif-axi", "lcdif-apb", "lcdif-pix", },
+		.num_clks = 3,
+		.gpc_name = "lcdif",
+		.clk_mask = BIT(6) | BIT(7),
+	},
+	[IMX8MM_DISPBLK_MIPI_DSI] = {
+		.name = "dispblk-mipi-dsi",
+		.clk_names = (const char *[]){ "dsi-pclk", "dsi-ref", },
+		.num_clks = 2,
+		.gpc_name = "mipi-dsi",
+		.rst_mask = BIT(5),
+		.clk_mask = BIT(8) | BIT(9),
+	},
+	[IMX8MM_DISPBLK_MIPI_CSI] = {
+		.name = "dispblk-mipi-csi",
+		.clk_names = (const char *[]){ "csi-aclk", "csi-pclk" },
+		.num_clks = 2,
+		.gpc_name = "mipi-csi",
+		.rst_mask = BIT(3) | BIT(4),
+		.clk_mask = BIT(10) | BIT(11),
+	},
+};
+
+static const struct imx8m_blk_ctrl_data imx8m_disp_blk_ctl_dev_data = {
+	.max_reg = 0x2c,
+	.power_notifier_fn = imx8mm_disp_power_notifier,
+	.domains = imx8m_disp_blk_ctl_domain_data,
+	.num_domains = ARRAY_SIZE(imx8m_disp_blk_ctl_domain_data),
+};
+
 static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
 	{
 		.compatible = "fsl,imx8mm-vpu-blk-ctrl",
 		.data = &imx8m_vpu_blk_ctl_dev_data
 	}, {
+		.compatible = "fsl,imx8mm-disp-blk-ctrl",
+		.data = &imx8m_disp_blk_ctl_dev_data
+	} ,{
 		/* Sentinel */
 	}
 };
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 14/18] arm64: dts: imx8mm: add GPC node
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Add the DT node for the GPC, including all the PGC power domains,
some of them are not fully functional yet, as they require interaction
with the blk-ctrls to properly power up/down the peripherals.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 107 ++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index e7648c3b8390..3922f26f8fd4 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -7,6 +7,8 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/power/imx8mm-power.h>
+#include <dt-bindings/reset/imx8mq-reset.h>
 #include <dt-bindings/thermal/thermal.h>
 
 #include "imx8mm-pinfunc.h"
@@ -609,6 +611,111 @@ src: reset-controller@30390000 {
 				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
 				#reset-cells = <1>;
 			};
+
+			gpc: gpc@303a0000 {
+				compatible = "fsl,imx8mm-gpc";
+				reg = <0x303a0000 0x10000>;
+				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <3>;
+
+				pgc {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					pgc_hsiomix: power-domain@0 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
+						clocks = <&clk IMX8MM_CLK_USB_BUS>;
+						assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>;
+						assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
+					};
+
+					pgc_pcie: power-domain@1 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_PCIE>;
+						power-domains = <&pgc_hsiomix>;
+						clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>;
+					};
+
+					pgc_otg1: power-domain@2 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_OTG1>;
+						power-domains = <&pgc_hsiomix>;
+					};
+
+					pgc_otg2: power-domain@3 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_OTG2>;
+						power-domains = <&pgc_hsiomix>;
+					};
+
+					pgc_gpumix: power-domain@4 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_GPUMIX>;
+						clocks = <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+							 <&clk IMX8MM_CLK_GPU_AHB>;
+						assigned-clocks = <&clk IMX8MM_CLK_GPU_AXI>,
+								  <&clk IMX8MM_CLK_GPU_AHB>;
+						assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>,
+									 <&clk IMX8MM_SYS_PLL1_800M>;
+						assigned-clock-rates = <800000000>, <400000000>;
+					};
+
+					pgc_gpu: power-domain@5 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_GPU>;
+						clocks = <&clk IMX8MM_CLK_GPU_AHB>,
+							 <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+							 <&clk IMX8MM_CLK_GPU2D_ROOT>,
+							 <&clk IMX8MM_CLK_GPU3D_ROOT>;
+						resets = <&src IMX8MQ_RESET_GPU_RESET>;
+						power-domains = <&pgc_gpumix>;
+					};
+
+					pgc_vpumix: power-domain@6 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_VPUMIX>;
+						clocks = <&clk IMX8MM_CLK_VPU_DEC_ROOT>;
+						assigned-clocks = <&clk IMX8MM_CLK_VPU_BUS>;
+						assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>;
+						resets = <&src IMX8MQ_RESET_VPU_RESET>;
+					};
+
+					pgc_vpu_g1: power-domain@7 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_VPUG1>;
+					};
+
+					pgc_vpu_g2: power-domain@8 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_VPUG2>;
+					};
+
+					pgc_vpu_h1: power-domain@9 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_VPUH1>;
+					};
+
+					pgc_dispmix: power-domain@10 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
+						clocks = <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+							 <&clk IMX8MM_CLK_DISP_AXI_ROOT>;
+						assigned-clocks = <&clk IMX8MM_CLK_DISP_AXI>,
+								  <&clk IMX8MM_CLK_DISP_APB>;
+						assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_1000M>,
+									 <&clk IMX8MM_SYS_PLL1_800M>;
+						assigned-clock-rates = <500000000>, <200000000>;
+					};
+
+					pgc_mipi: power-domain@11 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_MIPI>;
+					};
+				};
+			};
 		};
 
 		aips2: bus@30400000 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 14/18] arm64: dts: imx8mm: add GPC node
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Add the DT node for the GPC, including all the PGC power domains,
some of them are not fully functional yet, as they require interaction
with the blk-ctrls to properly power up/down the peripherals.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 107 ++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index e7648c3b8390..3922f26f8fd4 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -7,6 +7,8 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/power/imx8mm-power.h>
+#include <dt-bindings/reset/imx8mq-reset.h>
 #include <dt-bindings/thermal/thermal.h>
 
 #include "imx8mm-pinfunc.h"
@@ -609,6 +611,111 @@ src: reset-controller@30390000 {
 				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
 				#reset-cells = <1>;
 			};
+
+			gpc: gpc@303a0000 {
+				compatible = "fsl,imx8mm-gpc";
+				reg = <0x303a0000 0x10000>;
+				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <3>;
+
+				pgc {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					pgc_hsiomix: power-domain@0 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
+						clocks = <&clk IMX8MM_CLK_USB_BUS>;
+						assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>;
+						assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
+					};
+
+					pgc_pcie: power-domain@1 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_PCIE>;
+						power-domains = <&pgc_hsiomix>;
+						clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>;
+					};
+
+					pgc_otg1: power-domain@2 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_OTG1>;
+						power-domains = <&pgc_hsiomix>;
+					};
+
+					pgc_otg2: power-domain@3 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_OTG2>;
+						power-domains = <&pgc_hsiomix>;
+					};
+
+					pgc_gpumix: power-domain@4 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_GPUMIX>;
+						clocks = <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+							 <&clk IMX8MM_CLK_GPU_AHB>;
+						assigned-clocks = <&clk IMX8MM_CLK_GPU_AXI>,
+								  <&clk IMX8MM_CLK_GPU_AHB>;
+						assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>,
+									 <&clk IMX8MM_SYS_PLL1_800M>;
+						assigned-clock-rates = <800000000>, <400000000>;
+					};
+
+					pgc_gpu: power-domain@5 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_GPU>;
+						clocks = <&clk IMX8MM_CLK_GPU_AHB>,
+							 <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+							 <&clk IMX8MM_CLK_GPU2D_ROOT>,
+							 <&clk IMX8MM_CLK_GPU3D_ROOT>;
+						resets = <&src IMX8MQ_RESET_GPU_RESET>;
+						power-domains = <&pgc_gpumix>;
+					};
+
+					pgc_vpumix: power-domain@6 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_VPUMIX>;
+						clocks = <&clk IMX8MM_CLK_VPU_DEC_ROOT>;
+						assigned-clocks = <&clk IMX8MM_CLK_VPU_BUS>;
+						assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>;
+						resets = <&src IMX8MQ_RESET_VPU_RESET>;
+					};
+
+					pgc_vpu_g1: power-domain@7 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_VPUG1>;
+					};
+
+					pgc_vpu_g2: power-domain@8 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_VPUG2>;
+					};
+
+					pgc_vpu_h1: power-domain@9 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_VPUH1>;
+					};
+
+					pgc_dispmix: power-domain@10 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
+						clocks = <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+							 <&clk IMX8MM_CLK_DISP_AXI_ROOT>;
+						assigned-clocks = <&clk IMX8MM_CLK_DISP_AXI>,
+								  <&clk IMX8MM_CLK_DISP_APB>;
+						assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_1000M>,
+									 <&clk IMX8MM_SYS_PLL1_800M>;
+						assigned-clock-rates = <500000000>, <200000000>;
+					};
+
+					pgc_mipi: power-domain@11 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_MIPI>;
+					};
+				};
+			};
 		};
 
 		aips2: bus@30400000 {
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 15/18] arm64: dts: imx8mm: put USB controllers into power-domains
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Now that we have support for the power domain controller on the i.MX8MM
we can put the USB controllers in their respective power domains to allow
them to power down the PHY when possible.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 3922f26f8fd4..772f60bfe31e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1077,6 +1077,7 @@ usbotg1: usb@32e40000 {
 				assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
 				fsl,usbphy = <&usbphynop1>;
 				fsl,usbmisc = <&usbmisc1 0>;
+				power-domains = <&pgc_otg1>;
 				status = "disabled";
 			};
 
@@ -1096,6 +1097,7 @@ usbotg2: usb@32e50000 {
 				assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
 				fsl,usbphy = <&usbphynop2>;
 				fsl,usbmisc = <&usbmisc2 0>;
+				power-domains = <&pgc_otg2>;
 				status = "disabled";
 			};
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 15/18] arm64: dts: imx8mm: put USB controllers into power-domains
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Now that we have support for the power domain controller on the i.MX8MM
we can put the USB controllers in their respective power domains to allow
them to power down the PHY when possible.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 3922f26f8fd4..772f60bfe31e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1077,6 +1077,7 @@ usbotg1: usb@32e40000 {
 				assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
 				fsl,usbphy = <&usbphynop1>;
 				fsl,usbmisc = <&usbmisc1 0>;
+				power-domains = <&pgc_otg1>;
 				status = "disabled";
 			};
 
@@ -1096,6 +1097,7 @@ usbotg2: usb@32e50000 {
 				assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
 				fsl,usbphy = <&usbphynop2>;
 				fsl,usbmisc = <&usbmisc2 0>;
+				power-domains = <&pgc_otg2>;
 				status = "disabled";
 			};
 
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 16/18] arm64: dts: imx8mm: Add GPU nodes for 2D and 3D core
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

From: Frieder Schrempf <frieder.schrempf@kontron.de>

According to the documents, the i.MX8M-Mini features a GC320 and a
GCNanoUltra GPU core. Etnaviv detects them as:

	etnaviv-gpu 38000000.gpu: model: GC600, revision: 4653
	etnaviv-gpu 38008000.gpu: model: GC520, revision: 5341

This seems to work fine more or less without any changes to the HWDB,
which still might be needed in the future to correct some features,
etc.

[lst]: Added power domains and switched clock assignments to the
       new clock defines used for the composite clocks, instead of
       relying on the backwards compat defines.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 772f60bfe31e..87198f7ea6df 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1138,6 +1138,37 @@ gpmi: nand-controller@33002000{
 			status = "disabled";
 		};
 
+		gpu_3d: gpu@38000000 {
+			compatible = "vivante,gc";
+			reg = <0x38000000 0x8000>;
+			interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX8MM_CLK_GPU_AHB>,
+				 <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+				 <&clk IMX8MM_CLK_GPU3D_ROOT>,
+				 <&clk IMX8MM_CLK_GPU3D_ROOT>;
+			clock-names = "reg", "bus", "core", "shader";
+			assigned-clocks = <&clk IMX8MM_CLK_GPU3D_CORE>,
+					  <&clk IMX8MM_GPU_PLL_OUT>;
+			assigned-clock-parents = <&clk IMX8MM_GPU_PLL_OUT>;
+			assigned-clock-rates = <0>, <1000000000>;
+			power-domains = <&pgc_gpu>;
+		};
+
+		gpu_2d: gpu@38008000 {
+			compatible = "vivante,gc";
+			reg = <0x38008000 0x8000>;
+			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX8MM_CLK_GPU_AHB>,
+				 <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+				 <&clk IMX8MM_CLK_GPU2D_ROOT>;
+			clock-names = "reg", "bus", "core";
+			assigned-clocks = <&clk IMX8MM_CLK_GPU2D_CORE>,
+					  <&clk IMX8MM_GPU_PLL_OUT>;
+			assigned-clock-parents = <&clk IMX8MM_GPU_PLL_OUT>;
+			assigned-clock-rates = <0>, <1000000000>;
+			power-domains = <&pgc_gpu>;
+		};
+
 		gic: interrupt-controller@38800000 {
 			compatible = "arm,gic-v3";
 			reg = <0x38800000 0x10000>, /* GIC Dist */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 16/18] arm64: dts: imx8mm: Add GPU nodes for 2D and 3D core
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

From: Frieder Schrempf <frieder.schrempf@kontron.de>

According to the documents, the i.MX8M-Mini features a GC320 and a
GCNanoUltra GPU core. Etnaviv detects them as:

	etnaviv-gpu 38000000.gpu: model: GC600, revision: 4653
	etnaviv-gpu 38008000.gpu: model: GC520, revision: 5341

This seems to work fine more or less without any changes to the HWDB,
which still might be needed in the future to correct some features,
etc.

[lst]: Added power domains and switched clock assignments to the
       new clock defines used for the composite clocks, instead of
       relying on the backwards compat defines.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 772f60bfe31e..87198f7ea6df 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1138,6 +1138,37 @@ gpmi: nand-controller@33002000{
 			status = "disabled";
 		};
 
+		gpu_3d: gpu@38000000 {
+			compatible = "vivante,gc";
+			reg = <0x38000000 0x8000>;
+			interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX8MM_CLK_GPU_AHB>,
+				 <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+				 <&clk IMX8MM_CLK_GPU3D_ROOT>,
+				 <&clk IMX8MM_CLK_GPU3D_ROOT>;
+			clock-names = "reg", "bus", "core", "shader";
+			assigned-clocks = <&clk IMX8MM_CLK_GPU3D_CORE>,
+					  <&clk IMX8MM_GPU_PLL_OUT>;
+			assigned-clock-parents = <&clk IMX8MM_GPU_PLL_OUT>;
+			assigned-clock-rates = <0>, <1000000000>;
+			power-domains = <&pgc_gpu>;
+		};
+
+		gpu_2d: gpu@38008000 {
+			compatible = "vivante,gc";
+			reg = <0x38008000 0x8000>;
+			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX8MM_CLK_GPU_AHB>,
+				 <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+				 <&clk IMX8MM_CLK_GPU2D_ROOT>;
+			clock-names = "reg", "bus", "core";
+			assigned-clocks = <&clk IMX8MM_CLK_GPU2D_CORE>,
+					  <&clk IMX8MM_GPU_PLL_OUT>;
+			assigned-clock-parents = <&clk IMX8MM_GPU_PLL_OUT>;
+			assigned-clock-rates = <0>, <1000000000>;
+			power-domains = <&pgc_gpu>;
+		};
+
 		gic: interrupt-controller@38800000 {
 			compatible = "arm,gic-v3";
 			reg = <0x38800000 0x10000>, /* GIC Dist */
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 17/18] arm64: dts: imx8mm: add VPU blk-ctrl
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Add the DT node for the VPU blk-ctrl. With this in place the
VPU power domains are fully functional.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 87198f7ea6df..1452d60a0524 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1169,6 +1169,19 @@ gpu_2d: gpu@38008000 {
 			power-domains = <&pgc_gpu>;
 		};
 
+		vpu_blk_ctrl: blk-ctrl@38330000 {
+			compatible = "fsl,imx8mm-vpu-blk-ctrl", "syscon";
+			reg = <0x38330000 0x100>;
+			power-domains = <&pgc_vpumix>, <&pgc_vpu_g1>,
+					<&pgc_vpu_g2>, <&pgc_vpu_h1>;
+			power-domain-names = "bus", "g1", "g2", "h1";
+			clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>,
+				 <&clk IMX8MM_CLK_VPU_G2_ROOT>,
+				 <&clk IMX8MM_CLK_VPU_H1_ROOT>;
+			clock-names = "g1", "g2", "h1";
+			#power-domain-cells = <1>;
+		};
+
 		gic: interrupt-controller@38800000 {
 			compatible = "arm,gic-v3";
 			reg = <0x38800000 0x10000>, /* GIC Dist */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 17/18] arm64: dts: imx8mm: add VPU blk-ctrl
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Add the DT node for the VPU blk-ctrl. With this in place the
VPU power domains are fully functional.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 87198f7ea6df..1452d60a0524 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1169,6 +1169,19 @@ gpu_2d: gpu@38008000 {
 			power-domains = <&pgc_gpu>;
 		};
 
+		vpu_blk_ctrl: blk-ctrl@38330000 {
+			compatible = "fsl,imx8mm-vpu-blk-ctrl", "syscon";
+			reg = <0x38330000 0x100>;
+			power-domains = <&pgc_vpumix>, <&pgc_vpu_g1>,
+					<&pgc_vpu_g2>, <&pgc_vpu_h1>;
+			power-domain-names = "bus", "g1", "g2", "h1";
+			clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>,
+				 <&clk IMX8MM_CLK_VPU_G2_ROOT>,
+				 <&clk IMX8MM_CLK_VPU_H1_ROOT>;
+			clock-names = "g1", "g2", "h1";
+			#power-domain-cells = <1>;
+		};
+
 		gic: interrupt-controller@38800000 {
 			compatible = "arm,gic-v3";
 			reg = <0x38800000 0x10000>, /* GIC Dist */
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 18/18] arm64: dts: imx8mm: add DISP blk-ctrl
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 18:43   ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Add the DT node for the DISP blk-ctrl. With this in place the
display/mipi power domains are fully functional.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 1452d60a0524..3bec6b8d52a0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1067,6 +1067,33 @@ aips4: bus@32c00000 {
 			#size-cells = <1>;
 			ranges = <0x32c00000 0x32c00000 0x400000>;
 
+			disp_blk_ctrl: blk-ctrl@32e28000 {
+				compatible = "fsl,imx8mm-disp-blk-ctrl", "syscon";
+				reg = <0x32e28000 0x100>;
+				power-domains = <&pgc_dispmix>, <&pgc_dispmix>,
+						<&pgc_dispmix>, <&pgc_mipi>,
+						<&pgc_mipi>;
+				power-domain-names = "bus", "csi-bridge",
+						     "lcdif", "mipi-dsi",
+						     "mipi-csi";
+				clocks = <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
+					 <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+					 <&clk IMX8MM_CLK_CSI1_ROOT>,
+					 <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
+					 <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+					 <&clk IMX8MM_CLK_DISP_ROOT>,
+					 <&clk IMX8MM_CLK_DSI_CORE>,
+					 <&clk IMX8MM_CLK_DSI_PHY_REF>,
+					 <&clk IMX8MM_CLK_CSI1_CORE>,
+					 <&clk IMX8MM_CLK_CSI1_PHY_REF>;
+				clock-names = "csi-bridge-axi","csi-bridge-apb",
+					      "csi-bridge-core", "lcdif-axi",
+					      "lcdif-apb", "lcdif-pix",
+					      "dsi-pclk", "dsi-ref",
+					      "csi-aclk", "csi-pclk";
+				#power-domain-cells = <1>;
+			};
+
 			usbotg1: usb@32e40000 {
 				compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb";
 				reg = <0x32e40000 0x200>;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 50+ messages in thread
* [PATCH v3 18/18] arm64: dts: imx8mm: add DISP blk-ctrl
@ 2021-09-06 18:43   ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-06 18:43 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey, devicetree,
	linux-arm-kernel, kernel, patchwork-lst

Add the DT node for the DISP blk-ctrl. With this in place the
display/mipi power domains are fully functional.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 1452d60a0524..3bec6b8d52a0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1067,6 +1067,33 @@ aips4: bus@32c00000 {
 			#size-cells = <1>;
 			ranges = <0x32c00000 0x32c00000 0x400000>;
 
+			disp_blk_ctrl: blk-ctrl@32e28000 {
+				compatible = "fsl,imx8mm-disp-blk-ctrl", "syscon";
+				reg = <0x32e28000 0x100>;
+				power-domains = <&pgc_dispmix>, <&pgc_dispmix>,
+						<&pgc_dispmix>, <&pgc_mipi>,
+						<&pgc_mipi>;
+				power-domain-names = "bus", "csi-bridge",
+						     "lcdif", "mipi-dsi",
+						     "mipi-csi";
+				clocks = <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
+					 <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+					 <&clk IMX8MM_CLK_CSI1_ROOT>,
+					 <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
+					 <&clk IMX8MM_CLK_DISP_APB_ROOT>,
+					 <&clk IMX8MM_CLK_DISP_ROOT>,
+					 <&clk IMX8MM_CLK_DSI_CORE>,
+					 <&clk IMX8MM_CLK_DSI_PHY_REF>,
+					 <&clk IMX8MM_CLK_CSI1_CORE>,
+					 <&clk IMX8MM_CLK_CSI1_PHY_REF>;
+				clock-names = "csi-bridge-axi","csi-bridge-apb",
+					      "csi-bridge-core", "lcdif-axi",
+					      "lcdif-apb", "lcdif-pix",
+					      "dsi-pclk", "dsi-ref",
+					      "csi-aclk", "csi-pclk";
+				#power-domain-cells = <1>;
+			};
+
 			usbotg1: usb@32e40000 {
 				compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb";
 				reg = <0x32e40000 0x200>;
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 00/18] i.MX8MM GPC improvements and BLK_CTRL driver
  2021-09-06 18:43 ` Lucas Stach
@ 2021-09-06 19:01   ` Fabio Estevam
  -1 siblings, 0 replies; 50+ messages in thread
From: Fabio Estevam @ 2021-09-06 19:01 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Shawn Guo, Rob Herring, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Sascha Hauer, patchwork-lst

Hi Lucas,

On Mon, Sep 6, 2021 at 3:43 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi all,
>
> third revision of the GPC improvements and BLK_CTRL driver to enable
> all power domains on the i.MX8MM. Frieder is still hunting some
> sporadic and hard to reproduce issues and I currently don't have much
> time to help with that. Meanwhile here's a new revision with the
> obvious issues (DT binding validation fail) fixed to allow people to
> review the code, so we hopefully don't miss the next merge window again.
>
> I guess it would even be possible to merge the series with the
> sporadic issues still present, as it seems a lot of people are waiting
> for this to land, as it's blocking lots of other work.

Agreed, thanks!

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 00/18] i.MX8MM GPC improvements and BLK_CTRL driver
@ 2021-09-06 19:01   ` Fabio Estevam
  0 siblings, 0 replies; 50+ messages in thread
From: Fabio Estevam @ 2021-09-06 19:01 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Shawn Guo, Rob Herring, NXP Linux Team, Adam Ford,
	Frieder Schrempf, Marek Vasut, Tim Harvey,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Sascha Hauer, patchwork-lst

Hi Lucas,

On Mon, Sep 6, 2021 at 3:43 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi all,
>
> third revision of the GPC improvements and BLK_CTRL driver to enable
> all power domains on the i.MX8MM. Frieder is still hunting some
> sporadic and hard to reproduce issues and I currently don't have much
> time to help with that. Meanwhile here's a new revision with the
> obvious issues (DT binding validation fail) fixed to allow people to
> review the code, so we hopefully don't miss the next merge window again.
>
> I guess it would even be possible to merge the series with the
> sporadic issues still present, as it seems a lot of people are waiting
> for this to land, as it's blocking lots of other work.

Agreed, thanks!

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

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 00/18] i.MX8MM GPC improvements and BLK_CTRL driver
  2021-09-06 19:01   ` Fabio Estevam
@ 2021-09-07  6:52     ` Frieder Schrempf
  -1 siblings, 0 replies; 50+ messages in thread
From: Frieder Schrempf @ 2021-09-07  6:52 UTC (permalink / raw)
  To: Fabio Estevam, Lucas Stach
  Cc: Shawn Guo, Rob Herring, NXP Linux Team, Adam Ford, Marek Vasut,
	Tim Harvey,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Sascha Hauer, patchwork-lst

On 06.09.21 21:01, Fabio Estevam wrote:
> Hi Lucas,
> 
> On Mon, Sep 6, 2021 at 3:43 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>>
>> Hi all,
>>
>> third revision of the GPC improvements and BLK_CTRL driver to enable
>> all power domains on the i.MX8MM. Frieder is still hunting some
>> sporadic and hard to reproduce issues and I currently don't have much
>> time to help with that. Meanwhile here's a new revision with the
>> obvious issues (DT binding validation fail) fixed to allow people to
>> review the code, so we hopefully don't miss the next merge window again.
>>
>> I guess it would even be possible to merge the series with the
>> sporadic issues still present, as it seems a lot of people are waiting
>> for this to land, as it's blocking lots of other work.
> 
> Agreed, thanks!

Also agree. I think we should try to merge this even if the
rare/sporadic issues are not fixed.

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 00/18] i.MX8MM GPC improvements and BLK_CTRL driver
@ 2021-09-07  6:52     ` Frieder Schrempf
  0 siblings, 0 replies; 50+ messages in thread
From: Frieder Schrempf @ 2021-09-07  6:52 UTC (permalink / raw)
  To: Fabio Estevam, Lucas Stach
  Cc: Shawn Guo, Rob Herring, NXP Linux Team, Adam Ford, Marek Vasut,
	Tim Harvey,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Sascha Hauer, patchwork-lst

On 06.09.21 21:01, Fabio Estevam wrote:
> Hi Lucas,
> 
> On Mon, Sep 6, 2021 at 3:43 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>>
>> Hi all,
>>
>> third revision of the GPC improvements and BLK_CTRL driver to enable
>> all power domains on the i.MX8MM. Frieder is still hunting some
>> sporadic and hard to reproduce issues and I currently don't have much
>> time to help with that. Meanwhile here's a new revision with the
>> obvious issues (DT binding validation fail) fixed to allow people to
>> review the code, so we hopefully don't miss the next merge window again.
>>
>> I guess it would even be possible to merge the series with the
>> sporadic issues still present, as it seems a lot of people are waiting
>> for this to land, as it's blocking lots of other work.
> 
> Agreed, thanks!

Also agree. I think we should try to merge this even if the
rare/sporadic issues are not fixed.

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

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 09/18] dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains
  2021-09-06 18:43   ` Lucas Stach
@ 2021-09-07  7:06     ` Frieder Schrempf
  -1 siblings, 0 replies; 50+ messages in thread
From: Frieder Schrempf @ 2021-09-07  7:06 UTC (permalink / raw)
  To: Lucas Stach, Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Marek Vasut, Tim Harvey, devicetree, linux-arm-kernel, kernel,
	patchwork-lst

On 06.09.21 20:43, Lucas Stach wrote:
> This adds the defines for the power domains provided by the VPU
> blk-ctrl.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  include/dt-bindings/power/imx8mm-power.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
> index fc9c2e16aadc..38b0a56fd7d0 100644
> --- a/include/dt-bindings/power/imx8mm-power.h
> +++ b/include/dt-bindings/power/imx8mm-power.h
> @@ -19,4 +19,8 @@
>  #define IMX8MM_POWER_DOMAIN_DISPMIX	10
>  #define IMX8MM_POWER_DOMAIN_MIPI	11
>  
> +#define IMX8MM_VPUBLK_PD_G1		0
> +#define IMX8MM_VPUBLK_PD_G2		1
> +#define IMX8MM_VPUBLK_PD_H1		2

I wonder how these defines should be named. Here you have a
IMX8MM_*BLK_PD_*, but for the DISP BLK-CTRL you only have IMX8MM_*BLK_*
(without the PD).

Also in Peng's last approach for this we already have defines for this
[1] that have been acked by Rob and might be useful as a reference or
you could even pick up Peng's patch and by that carry over the existing
R-b/A-b tags.

Though, in general I like the shorter versions you provided better.

[1]
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210629072941.7980-2-peng.fan@oss.nxp.com/

> +
>  #endif
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 09/18] dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains
@ 2021-09-07  7:06     ` Frieder Schrempf
  0 siblings, 0 replies; 50+ messages in thread
From: Frieder Schrempf @ 2021-09-07  7:06 UTC (permalink / raw)
  To: Lucas Stach, Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Marek Vasut, Tim Harvey, devicetree, linux-arm-kernel, kernel,
	patchwork-lst

On 06.09.21 20:43, Lucas Stach wrote:
> This adds the defines for the power domains provided by the VPU
> blk-ctrl.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  include/dt-bindings/power/imx8mm-power.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
> index fc9c2e16aadc..38b0a56fd7d0 100644
> --- a/include/dt-bindings/power/imx8mm-power.h
> +++ b/include/dt-bindings/power/imx8mm-power.h
> @@ -19,4 +19,8 @@
>  #define IMX8MM_POWER_DOMAIN_DISPMIX	10
>  #define IMX8MM_POWER_DOMAIN_MIPI	11
>  
> +#define IMX8MM_VPUBLK_PD_G1		0
> +#define IMX8MM_VPUBLK_PD_G2		1
> +#define IMX8MM_VPUBLK_PD_H1		2

I wonder how these defines should be named. Here you have a
IMX8MM_*BLK_PD_*, but for the DISP BLK-CTRL you only have IMX8MM_*BLK_*
(without the PD).

Also in Peng's last approach for this we already have defines for this
[1] that have been acked by Rob and might be useful as a reference or
you could even pick up Peng's patch and by that carry over the existing
R-b/A-b tags.

Though, in general I like the shorter versions you provided better.

[1]
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210629072941.7980-2-peng.fan@oss.nxp.com/

> +
>  #endif
> 

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

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 10/18] soc: imx: add i.MX8M blk-ctrl driver
  2021-09-06 18:43   ` Lucas Stach
@ 2021-09-07  7:28     ` Frieder Schrempf
  -1 siblings, 0 replies; 50+ messages in thread
From: Frieder Schrempf @ 2021-09-07  7:28 UTC (permalink / raw)
  To: Lucas Stach, Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Marek Vasut, Tim Harvey, devicetree, linux-arm-kernel, kernel,
	patchwork-lst

On 06.09.21 20:43, Lucas Stach wrote:
> This adds a driver for the blk-ctrl blocks found in the i.MX8M* line of
> SoCs. The blk-ctrl is a top-level peripheral located in the various *MIX
> power domains and interacts with the GPC power controller to provide the
> peripherals in the power domain access to the NoC and ensures that those
> peripherals are properly reset when their respective power domain is
> brought back to life.
> 
> Software needs to do different things to make the bus handshake happen
> after the the GPC *MIX domain is power up and before it is powered down.

        ^ double "the"             ^ powered

> As the requirements are quite different between the various blk-ctrls
> there is a callback function provided to hook in the proper sequence.
> 
> The peripheral domains are quite uniform, they handle the soft clock
> enables and resets in the blk-ctrl address space and sequencing with the
> upstream GPC power domains.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

I don't understand all of the logic behind this implementation, but
looking at the code I don't see any obvious problems. So FWIW:

Acked-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
> This commit includes the full code to drive the VPUMIX domain on the
> i.MX8MM, as the skeleton driver would probably be harder to review
> without the context provided by one blk-ctrl implementation. Other
> blk-ctrl implementations will follow, based on this overall structure.
> ---
>  drivers/soc/imx/Makefile         |   1 +
>  drivers/soc/imx/imx8m-blk-ctrl.c | 455 +++++++++++++++++++++++++++++++
>  2 files changed, 456 insertions(+)
>  create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c
> 
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> index 078dc918f4f3..8a707077914c 100644
> --- a/drivers/soc/imx/Makefile
> +++ b/drivers/soc/imx/Makefile
> @@ -5,3 +5,4 @@ endif
>  obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
>  obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
>  obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> +obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> new file mode 100644
> index 000000000000..3dd17b903636
> --- /dev/null
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +
> +#include <dt-bindings/power/imx8mm-power.h>
> +
> +#define BLK_SFT_RSTN	0x0
> +#define BLK_CLK_EN	0x4
> +
> +struct imx8m_blk_ctrl_domain;
> +
> +struct imx8m_blk_ctrl {
> +	struct device *dev;
> +	struct notifier_block power_nb;
> +	struct device *bus_power_dev;
> +	struct regmap *regmap;
> +	struct imx8m_blk_ctrl_domain *domains;
> +	struct genpd_onecell_data onecell_data;
> +};
> +
> +struct imx8m_blk_ctrl_domain_data {
> +	const char *name;
> +	const char **clk_names;
> +	int num_clks;
> +	const char *gpc_name;
> +	u32 rst_mask;
> +	u32 clk_mask;
> +};
> +
> +#define DOMAIN_MAX_CLKS 3
> +
> +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 device *power_dev;
> +	struct imx8m_blk_ctrl *bc;
> +};
> +
> +struct imx8m_blk_ctrl_data {
> +	int max_reg;
> +	notifier_fn_t power_notifier_fn;
> +	const struct imx8m_blk_ctrl_domain_data *domains;
> +	int num_domains;
> +};
> +
> +static inline struct imx8m_blk_ctrl_domain *
> +to_imx8m_blk_ctrl_domain(struct generic_pm_domain *genpd)
> +{
> +	return container_of(genpd, struct imx8m_blk_ctrl_domain, genpd);
> +}
> +
> +static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> +{
> +	struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
> +	const struct imx8m_blk_ctrl_domain_data *data = domain->data;
> +	struct imx8m_blk_ctrl *bc = domain->bc;
> +	int ret;
> +
> +	/* make sure bus domain is awake */
> +	ret = pm_runtime_get_sync(bc->bus_power_dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(bc->bus_power_dev);
> +		dev_err(bc->dev, "failed to power up bus domain\n");
> +		return ret;
> +	}
> +
> +	/* put devices into reset */
> +	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> +
> +	/* enable upstream and blk-ctrl clocks to allow reset to propagate */
> +	ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
> +	if (ret) {
> +		dev_err(bc->dev, "failed to enable clocks\n");
> +		goto bus_put;
> +	}
> +	regmap_set_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
> +
> +	/* power up upstream GPC domain */
> +	ret = pm_runtime_get_sync(domain->power_dev);
> +	if (ret < 0) {
> +		dev_err(bc->dev, "failed to power up peripheral domain\n");
> +		goto clk_disable;
> +	}
> +
> +	/* wait for reset to propagate */
> +	udelay(5);
> +
> +	/* release reset */
> +	regmap_set_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> +
> +	/* disable upstream clocks */
> +	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> +
> +	return 0;
> +
> +clk_disable:
> +	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> +bus_put:
> +	pm_runtime_put(bc->bus_power_dev);
> +
> +	return ret;
> +}
> +
> +static int imx8m_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> +{
> +	struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
> +	const struct imx8m_blk_ctrl_domain_data *data = domain->data;
> +	struct imx8m_blk_ctrl *bc = domain->bc;
> +
> +	/* put devices into reset and disable clocks */
> +	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> +	regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
> +
> +	/* power down upstream GPC domain */
> +	pm_runtime_put(domain->power_dev);
> +
> +	/* allow bus domain to suspend */
> +	pm_runtime_put(bc->bus_power_dev);
> +
> +	return 0;
> +}
> +
> +static struct generic_pm_domain *
> +imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
> +{
> +	struct genpd_onecell_data *onecell_data = data;
> +	unsigned int index = args->args[0];
> +
> +	if (args->args_count != 1 ||
> +	    index > onecell_data->num_domains)
> +		return ERR_PTR(-EINVAL);
> +
> +	return onecell_data->domains[index];
> +}
> +
> +static struct lock_class_key blk_ctrl_genpd_lock_class;
> +
> +static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
> +{
> +	const struct imx8m_blk_ctrl_data *bc_data;
> +	struct device *dev = &pdev->dev;
> +	struct imx8m_blk_ctrl *bc;
> +	void __iomem *base;
> +	int i, ret;
> +
> +	struct regmap_config regmap_config = {
> +		.reg_bits	= 32,
> +		.val_bits	= 32,
> +		.reg_stride	= 4,
> +	};
> +
> +	bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
> +	if (!bc)
> +		return -ENOMEM;
> +
> +	bc->dev = dev;
> +
> +	bc_data = of_device_get_match_data(dev);
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap_config.max_register = bc_data->max_reg;
> +	bc->regmap = devm_regmap_init_mmio(dev, base, &regmap_config);
> +	if (IS_ERR(bc->regmap))
> +		return dev_err_probe(dev, PTR_ERR(bc->regmap),
> +				     "failed to init regmap \n");
> +
> +	bc->domains = devm_kcalloc(dev, bc_data->num_domains,
> +				    sizeof(struct imx8m_blk_ctrl_domain),
> +				    GFP_KERNEL);
> +	if (!bc->domains)
> +		return -ENOMEM;
> +
> +	bc->onecell_data.num_domains = bc_data->num_domains;
> +	bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
> +	bc->onecell_data.domains =
> +		devm_kcalloc(dev, bc_data->num_domains,
> +			     sizeof(struct generic_pm_domain *), GFP_KERNEL);
> +	if (!bc->onecell_data.domains)
> +		return -ENOMEM;
> +
> +	bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
> +	if (IS_ERR(bc->bus_power_dev))
> +		return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> +				     "failed to attach power domain\n");
> +
> +	for (i = 0; i < bc_data->num_domains; i++) {
> +		const struct imx8m_blk_ctrl_domain_data *data = &bc_data->domains[i];
> +		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
> +		int j;
> +
> +		domain->data = data;
> +
> +		for (j = 0; j < data->num_clks; j++)
> +			domain->clks[j].id = data->clk_names[j];
> +
> +		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
> +		if (ret) {
> +			dev_err_probe(dev, ret, "failed to get clock\n");
> +			goto cleanup_pds;
> +		}
> +
> +		domain->power_dev =
> +			dev_pm_domain_attach_by_name(dev, data->gpc_name);
> +		if (IS_ERR(domain->power_dev)) {
> +			dev_err_probe(dev, PTR_ERR(domain->power_dev),
> +				      "failed to attach power domain\n");
> +			ret = PTR_ERR(domain->power_dev);
> +			goto cleanup_pds;
> +		}
> +
> +		domain->genpd.name = data->name;
> +		domain->genpd.power_on = imx8m_blk_ctrl_power_on;
> +		domain->genpd.power_off = imx8m_blk_ctrl_power_off;
> +		domain->bc = bc;
> +
> +		ret = pm_genpd_init(&domain->genpd, NULL, true);
> +		if (ret) {
> +			dev_err_probe(dev, ret, "failed to init power domain\n");
> +			dev_pm_domain_detach(domain->power_dev, true);
> +			goto cleanup_pds;
> +		}
> +
> +		/*
> +		 * We use runtime PM to trigger power on/off of the upstream GPC
> +		 * domain, as a strict hierarchical parent/child power domain
> +		 * setup doesn't allow us to meet the sequencing requirements.
> +		 * This means we have nested locking of genpd locks, without the
> +		 * nesting being visible at the genpd level, so we need a
> +		 * separate lock class to make lockdep aware of the fact that
> +		 * this are separate domain locks that can be nested without a
> +		 * self-deadlock.
> +		 */
> +		lockdep_set_class(&domain->genpd.mlock,
> +				  &blk_ctrl_genpd_lock_class);
> +
> +		bc->onecell_data.domains[i] = &domain->genpd;
> +	}
> +
> +	ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to add power domain provider\n");
> +		goto cleanup_pds;
> +	}
> +
> +	bc->power_nb.notifier_call = bc_data->power_notifier_fn;
> +	ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to add power notifier\n");
> +		goto cleanup_provider;
> +	}
> +
> +	dev_set_drvdata(dev, bc);
> +
> +	return 0;
> +
> +cleanup_provider:
> +	of_genpd_del_provider(dev->of_node);
> +cleanup_pds:
> +	for (i--; i >= 0; i--) {
> +		pm_genpd_remove(&bc->domains[i].genpd);
> +		dev_pm_domain_detach(bc->domains[i].power_dev, true);
> +	}
> +
> +	dev_pm_domain_detach(bc->bus_power_dev, true);
> +
> +	return ret;
> +}
> +
> +static int imx8m_blk_ctrl_remove(struct platform_device *pdev)
> +{
> +	struct imx8m_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
> +	int i;
> +
> +	of_genpd_del_provider(pdev->dev.of_node);
> +
> +	for (i = 0; bc->onecell_data.num_domains; i++) {
> +		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
> +
> +		pm_genpd_remove(&domain->genpd);
> +		dev_pm_domain_detach(domain->power_dev, true);
> +	}
> +
> +	dev_pm_genpd_remove_notifier(bc->bus_power_dev);
> +
> +	dev_pm_domain_detach(bc->bus_power_dev, true);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int imx8m_blk_ctrl_suspend(struct device *dev)
> +{
> +	struct imx8m_blk_ctrl *bc = dev_get_drvdata(dev);
> +	int ret, i;
> +
> +	/*
> +	 * This may look strange, but is done so the generic PM_SLEEP code
> +	 * can power down our domains and more importantly power them up again
> +	 * after resume, without tripping over our usage of runtime PM to
> +	 * control the upstream GPC domains. Things happen in the right order
> +	 * in the system suspend/resume paths due to the device parent/child
> +	 * hierarchy.
> +	 */
> +	ret = pm_runtime_get_sync(bc->bus_power_dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(bc->bus_power_dev);
> +		return ret;
> +	}
> +
> +	for (i = 0; i < bc->onecell_data.num_domains; i++) {
> +		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
> +
> +		ret = pm_runtime_get_sync(domain->power_dev);
> +		if (ret < 0) {
> +			pm_runtime_put_noidle(domain->power_dev);
> +			goto out_fail;
> +		}
> +	}
> +
> +	return 0;
> +
> +out_fail:
> +	for (i--; i >= 0; i--)
> +		pm_runtime_put(bc->domains[i].power_dev);
> +
> +	pm_runtime_put(bc->bus_power_dev);
> +
> +	return ret;
> +}
> +
> +static int imx8m_blk_ctrl_resume(struct device *dev)
> +{
> +	struct imx8m_blk_ctrl *bc = dev_get_drvdata(dev);
> +	int i;
> +
> +	for (i = 0; i < bc->onecell_data.num_domains; i++)
> +		pm_runtime_put(bc->domains[i].power_dev);
> +
> +	pm_runtime_put(bc->bus_power_dev);
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops imx8m_blk_ctrl_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(imx8m_blk_ctrl_suspend, imx8m_blk_ctrl_resume)
> +};
> +
> +static int imx8mm_vpu_power_notifier(struct notifier_block *nb,
> +				     unsigned long action, void *data)
> +{
> +	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
> +						 power_nb);
> +
> +	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
> +		return NOTIFY_OK;
> +
> +	/*
> +	 * The ADB in the VPUMIX domain has no separate reset and clock
> +	 * enable bits, but is ungated together with the VPU clocks. To
> +	 * allow the handshake with the GPC to progress we put the VPUs
> +	 * in reset and ungate the clocks.
> +	 */
> +	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN,
> +			  BIT(0) | BIT(1) | BIT(2));
> +	regmap_set_bits(bc->regmap, BLK_CLK_EN,
> +			BIT(0) | BIT(1) | BIT(2));
> +
> +	if (action == GENPD_NOTIFY_ON) {
> +		/*
> +		 * On power up we have no software backchannel to the GPC to
> +		 * wait for the ADB handshake to happen, so we just delay for a
> +		 * bit. On power down the GPC driver waits for the handshake.
> +		 */
> +		udelay(5);
> +
> +		/* set "fuse" bits to enable the VPUs */
> +		regmap_set_bits(bc->regmap, 0x8, 0xffffffff);
> +		regmap_set_bits(bc->regmap, 0xc, 0xffffffff);
> +		regmap_set_bits(bc->regmap, 0x10, 0xffffffff);
> +		regmap_set_bits(bc->regmap, 0x14, 0xffffffff);
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static const struct imx8m_blk_ctrl_domain_data imx8m_vpu_blk_ctl_domain_data[] = {
> +	[IMX8MM_VPUBLK_PD_G1] = {
> +		.name = "vpublk-g1",
> +		.clk_names = (const char *[]){ "g1", },
> +		.num_clks = 1,
> +		.gpc_name = "g1",
> +		.rst_mask = BIT(1),
> +		.clk_mask = BIT(1),
> +	},
> +	[IMX8MM_VPUBLK_PD_G2] = {
> +		.name = "vpublk-g2",
> +		.clk_names = (const char *[]){ "g2", },
> +		.num_clks = 1,
> +		.gpc_name = "g2",
> +		.rst_mask = BIT(0),
> +		.clk_mask = BIT(0),
> +	},
> +	[IMX8MM_VPUBLK_PD_H1] = {
> +		.name = "vpublk-h1",
> +		.clk_names = (const char *[]){ "h1", },
> +		.num_clks = 1,
> +		.gpc_name = "h1",
> +		.rst_mask = BIT(2),
> +		.clk_mask = BIT(2),
> +	},
> +};
> +
> +static const struct imx8m_blk_ctrl_data imx8m_vpu_blk_ctl_dev_data = {
> +	.max_reg = 0x18,
> +	.power_notifier_fn = imx8mm_vpu_power_notifier,
> +	.domains = imx8m_vpu_blk_ctl_domain_data,
> +	.num_domains = ARRAY_SIZE(imx8m_vpu_blk_ctl_domain_data),
> +};
> +
> +static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
> +	{
> +		.compatible = "fsl,imx8mm-vpu-blk-ctrl",
> +		.data = &imx8m_vpu_blk_ctl_dev_data
> +	}, {
> +		/* Sentinel */
> +	}
> +};
> +MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
> +
> +static struct platform_driver imx8m_blk_ctrl_driver = {
> +	.probe = imx8m_blk_ctrl_probe,
> +	.remove = imx8m_blk_ctrl_remove,
> +	.driver = {
> +		.name = "imx8m-blk-ctrl",
> +		.pm = &imx8m_blk_ctrl_pm_ops,
> +		.of_match_table = imx8m_blk_ctrl_of_match,
> +	},
> +};
> +module_platform_driver(imx8m_blk_ctrl_driver);
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 10/18] soc: imx: add i.MX8M blk-ctrl driver
@ 2021-09-07  7:28     ` Frieder Schrempf
  0 siblings, 0 replies; 50+ messages in thread
From: Frieder Schrempf @ 2021-09-07  7:28 UTC (permalink / raw)
  To: Lucas Stach, Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Marek Vasut, Tim Harvey, devicetree, linux-arm-kernel, kernel,
	patchwork-lst

On 06.09.21 20:43, Lucas Stach wrote:
> This adds a driver for the blk-ctrl blocks found in the i.MX8M* line of
> SoCs. The blk-ctrl is a top-level peripheral located in the various *MIX
> power domains and interacts with the GPC power controller to provide the
> peripherals in the power domain access to the NoC and ensures that those
> peripherals are properly reset when their respective power domain is
> brought back to life.
> 
> Software needs to do different things to make the bus handshake happen
> after the the GPC *MIX domain is power up and before it is powered down.

        ^ double "the"             ^ powered

> As the requirements are quite different between the various blk-ctrls
> there is a callback function provided to hook in the proper sequence.
> 
> The peripheral domains are quite uniform, they handle the soft clock
> enables and resets in the blk-ctrl address space and sequencing with the
> upstream GPC power domains.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

I don't understand all of the logic behind this implementation, but
looking at the code I don't see any obvious problems. So FWIW:

Acked-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
> This commit includes the full code to drive the VPUMIX domain on the
> i.MX8MM, as the skeleton driver would probably be harder to review
> without the context provided by one blk-ctrl implementation. Other
> blk-ctrl implementations will follow, based on this overall structure.
> ---
>  drivers/soc/imx/Makefile         |   1 +
>  drivers/soc/imx/imx8m-blk-ctrl.c | 455 +++++++++++++++++++++++++++++++
>  2 files changed, 456 insertions(+)
>  create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c
> 
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> index 078dc918f4f3..8a707077914c 100644
> --- a/drivers/soc/imx/Makefile
> +++ b/drivers/soc/imx/Makefile
> @@ -5,3 +5,4 @@ endif
>  obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
>  obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
>  obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> +obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> new file mode 100644
> index 000000000000..3dd17b903636
> --- /dev/null
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +
> +#include <dt-bindings/power/imx8mm-power.h>
> +
> +#define BLK_SFT_RSTN	0x0
> +#define BLK_CLK_EN	0x4
> +
> +struct imx8m_blk_ctrl_domain;
> +
> +struct imx8m_blk_ctrl {
> +	struct device *dev;
> +	struct notifier_block power_nb;
> +	struct device *bus_power_dev;
> +	struct regmap *regmap;
> +	struct imx8m_blk_ctrl_domain *domains;
> +	struct genpd_onecell_data onecell_data;
> +};
> +
> +struct imx8m_blk_ctrl_domain_data {
> +	const char *name;
> +	const char **clk_names;
> +	int num_clks;
> +	const char *gpc_name;
> +	u32 rst_mask;
> +	u32 clk_mask;
> +};
> +
> +#define DOMAIN_MAX_CLKS 3
> +
> +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 device *power_dev;
> +	struct imx8m_blk_ctrl *bc;
> +};
> +
> +struct imx8m_blk_ctrl_data {
> +	int max_reg;
> +	notifier_fn_t power_notifier_fn;
> +	const struct imx8m_blk_ctrl_domain_data *domains;
> +	int num_domains;
> +};
> +
> +static inline struct imx8m_blk_ctrl_domain *
> +to_imx8m_blk_ctrl_domain(struct generic_pm_domain *genpd)
> +{
> +	return container_of(genpd, struct imx8m_blk_ctrl_domain, genpd);
> +}
> +
> +static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> +{
> +	struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
> +	const struct imx8m_blk_ctrl_domain_data *data = domain->data;
> +	struct imx8m_blk_ctrl *bc = domain->bc;
> +	int ret;
> +
> +	/* make sure bus domain is awake */
> +	ret = pm_runtime_get_sync(bc->bus_power_dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(bc->bus_power_dev);
> +		dev_err(bc->dev, "failed to power up bus domain\n");
> +		return ret;
> +	}
> +
> +	/* put devices into reset */
> +	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> +
> +	/* enable upstream and blk-ctrl clocks to allow reset to propagate */
> +	ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
> +	if (ret) {
> +		dev_err(bc->dev, "failed to enable clocks\n");
> +		goto bus_put;
> +	}
> +	regmap_set_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
> +
> +	/* power up upstream GPC domain */
> +	ret = pm_runtime_get_sync(domain->power_dev);
> +	if (ret < 0) {
> +		dev_err(bc->dev, "failed to power up peripheral domain\n");
> +		goto clk_disable;
> +	}
> +
> +	/* wait for reset to propagate */
> +	udelay(5);
> +
> +	/* release reset */
> +	regmap_set_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> +
> +	/* disable upstream clocks */
> +	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> +
> +	return 0;
> +
> +clk_disable:
> +	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
> +bus_put:
> +	pm_runtime_put(bc->bus_power_dev);
> +
> +	return ret;
> +}
> +
> +static int imx8m_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> +{
> +	struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
> +	const struct imx8m_blk_ctrl_domain_data *data = domain->data;
> +	struct imx8m_blk_ctrl *bc = domain->bc;
> +
> +	/* put devices into reset and disable clocks */
> +	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask);
> +	regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask);
> +
> +	/* power down upstream GPC domain */
> +	pm_runtime_put(domain->power_dev);
> +
> +	/* allow bus domain to suspend */
> +	pm_runtime_put(bc->bus_power_dev);
> +
> +	return 0;
> +}
> +
> +static struct generic_pm_domain *
> +imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
> +{
> +	struct genpd_onecell_data *onecell_data = data;
> +	unsigned int index = args->args[0];
> +
> +	if (args->args_count != 1 ||
> +	    index > onecell_data->num_domains)
> +		return ERR_PTR(-EINVAL);
> +
> +	return onecell_data->domains[index];
> +}
> +
> +static struct lock_class_key blk_ctrl_genpd_lock_class;
> +
> +static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
> +{
> +	const struct imx8m_blk_ctrl_data *bc_data;
> +	struct device *dev = &pdev->dev;
> +	struct imx8m_blk_ctrl *bc;
> +	void __iomem *base;
> +	int i, ret;
> +
> +	struct regmap_config regmap_config = {
> +		.reg_bits	= 32,
> +		.val_bits	= 32,
> +		.reg_stride	= 4,
> +	};
> +
> +	bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
> +	if (!bc)
> +		return -ENOMEM;
> +
> +	bc->dev = dev;
> +
> +	bc_data = of_device_get_match_data(dev);
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap_config.max_register = bc_data->max_reg;
> +	bc->regmap = devm_regmap_init_mmio(dev, base, &regmap_config);
> +	if (IS_ERR(bc->regmap))
> +		return dev_err_probe(dev, PTR_ERR(bc->regmap),
> +				     "failed to init regmap \n");
> +
> +	bc->domains = devm_kcalloc(dev, bc_data->num_domains,
> +				    sizeof(struct imx8m_blk_ctrl_domain),
> +				    GFP_KERNEL);
> +	if (!bc->domains)
> +		return -ENOMEM;
> +
> +	bc->onecell_data.num_domains = bc_data->num_domains;
> +	bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
> +	bc->onecell_data.domains =
> +		devm_kcalloc(dev, bc_data->num_domains,
> +			     sizeof(struct generic_pm_domain *), GFP_KERNEL);
> +	if (!bc->onecell_data.domains)
> +		return -ENOMEM;
> +
> +	bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
> +	if (IS_ERR(bc->bus_power_dev))
> +		return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> +				     "failed to attach power domain\n");
> +
> +	for (i = 0; i < bc_data->num_domains; i++) {
> +		const struct imx8m_blk_ctrl_domain_data *data = &bc_data->domains[i];
> +		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
> +		int j;
> +
> +		domain->data = data;
> +
> +		for (j = 0; j < data->num_clks; j++)
> +			domain->clks[j].id = data->clk_names[j];
> +
> +		ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
> +		if (ret) {
> +			dev_err_probe(dev, ret, "failed to get clock\n");
> +			goto cleanup_pds;
> +		}
> +
> +		domain->power_dev =
> +			dev_pm_domain_attach_by_name(dev, data->gpc_name);
> +		if (IS_ERR(domain->power_dev)) {
> +			dev_err_probe(dev, PTR_ERR(domain->power_dev),
> +				      "failed to attach power domain\n");
> +			ret = PTR_ERR(domain->power_dev);
> +			goto cleanup_pds;
> +		}
> +
> +		domain->genpd.name = data->name;
> +		domain->genpd.power_on = imx8m_blk_ctrl_power_on;
> +		domain->genpd.power_off = imx8m_blk_ctrl_power_off;
> +		domain->bc = bc;
> +
> +		ret = pm_genpd_init(&domain->genpd, NULL, true);
> +		if (ret) {
> +			dev_err_probe(dev, ret, "failed to init power domain\n");
> +			dev_pm_domain_detach(domain->power_dev, true);
> +			goto cleanup_pds;
> +		}
> +
> +		/*
> +		 * We use runtime PM to trigger power on/off of the upstream GPC
> +		 * domain, as a strict hierarchical parent/child power domain
> +		 * setup doesn't allow us to meet the sequencing requirements.
> +		 * This means we have nested locking of genpd locks, without the
> +		 * nesting being visible at the genpd level, so we need a
> +		 * separate lock class to make lockdep aware of the fact that
> +		 * this are separate domain locks that can be nested without a
> +		 * self-deadlock.
> +		 */
> +		lockdep_set_class(&domain->genpd.mlock,
> +				  &blk_ctrl_genpd_lock_class);
> +
> +		bc->onecell_data.domains[i] = &domain->genpd;
> +	}
> +
> +	ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to add power domain provider\n");
> +		goto cleanup_pds;
> +	}
> +
> +	bc->power_nb.notifier_call = bc_data->power_notifier_fn;
> +	ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to add power notifier\n");
> +		goto cleanup_provider;
> +	}
> +
> +	dev_set_drvdata(dev, bc);
> +
> +	return 0;
> +
> +cleanup_provider:
> +	of_genpd_del_provider(dev->of_node);
> +cleanup_pds:
> +	for (i--; i >= 0; i--) {
> +		pm_genpd_remove(&bc->domains[i].genpd);
> +		dev_pm_domain_detach(bc->domains[i].power_dev, true);
> +	}
> +
> +	dev_pm_domain_detach(bc->bus_power_dev, true);
> +
> +	return ret;
> +}
> +
> +static int imx8m_blk_ctrl_remove(struct platform_device *pdev)
> +{
> +	struct imx8m_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
> +	int i;
> +
> +	of_genpd_del_provider(pdev->dev.of_node);
> +
> +	for (i = 0; bc->onecell_data.num_domains; i++) {
> +		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
> +
> +		pm_genpd_remove(&domain->genpd);
> +		dev_pm_domain_detach(domain->power_dev, true);
> +	}
> +
> +	dev_pm_genpd_remove_notifier(bc->bus_power_dev);
> +
> +	dev_pm_domain_detach(bc->bus_power_dev, true);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int imx8m_blk_ctrl_suspend(struct device *dev)
> +{
> +	struct imx8m_blk_ctrl *bc = dev_get_drvdata(dev);
> +	int ret, i;
> +
> +	/*
> +	 * This may look strange, but is done so the generic PM_SLEEP code
> +	 * can power down our domains and more importantly power them up again
> +	 * after resume, without tripping over our usage of runtime PM to
> +	 * control the upstream GPC domains. Things happen in the right order
> +	 * in the system suspend/resume paths due to the device parent/child
> +	 * hierarchy.
> +	 */
> +	ret = pm_runtime_get_sync(bc->bus_power_dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(bc->bus_power_dev);
> +		return ret;
> +	}
> +
> +	for (i = 0; i < bc->onecell_data.num_domains; i++) {
> +		struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
> +
> +		ret = pm_runtime_get_sync(domain->power_dev);
> +		if (ret < 0) {
> +			pm_runtime_put_noidle(domain->power_dev);
> +			goto out_fail;
> +		}
> +	}
> +
> +	return 0;
> +
> +out_fail:
> +	for (i--; i >= 0; i--)
> +		pm_runtime_put(bc->domains[i].power_dev);
> +
> +	pm_runtime_put(bc->bus_power_dev);
> +
> +	return ret;
> +}
> +
> +static int imx8m_blk_ctrl_resume(struct device *dev)
> +{
> +	struct imx8m_blk_ctrl *bc = dev_get_drvdata(dev);
> +	int i;
> +
> +	for (i = 0; i < bc->onecell_data.num_domains; i++)
> +		pm_runtime_put(bc->domains[i].power_dev);
> +
> +	pm_runtime_put(bc->bus_power_dev);
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops imx8m_blk_ctrl_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(imx8m_blk_ctrl_suspend, imx8m_blk_ctrl_resume)
> +};
> +
> +static int imx8mm_vpu_power_notifier(struct notifier_block *nb,
> +				     unsigned long action, void *data)
> +{
> +	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
> +						 power_nb);
> +
> +	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
> +		return NOTIFY_OK;
> +
> +	/*
> +	 * The ADB in the VPUMIX domain has no separate reset and clock
> +	 * enable bits, but is ungated together with the VPU clocks. To
> +	 * allow the handshake with the GPC to progress we put the VPUs
> +	 * in reset and ungate the clocks.
> +	 */
> +	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN,
> +			  BIT(0) | BIT(1) | BIT(2));
> +	regmap_set_bits(bc->regmap, BLK_CLK_EN,
> +			BIT(0) | BIT(1) | BIT(2));
> +
> +	if (action == GENPD_NOTIFY_ON) {
> +		/*
> +		 * On power up we have no software backchannel to the GPC to
> +		 * wait for the ADB handshake to happen, so we just delay for a
> +		 * bit. On power down the GPC driver waits for the handshake.
> +		 */
> +		udelay(5);
> +
> +		/* set "fuse" bits to enable the VPUs */
> +		regmap_set_bits(bc->regmap, 0x8, 0xffffffff);
> +		regmap_set_bits(bc->regmap, 0xc, 0xffffffff);
> +		regmap_set_bits(bc->regmap, 0x10, 0xffffffff);
> +		regmap_set_bits(bc->regmap, 0x14, 0xffffffff);
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static const struct imx8m_blk_ctrl_domain_data imx8m_vpu_blk_ctl_domain_data[] = {
> +	[IMX8MM_VPUBLK_PD_G1] = {
> +		.name = "vpublk-g1",
> +		.clk_names = (const char *[]){ "g1", },
> +		.num_clks = 1,
> +		.gpc_name = "g1",
> +		.rst_mask = BIT(1),
> +		.clk_mask = BIT(1),
> +	},
> +	[IMX8MM_VPUBLK_PD_G2] = {
> +		.name = "vpublk-g2",
> +		.clk_names = (const char *[]){ "g2", },
> +		.num_clks = 1,
> +		.gpc_name = "g2",
> +		.rst_mask = BIT(0),
> +		.clk_mask = BIT(0),
> +	},
> +	[IMX8MM_VPUBLK_PD_H1] = {
> +		.name = "vpublk-h1",
> +		.clk_names = (const char *[]){ "h1", },
> +		.num_clks = 1,
> +		.gpc_name = "h1",
> +		.rst_mask = BIT(2),
> +		.clk_mask = BIT(2),
> +	},
> +};
> +
> +static const struct imx8m_blk_ctrl_data imx8m_vpu_blk_ctl_dev_data = {
> +	.max_reg = 0x18,
> +	.power_notifier_fn = imx8mm_vpu_power_notifier,
> +	.domains = imx8m_vpu_blk_ctl_domain_data,
> +	.num_domains = ARRAY_SIZE(imx8m_vpu_blk_ctl_domain_data),
> +};
> +
> +static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
> +	{
> +		.compatible = "fsl,imx8mm-vpu-blk-ctrl",
> +		.data = &imx8m_vpu_blk_ctl_dev_data
> +	}, {
> +		/* Sentinel */
> +	}
> +};
> +MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
> +
> +static struct platform_driver imx8m_blk_ctrl_driver = {
> +	.probe = imx8m_blk_ctrl_probe,
> +	.remove = imx8m_blk_ctrl_remove,
> +	.driver = {
> +		.name = "imx8m-blk-ctrl",
> +		.pm = &imx8m_blk_ctrl_pm_ops,
> +		.of_match_table = imx8m_blk_ctrl_of_match,
> +	},
> +};
> +module_platform_driver(imx8m_blk_ctrl_driver);
> 

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

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 09/18] dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains
  2021-09-07  7:06     ` Frieder Schrempf
@ 2021-09-07  8:11       ` Lucas Stach
  -1 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-07  8:11 UTC (permalink / raw)
  To: Frieder Schrempf, Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Marek Vasut, Tim Harvey, devicetree, linux-arm-kernel, kernel,
	patchwork-lst

Am Dienstag, dem 07.09.2021 um 09:06 +0200 schrieb Frieder Schrempf:
> On 06.09.21 20:43, Lucas Stach wrote:
> > This adds the defines for the power domains provided by the VPU
> > blk-ctrl.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  include/dt-bindings/power/imx8mm-power.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
> > index fc9c2e16aadc..38b0a56fd7d0 100644
> > --- a/include/dt-bindings/power/imx8mm-power.h
> > +++ b/include/dt-bindings/power/imx8mm-power.h
> > @@ -19,4 +19,8 @@
> >  #define IMX8MM_POWER_DOMAIN_DISPMIX	10
> >  #define IMX8MM_POWER_DOMAIN_MIPI	11
> >  
> > +#define IMX8MM_VPUBLK_PD_G1		0
> > +#define IMX8MM_VPUBLK_PD_G2		1
> > +#define IMX8MM_VPUBLK_PD_H1		2
> 
> I wonder how these defines should be named. Here you have a
> IMX8MM_*BLK_PD_*, but for the DISP BLK-CTRL you only have IMX8MM_*BLK_*
> (without the PD).
> 
> Also in Peng's last approach for this we already have defines for this
> [1] that have been acked by Rob and might be useful as a reference or
> you could even pick up Peng's patch and by that carry over the existing
> R-b/A-b tags.
> 
> Though, in general I like the shorter versions you provided better.

Good point, we should try to be consistent here. I don't want those
names to be overly long, but it should be clear that those are power
domain specifiers. IMO the best option is the *BLK_PD* naming, as used
in the VPUMIX binding.

Regards,
Lucas


^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 09/18] dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains
@ 2021-09-07  8:11       ` Lucas Stach
  0 siblings, 0 replies; 50+ messages in thread
From: Lucas Stach @ 2021-09-07  8:11 UTC (permalink / raw)
  To: Frieder Schrempf, Shawn Guo
  Cc: Rob Herring, Fabio Estevam, NXP Linux Team, Adam Ford,
	Marek Vasut, Tim Harvey, devicetree, linux-arm-kernel, kernel,
	patchwork-lst

Am Dienstag, dem 07.09.2021 um 09:06 +0200 schrieb Frieder Schrempf:
> On 06.09.21 20:43, Lucas Stach wrote:
> > This adds the defines for the power domains provided by the VPU
> > blk-ctrl.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  include/dt-bindings/power/imx8mm-power.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
> > index fc9c2e16aadc..38b0a56fd7d0 100644
> > --- a/include/dt-bindings/power/imx8mm-power.h
> > +++ b/include/dt-bindings/power/imx8mm-power.h
> > @@ -19,4 +19,8 @@
> >  #define IMX8MM_POWER_DOMAIN_DISPMIX	10
> >  #define IMX8MM_POWER_DOMAIN_MIPI	11
> >  
> > +#define IMX8MM_VPUBLK_PD_G1		0
> > +#define IMX8MM_VPUBLK_PD_G2		1
> > +#define IMX8MM_VPUBLK_PD_H1		2
> 
> I wonder how these defines should be named. Here you have a
> IMX8MM_*BLK_PD_*, but for the DISP BLK-CTRL you only have IMX8MM_*BLK_*
> (without the PD).
> 
> Also in Peng's last approach for this we already have defines for this
> [1] that have been acked by Rob and might be useful as a reference or
> you could even pick up Peng's patch and by that carry over the existing
> R-b/A-b tags.
> 
> Though, in general I like the shorter versions you provided better.

Good point, we should try to be consistent here. I don't want those
names to be overly long, but it should be clear that those are power
domain specifiers. IMO the best option is the *BLK_PD* naming, as used
in the VPUMIX binding.

Regards,
Lucas


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

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 10/18] soc: imx: add i.MX8M blk-ctrl driver
  2021-09-06 18:43   ` Lucas Stach
@ 2021-09-07  9:08     ` Philipp Zabel
  -1 siblings, 0 replies; 50+ messages in thread
From: Philipp Zabel @ 2021-09-07  9:08 UTC (permalink / raw)
  To: Lucas Stach, Shawn Guo
  Cc: Marek Vasut, devicetree, Fabio Estevam, Tim Harvey,
	Frieder Schrempf, patchwork-lst, Rob Herring, NXP Linux Team,
	kernel, Adam Ford, linux-arm-kernel

Hi Lucas,

On Mon, 2021-09-06 at 20:43 +0200, Lucas Stach wrote:
> This adds a driver for the blk-ctrl blocks found in the i.MX8M* line of
> SoCs. The blk-ctrl is a top-level peripheral located in the various *MIX
> power domains and interacts with the GPC power controller to provide the
> peripherals in the power domain access to the NoC and ensures that those
> peripherals are properly reset when their respective power domain is
> brought back to life.
> 
> Software needs to do different things to make the bus handshake happen
> after the the GPC *MIX domain is power up and before it is powered down.
> As the requirements are quite different between the various blk-ctrls
> there is a callback function provided to hook in the proper sequence.
> 
> The peripheral domains are quite uniform, they handle the soft clock
> enables and resets in the blk-ctrl address space and sequencing with the
> upstream GPC power domains.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> This commit includes the full code to drive the VPUMIX domain on the
> i.MX8MM, as the skeleton driver would probably be harder to review
> without the context provided by one blk-ctrl implementation. Other
> blk-ctrl implementations will follow, based on this overall structure.
> ---
>  drivers/soc/imx/Makefile         |   1 +
>  drivers/soc/imx/imx8m-blk-ctrl.c | 455 +++++++++++++++++++++++++++++++
>  2 files changed, 456 insertions(+)
>  create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c
> 
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> index 078dc918f4f3..8a707077914c 100644
> --- a/drivers/soc/imx/Makefile
> +++ b/drivers/soc/imx/Makefile
> @@ -5,3 +5,4 @@ endif
>  obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
>  obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
>  obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> +obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> new file mode 100644
> index 000000000000..3dd17b903636
> --- /dev/null
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +
> +#include <dt-bindings/power/imx8mm-power.h>
> +
> +#define BLK_SFT_RSTN	0x0
> +#define BLK_CLK_EN	0x4
> +
> +struct imx8m_blk_ctrl_domain;
> +
> +struct imx8m_blk_ctrl {
> +	struct device *dev;
> +	struct notifier_block power_nb;
> +	struct device *bus_power_dev;
> +	struct regmap *regmap;
> +	struct imx8m_blk_ctrl_domain *domains;
> +	struct genpd_onecell_data onecell_data;
> +};
> +
> +struct imx8m_blk_ctrl_domain_data {
> +	const char *name;
> +	const char **clk_names;

	const char * const *clk_names;
even?

> +	int num_clks;
> +	const char *gpc_name;
> +	u32 rst_mask;
> +	u32 clk_mask;
> +};
> +
[...]
> +static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
> +{
[...]
> +	regmap_config.max_register = bc_data->max_reg;
> +	bc->regmap = devm_regmap_init_mmio(dev, base, &regmap_config);
> +	if (IS_ERR(bc->regmap))
> +		return dev_err_probe(dev, PTR_ERR(bc->regmap),
> +				     "failed to init regmap \n");
                                                           ^
Superfluous whitespace.

> +	bc->domains = devm_kcalloc(dev, bc_data->num_domains,
> +				    sizeof(struct imx8m_blk_ctrl_domain),
> +				    GFP_KERNEL);
                                   ^
Misalignment (also superfluous whitespace).

[...]
> +static int imx8mm_vpu_power_notifier(struct notifier_block *nb,
> +				     unsigned long action, void *data)
> +{
> +	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
> +						 power_nb);
> +
> +	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
> +		return NOTIFY_OK;
> +
> +	/*
> +	 * The ADB in the VPUMIX domain has no separate reset and clock
> +	 * enable bits, but is ungated together with the VPU clocks. To
> +	 * allow the handshake with the GPC to progress we put the VPUs
> +	 * in reset and ungate the clocks.
> +	 */
> +	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN,
> +			  BIT(0) | BIT(1) | BIT(2));
> +	regmap_set_bits(bc->regmap, BLK_CLK_EN,
> +			BIT(0) | BIT(1) | BIT(2));

These can be put on a single line each.

Otherwise,
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

^ permalink raw reply	[flat|nested] 50+ messages in thread
* Re: [PATCH v3 10/18] soc: imx: add i.MX8M blk-ctrl driver
@ 2021-09-07  9:08     ` Philipp Zabel
  0 siblings, 0 replies; 50+ messages in thread
From: Philipp Zabel @ 2021-09-07  9:08 UTC (permalink / raw)
  To: Lucas Stach, Shawn Guo
  Cc: Marek Vasut, devicetree, Fabio Estevam, Tim Harvey,
	Frieder Schrempf, patchwork-lst, Rob Herring, NXP Linux Team,
	kernel, Adam Ford, linux-arm-kernel

Hi Lucas,

On Mon, 2021-09-06 at 20:43 +0200, Lucas Stach wrote:
> This adds a driver for the blk-ctrl blocks found in the i.MX8M* line of
> SoCs. The blk-ctrl is a top-level peripheral located in the various *MIX
> power domains and interacts with the GPC power controller to provide the
> peripherals in the power domain access to the NoC and ensures that those
> peripherals are properly reset when their respective power domain is
> brought back to life.
> 
> Software needs to do different things to make the bus handshake happen
> after the the GPC *MIX domain is power up and before it is powered down.
> As the requirements are quite different between the various blk-ctrls
> there is a callback function provided to hook in the proper sequence.
> 
> The peripheral domains are quite uniform, they handle the soft clock
> enables and resets in the blk-ctrl address space and sequencing with the
> upstream GPC power domains.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> This commit includes the full code to drive the VPUMIX domain on the
> i.MX8MM, as the skeleton driver would probably be harder to review
> without the context provided by one blk-ctrl implementation. Other
> blk-ctrl implementations will follow, based on this overall structure.
> ---
>  drivers/soc/imx/Makefile         |   1 +
>  drivers/soc/imx/imx8m-blk-ctrl.c | 455 +++++++++++++++++++++++++++++++
>  2 files changed, 456 insertions(+)
>  create mode 100644 drivers/soc/imx/imx8m-blk-ctrl.c
> 
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> index 078dc918f4f3..8a707077914c 100644
> --- a/drivers/soc/imx/Makefile
> +++ b/drivers/soc/imx/Makefile
> @@ -5,3 +5,4 @@ endif
>  obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
>  obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
>  obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> +obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> new file mode 100644
> index 000000000000..3dd17b903636
> --- /dev/null
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +
> +#include <dt-bindings/power/imx8mm-power.h>
> +
> +#define BLK_SFT_RSTN	0x0
> +#define BLK_CLK_EN	0x4
> +
> +struct imx8m_blk_ctrl_domain;
> +
> +struct imx8m_blk_ctrl {
> +	struct device *dev;
> +	struct notifier_block power_nb;
> +	struct device *bus_power_dev;
> +	struct regmap *regmap;
> +	struct imx8m_blk_ctrl_domain *domains;
> +	struct genpd_onecell_data onecell_data;
> +};
> +
> +struct imx8m_blk_ctrl_domain_data {
> +	const char *name;
> +	const char **clk_names;

	const char * const *clk_names;
even?

> +	int num_clks;
> +	const char *gpc_name;
> +	u32 rst_mask;
> +	u32 clk_mask;
> +};
> +
[...]
> +static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
> +{
[...]
> +	regmap_config.max_register = bc_data->max_reg;
> +	bc->regmap = devm_regmap_init_mmio(dev, base, &regmap_config);
> +	if (IS_ERR(bc->regmap))
> +		return dev_err_probe(dev, PTR_ERR(bc->regmap),
> +				     "failed to init regmap \n");
                                                           ^
Superfluous whitespace.

> +	bc->domains = devm_kcalloc(dev, bc_data->num_domains,
> +				    sizeof(struct imx8m_blk_ctrl_domain),
> +				    GFP_KERNEL);
                                   ^
Misalignment (also superfluous whitespace).

[...]
> +static int imx8mm_vpu_power_notifier(struct notifier_block *nb,
> +				     unsigned long action, void *data)
> +{
> +	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
> +						 power_nb);
> +
> +	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
> +		return NOTIFY_OK;
> +
> +	/*
> +	 * The ADB in the VPUMIX domain has no separate reset and clock
> +	 * enable bits, but is ungated together with the VPU clocks. To
> +	 * allow the handshake with the GPC to progress we put the VPUs
> +	 * in reset and ungate the clocks.
> +	 */
> +	regmap_clear_bits(bc->regmap, BLK_SFT_RSTN,
> +			  BIT(0) | BIT(1) | BIT(2));
> +	regmap_set_bits(bc->regmap, BLK_CLK_EN,
> +			BIT(0) | BIT(1) | BIT(2));

These can be put on a single line each.

Otherwise,
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

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

^ permalink raw reply	[flat|nested] 50+ messages in thread
end of thread, other threads:[~2021-09-07  9:10 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-06 18:43 [PATCH v3 00/18] i.MX8MM GPC improvements and BLK_CTRL driver Lucas Stach
2021-09-06 18:43 ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 01/18] Revert "soc: imx: gpcv2: move reset assert after requesting domain power up" Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 02/18] soc: imx: gpcv2: Turn domain->pgc into bitfield Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 03/18] soc: imx: gpcv2: Set both GPC_PGC_nCTRL(GPU_2D|GPU_3D) for MX8MM GPU domain Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 04/18] soc: imx: gpcv2: add lockdep annotation Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 05/18] soc: imx: gpcv2: add domain option to keep domain clocks enabled Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 06/18] soc: imx: gpcv2: keep i.MX8M* bus " Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 07/18] soc: imx: gpcv2: support system suspend/resume Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 08/18] dt-bindings: soc: add binding for i.MX8MM VPU blk-ctrl Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 09/18] dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-07  7:06   ` Frieder Schrempf
2021-09-07  7:06     ` Frieder Schrempf
2021-09-07  8:11     ` Lucas Stach
2021-09-07  8:11       ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 10/18] soc: imx: add i.MX8M blk-ctrl driver Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-07  7:28   ` Frieder Schrempf
2021-09-07  7:28     ` Frieder Schrempf
2021-09-07  9:08   ` Philipp Zabel
2021-09-07  9:08     ` Philipp Zabel
2021-09-06 18:43 ` [PATCH v3 11/18] dt-bindings: soc: add binding for i.MX8MM DISP blk-ctrl Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 12/18] dt-bindings: power: imx8mm: add defines for DISP blk-ctrl domains Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 13/18] soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 14/18] arm64: dts: imx8mm: add GPC node Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 15/18] arm64: dts: imx8mm: put USB controllers into power-domains Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 16/18] arm64: dts: imx8mm: Add GPU nodes for 2D and 3D core Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 17/18] arm64: dts: imx8mm: add VPU blk-ctrl Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 18:43 ` [PATCH v3 18/18] arm64: dts: imx8mm: add DISP blk-ctrl Lucas Stach
2021-09-06 18:43   ` Lucas Stach
2021-09-06 19:01 ` [PATCH v3 00/18] i.MX8MM GPC improvements and BLK_CTRL driver Fabio Estevam
2021-09-06 19:01   ` Fabio Estevam
2021-09-07  6:52   ` Frieder Schrempf
2021-09-07  6:52     ` Frieder Schrempf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.