[PATCH V3] ARM: tegra: disable LP2 cpuidle state if PCIe is enabled

[PATCH V3] ARM: tegra: disable LP2 cpuidle state if PCIe is enabled

[Stephen Warren]

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
this, simply disable LP2 if the PCI driver and DT node are both enabled.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
I intend to apply this on top of Thierry's Tegra PCIe driver series.

v3: Only disable LP2 if a PCIe device that requests an interrupt is
actually present. This prevents the loss of power-saving when the WAR
isn't required.
---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 12 ++++++++++++
 arch/arm/mach-tegra/cpuidle.c         | 10 ++++++++++
 arch/arm/mach-tegra/cpuidle.h         |  1 +
 drivers/pci/host/pci-tegra.c          |  5 +++++
 4 files changed, 28 insertions(+)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 706aa42..b82dcae 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -211,6 +211,18 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 }
 #endif
 
+/*
+ * Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
+ * they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
+ * this, simply disable LP2 if the PCI driver and DT node are both enabled.
+ */
+void tegra20_cpuidle_pcie_irqs_in_use(void)
+{
+	pr_info_once(
+		"Disabling cpuidle LP2 state, since PCIe IRQs are in use\n");
+	tegra_idle_driver.states[1].disabled = true;
+}
+
 int __init tegra20_cpuidle_init(void)
 {
 	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
index e85973c..0961dfc 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle.c
@@ -44,3 +44,13 @@ void __init tegra_cpuidle_init(void)
 		break;
 	}
 }
+
+void tegra_cpuidle_pcie_irqs_in_use(void)
+{
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
+			tegra20_cpuidle_pcie_irqs_in_use();
+		break;
+	}
+}
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
index 9ec2c1a..c017dab 100644
--- a/arch/arm/mach-tegra/cpuidle.h
+++ b/arch/arm/mach-tegra/cpuidle.h
@@ -19,6 +19,7 @@
 
 #ifdef CONFIG_CPU_IDLE
 int tegra20_cpuidle_init(void);
+void tegra20_cpuidle_pcie_irqs_in_use(void);
 int tegra30_cpuidle_init(void);
 int tegra114_cpuidle_init(void);
 void tegra_cpuidle_init(void);
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index ad95c40..7356741 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -41,6 +41,7 @@
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/tegra-cpuidle.h>
 #include <linux/tegra-powergate.h>
 #include <linux/vmalloc.h>
 #include <linux/regulator/consumer.h>
@@ -636,6 +637,8 @@ static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
 	struct tegra_pcie *pcie = sys_to_pcie(pdev->bus->sysdata);
 
+	tegra_cpuidle_pcie_irqs_in_use();
+
 	return pcie->irq;
 }
 
@@ -1221,6 +1224,8 @@ static int tegra_msi_map(struct irq_domain *domain, unsigned int irq,
 	irq_set_chip_data(irq, domain->host_data);
 	set_irq_flags(irq, IRQF_VALID);
 
+	tegra_cpuidle_pcie_irqs_in_use();
+
 	return 0;
 }
 
-- 
1.8.1.5

[PATCH V3] ARM: tegra: disable LP2 cpuidle state if PCIe is enabled

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
this, simply disable LP2 if the PCI driver and DT node are both enabled.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
I intend to apply this on top of Thierry's Tegra PCIe driver series.

v3: Only disable LP2 if a PCIe device that requests an interrupt is
actually present. This prevents the loss of power-saving when the WAR
isn't required.
---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 12 ++++++++++++
 arch/arm/mach-tegra/cpuidle.c         | 10 ++++++++++
 arch/arm/mach-tegra/cpuidle.h         |  1 +
 drivers/pci/host/pci-tegra.c          |  5 +++++
 4 files changed, 28 insertions(+)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 706aa42..b82dcae 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -211,6 +211,18 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 }
 #endif
 
+/*
+ * Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
+ * they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
+ * this, simply disable LP2 if the PCI driver and DT node are both enabled.
+ */
+void tegra20_cpuidle_pcie_irqs_in_use(void)
+{
+	pr_info_once(
+		"Disabling cpuidle LP2 state, since PCIe IRQs are in use\n");
+	tegra_idle_driver.states[1].disabled = true;
+}
+
 int __init tegra20_cpuidle_init(void)
 {
 	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
index e85973c..0961dfc 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle.c
@@ -44,3 +44,13 @@ void __init tegra_cpuidle_init(void)
 		break;
 	}
 }
+
+void tegra_cpuidle_pcie_irqs_in_use(void)
+{
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
+			tegra20_cpuidle_pcie_irqs_in_use();
+		break;
+	}
+}
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
index 9ec2c1a..c017dab 100644
--- a/arch/arm/mach-tegra/cpuidle.h
+++ b/arch/arm/mach-tegra/cpuidle.h
@@ -19,6 +19,7 @@
 
 #ifdef CONFIG_CPU_IDLE
 int tegra20_cpuidle_init(void);
