1. Patchew
  2. linux
  3. View series

[PATCH] platform/x86:intel/pmc: Ignore all LTRs during suspend

Xi Pardee posted 1 patch 1 year, 6 months ago
There is a newer version of this series
drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++
drivers/platform/x86/intel/pmc/core.h |  2 +
2 files changed, 55 insertions(+)
[PATCH] platform/x86:intel/pmc: Ignore all LTRs during suspend
Posted by Xi Pardee 1 year, 6 months ago 
From: Xi Pardee <xi.pardee@intel.com>

Add support to ignore all LTRs before suspend and restore the previous
LTR values after suspend. This feature could be turned off with module
parameter ltr_ignore_all_suspend.

Suggested-by: Rafael J. Wysocki<rafael.j.wysocki@intel.com>
Signed-off-by: Xi Pardee <xi.pardee@intel.com>
---
 drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++
 drivers/platform/x86/intel/pmc/core.h |  2 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 01ae71c6df59..f364a011721d 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker);
 
+static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) {
+		struct pmc *pmc;
+		u32 ltr_ign;
+
+		pmc = pmcdev->pmcs[i];
+		if (!pmc)
+			continue;
+
+		guard(mutex)(&pmcdev->lock);
+		pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset);
+
+		/* ltr_ignore_max is the max index value for ltr ignore register */
+		ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0);
+		pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign);
+	}
+
+	/*
+	 * Ignoring ME during suspend is blocking platforms with ADL PCH to get to
+	 * deeper S0IX substate.
+	 */
+	pmc_core_send_ltr_ignore(pmcdev, 6, 0);
+}
+
+static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) {
+		struct pmc *pmc;
+
+		pmc = pmcdev->pmcs[i];
+		if (!pmc)
+			continue;
+
+		guard(mutex)(&pmcdev->lock);
+		pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign);
+	}
+}
+
 static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset,
 				       const int lpm_adj_x2)
 {
@@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures;
 module_param(warn_on_s0ix_failures, bool, 0644);
 MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures");
 
+static bool ltr_ignore_all_suspend = true;
+module_param(ltr_ignore_all_suspend, bool, 0644);
+MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend");
+
 static __maybe_unused int pmc_core_suspend(struct device *dev)
 {
 	struct pmc_dev *pmcdev = dev_get_drvdata(dev);
@@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev)
 	if (pmcdev->suspend)
 		pmcdev->suspend(pmcdev);
 
+	if (ltr_ignore_all_suspend)
+		pmc_core_ltr_ignore_all(pmcdev);
+
 	/* Check if the syspend will actually use S0ix */
 	if (pm_suspend_via_firmware())
 		return 0;
@@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev)
 {
 	struct pmc_dev *pmcdev = dev_get_drvdata(dev);
 
+	if (ltr_ignore_all_suspend)
+		pmc_core_ltr_restore_all(pmcdev);
+
 	if (pmcdev->resume)
 		return pmcdev->resume(pmcdev);
 
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index ea04de7eb9e8..e862ea88b816 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -372,6 +372,7 @@ struct pmc_info {
  * @map:		pointer to pmc_reg_map struct that contains platform
  *			specific attributes
  * @lpm_req_regs:	List of substate requirements
+ * @ltr_ign:		Holds LTR ignore data while suspended
  *
  * pmc contains info about one power management controller device.
  */
@@ -380,6 +381,7 @@ struct pmc {
 	void __iomem *regbase;
 	const struct pmc_reg_map *map;
 	u32 *lpm_req_regs;
+	u32 ltr_ign;
 };
 
 /**
-- 
2.43.0
Re: [PATCH] platform/x86:intel/pmc: Ignore all LTRs during suspend
Posted by Ilpo Järvinen 1 year, 5 months ago 
On Thu, 8 Aug 2024, Xi Pardee wrote:

> From: Xi Pardee <xi.pardee@intel.com>
> 
> Add support to ignore all LTRs before suspend and restore the previous
> LTR values after suspend. This feature could be turned off with module
> parameter ltr_ignore_all_suspend.

This does not answer why? question.

> Suggested-by: Rafael J. Wysocki<rafael.j.wysocki@intel.com>

Please add a space between the name and email.

This patch seems to have fallen through cracks and did not appear in the 
patchwork at all.

> Signed-off-by: Xi Pardee <xi.pardee@intel.com>
> ---
>  drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++
>  drivers/platform/x86/intel/pmc/core.h |  2 +
>  2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 01ae71c6df59..f364a011721d 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused)
>  }
>  DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker);
>  
> +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) {
> +		struct pmc *pmc;
> +		u32 ltr_ign;
> +
> +		pmc = pmcdev->pmcs[i];
> +		if (!pmc)
> +			continue;
> +
> +		guard(mutex)(&pmcdev->lock);
> +		pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset);
> +
> +		/* ltr_ignore_max is the max index value for ltr ignore register */

ltr -> LTR

> +		ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0);
> +		pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign);
> +	}
> +
> +	/*
> +	 * Ignoring ME during suspend is blocking platforms with ADL PCH to get to
> +	 * deeper S0IX substate.

S0ix

--
 i.

> +	 */
> +	pmc_core_send_ltr_ignore(pmcdev, 6, 0);
> +}
> +
> +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) {
> +		struct pmc *pmc;
> +
> +		pmc = pmcdev->pmcs[i];
> +		if (!pmc)
> +			continue;
> +
> +		guard(mutex)(&pmcdev->lock);
> +		pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign);
> +	}
> +}
> +
>  static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset,
>  				       const int lpm_adj_x2)
>  {
> @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures;
>  module_param(warn_on_s0ix_failures, bool, 0644);
>  MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures");
>  
> +static bool ltr_ignore_all_suspend = true;
> +module_param(ltr_ignore_all_suspend, bool, 0644);
> +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend");
> +
>  static __maybe_unused int pmc_core_suspend(struct device *dev)
>  {
>  	struct pmc_dev *pmcdev = dev_get_drvdata(dev);
> @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev)
>  	if (pmcdev->suspend)
>  		pmcdev->suspend(pmcdev);
>  
> +	if (ltr_ignore_all_suspend)
> +		pmc_core_ltr_ignore_all(pmcdev);
> +
>  	/* Check if the syspend will actually use S0ix */
>  	if (pm_suspend_via_firmware())
>  		return 0;
> @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev)
>  {
>  	struct pmc_dev *pmcdev = dev_get_drvdata(dev);
>  
> +	if (ltr_ignore_all_suspend)
> +		pmc_core_ltr_restore_all(pmcdev);
> +
>  	if (pmcdev->resume)
>  		return pmcdev->resume(pmcdev);
>  
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index ea04de7eb9e8..e862ea88b816 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -372,6 +372,7 @@ struct pmc_info {
>   * @map:		pointer to pmc_reg_map struct that contains platform
>   *			specific attributes
>   * @lpm_req_regs:	List of substate requirements
> + * @ltr_ign:		Holds LTR ignore data while suspended
>   *
>   * pmc contains info about one power management controller device.
>   */
> @@ -380,6 +381,7 @@ struct pmc {
>  	void __iomem *regbase;
>  	const struct pmc_reg_map *map;
>  	u32 *lpm_req_regs;
> +	u32 ltr_ign;
>  };
>  
>  /**
>
Re: [PATCH] platform/x86:intel/pmc: Ignore all LTRs during suspend
Posted by Xi Pardee 1 year, 5 months ago 
Hi,

On 8/30/2024 2:53 AM, Ilpo Järvinen wrote:
> On Thu, 8 Aug 2024, Xi Pardee wrote:
>
>> From: Xi Pardee <xi.pardee@intel.com>
>>
>> Add support to ignore all LTRs before suspend and restore the previous
>> LTR values after suspend. This feature could be turned off with module
>> parameter ltr_ignore_all_suspend.
> This does not answer why? question.

I will add the reason to commit message in next version.

>
>> Suggested-by: Rafael J. Wysocki<rafael.j.wysocki@intel.com>
> Please add a space between the name and email.
>
> This patch seems to have fallen through cracks and did not appear in the
> patchwork at all.
Will add a space between the name and email.
>
>> Signed-off-by: Xi Pardee <xi.pardee@intel.com>
>> ---
>>   drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++
>>   drivers/platform/x86/intel/pmc/core.h |  2 +
>>   2 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
>> index 01ae71c6df59..f364a011721d 100644
>> --- a/drivers/platform/x86/intel/pmc/core.c
>> +++ b/drivers/platform/x86/intel/pmc/core.c
>> @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused)
>>   }
>>   DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker);
>>   
>> +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev)
>> +{
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) {
>> +		struct pmc *pmc;
>> +		u32 ltr_ign;
>> +
>> +		pmc = pmcdev->pmcs[i];
>> +		if (!pmc)
>> +			continue;
>> +
>> +		guard(mutex)(&pmcdev->lock);
>> +		pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset);
>> +
>> +		/* ltr_ignore_max is the max index value for ltr ignore register */
> ltr -> LTR
Will change it in next version.
>
>> +		ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0);
>> +		pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign);
>> +	}
>> +
>> +	/*
>> +	 * Ignoring ME during suspend is blocking platforms with ADL PCH to get to
>> +	 * deeper S0IX substate.
> S0ix
>
> --
>   i.

Will change it in next version.

Thanks!

Xi

>
>> +	 */
>> +	pmc_core_send_ltr_ignore(pmcdev, 6, 0);
>> +}
>> +
>> +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev)
>> +{
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) {
>> +		struct pmc *pmc;
>> +
>> +		pmc = pmcdev->pmcs[i];
>> +		if (!pmc)
>> +			continue;
>> +
>> +		guard(mutex)(&pmcdev->lock);
>> +		pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign);
>> +	}
>> +}
>> +
>>   static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset,
>>   				       const int lpm_adj_x2)
>>   {
>> @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures;
>>   module_param(warn_on_s0ix_failures, bool, 0644);
>>   MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures");
>>   
>> +static bool ltr_ignore_all_suspend = true;
>> +module_param(ltr_ignore_all_suspend, bool, 0644);
>> +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend");
>> +
>>   static __maybe_unused int pmc_core_suspend(struct device *dev)
>>   {
>>   	struct pmc_dev *pmcdev = dev_get_drvdata(dev);
>> @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev)
>>   	if (pmcdev->suspend)
>>   		pmcdev->suspend(pmcdev);
>>   
>> +	if (ltr_ignore_all_suspend)
>> +		pmc_core_ltr_ignore_all(pmcdev);
>> +
>>   	/* Check if the syspend will actually use S0ix */
>>   	if (pm_suspend_via_firmware())
>>   		return 0;
>> @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev)
>>   {
>>   	struct pmc_dev *pmcdev = dev_get_drvdata(dev);
>>   
>> +	if (ltr_ignore_all_suspend)
>> +		pmc_core_ltr_restore_all(pmcdev);
>> +
>>   	if (pmcdev->resume)
>>   		return pmcdev->resume(pmcdev);
>>   
>> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
>> index ea04de7eb9e8..e862ea88b816 100644
>> --- a/drivers/platform/x86/intel/pmc/core.h
>> +++ b/drivers/platform/x86/intel/pmc/core.h
>> @@ -372,6 +372,7 @@ struct pmc_info {
>>    * @map:		pointer to pmc_reg_map struct that contains platform
>>    *			specific attributes
>>    * @lpm_req_regs:	List of substate requirements
>> + * @ltr_ign:		Holds LTR ignore data while suspended
>>    *
>>    * pmc contains info about one power management controller device.
>>    */
>> @@ -380,6 +381,7 @@ struct pmc {
>>   	void __iomem *regbase;
>>   	const struct pmc_reg_map *map;
>>   	u32 *lpm_req_regs;
>> +	u32 ltr_ign;
>>   };
>>   
>>   /**
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.