| Message ID | cover.1539020938.git.leonard.crestez@nxp.com |
|---|---|
| Headers | show |
| Series | ARM: dts: imx6sx: Add DISPLAY power domain support | expand |
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.
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.
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.
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
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.
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.
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 >
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
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