Skip to contentSkip to conversation

Fix imx6sl display power domain

PATCH Applied Leonard Crestez 4 participants 9 messages Jul 11, 2018

Subsystems (2)
Device tree bindings Arm i.MX and Layerscape
lore
Share & tools
lore

Leonard Crestez

author
#0 lore raw

v3 cover Fix imx6sl display power domain

Changes since v2:
* Don't mistakenly include IMX6SL_CLK_IPG in pd_disp
* Use GENPD_FLAG_ALWAYS_ON, as suggested by Ulf

Errata workaround could be refined later, however the power savings are small and it would require a lot of testing. So push the safest fix first to get basic hardware support.

Link to v2: https://lkml.org/lkml/2018/7/9/447

Leonard Crestez (2):
soc: imx: gpc: Disable 6sl display power gating for ERR006287
ARM: dts: imx6sl: Convert gpc to new bindings

 arch/arm/boot/dts/imx6sl.dtsi | 35 +++++++++++++++++++++++++++++++----
 drivers/soc/imx/gpc.c         | 10 ++++++++++
 2 files changed, 41 insertions(+), 4 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v3 1/2 soc: imx: gpc: Disable 6sl display power gating for ERR006287

The imx6sl chip errata document describes ERR006287 like this:

Quoted 1 line

Upon resuming from power gating, the modules in the display power domain

(eLCDIF, EPDC, PXP and SPDC) might fail to perform register reads correctly.

Quoted 1 line

When the modules listed above are used, do not use power gating on the

display power domain.

Link https://www.nxp.com/docs/en/errata/IMX6SLCE.pdf#page=62

Handle this in the safest possible way by keeping the DISP domain always-on.

Signed-off-by Leonard Crestez <[email protected]>
---
 drivers/soc/imx/gpc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/soc/imx/gpc.c +10 -0