+void tegra20_cpuidle_pcie_irqs_in_use(void);
 int tegra30_cpuidle_init(void);
 int tegra114_cpuidle_init(void);
 void tegra_cpuidle_init(void);
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index ad95c40..7356741 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -41,6 +41,7 @@
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/tegra-cpuidle.h>
 #include <linux/tegra-powergate.h>
 #include <linux/vmalloc.h>
 #include <linux/regulator/consumer.h>
@@ -636,6 +637,8 @@ static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
 	struct tegra_pcie *pcie = sys_to_pcie(pdev->bus->sysdata);
 
+	tegra_cpuidle_pcie_irqs_in_use();
+
 	return pcie->irq;
 }
 
@@ -1221,6 +1224,8 @@ static int tegra_msi_map(struct irq_domain *domain, unsigned int irq,
 	irq_set_chip_data(irq, domain->host_data);
 	set_irq_flags(irq, IRQF_VALID);
 
+	tegra_cpuidle_pcie_irqs_in_use();
+
 	return 0;
 }
 
-- 
1.8.1.5

[PATCH V3] ARM: tegra: disable LP2 cpuidle state if PCIe is enabled

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
this, simply disable LP2 if the PCI driver and DT node are both enabled.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
I intend to apply this on top of Thierry's Tegra PCIe driver series.

v3: Only disable LP2 if a PCIe device that requests an interrupt is
actually present. This prevents the loss of power-saving when the WAR
isn't required.
---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 12 ++++++++++++
 arch/arm/mach-tegra/cpuidle.c         | 10 ++++++++++
 arch/arm/mach-tegra/cpuidle.h         |  1 +
 drivers/pci/host/pci-tegra.c          |  5 +++++
 4 files changed, 28 insertions(+)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 706aa42..b82dcae 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -211,6 +211,18 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 }
 #endif
 
+/*
+ * Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
+ * they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
+ * this, simply disable LP2 if the PCI driver and DT node are both enabled.
+ */
+void tegra20_cpuidle_pcie_irqs_in_use(void)
+{
+	pr_info_once(
+		"Disabling cpuidle LP2 state, since PCIe IRQs are in use\n");
+	tegra_idle_driver.states[1].disabled = true;
+}
+
 int __init tegra20_cpuidle_init(void)
 {
 	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
index e85973c..0961dfc 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle.c
@@ -44,3 +44,13 @@ void __init tegra_cpuidle_init(void)
 		break;
 	}
 }
+
+void tegra_cpuidle_pcie_irqs_in_use(void)
+{
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
+			tegra20_cpuidle_pcie_irqs_in_use();
+		break;
+	}
+}
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
index 9ec2c1a..c017dab 100644
--- a/arch/arm/mach-tegra/cpuidle.h
+++ b/arch/arm/mach-tegra/cpuidle.h
@@ -19,6 +19,7 @@
 
 #ifdef CONFIG_CPU_IDLE
 int tegra20_cpuidle_init(void);
+void tegra20_cpuidle_pcie_irqs_in_use(void);
 int tegra30_cpuidle_init(void);
 int tegra114_cpuidle_init(void);
 void tegra_cpuidle_init(void);
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index ad95c40..7356741 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -41,6 +41,7 @@
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/tegra-cpuidle.h>
 #include <linux/tegra-powergate.h>
 #include <linux/vmalloc.h>
 #include <linux/regulator/consumer.h>
@@ -636,6 +637,8 @@ static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
 	struct tegra_pcie *pcie = sys_to_pcie(pdev->bus->sysdata);
 
+	tegra_cpuidle_pcie_irqs_in_use();
+
 	return pcie->irq;
 }
 
@@ -1221,6 +1224,8 @@ static int tegra_msi_map(struct irq_domain *domain, unsigned int irq,
 	irq_set_chip_data(irq, domain->host_data);
 	set_irq_flags(irq, IRQF_VALID);
 
+	tegra_cpuidle_pcie_irqs_in_use();
+
 	return 0;
 }
 
-- 
1.8.1.5

Re: [PATCH V3] ARM: tegra: disable LP2 cpuidle state if PCIe is enabled

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2556 bytes --]

On Fri, Aug 09, 2013 at 02:04:16PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
> they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
> this, simply disable LP2 if the PCI driver and DT node are both enabled.

This description is no longer accurate.

> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> I intend to apply this on top of Thierry's Tegra PCIe driver series.
> 
> v3: Only disable LP2 if a PCIe device that requests an interrupt is
> actually present. This prevents the loss of power-saving when the WAR
> isn't required.

Currently I see the informational message about LP2 being disabled
rather early in PCI setup. That's probably caused by the .map_irq()
being called during enumeration/fixup. So effectively this disables LP2
as soon as a PCI device is found, whether it needs the IRQ or not. But I
don't think we can do any better than this, so that's fine.

