[V3,2/3] PCI: tegra: fixups to avoid unnecessary wakeup from ASPM-L1.2

Message ID 1510492674-12786-3-git-send-email-vidyas@nvidia.com
State New
Headers show
Series
  • Add ASPM-L1 Substates support for Tegra
Related show

Commit Message

Vidya Sagar Nov. 12, 2017, 1:17 p.m.
sets CLKREQ asserted delay to a higher value to avoid
unnecessary wake up from L1.2_ENTRY state for Tegra210

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* no change in this patch

V3:
* no change in this patch

 drivers/pci/host/pci-tegra.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Bjorn Helgaas Nov. 20, 2017, 9:30 p.m. | #1
s/PCI: tegra: fixups to avoid unnecessary .../
 /PCI: tegra: <Capitalized verb> .../

On Sun, Nov 12, 2017 at 06:47:53PM +0530, Vidya Sagar wrote:
> sets CLKREQ asserted delay to a higher value to avoid
> unnecessary wake up from L1.2_ENTRY state for Tegra210

"L1.2.Entry" to match the spec sec 5.5.3.1 (at least I assume that's
what this refers to).

> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * no change in this patch
> 
> V3:
> * no change in this patch
> 
>  drivers/pci/host/pci-tegra.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index 6d68f49f152e..29ee4bb0b7c6 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -204,6 +204,8 @@
>  #define RP_L1_PM_SUBSTATES_1_CTL			0xC04
>  #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
>  #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
> +#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK		(0x1FF << 13)
> +#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY		(0x27 << 13)
>  
>  #define RP_L1_PM_SUBSTATES_2_CTL			0xC08
>  #define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
> @@ -350,6 +352,7 @@ struct tegra_pcie_soc {
>  	bool program_deskew_time;
>  	bool updateFC_threshold;
>  	bool has_aspm_l1ss;
> +	bool l1ss_rp_wake_fixup;
>  };
>  
>  static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
> @@ -2290,6 +2293,16 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
>  			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
>  		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
>  
> +		if (soc->l1ss_rp_wake_fixup) {
> +			/* Set CLKREQ asserted delay greater than Power_Off
> +			 * time (2us) to avoid RP wakeup in L1.2_ENTRY
> +			 */

Use standard multi-line comment formatting (and "L1.2.Entry" as above).

I assume "CLKREQ asserted delay" is a Tegra-internal thing, since it
doesn't obviously correspond to a timing parameter in the spec.

And I assume it doesn't depend on any of the OS-writable things in the
L1 PM Substates control registers, since this fixup isn't executed
during config writes to those registers?

Does this depend on any circuit details outside of Tegra, i.e., should
it be described via DT?

> +			value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> +			value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
> +			value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
> +			writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
> +		}
> +
>  		/* Following is based on clk_m being 19.2 MHz */
>  		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
>  		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
> @@ -2446,6 +2459,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
>  	.has_aspm_l1ss = false,
> +	.l1ss_rp_wake_fixup = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra30_pcie = {
> @@ -2468,6 +2482,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
>  	.has_aspm_l1ss = false,
> +	.l1ss_rp_wake_fixup = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra124_pcie = {
> @@ -2489,6 +2504,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
>  	.has_aspm_l1ss = false,
> +	.l1ss_rp_wake_fixup = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra210_pcie = {
> @@ -2518,6 +2534,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
>  	.program_deskew_time = true,
>  	.updateFC_threshold = true,
>  	.has_aspm_l1ss = true,
> +	.l1ss_rp_wake_fixup = true,
>  };
>  
>  static const struct tegra_pcie_soc tegra186_pcie = {
> @@ -2540,6 +2557,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
>  	.has_aspm_l1ss = true,
> +	.l1ss_rp_wake_fixup = false,
>  };
>  
>  static const struct of_device_id tegra_pcie_of_match[] = {
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lorenzo Pieralisi Nov. 21, 2017, 3:08 p.m. | #2
On Mon, Nov 20, 2017 at 03:30:52PM -0600, Bjorn Helgaas wrote:

[...]

> >  static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
> > @@ -2290,6 +2293,16 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
> >  			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
> >  		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
> >  
> > +		if (soc->l1ss_rp_wake_fixup) {
> > +			/* Set CLKREQ asserted delay greater than Power_Off
> > +			 * time (2us) to avoid RP wakeup in L1.2_ENTRY
> > +			 */
> 
> Use standard multi-line comment formatting (and "L1.2.Entry" as above).
> 
> I assume "CLKREQ asserted delay" is a Tegra-internal thing, since it
> doesn't obviously correspond to a timing parameter in the spec.
> 
> And I assume it doesn't depend on any of the OS-writable things in the
> L1 PM Substates control registers, since this fixup isn't executed
> during config writes to those registers?
> 
> Does this depend on any circuit details outside of Tegra, i.e., should
> it be described via DT?

Yes, I have noticed too that there are lots of hardcoded values in this
driver that may benefit from some Tegra DT bindings instead of
hardcoding everything in the kernel.

Thanks,
Lorenzo

> > +			value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> > +			value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
> > +			value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
> > +			writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
> > +		}
> > +
> >  		/* Following is based on clk_m being 19.2 MHz */
> >  		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> >  		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
> > @@ -2446,6 +2459,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
> >  	.program_deskew_time = false,
> >  	.updateFC_threshold = false,
> >  	.has_aspm_l1ss = false,
> > +	.l1ss_rp_wake_fixup = false,
> >  };
> >  
> >  static const struct tegra_pcie_soc tegra30_pcie = {
> > @@ -2468,6 +2482,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
> >  	.program_deskew_time = false,
> >  	.updateFC_threshold = false,
> >  	.has_aspm_l1ss = false,
> > +	.l1ss_rp_wake_fixup = false,
> >  };
> >  
> >  static const struct tegra_pcie_soc tegra124_pcie = {
> > @@ -2489,6 +2504,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
> >  	.program_deskew_time = false,
> >  	.updateFC_threshold = false,
> >  	.has_aspm_l1ss = false,
> > +	.l1ss_rp_wake_fixup = false,
> >  };
> >  
> >  static const struct tegra_pcie_soc tegra210_pcie = {
> > @@ -2518,6 +2534,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
> >  	.program_deskew_time = true,
> >  	.updateFC_threshold = true,
> >  	.has_aspm_l1ss = true,
> > +	.l1ss_rp_wake_fixup = true,
> >  };
> >  
> >  static const struct tegra_pcie_soc tegra186_pcie = {
> > @@ -2540,6 +2557,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
> >  	.program_deskew_time = false,
> >  	.updateFC_threshold = false,
> >  	.has_aspm_l1ss = true,
> > +	.l1ss_rp_wake_fixup = false,
> >  };
> >  
> >  static const struct of_device_id tegra_pcie_of_match[] = {
> > -- 
> > 2.7.4
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vidya Sagar Dec. 14, 2017, 4:32 p.m. | #3
On Tuesday 21 November 2017 03:00 AM, Bjorn Helgaas wrote:
> s/PCI: tegra: fixups to avoid unnecessary .../
>   /PCI: tegra: <Capitalized verb> .../
I'll take care of this in next patch
> On Sun, Nov 12, 2017 at 06:47:53PM +0530, Vidya Sagar wrote:
>> sets CLKREQ asserted delay to a higher value to avoid
>> unnecessary wake up from L1.2_ENTRY state for Tegra210
> "L1.2.Entry" to match the spec sec 5.5.3.1 (at least I assume that's
> what this refers to).
I'll take care of this in next patch
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> V2:
>> * no change in this patch
>>
>> V3:
>> * no change in this patch
>>
>>   drivers/pci/host/pci-tegra.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>> index 6d68f49f152e..29ee4bb0b7c6 100644
>> --- a/drivers/pci/host/pci-tegra.c
>> +++ b/drivers/pci/host/pci-tegra.c
>> @@ -204,6 +204,8 @@
>>   #define RP_L1_PM_SUBSTATES_1_CTL			0xC04
>>   #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
>>   #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
>> +#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK		(0x1FF << 13)
>> +#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY		(0x27 << 13)
>>   
>>   #define RP_L1_PM_SUBSTATES_2_CTL			0xC08
>>   #define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
>> @@ -350,6 +352,7 @@ struct tegra_pcie_soc {
>>   	bool program_deskew_time;
>>   	bool updateFC_threshold;
>>   	bool has_aspm_l1ss;
>> +	bool l1ss_rp_wake_fixup;
>>   };
>>   
>>   static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
>> @@ -2290,6 +2293,16 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
>>   			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
>>   		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
>>   
>> +		if (soc->l1ss_rp_wake_fixup) {
>> +			/* Set CLKREQ asserted delay greater than Power_Off
>> +			 * time (2us) to avoid RP wakeup in L1.2_ENTRY
>> +			 */
> Use standard multi-line comment formatting (and "L1.2.Entry" as above).
I'll take care of it in next patch
> I assume "CLKREQ asserted delay" is a Tegra-internal thing, since it
> doesn't obviously correspond to a timing parameter in the spec.
Yes. Its Tegra's internal thing
> And I assume it doesn't depend on any of the OS-writable things in the
> L1 PM Substates control registers, since this fixup isn't executed
> during config writes to those registers?
Yes. It doesn't depend on any OS-writable value in L1 PM Substates 
control registers.
Its a one time programmable value to be written before root port's LTSSM 
starts.
> Does this depend on any circuit details outside of Tegra, i.e., should
> it be described via DT?	
Nope. It doesn't depend on any external (to Tegra) circuitry.
>> +			value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
>> +			value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
>> +			value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
>> +			writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
>> +		}
>> +
>>   		/* Following is based on clk_m being 19.2 MHz */
>>   		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
>>   		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
>> @@ -2446,6 +2459,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>>   	.has_aspm_l1ss = false,
>> +	.l1ss_rp_wake_fixup = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra30_pcie = {
>> @@ -2468,6 +2482,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>>   	.has_aspm_l1ss = false,
>> +	.l1ss_rp_wake_fixup = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra124_pcie = {
>> @@ -2489,6 +2504,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>>   	.has_aspm_l1ss = false,
>> +	.l1ss_rp_wake_fixup = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra210_pcie = {
>> @@ -2518,6 +2534,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
>>   	.program_deskew_time = true,
>>   	.updateFC_threshold = true,
>>   	.has_aspm_l1ss = true,
>> +	.l1ss_rp_wake_fixup = true,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra186_pcie = {
>> @@ -2540,6 +2557,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>>   	.has_aspm_l1ss = true,
>> +	.l1ss_rp_wake_fixup = false,
>>   };
>>   
>>   static const struct of_device_id tegra_pcie_of_match[] = {
>> -- 
>> 2.7.4
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 6d68f49f152e..29ee4bb0b7c6 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -204,6 +204,8 @@ 
 #define RP_L1_PM_SUBSTATES_1_CTL			0xC04
 #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
 #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
+#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK		(0x1FF << 13)
+#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY		(0x27 << 13)
 
 #define RP_L1_PM_SUBSTATES_2_CTL			0xC08
 #define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
@@ -350,6 +352,7 @@  struct tegra_pcie_soc {
 	bool program_deskew_time;
 	bool updateFC_threshold;
 	bool has_aspm_l1ss;
+	bool l1ss_rp_wake_fixup;
 };
 
 static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
@@ -2290,6 +2293,16 @@  static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
 			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
 		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
 
+		if (soc->l1ss_rp_wake_fixup) {
+			/* Set CLKREQ asserted delay greater than Power_Off
+			 * time (2us) to avoid RP wakeup in L1.2_ENTRY
+			 */
+			value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
+			value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
+			value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
+			writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
+		}
+
 		/* Following is based on clk_m being 19.2 MHz */
 		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
 		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
@@ -2446,6 +2459,7 @@  static const struct tegra_pcie_soc tegra20_pcie = {
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
 	.has_aspm_l1ss = false,
+	.l1ss_rp_wake_fixup = false,
 };
 
 static const struct tegra_pcie_soc tegra30_pcie = {
@@ -2468,6 +2482,7 @@  static const struct tegra_pcie_soc tegra30_pcie = {
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
 	.has_aspm_l1ss = false,
+	.l1ss_rp_wake_fixup = false,
 };
 
 static const struct tegra_pcie_soc tegra124_pcie = {
@@ -2489,6 +2504,7 @@  static const struct tegra_pcie_soc tegra124_pcie = {
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
 	.has_aspm_l1ss = false,
+	.l1ss_rp_wake_fixup = false,
 };
 
 static const struct tegra_pcie_soc tegra210_pcie = {
@@ -2518,6 +2534,7 @@  static const struct tegra_pcie_soc tegra210_pcie = {
 	.program_deskew_time = true,
 	.updateFC_threshold = true,
 	.has_aspm_l1ss = true,
+	.l1ss_rp_wake_fixup = true,
 };
 
 static const struct tegra_pcie_soc tegra186_pcie = {
@@ -2540,6 +2557,7 @@  static const struct tegra_pcie_soc tegra186_pcie = {
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
 	.has_aspm_l1ss = true,
+	.l1ss_rp_wake_fixup = false,
 };
 
 static const struct of_device_id tegra_pcie_of_match[] = {