index 526f2d02dc78..f7960f773019 100644
@@ -339,30 +339,35 @@ static struct imx_pm_domain imx_gpc_domains[] = {
 };
 
 struct imx_gpc_dt_data {
 	int num_domains;
 	bool err009619_present;
+	bool err006287_present;
 };
 
 static const struct imx_gpc_dt_data imx6q_dt_data = {
 	.num_domains = 2,
 	.err009619_present = false,
+	.err006287_present = false,
 };
 
 static const struct imx_gpc_dt_data imx6qp_dt_data = {
 	.num_domains = 2,
 	.err009619_present = true,
+	.err006287_present = false,
 };
 
 static const struct imx_gpc_dt_data imx6sl_dt_data = {
 	.num_domains = 3,
 	.err009619_present = false,
+	.err006287_present = true,
 };
 
 static const struct imx_gpc_dt_data imx6sx_dt_data = {
 	.num_domains = 4,
 	.err009619_present = false,
+	.err006287_present = false,
 };
 
 static const struct of_device_id imx_gpc_dt_ids[] = {
 	{ .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
 	{ .compatible = "fsl,imx6qp-gpc", .data = &imx6qp_dt_data },

@@ -467,10 +472,15 @@ static int imx_gpc_probe(struct platform_device *pdev)
 	/* Disable PU power down in normal operation if ERR009619 is present */
 	if (of_id_data->err009619_present)
 		imx_gpc_domains[GPC_PGC_DOMAIN_PU].flags |=
 				PGC_DOMAIN_FLAG_NO_PD;
 
+	/* Keep DISP always on if ERR006287 is present */
+	if (of_id_data->err006287_present)
+		imx_gpc_domains[GPC_PGC_DOMAIN_DISPLAY].base.flags |=
+				GENPD_FLAG_ALWAYS_ON;
+
 	if (!pgc_node) {
 		ret = imx_gpc_old_dt_init(&pdev->dev, regmap,
 					  of_id_data->num_domains);
 		if (ret)
 			return ret;
Signature · 1 line
2.17.1
Reviewed-by on v3 1/2
#2 lore raw

Re: [PATCH v3 1/2] soc: imx: gpc: Disable 6sl display power gating for ERR006287

Am Mittwoch, den 11.07.2018, 15:11 +0300 schrieb Leonard Crestez:

Quoted 9 lines

The imx6sl chip errata document describes ERR006287 like this:

Quoted 1 line

Upon resuming from power gating, the modules in the display power domain

(eLCDIF, EPDC, PXP and SPDC) might fail to perform register reads correctly.

Quoted 1 line

When the modules listed above are used, do not use power gating on the

Quoted 8 lines

display power domain.

Link https://www.nxp.com/docs/en/errata/IMX6SLCE.pdf#page=62

Handle this in the safest possible way by keeping the DISP domain always-on.

Signed-off-by Leonard Crestez <[email protected]>
Reviewed-by Lucas Stach <[email protected]>

Can you send a follow on patch to switch the i.MX6QP errata workaround to use GENPD_FLAG_ALWAYS_ON and remove the -EBUSY stuff in the power down path?

Regards,
Lucas

Quoted 60 lines

---
?drivers/soc/imx/gpc.c | 10 ++++++++++
?1 file changed, 10 insertions(+)

drivers/soc/imx/gpc.c +0 -0
index 526f2d02dc78..f7960f773019 100644
@@ -339,30 +339,35 @@ static struct imx_pm_domain imx_gpc_domains[] = {
 ?};

?
?struct imx_gpc_dt_data {

drivers/soc/imx/gpc.c static struct imx_pm_domain imx_gpc_domains[] = { · line 344

Quoted 3 lines

? int num_domains;
? bool err009619_present;

drivers/soc/imx/gpc.c +1 -0
+	bool err006287_present;

?};
?
?static const struct imx_gpc_dt_data imx6q_dt_data = {

Review

Quoted 3 lines

? .num_domains = 2,
? .err009619_present = false,

(quoted diff) +1 -0
+	.err006287_present = false,

?};
?
?static const struct imx_gpc_dt_data imx6qp_dt_data = {

Review

Quoted 3 lines

? .num_domains = 2,
? .err009619_present = true,

(quoted diff) +1 -0
+	.err006287_present = false,

?};
?
?static const struct imx_gpc_dt_data imx6sl_dt_data = {

drivers/soc/imx/gpc.c static struct imx_pm_domain imx_gpc_domains[] = { · line 362

Quoted 3 lines

? .num_domains = 3,
? .err009619_present = false,

drivers/soc/imx/gpc.c +1 -0
+	.err006287_present = true,

?};
?
?static const struct imx_gpc_dt_data imx6sx_dt_data = {

Review

Quoted 3 lines

? .num_domains = 4,
? .err009619_present = false,

(quoted diff) +1 -0
+	.err006287_present = false,

?};
?
?static const struct of_device_id imx_gpc_dt_ids[] = {

Quoted 2 lines

? { .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
? { .compatible = "fsl,imx6qp-gpc", .data = &imx6qp_dt_data },

drivers/soc/imx/gpc.c +0 -0
@@ -467,10 +472,15 @@ static int imx_gpc_probe(struct platform_device *pdev)
Quoted 4 lines

? /* Disable PU power down in normal operation if ERR009619 is present */
? if (of_id_data->err009619_present)
? imx_gpc_domains[GPC_PGC_DOMAIN_PU].flags |=
? PGC_DOMAIN_FLAG_NO_PD;

?

drivers/soc/imx/gpc.c static int imx_gpc_probe(struct platform_device *pdev) · line 480

Quoted 4 lines
drivers/soc/imx/gpc.c +4 -0
+	/* Keep DISP always on if ERR006287 is present */
+	if (of_id_data->err006287_present)
+		imx_gpc_domains[GPC_PGC_DOMAIN_DISPLAY].base.flags |=
+				GENPD_FLAG_ALWAYS_ON;

+

Quoted 5 lines

? if (!pgc_node) {
? ret = imx_gpc_old_dt_init(&pdev->dev, regmap,
? ??of_id_data->num_domains);
? if (ret)
? return ret;

author
#3 lore raw

Re: [PATCH v3 1/2] soc: imx: gpc: Disable 6sl display power gating for ERR006287

Quoted 11 lines · Lucas Stach

Am Mittwoch, den 11.07.2018, 15:11 +0300 schrieb Leonard Crestez:

Quoted 4 lines

Handle this in the safest possible way by keeping the DISP domain always-on.

Signed-off-by Leonard Crestez <[email protected]>
Reviewed-by Lucas Stach <[email protected]>

Can you send a follow on patch to switch the i.MX6QP errata workaround to use GENPD_FLAG_ALWAYS_ON and remove the -EBUSY stuff in the power down path?

Sure.

I was thinking of converting it to a new GENPD_FLAG which allows power_off in suspend (as suggested by Ulf) but switching to GENPD_FLAG_ALWAYS_ON in order to simplify the code could be done first.

The -EBUSY stuff is not very harmful.

--
Regards,
Leonard

#4 lore raw

Re: [PATCH v3 1/2] soc: imx: gpc: Disable 6sl display power gating for ERR006287

Am Mittwoch, den 11.07.2018, 15:21 +0300 schrieb Leonard Crestez:

Quoted 15 lines · Lucas Stach
Quoted 6 lines

Am Mittwoch, den 11.07.2018, 15:11 +0300 schrieb Leonard Crestez:

Quoted 4 lines

Handle this in the safest possible way by keeping the DISP domain always-on.

Signed-off-by Leonard Crestez <[email protected]>
Quoted 7 lines
Reviewed-by Lucas Stach <[email protected]>

Can you send a follow on patch to switch the i.MX6QP errata workaround
to use GENPD_FLAG_ALWAYS_ON and remove the -EBUSY stuff in the power
down path?

Quoted 6 lines

Sure.

I was thinking of converting it to a new GENPD_FLAG which allows power_off in suspend (as suggested by Ulf) but switching to GENPD_FLAG_ALWAYS_ON in order to simplify the code could be done first.

Sure, if you are going to work on this I'm fine with converting it over to this without first going for GENPD_FLAG_ALWAYS_ON. Just wanted to make sure that things are consistent.

Quoted 1 line

The -EBUSY stuff is not very harmful.

It's dead code once the appropriate flags have been added to the domain, so should be removed.

Regards,
Lucas

Reviewed-by on v3 1/2
#5 lore raw

Re: [PATCH v3 1/2] soc: imx: gpc: Disable 6sl display power gating for ERR006287

In reply to Leonard Crestez
Quoted 7 lines · Leonard Crestez

The imx6sl chip errata document describes ERR006287 like this:

Quoted 1 line

Upon resuming from power gating, the modules in the display power domain

(eLCDIF, EPDC, PXP and SPDC) might fail to perform register reads correctly.

Quoted 1 line

When the modules listed above are used, do not use power gating on the

Quoted 8 lines

display power domain.

Link https://www.nxp.com/docs/en/errata/IMX6SLCE.pdf#page=62

Handle this in the safest possible way by keeping the DISP domain always-on.

Signed-off-by Leonard Crestez <[email protected]>
Reviewed-by Ulf Hansson <[email protected]>

Kind regards
Uffe

Quoted 63 lines
---
 drivers/soc/imx/gpc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/soc/imx/gpc.c +10 -1
index 526f2d02dc78..f7960f773019 100644
@@ -339,30 +339,35 @@ static struct imx_pm_domain imx_gpc_domains[] = {
 };
 
 struct imx_gpc_dt_data {
        int num_domains;
        bool err009619_present;
+       bool err006287_present;
 };
 
 static const struct imx_gpc_dt_data imx6q_dt_data = {
        .num_domains = 2,
        .err009619_present = false,
+       .err006287_present = false,
 };
 
 static const struct imx_gpc_dt_data imx6qp_dt_data = {
        .num_domains = 2,
        .err009619_present = true,
+       .err006287_present = false,
 };
 
 static const struct imx_gpc_dt_data imx6sl_dt_data = {
        .num_domains = 3,
        .err009619_present = false,
+       .err006287_present = true,
 };
 
 static const struct imx_gpc_dt_data imx6sx_dt_data = {
        .num_domains = 4,
        .err009619_present = false,
+       .err006287_present = false,
 };
 
 static const struct of_device_id imx_gpc_dt_ids[] = {
        { .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
        { .compatible = "fsl,imx6qp-gpc", .data = &imx6qp_dt_data },

@@ -467,10 +472,15 @@ static int imx_gpc_probe(struct platform_device *pdev)
        /* Disable PU power down in normal operation if ERR009619 is present */
        if (of_id_data->err009619_present)
                imx_gpc_domains[GPC_PGC_DOMAIN_PU].flags |=
                                PGC_DOMAIN_FLAG_NO_PD;
 
+       /* Keep DISP always on if ERR006287 is present */
+       if (of_id_data->err006287_present)
+               imx_gpc_domains[GPC_PGC_DOMAIN_DISPLAY].base.flags |=
+                               GENPD_FLAG_ALWAYS_ON;
+
        if (!pgc_node) {
                ret = imx_gpc_old_dt_init(&pdev->dev, regmap,
                                          of_id_data->num_domains);
                if (ret)
                        return ret;
--

2.17.1

author
#6 lore raw

PATCH v3 2/2 ARM: dts: imx6sl: Convert gpc to new bindings

In reply to Leonard Crestez

With old bindings imx_gpc_onecell_data always sets num_domains to 2 so the DISPMIX domain can't actually be referenced. The pd is still defined and pm core shuts it down as "unused" so display can't work.

Fix this by converting to new gpc bindings by adding pgc nodes and referencing the newly-defined &pu_disp domain from &lcdif.

Signed-off-by Leonard Crestez <[email protected]>
---
 arch/arm/boot/dts/imx6sl.dtsi | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)
arch/arm/boot/dts/imx6sl.dtsi +31 -4
index 102c575a6025..49a56b4fd393 100644
@@ -698,14 +698,40 @@
 				reg = <0x020dc000 0x4000>;
 				interrupt-controller;
 				#interrupt-cells = <3>;
 				interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-parent = <&intc>;
-				pu-supply = <&reg_pu>;
-				clocks = <&clks IMX6SL_CLK_GPU2D_OVG>,
-					 <&clks IMX6SL_CLK_GPU2D_PODF>;
-				#power-domain-cells = <1>;
+				clocks = <&clks IMX6SL_CLK_IPG>;
+				clock-names = "ipg";
+
+				pgc {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					power-domain at 0 {
+						reg = <0>;
+						#power-domain-cells = <0>;
+					};
+
+					pd_pu: power-domain at 1 {
+						reg = <1>;
+						#power-domain-cells = <0>;
+						power-supply = <&reg_pu>;
+						clocks = <&clks IMX6SL_CLK_GPU2D_OVG>,
+						         <&clks IMX6SL_CLK_GPU2D_PODF>;
+					};
+
+					pd_disp: power-domain at 2 {
+						reg = <2>;
+						#power-domain-cells = <0>;
+						clocks = <&clks IMX6SL_CLK_LCDIF_AXI>,
+							 <&clks IMX6SL_CLK_LCDIF_PIX>,
+							 <&clks IMX6SL_CLK_EPDC_AXI>,
+							 <&clks IMX6SL_CLK_EPDC_PIX>,
+							 <&clks IMX6SL_CLK_PXP_AXI>;
+					};
+				};
 			};
 
 			gpr: iomuxc-gpr at 20e0000 {
 				compatible = "fsl,imx6sl-iomuxc-gpr",
 					     "fsl,imx6q-iomuxc-gpr", "syscon";

@@ -756,10 +782,11 @@
 				clocks = <&clks IMX6SL_CLK_LCDIF_PIX>,
 					 <&clks IMX6SL_CLK_LCDIF_AXI>,
 					 <&clks IMX6SL_CLK_DUMMY>;
 				clock-names = "pix", "axi", "disp_axi";
 				status = "disabled";
+				power-domains = <&pd_disp>;
 			};
 
 			dcp: dcp at 20fc000 {
 				compatible = "fsl,imx6sl-dcp", "fsl,imx28-dcp";
 				reg = <0x020fc000 0x4000>;
Signature · 1 line
2.17.1
Reviewed-by on v3 2/2
#7 lore raw

Re: [PATCH v3 2/2] ARM: dts: imx6sl: Convert gpc to new bindings

Am Mittwoch, den 11.07.2018, 15:11 +0300 schrieb Leonard Crestez:

Quoted 8 lines

With old bindings imx_gpc_onecell_data always sets num_domains to 2 so the DISPMIX domain can't actually be referenced. The pd is still defined and pm core shuts it down as "unused" so display can't work.

Fix this by converting to new gpc bindings by adding pgc nodes and referencing the newly-defined &pu_disp domain from &lcdif.

Signed-off-by Leonard Crestez <[email protected]>
Reviewed-by Lucas Stach <[email protected]>
Quoted 65 lines

---
?arch/arm/boot/dts/imx6sl.dtsi | 35 +++++++++++++++++++++++++++++++---- ?1 file changed, 31 insertions(+), 4 deletions(-)

arch/arm/boot/dts/imx6sl.dtsi +2 -0
index 102c575a6025..49a56b4fd393 100644
@@ -698,14 +698,40 @@
 > ?				reg = <0x020dc000 0x4000>;
 > ?				interrupt-controller;
 > ?				#interrupt-cells = <3>;
 > ?				interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>;
 > ?				interrupt-parent = <&intc>;
 > -				pu-supply = <&reg_pu>;
 > -				clocks = <&clks IMX6SL_CLK_GPU2D_OVG>,
 > -					?<&clks IMX6SL_CLK_GPU2D_PODF>;
 > -				#power-domain-cells = <1>;
 > +				clocks = <&clks IMX6SL_CLK_IPG>;
 > +				clock-names = "ipg";
+
 > +				pgc {
 > +					#address-cells = <1>;
 > +					#size-cells = <0>;
+

arch/arm/boot/dts/imx6sl.dtsi line 713

Quoted 4 lines
arch/arm/boot/dts/imx6sl.dtsi +4 -0
+					power-domain at 0 {
+						reg = <0>;
+						#power-domain-cells = <0>;
+					};

+

arch/arm/boot/dts/imx6sl.dtsi line 721

Quoted 7 lines
Quoted 1 line
arch/arm/boot/dts/imx6sl.dtsi +1 -0
+					pd_pu: power-domain at 1 {
arch/arm/boot/dts/imx6sl.dtsi +6 -0
+						reg = <1>;
+						#power-domain-cells = <0>;
+						power-supply = <&reg_pu>;
+						clocks = <&clks IMX6SL_CLK_GPU2D_OVG>,
+						?????????<&clks IMX6SL_CLK_GPU2D_PODF>;
+					};

+

Quoted 11 lines
Quoted 1 line
arch/arm/boot/dts/imx6sl.dtsi +1 -0
+					pd_disp: power-domain at 2 {
arch/arm/boot/dts/imx6sl.dtsi +9 -0
+						reg = <2>;
+						#power-domain-cells = <0>;
+						clocks = <&clks IMX6SL_CLK_LCDIF_AXI>,
+							?<&clks IMX6SL_CLK_LCDIF_PIX>,
+							?<&clks IMX6SL_CLK_EPDC_AXI>,
+							?<&clks IMX6SL_CLK_EPDC_PIX>,
+							?<&clks IMX6SL_CLK_PXP_AXI>;
+					};
+				};

? };

?

Quoted 3 lines
Quoted 1 line

? gpr: iomuxc-gpr at 20e0000 {

? compatible = "fsl,imx6sl-iomuxc-gpr",
? ?????"fsl,imx6q-iomuxc-gpr", "syscon";

arch/arm/boot/dts/imx6sl.dtsi +0 -0
@@ -756,10 +782,11 @@
 > ?				clocks = <&clks IMX6SL_CLK_LCDIF_PIX>,
 > ?					?<&clks IMX6SL_CLK_LCDIF_AXI>,
 > ?					?<&clks IMX6SL_CLK_DUMMY>;
 > ?				clock-names = "pix", "axi", "disp_axi";
 > ?				status = "disabled";
 > +				power-domains = <&pd_disp>;
 > ?			};

?

Quoted 3 lines
Quoted 1 line

? dcp: dcp at 20fc000 {

? compatible = "fsl,imx6sl-dcp", "fsl,imx28-dcp";
? reg = <0x020fc000 0x4000>;

Applied
#8 lore raw
In reply to Leonard Crestez
Quoted 3 lines · Leonard Crestez

Leonard Crestez (2):
soc: imx: gpc: Disable 6sl display power gating for ERR006287
ARM: dts: imx6sl: Convert gpc to new bindings

Applied both, thanks.