[...]
> diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
[...]
> +void tegra_cpuidle_pcie_irqs_in_use(void)
> +{
> +	switch (tegra_chip_id) {
> +	case TEGRA20:
> +		if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> +			tegra20_cpuidle_pcie_irqs_in_use();
> +		break;
> +	}
> +}

I plan to eventually (3.13 perhaps) support building the pci-tegra
driver as a module since most or all prerequisites are now in place, so
we'll need to export this function eventually. But we can do that when
we actually need it. Perhaps somebody will find a better fix for the LP2
problem until then.

> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
[...]
> @@ -41,6 +41,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> +#include <linux/tegra-cpuidle.h>

That file doesn't exist in any tree I have. Perhaps it's a new file and
you forgot to add it in this commit?

I've added that file locally to test the patch and verified that it
works around the issue on TrimSlice while not causing any regressions on
Beaver (it doesn't run any code on !ARCH_TEGRA_2x_SOC anyway, but better
safe than sorry).

So with the missing header file added:

Tested-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

And with an updated commit message:

Acked-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V3] ARM: tegra: disable LP2 cpuidle state if PCIe is enabled

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2440 bytes --]

On Fri, Aug 09, 2013 at 02:04:16PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
> they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
> this, simply disable LP2 if the PCI driver and DT node are both enabled.

This description is no longer accurate.

> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> I intend to apply this on top of Thierry's Tegra PCIe driver series.
> 
> v3: Only disable LP2 if a PCIe device that requests an interrupt is
> actually present. This prevents the loss of power-saving when the WAR
> isn't required.

Currently I see the informational message about LP2 being disabled
rather early in PCI setup. That's probably caused by the .map_irq()
being called during enumeration/fixup. So effectively this disables LP2
as soon as a PCI device is found, whether it needs the IRQ or not. But I
don't think we can do any better than this, so that's fine.

[...]
> diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
[...]
> +void tegra_cpuidle_pcie_irqs_in_use(void)
> +{
> +	switch (tegra_chip_id) {
> +	case TEGRA20:
> +		if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> +			tegra20_cpuidle_pcie_irqs_in_use();
> +		break;
> +	}
> +}

I plan to eventually (3.13 perhaps) support building the pci-tegra
driver as a module since most or all prerequisites are now in place, so
we'll need to export this function eventually. But we can do that when
we actually need it. Perhaps somebody will find a better fix for the LP2
problem until then.

> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
[...]
> @@ -41,6 +41,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> +#include <linux/tegra-cpuidle.h>

That file doesn't exist in any tree I have. Perhaps it's a new file and
you forgot to add it in this commit?

I've added that file locally to test the patch and verified that it
works around the issue on TrimSlice while not causing any regressions on
Beaver (it doesn't run any code on !ARCH_TEGRA_2x_SOC anyway, but better
safe than sorry).

So with the missing header file added:

Tested-by: Thierry Reding <treding@nvidia.com>

And with an updated commit message:

Acked-by: Thierry Reding <treding@nvidia.com>

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH V3] ARM: tegra: disable LP2 cpuidle state if PCIe is enabled

[Thierry Reding]

On Fri, Aug 09, 2013 at 02:04:16PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
> they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
> this, simply disable LP2 if the PCI driver and DT node are both enabled.

This description is no longer accurate.

> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> I intend to apply this on top of Thierry's Tegra PCIe driver series.
> 
> v3: Only disable LP2 if a PCIe device that requests an interrupt is
> actually present. This prevents the loss of power-saving when the WAR
> isn't required.

Currently I see the informational message about LP2 being disabled
rather early in PCI setup. That's probably caused by the .map_irq()
being called during enumeration/fixup. So effectively this disables LP2
as soon as a PCI device is found, whether it needs the IRQ or not. But I
don't think we can do any better than this, so that's fine.

[...]
> diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
[...]
> +void tegra_cpuidle_pcie_irqs_in_use(void)
> +{
> +	switch (tegra_chip_id) {
> +	case TEGRA20:
> +		if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> +			tegra20_cpuidle_pcie_irqs_in_use();
> +		break;
> +	}
> +}

I plan to eventually (3.13 perhaps) support building the pci-tegra
driver as a module since most or all prerequisites are now in place, so
we'll need to export this function eventually. But we can do that when
we actually need it. Perhaps somebody will find a better fix for the LP2
problem until then.

> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
[...]
> @@ -41,6 +41,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> +#include <linux/tegra-cpuidle.h>

That file doesn't exist in any tree I have. Perhaps it's a new file and
you forgot to add it in this commit?

I've added that file locally to test the patch and verified that it
works around the issue on TrimSlice while not causing any regressions on
Beaver (it doesn't run any code on !ARCH_TEGRA_2x_SOC anyway, but better
safe than sorry).

So with the missing header file added:

Tested-by: Thierry Reding <treding@nvidia.com>

And with an updated commit message:

Acked-by: Thierry Reding <treding@nvidia.com>

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130812/d8727962/attachment.sig>