[PATCH] ata: libata: add workaround to flip LPM during suspend/resume

Koba Ko posted 1 patch 2 years, 5 months ago
There is a newer version of this series
drivers/ata/ahci.c      | 29 +++++++++++++++++++++++++++++
drivers/ata/libata-eh.c |  8 ++++++++
include/linux/libata.h  |  1 +
3 files changed, 38 insertions(+)
[PATCH] ata: libata: add workaround to flip LPM during suspend/resume
Posted by Koba Ko 2 years, 5 months ago
Due to TigerLake/Adler Lake AHCI controller's LPM regression,
can't apply LPM on TigerLake/AdlerLake AHCI controller.

Add a workaround to flip LPM during suspend/resume.
When suspneding, apply LPM on TigerLake/AdlerLake AHCI.
Restore it to target_lpm_policy after resuming.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217775
Signed-off-by: Koba Ko <koba.ko@canonical.com>
---
 drivers/ata/ahci.c      | 29 +++++++++++++++++++++++++++++
 drivers/ata/libata-eh.c |  8 ++++++++
 include/linux/libata.h  |  1 +
 3 files changed, 38 insertions(+)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 02503e903e4a8..5c9196cd2c738 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -95,6 +95,8 @@ static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
 #ifdef CONFIG_PM
 static int ahci_pci_device_runtime_suspend(struct device *dev);
 static int ahci_pci_device_runtime_resume(struct device *dev);
+static int ahci_intel_port_suspend(struct ata_port *ap, pm_message_t mesg);
+static int ahci_intel_port_resume(struct ata_port *ap);
 #ifdef CONFIG_PM_SLEEP
 static int ahci_pci_device_suspend(struct device *dev);
 static int ahci_pci_device_resume(struct device *dev);
@@ -1025,6 +1027,30 @@ static void ahci_p5wdh_workaround(struct ata_host *host)
 		ap->link.flags |= ATA_LFLAG_NO_SRST | ATA_LFLAG_ASSUME_ATA;
 	}
 }
+/*
+ * Intel TGL/ADL workaround, when suspending, put port into LPM,
+ * recover to max power after resuming.
+ */
+static void ahci_intel_ahci_workaround(struct ata_host *host)
+{
+	struct pci_dev *pdev = to_pci_dev(host->dev);
+	int i;
+	static const struct pci_device_id ids[] = {
+		{ PCI_VDEVICE(INTEL, 0xa0d3)}, /* Tiger Lake UP{3,4} AHCI */
+		{ PCI_VDEVICE(INTEL, 0x7ae2)}, /* Alder Lake AHCI*/
+		{}
+	};
+
+	dev_info(&pdev->dev, "enabling Intel AHCI workaround\n");
+
+	if (pci_match_id(ids, pdev)) {
+		for (i = 0; i < host->n_ports; i++) {
+			struct ata_port *ap = host->ports[i];
+
+			ap->flags |= ATA_LFLAG_NO_LPM_RECOVER;
+		}
+	}
+}
 
 /*
  * Macbook7,1 firmware forcibly disables MCP89 AHCI and changes PCI ID when
@@ -1905,6 +1931,9 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* apply workaround for ASUS P5W DH Deluxe mainboard */
 	ahci_p5wdh_workaround(host);
 
+	/* apply workaround for Intel AHCI */
+	ahci_intel_ahci_workaround(host);
+
 	/* apply gtf filter quirk */
 	ahci_gtf_filter_workaround(host);
 
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 159ba6ba19ebb..de29e50843f99 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -4000,6 +4000,11 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap)
 	int rc = 0;
 	struct ata_device *dev;
 
