[PATCH v2 0/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[PATCH v2 0/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[Leonard Crestez]

Now that mxsfb has functional runtime_pm support we can enable power
gating for the DISPLAY power domain on imx6sx.

Since pci-imx doesn't support runtime power gating this is still
always-on unless PCI is disabled/unused. But it's reasonable for an imx
board to not have PCI.

The current pd_pcie is actually only for PCIE_PHY, the PCIE ip block is
actually inside the DISPLAY domain. This is handled by adding the pcie
node in both power domains, using multi-pd support.

Series is against linux-next. The last patch needs runtime PM in all
drivers or it will cause hangs. This means Shawn should hold back on it
until the PCI and mxsfb dependencies get merged (both might take a long
time). So you can just review that patch and I will resend it later,
perhaps in another cycle.

---

Changes since v1:
* Document multi-pd in imx6q-pcie bindings and add DT list
* Don't refer to "DISPLAY" as "DISPMIX" because the Reference Manual
doesn't do that.
* Link to v1: https://lore.kernel.org/patchwork/cover/994092/

This is independent of other imx6-pci PM patches such as this one:
* https://lore.kernel.org/patchwork/patch/996806/
Device links are quite nice!

Link to mxsfb thread: https://lkml.org/lkml/2018/9/26/1185

Leonard Crestez (4):
  soc: imx: gpc: Increase GPC_CLK_MAX to 7
  dt-bindings: imx6q-pcie: Add multi-pd bindings for imx6sx
  PCI: imx: Add multi-pd support
  ARM: dts: imx6sx: Add DISPLAY power domain support

 .../bindings/pci/fsl,imx6q-pcie.txt           |  4 +-
 arch/arm/boot/dts/imx6sx.dtsi                 | 19 +++++++-
 drivers/pci/controller/dwc/pci-imx6.c         | 48 +++++++++++++++++++
 drivers/soc/imx/gpc.c                         |  2 +-
 4 files changed, 70 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] soc: imx: gpc: Increase GPC_CLK_MAX to 7

[Leonard Crestez]

The DISPLAY power domain on imx6sx has 7 clocks.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/soc/imx/gpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c
index aa3729ecaa9e..7d14a4b4e82a 100644
--- a/drivers/soc/imx/gpc.c
+++ b/drivers/soc/imx/gpc.c
@@ -33,11 +33,11 @@
 #define GPC_PGC_DISP_SR		0x24c
 
 #define GPU_VPU_PUP_REQ		BIT(1)
 #define GPU_VPU_PDN_REQ		BIT(0)
 
-#define GPC_CLK_MAX		6
+#define GPC_CLK_MAX		7
 
 #define PGC_DOMAIN_FLAG_NO_PD		BIT(0)
 
 struct imx_pm_domain {
 	struct generic_pm_domain base;
-- 
2.17.1

Re: [PATCH v2 1/4] soc: imx: gpc: Increase GPC_CLK_MAX to 7

[Shawn Guo]

On Mon, Oct 08, 2018 at 06:06:19PM +0000, Leonard Crestez wrote:
> The DISPLAY power domain on imx6sx has 7 clocks.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

Applied, thanks.

[PATCH v2 2/4] dt-bindings: imx6q-pcie: Add multi-pd bindings for imx6sx

[Leonard Crestez]

The PCIE and PCIE_PHY blocks are in different power domains on imx6sx
and this needs to be described using multi-pd bindings.

This was not required until now because the power-domain of the PCIE
block (DISPLAY) was always on.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index f37494d5a7be..d514c1f2365f 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -39,11 +39,13 @@ Optional properties:
   host.
 
 Additional required properties for imx6sx-pcie:
 - clock names: Must include the following additional entries:
 	- "pcie_inbound_axi"
-- power-domains: Must be set to a phandle pointing to the PCIE_PHY power domain
+- power-domains: Must be set to phandles pointing to the DISPLAY and
+  PCIE_PHY power domains
+- power-domain-names: Must be "pcie", "pcie_phy"
 
 Additional required properties for imx7d-pcie:
 - power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
 - resets: Must contain phandles to PCIe-related reset lines exposed by SRC
   IP block
-- 
2.17.1

Re: [PATCH v2 2/4] dt-bindings: imx6q-pcie: Add multi-pd bindings for imx6sx

[Rob Herring]

On Mon, 8 Oct 2018 18:06:20 +0000, Leonard Crestez wrote:
> The PCIE and PCIE_PHY blocks are in different power domains on imx6sx
> and this needs to be described using multi-pd bindings.
> 
> This was not required until now because the power-domain of the PCIE
> block (DISPLAY) was always on.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v2 3/4] PCI: imx: Add multi-pd support

[Leonard Crestez]

On some chips the PCIE and PCIE_PHY blocks are in separate power domains
which can be power-gated independently. The pci driver needs to handle
this by keeping both domain active.

This is intended for imx6sx where PCIE is in DISPLAY and PCIE_PHY in
it's own domain. Defining the DISPLAY domain requires a way for pcie to
keep it active or it will break when displays are off.

The power-domains on imx6sx are meant to look like this:
	power-domains = <&pd_disp>, <&pd_pci>;
	power-domain-names = "pcie", "pcie_phy";

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pci/controller/dwc/pci-imx6.c | 48 +++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 6171171db1fc..a482f86b02e6 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -25,10 +25,12 @@
 #include <linux/resource.h>
 #include <linux/signal.h>
 #include <linux/types.h>
 #include <linux/interrupt.h>
 #include <linux/reset.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
 
 #include "pcie-designware.h"
 
 #define to_imx6_pcie(x)	dev_get_drvdata((x)->dev)
 
@@ -57,10 +59,15 @@ struct imx6_pcie {
 	u32			tx_deemph_gen2_6db;
 	u32			tx_swing_full;
 	u32			tx_swing_low;
 	int			link_gen;
 	struct regulator	*vpcie;
+
+	/* power domain for pcie */
+	struct device		*pd_pcie;
+	/* power domain for pcie phy */
+	struct device		*pd_pcie_phy;
 };
 
 /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
 #define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
 #define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
@@ -290,10 +297,47 @@ static int imx6q_pcie_abort_handler(unsigned long addr,
 	}
 
 	return 1;
 }
 
+static int imx6_pcie_attach_pd(struct device *dev)
+{
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+	struct device_link *link;
+
+	/* Do nothing when in a single power domain */
+	if (dev->pm_domain)
+		return 0;
+
+	imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie");
+	if (IS_ERR(imx6_pcie->pd_pcie))
+		return PTR_ERR(imx6_pcie->pd_pcie);
+	link = device_link_add(dev, imx6_pcie->pd_pcie,
+			DL_FLAG_STATELESS |
+			DL_FLAG_PM_RUNTIME |
+			DL_FLAG_RPM_ACTIVE);
+	if (IS_ERR(link)) {
+		dev_err(dev, "Failed to add device_link to pcie pd: %ld\n", PTR_ERR(link));
+		return PTR_ERR(link);
+	}
+
+	imx6_pcie->pd_pcie_phy = dev_pm_domain_attach_by_name(dev, "pcie_phy");
+	if (IS_ERR(imx6_pcie->pd_pcie_phy))
+		return PTR_ERR(imx6_pcie->pd_pcie_phy);
+
+	device_link_add(dev, imx6_pcie->pd_pcie_phy,
+			DL_FLAG_STATELESS |
+			DL_FLAG_PM_RUNTIME |
+			DL_FLAG_RPM_ACTIVE);
+	if (IS_ERR(link)) {
+		dev_err(dev, "Failed to add device_link to pcie_phy pd: %ld\n", PTR_ERR(link));
+		return PTR_ERR(link);
+	}
+
+	return 0;
+}
+
 static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
 {
 	struct device *dev = imx6_pcie->pci->dev;
 
 	switch (imx6_pcie->variant) {
@@ -1013,10 +1057,14 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 		imx6_pcie->vpcie = NULL;
 	}
 
 	platform_set_drvdata(pdev, imx6_pcie);
 
+	ret = imx6_pcie_attach_pd(dev);
+	if (ret)
+		return ret;
+
 	ret = imx6_add_pcie_port(imx6_pcie, pdev);
 	if (ret < 0)
 		return ret;
 
 	return 0;
-- 
2.17.1

Re: [PATCH v2 3/4] PCI: imx: Add multi-pd support

[Lorenzo Pieralisi]

On Mon, Oct 08, 2018 at 06:06:21PM +0000, Leonard Crestez wrote:
> On some chips the PCIE and PCIE_PHY blocks are in separate power domains
> which can be power-gated independently. The pci driver needs to handle
> this by keeping both domain active.
> 
> This is intended for imx6sx where PCIE is in DISPLAY and PCIE_PHY in
> it's own domain. Defining the DISPLAY domain requires a way for pcie to
> keep it active or it will break when displays are off.
> 
> The power-domains on imx6sx are meant to look like this:
> 	power-domains = <&pd_disp>, <&pd_pci>;
> 	power-domain-names = "pcie", "pcie_phy";
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 48 +++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)

I expect Shawn to pick the whole series up and therefore I am dropping
this series from the PCI tree.

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 6171171db1fc..a482f86b02e6 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -25,10 +25,12 @@
>  #include <linux/resource.h>
>  #include <linux/signal.h>
>  #include <linux/types.h>
>  #include <linux/interrupt.h>
>  #include <linux/reset.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
>  
>  #include "pcie-designware.h"
>  
>  #define to_imx6_pcie(x)	dev_get_drvdata((x)->dev)
>  
> @@ -57,10 +59,15 @@ struct imx6_pcie {
>  	u32			tx_deemph_gen2_6db;
>  	u32			tx_swing_full;
>  	u32			tx_swing_low;
>  	int			link_gen;
>  	struct regulator	*vpcie;
> +
> +	/* power domain for pcie */
> +	struct device		*pd_pcie;
> +	/* power domain for pcie phy */
> +	struct device		*pd_pcie_phy;
>  };
>  
>  /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
>  #define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
>  #define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
> @@ -290,10 +297,47 @@ static int imx6q_pcie_abort_handler(unsigned long addr,
>  	}
>  
>  	return 1;
>  }
>  
> +static int imx6_pcie_attach_pd(struct device *dev)
> +{
> +	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +	struct device_link *link;
> +
> +	/* Do nothing when in a single power domain */
> +	if (dev->pm_domain)
> +		return 0;
> +
> +	imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie");
> +	if (IS_ERR(imx6_pcie->pd_pcie))
> +		return PTR_ERR(imx6_pcie->pd_pcie);
> +	link = device_link_add(dev, imx6_pcie->pd_pcie,
> +			DL_FLAG_STATELESS |
> +			DL_FLAG_PM_RUNTIME |
> +			DL_FLAG_RPM_ACTIVE);
> +	if (IS_ERR(link)) {
> +		dev_err(dev, "Failed to add device_link to pcie pd: %ld\n", PTR_ERR(link));
> +		return PTR_ERR(link);
> +	}
> +
> +	imx6_pcie->pd_pcie_phy = dev_pm_domain_attach_by_name(dev, "pcie_phy");
> +	if (IS_ERR(imx6_pcie->pd_pcie_phy))
> +		return PTR_ERR(imx6_pcie->pd_pcie_phy);
> +
> +	device_link_add(dev, imx6_pcie->pd_pcie_phy,
> +			DL_FLAG_STATELESS |
> +			DL_FLAG_PM_RUNTIME |
> +			DL_FLAG_RPM_ACTIVE);
> +	if (IS_ERR(link)) {
> +		dev_err(dev, "Failed to add device_link to pcie_phy pd: %ld\n", PTR_ERR(link));
> +		return PTR_ERR(link);
> +	}
> +
> +	return 0;
> +}
> +
>  static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  {
>  	struct device *dev = imx6_pcie->pci->dev;
>  
>  	switch (imx6_pcie->variant) {
> @@ -1013,10 +1057,14 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		imx6_pcie->vpcie = NULL;
>  	}
>  
>  	platform_set_drvdata(pdev, imx6_pcie);
>  
> +	ret = imx6_pcie_attach_pd(dev);
> +	if (ret)
> +		return ret;
> +
>  	ret = imx6_add_pcie_port(imx6_pcie, pdev);
>  	if (ret < 0)
>  		return ret;
>  
>  	return 0;
> -- 
> 2.17.1
> 

Re: [PATCH v2 3/4] PCI: imx: Add multi-pd support

[Shawn Guo]

On Fri, Nov 16, 2018 at 12:25:41PM +0000, Lorenzo Pieralisi wrote:
> On Mon, Oct 08, 2018 at 06:06:21PM +0000, Leonard Crestez wrote:
> > On some chips the PCIE and PCIE_PHY blocks are in separate power domains
> > which can be power-gated independently. The pci driver needs to handle
> > this by keeping both domain active.
> > 
> > This is intended for imx6sx where PCIE is in DISPLAY and PCIE_PHY in
> > it's own domain. Defining the DISPLAY domain requires a way for pcie to
> > keep it active or it will break when displays are off.
> > 
> > The power-domains on imx6sx are meant to look like this:
> > 	power-domains = <&pd_disp>, <&pd_pci>;
> > 	power-domain-names = "pcie", "pcie_phy";
> > 
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 48 +++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> 
> I expect Shawn to pick the whole series up and therefore I am dropping
> this series from the PCI tree.

Lorenzo,

I think the best approach is that you send patch #2 and #3 for 4.21
through PCI tree, and we will be able to apply patch #4 in 4.22
development cycle.

Queuing patch #3 on IMX tree will stands a good chance for conflicts
with other pci-imx6.c changes on PCI tree.

Shawn

[PATCH v2 4/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[Leonard Crestez]

This was implemented in the driver but not actually defined and
referenced in dts. This makes it always on.

From reference manual in section "10.4.1.4.1 Power Distribution":

"Display domain - The DISPLAY domain contains GIS, CSI, PXP, LCDIF,
PCIe, DCIC, and LDB. It is supplied by internal regulator."

The current pd_pcie is actually only for PCIE_PHY, the PCIE ip block is
actually inside the DISPLAY domain. Handle this by adding the pcie node
in both power domains.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm/boot/dts/imx6sx.dtsi | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 95a3c1cb877d..b7f815d82d1b 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -778,10 +778,22 @@
 						#power-domain-cells = <0>;
 						power-supply = <&reg_soc>;
 						clocks = <&clks IMX6SX_CLK_GPU>;
 					};
 
+					pd_disp: power-domain@2 {
+						reg = <2>;
+						#power-domain-cells = <0>;
+						clocks = <&clks IMX6SX_CLK_PXP_AXI>,
+							 <&clks IMX6SX_CLK_DISPLAY_AXI>,
+							 <&clks IMX6SX_CLK_LCDIF1_PIX>,
+							 <&clks IMX6SX_CLK_LCDIF_APB>,
+							 <&clks IMX6SX_CLK_LCDIF2_PIX>,
+							 <&clks IMX6SX_CLK_CSI>,
+							 <&clks IMX6SX_CLK_VADC>;
+					};
+
 					pd_pci: power-domain@3 {
 						reg = <3>;
 						#power-domain-cells = <0>;
 						power-supply = <&reg_pcie>;
 					};
@@ -1196,10 +1208,11 @@
 					reg = <0x02218000 0x4000>;
 					interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&clks IMX6SX_CLK_PXP_AXI>,
 						 <&clks IMX6SX_CLK_DISPLAY_AXI>;
 					clock-names = "pxp-axi", "disp-axi";
+					power-domains = <&pd_disp>;
 					status = "disabled";
 				};
 
 				csi2: csi@221c000 {
 					reg = <0x0221c000 0x4000>;
@@ -1217,10 +1230,11 @@
 					interrupts = <GIC_SPI 5 IRQ_TYPE_EDGE_RISING>;
 					clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,
 						 <&clks IMX6SX_CLK_LCDIF_APB>,
 						 <&clks IMX6SX_CLK_DISPLAY_AXI>;
 					clock-names = "pix", "axi", "disp_axi";
+					power-domains = <&pd_disp>;
 					status = "disabled";
 				};
 
 				lcdif2: lcdif@2224000 {
 					compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif";
@@ -1228,19 +1242,21 @@
 					interrupts = <GIC_SPI 6 IRQ_TYPE_EDGE_RISING>;
 					clocks = <&clks IMX6SX_CLK_LCDIF2_PIX>,
 						 <&clks IMX6SX_CLK_LCDIF_APB>,
 						 <&clks IMX6SX_CLK_DISPLAY_AXI>;
 					clock-names = "pix", "axi", "disp_axi";
+					power-domains = <&pd_disp>;
 					status = "disabled";
 				};
 
 				vadc: vadc@2228000 {
 					reg = <0x02228000 0x4000>, <0x0222c000 0x4000>;
 					reg-names = "vadc-vafe", "vadc-vdec";
 					clocks = <&clks IMX6SX_CLK_VADC>,
 						 <&clks IMX6SX_CLK_CSI>;
 					clock-names = "vadc", "csi";
+					power-domains = <&pd_disp>;
 					status = "disabled";
 				};
 			};
 
 			adc1: adc@2280000 {
@@ -1361,10 +1377,11 @@
 			clocks = <&clks IMX6SX_CLK_PCIE_AXI>,
 				 <&clks IMX6SX_CLK_LVDS1_OUT>,
 				 <&clks IMX6SX_CLK_PCIE_REF_125M>,
 				 <&clks IMX6SX_CLK_DISPLAY_AXI>;
 			clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_inbound_axi";
-			power-domains = <&pd_pci>;
+			power-domains = <&pd_disp>, <&pd_pci>;
+			power-domain-names = "pcie", "pcie_phy";
 			status = "disabled";
 		};
 	};
 };
-- 
2.17.1

Re: [PATCH v2 4/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[Shawn Guo]

On Mon, Oct 08, 2018 at 06:06:23PM +0000, Leonard Crestez wrote:
> This was implemented in the driver but not actually defined and
> referenced in dts. This makes it always on.
> 
> From reference manual in section "10.4.1.4.1 Power Distribution":
> 
> "Display domain - The DISPLAY domain contains GIS, CSI, PXP, LCDIF,
> PCIe, DCIC, and LDB. It is supplied by internal regulator."
> 
> The current pd_pcie is actually only for PCIE_PHY, the PCIE ip block is
> actually inside the DISPLAY domain. Handle this by adding the pcie node
> in both power domains.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

Applied, thanks.

Re: [PATCH v2 4/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[Leonard Crestez]

On 10/31/2018 8:12 AM, Shawn Guo wrote:
> On Mon, Oct 08, 2018 at 06:06:23PM +0000, Leonard Crestez wrote:
>> This was implemented in the driver but not actually defined and
>> referenced in dts. This makes it always on.
>>
>>  From reference manual in section "10.4.1.4.1 Power Distribution":
>>
>> "Display domain - The DISPLAY domain contains GIS, CSI, PXP, LCDIF,
>> PCIe, DCIC, and LDB. It is supplied by internal regulator."
>>
>> The current pd_pcie is actually only for PCIE_PHY, the PCIE ip block is
>> actually inside the DISPLAY domain. Handle this by adding the pcie node
>> in both power domains.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> 
> Applied, thanks.

As mentioned in the cover letter this requires multi-PD support in 
imx-pci to be implemented, specifically PATCH 3/4 of this series:

	https://lore.kernel.org/patchwork/patch/996810/

Unless that also gets merged soon via pci I expect issues in linux-next. 
The patch already has reviewed-by tags so "merging it soon" is not 
unreasonable.

Link to cover: https://lore.kernel.org/patchwork/cover/996807/

There is also an mxsfb dependency but that's already merged in 
torvalds/master.

Re: [PATCH v2 4/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[Lorenzo Pieralisi]

On Wed, Oct 31, 2018 at 12:17:50PM +0000, Leonard Crestez wrote:
> On 10/31/2018 8:12 AM, Shawn Guo wrote:
> > On Mon, Oct 08, 2018 at 06:06:23PM +0000, Leonard Crestez wrote:
> >> This was implemented in the driver but not actually defined and
> >> referenced in dts. This makes it always on.
> >>
> >>  From reference manual in section "10.4.1.4.1 Power Distribution":
> >>
> >> "Display domain - The DISPLAY domain contains GIS, CSI, PXP, LCDIF,
> >> PCIe, DCIC, and LDB. It is supplied by internal regulator."
> >>
> >> The current pd_pcie is actually only for PCIE_PHY, the PCIE ip block is
> >> actually inside the DISPLAY domain. Handle this by adding the pcie node
> >> in both power domains.
> >>
> >> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > 
> > Applied, thanks.
> 
> As mentioned in the cover letter this requires multi-PD support in 
> imx-pci to be implemented, specifically PATCH 3/4 of this series:
> 
> 	https://lore.kernel.org/patchwork/patch/996810/
> 
> Unless that also gets merged soon via pci I expect issues in linux-next. 

I do not know what you mean by "issues in linux-next" (I assume you mean
when Shawn sends the patches to linux-next in preparation for v4.21); we
have not planned any other PCI pull request for v4.20-rc1 material.

> The patch already has reviewed-by tags so "merging it soon" is not 
> unreasonable.

I can ACK it if Shawn wants to pull it, it may take a while to see it
in -next if it has to go through the PCI tree, I think it is better
to queue the series without splitting the patches across multiple
channels though.

Lorenzo

Re: [PATCH v2 4/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[Leonard Crestez]

On 10/31/2018 5:06 PM, Lorenzo Pieralisi wrote:
> On Wed, Oct 31, 2018 at 12:17:50PM +0000, Leonard Crestez wrote:
>> On 10/31/2018 8:12 AM, Shawn Guo wrote:
>>> On Mon, Oct 08, 2018 at 06:06:23PM +0000, Leonard Crestez wrote:
>>>> This was implemented in the driver but not actually defined and
>>>> referenced in dts. This makes it always on.
>>>>
>>>>   From reference manual in section "10.4.1.4.1 Power Distribution":
>>>>
>>>> "Display domain - The DISPLAY domain contains GIS, CSI, PXP, LCDIF,
>>>> PCIe, DCIC, and LDB. It is supplied by internal regulator."
>>>>
>>>> The current pd_pcie is actually only for PCIE_PHY, the PCIE ip block is
>>>> actually inside the DISPLAY domain. Handle this by adding the pcie node
>>>> in both power domains.
>>>>
>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>
>>> Applied, thanks.
>>
>> As mentioned in the cover letter this requires multi-PD support in
>> imx-pci to be implemented, specifically PATCH 3/4 of this series:
>>
>> Unless that also gets merged soon via pci I expect issues in linux-next.
> 
> I do not know what you mean by "issues in linux-next" (I assume you mean
> when Shawn sends the patches to linux-next in preparation for v4.21); we
> have not planned any other PCI pull request for v4.20-rc1 material.

Yes my concern is linux-next for 4.21

>> The patch already has reviewed-by tags so "merging it soon" is not
>> unreasonable.
> 
> I can ACK it if Shawn wants to pull it, it may take a while to see it
> in -next if it has to go through the PCI tree, I think it is better
> to queue the series without splitting the patches across multiple
> channels though.

This sounds great to me, merging the whole series through Shawn's imx 
tree would prevent issues in linux-next or possible bisect failures in 4.21.

Re: [PATCH v2 4/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[Shawn Guo]

On Wed, Oct 31, 2018 at 12:17:50PM +0000, Leonard Crestez wrote:
> On 10/31/2018 8:12 AM, Shawn Guo wrote:
> > On Mon, Oct 08, 2018 at 06:06:23PM +0000, Leonard Crestez wrote:
> >> This was implemented in the driver but not actually defined and
> >> referenced in dts. This makes it always on.
> >>
> >>  From reference manual in section "10.4.1.4.1 Power Distribution":
> >>
> >> "Display domain - The DISPLAY domain contains GIS, CSI, PXP, LCDIF,
> >> PCIe, DCIC, and LDB. It is supplied by internal regulator."
> >>
> >> The current pd_pcie is actually only for PCIE_PHY, the PCIE ip block is
> >> actually inside the DISPLAY domain. Handle this by adding the pcie node
> >> in both power domains.
> >>
> >> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > 
> > Applied, thanks.
> 
> As mentioned in the cover letter this requires multi-PD support in 
> imx-pci to be implemented, specifically PATCH 3/4 of this series:
> 
> 	https://lore.kernel.org/patchwork/patch/996810/
> 
> Unless that also gets merged soon via pci I expect issues in linux-next. 
> The patch already has reviewed-by tags so "merging it soon" is not 
> unreasonable.

Oops, I overlooked the notes in cover-letter.  Let's use the approach as
suggested there - applying the dts change after all driver dependencies
are landed.

Patch dropped, sorry.

Re: [PATCH v2 0/4] ARM: dts: imx6sx: Add DISPLAY power domain support

[Lorenzo Pieralisi]

On Mon, Oct 08, 2018 at 06:06:18PM +0000, Leonard Crestez wrote:
> Now that mxsfb has functional runtime_pm support we can enable power
> gating for the DISPLAY power domain on imx6sx.
> 
> Since pci-imx doesn't support runtime power gating this is still
> always-on unless PCI is disabled/unused. But it's reasonable for an imx
> board to not have PCI.
> 
> The current pd_pcie is actually only for PCIE_PHY, the PCIE ip block is
> actually inside the DISPLAY domain. This is handled by adding the pcie
> node in both power domains, using multi-pd support.
> 
> Series is against linux-next. The last patch needs runtime PM in all
> drivers or it will cause hangs. This means Shawn should hold back on it
> until the PCI and mxsfb dependencies get merged (both might take a long
> time). So you can just review that patch and I will resend it later,
> perhaps in another cycle.
> 
> ---
> 
> Changes since v1:
> * Document multi-pd in imx6q-pcie bindings and add DT list
> * Don't refer to "DISPLAY" as "DISPMIX" because the Reference Manual
> doesn't do that.
> * Link to v1: https://lore.kernel.org/patchwork/cover/994092/
> 
> This is independent of other imx6-pci PM patches such as this one:
> * https://lore.kernel.org/patchwork/patch/996806/
> Device links are quite nice!
> 
> Link to mxsfb thread: https://lkml.org/lkml/2018/9/26/1185
> 
> Leonard Crestez (4):
>   soc: imx: gpc: Increase GPC_CLK_MAX to 7
>   dt-bindings: imx6q-pcie: Add multi-pd bindings for imx6sx
>   PCI: imx: Add multi-pd support
>   ARM: dts: imx6sx: Add DISPLAY power domain support
> 
>  .../bindings/pci/fsl,imx6q-pcie.txt           |  4 +-
>  arch/arm/boot/dts/imx6sx.dtsi                 | 19 +++++++-
>  drivers/pci/controller/dwc/pci-imx6.c         | 48 +++++++++++++++++++
>  drivers/soc/imx/gpc.c                         |  2 +-
>  4 files changed, 70 insertions(+), 3 deletions(-)

As agreed with Shawn, I have applied patches 2-3 to pci/dwc for v4.21,
thanks.

Lorenzo