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

Message ID a3265a84052a7c11ae96e61e39adc988ca980d66.1539020938.git.leonard.crestez@nxp.com
State Not Applicable
Delegated to: Lorenzo Pieralisi
Headers show
Series
  • ARM: dts: imx6sx: Add DISPLAY power domain support
Related show

Commit Message

Leonard Crestez Oct. 8, 2018, 6:06 p.m.
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(-)

Comments

Shawn Guo Oct. 31, 2018, 6:11 a.m. | #1
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.
Leonard Crestez Oct. 31, 2018, 12:17 p.m. | #2
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.
Lorenzo Pieralisi Oct. 31, 2018, 3:05 p.m. | #3
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
Leonard Crestez Nov. 3, 2018, 3:54 p.m. | #4
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.
Shawn Guo Nov. 4, 2018, 3:54 a.m. | #5
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.

Patch

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";
 		};
 	};
 };