+	ata_for_each_dev(dev, &ap->link, ENABLED) {
+		ata_eh_set_lpm(&ap->link, ATA_LPM_MED_POWER_WITH_DIPM, &dev);
+	}
+
+
 	/* are we suspending? */
 	spin_lock_irqsave(ap->lock, flags);
 	if (!(ap->pflags & ATA_PFLAG_PM_PENDING) ||
@@ -4087,6 +4092,9 @@ static void ata_eh_handle_port_resume(struct ata_port *ap)
 	if (ap->ops->port_resume)
 		ap->ops->port_resume(ap);
 
+	ata_for_each_dev(dev, &ap->link, ENABLED) {
+		ata_eh_set_lpm(&ap->link, ap->target_lpm_policy, &dev);
+	}
 	/* tell ACPI that we're resuming */
 	ata_acpi_on_resume(ap);
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 52d58b13e5eee..e720fed6dbd7f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -147,6 +147,7 @@ enum {
 	ATA_LFLAG_RST_ONCE	= (1 << 9), /* limit recovery to one reset */
 	ATA_LFLAG_CHANGED	= (1 << 10), /* LPM state changed on this link */
 	ATA_LFLAG_NO_DEBOUNCE_DELAY = (1 << 11), /* no debounce delay on link resume */
+	ATA_LFLAG_NO_LPM_RECOVER = (1 << 12), /* disable LPM on this link */
 
 	/* struct ata_port flags */
 	ATA_FLAG_SLAVE_POSS	= (1 << 0), /* host supports slave dev */
-- 
2.34.1
Re: [PATCH] ata: libata: add workaround to flip LPM during suspend/resume
Posted by Koba Ko 2 years, 5 months ago
Nack, due to some errors
1. unused declarations
2. missing judgement for ATA_LFLAG_NO_LPM_RECOVER

On Fri, Sep 1, 2023 at 3:08 AM Koba Ko <koba.ko@canonical.com> wrote:
>
> Due to TigerLake/Adler Lake AHCI controller's LPM regression,
> can't apply LPM on TigerLake/AdlerLake AHCI controller.
>
> Add a workaround to flip LPM during suspend/resume.
> When suspneding, apply LPM on TigerLake/AdlerLake AHCI.
> Restore it to target_lpm_policy after resuming.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217775
> Signed-off-by: Koba Ko <koba.ko@canonical.com>
> ---
>  drivers/ata/ahci.c      | 29 +++++++++++++++++++++++++++++
>  drivers/ata/libata-eh.c |  8 ++++++++
>  include/linux/libata.h  |  1 +
>  3 files changed, 38 insertions(+)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 02503e903e4a8..5c9196cd2c738 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -95,6 +95,8 @@ static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
>  #ifdef CONFIG_PM
>  static int ahci_pci_device_runtime_suspend(struct device *dev);
>  static int ahci_pci_device_runtime_resume(struct device *dev);
> +static int ahci_intel_port_suspend(struct ata_port *ap, pm_message_t mesg);
> +static int ahci_intel_port_resume(struct ata_port *ap);
>  #ifdef CONFIG_PM_SLEEP
>  static int ahci_pci_device_suspend(struct device *dev);
>  static int ahci_pci_device_resume(struct device *dev);
> @@ -1025,6 +1027,30 @@ static void ahci_p5wdh_workaround(struct ata_host *host)
>                 ap->link.flags |= ATA_LFLAG_NO_SRST | ATA_LFLAG_ASSUME_ATA;
>         }
>  }
> +/*
> + * Intel TGL/ADL workaround, when suspending, put port into LPM,
> + * recover to max power after resuming.
> + */
> +static void ahci_intel_ahci_workaround(struct ata_host *host)
> +{
> +       struct pci_dev *pdev = to_pci_dev(host->dev);
> +       int i;
> +       static const struct pci_device_id ids[] = {
> +               { PCI_VDEVICE(INTEL, 0xa0d3)}, /* Tiger Lake UP{3,4} AHCI */
> +               { PCI_VDEVICE(INTEL, 0x7ae2)}, /* Alder Lake AHCI*/
> +               {}
> +       };
> +
> +       dev_info(&pdev->dev, "enabling Intel AHCI workaround\n");
> +
> +       if (pci_match_id(ids, pdev)) {
> +               for (i = 0; i < host->n_ports; i++) {
> +                       struct ata_port *ap = host->ports[i];
> +
> +                       ap->flags |= ATA_LFLAG_NO_LPM_RECOVER;
> +               }
> +       }
> +}
>
>  /*
>   * Macbook7,1 firmware forcibly disables MCP89 AHCI and changes PCI ID when
> @@ -1905,6 +1931,9 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>         /* apply workaround for ASUS P5W DH Deluxe mainboard */
>         ahci_p5wdh_workaround(host);
>
> +       /* apply workaround for Intel AHCI */
> +       ahci_intel_ahci_workaround(host);
> +
>         /* apply gtf filter quirk */
>         ahci_gtf_filter_workaround(host);
>
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index 159ba6ba19ebb..de29e50843f99 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -4000,6 +4000,11 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap)
>         int rc = 0;
>         struct ata_device *dev;
>
> +       ata_for_each_dev(dev, &ap->link, ENABLED) {
> +               ata_eh_set_lpm(&ap->link, ATA_LPM_MED_POWER_WITH_DIPM, &dev);
> +       }
> +
> +
>         /* are we suspending? */
>         spin_lock_irqsave(ap->lock, flags);
>         if (!(ap->pflags & ATA_PFLAG_PM_PENDING) ||
> @@ -4087,6 +4092,9 @@ static void ata_eh_handle_port_resume(struct ata_port *ap)
>         if (ap->ops->port_resume)
>                 ap->ops->port_resume(ap);
>
> +       ata_for_each_dev(dev, &ap->link, ENABLED) {
> +               ata_eh_set_lpm(&ap->link, ap->target_lpm_policy, &dev);
> +       }
>         /* tell ACPI that we're resuming */
>         ata_acpi_on_resume(ap);
>
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 52d58b13e5eee..e720fed6dbd7f 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -147,6 +147,7 @@ enum {
>         ATA_LFLAG_RST_ONCE      = (1 << 9), /* limit recovery to one reset */
>         ATA_LFLAG_CHANGED       = (1 << 10), /* LPM state changed on this link */
>         ATA_LFLAG_NO_DEBOUNCE_DELAY = (1 << 11), /* no debounce delay on link resume */
> +       ATA_LFLAG_NO_LPM_RECOVER = (1 << 12), /* disable LPM on this link */
>
>         /* struct ata_port flags */
>         ATA_FLAG_SLAVE_POSS     = (1 << 0), /* host supports slave dev */
> --
> 2.34.1
>
Re: [PATCH] ata: libata: add workaround to flip LPM during suspend/resume
Posted by kernel test robot 2 years, 5 months ago
Hi Koba,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.5 next-20230831]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Koba-Ko/ata-libata-add-workaround-to-flip-LPM-during-suspend-resume/20230901-030957
base:   linus/master
patch link:    https://lore.kernel.org/r/20230831190828.1171119-1-koba.ko%40canonical.com
patch subject: [PATCH] ata: libata: add workaround to flip LPM during suspend/resume
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20230901/202309010504.kO8lW46F-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230901/202309010504.kO8lW46F-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309010504.kO8lW46F-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/ata/ahci.c:98:12: warning: 'ahci_intel_port_suspend' declared 'static' but never defined [-Wunused-function]
      98 | static int ahci_intel_port_suspend(struct ata_port *ap, pm_message_t mesg);
         |            ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/ata/ahci.c:99:12: warning: 'ahci_intel_port_resume' declared 'static' but never defined [-Wunused-function]
      99 | static int ahci_intel_port_resume(struct ata_port *ap);
         |            ^~~~~~~~~~~~~~~~~~~~~~


vim +98 drivers/ata/ahci.c

    82	
    83	static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
    84	static void ahci_remove_one(struct pci_dev *dev);
    85	static void ahci_shutdown_one(struct pci_dev *dev);
    86	static void ahci_intel_pcs_quirk(struct pci_dev *pdev, struct ahci_host_priv *hpriv);
    87	static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
    88					 unsigned long deadline);
    89	static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class,
    90				      unsigned long deadline);
    91	static void ahci_mcp89_apple_enable(struct pci_dev *pdev);
    92	static bool is_mcp89_apple(struct pci_dev *pdev);
    93	static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
    94					unsigned long deadline);
    95	#ifdef CONFIG_PM
    96	static int ahci_pci_device_runtime_suspend(struct device *dev);
    97	static int ahci_pci_device_runtime_resume(struct device *dev);
  > 98	static int ahci_intel_port_suspend(struct ata_port *ap, pm_message_t mesg);
  > 99	static int ahci_intel_port_resume(struct ata_port *ap);
   100	#ifdef CONFIG_PM_SLEEP
   101	static int ahci_pci_device_suspend(struct device *dev);
   102	static int ahci_pci_device_resume(struct device *dev);
   103	#endif
   104	#endif /* CONFIG_PM */
   105	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki