All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mario Limonciello <mario.limonciello@amd.com>
To: Hans de Goede <hdegoede@redhat.com>,
	Mark Gross <mgross@linux.intel.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>
Cc: "open list:X86 PLATFORM DRIVERS" 
	<platform-driver-x86@vger.kernel.org>,
	<linux-acpi@vger.kernel.org>,
	Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
	Goswami Sanket <Sanket.Goswami@amd.com>,
	Mario Limonciello <mario.limonciello@amd.com>
Subject: [PATCH v3 4/5] platform/x86: amd-pmc: Move to later in the suspend process
Date: Mon, 14 Mar 2022 00:03:38 -0500	[thread overview]
Message-ID: <20220314050340.1176-4-mario.limonciello@amd.com> (raw)
In-Reply-To: <20220314050340.1176-1-mario.limonciello@amd.com>

The `OS_HINT` message is supposed to indicate that everything else
that is supposed to go into the deepest state has done so.

This assumption is invalid as:
1) The CPUs will still go in and out of the deepest state
2) Other devices may still run their `noirq` suspend routines
3) The LPS0 ACPI device will still run

To more closely mirror how this works on other operating systems,
move the `amd-pmc` suspend to the very last thing before the s2idle
loop via an lps0 callback.

Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-to-idle path")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
changes from v2->v3:
 * no changes
changes from v1->v2:
 * adjust for changes in previous patches
 * display a debugging message for constraints

 drivers/platform/x86/amd-pmc.c | 42 +++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
index 971aaabaa9c8..752a99d759d2 100644
--- a/drivers/platform/x86/amd-pmc.c
+++ b/drivers/platform/x86/amd-pmc.c
@@ -639,13 +639,16 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
 	return rc;
 }
 
-static int __maybe_unused amd_pmc_suspend(struct device *dev)
+static int amd_pmc_suspend(void *context, bool constraints)
 {
-	struct amd_pmc_dev *pdev = dev_get_drvdata(dev);
+	struct amd_pmc_dev *pdev = dev_get_drvdata((struct device *)context);
 	int rc;
 	u8 msg;
 	u32 arg = 1;
 
+	/* for enabling constraints checking in the future */
+	dev_dbg(pdev->dev, "LPI constraints were%smet.\n", constraints ? " " : " not ");
+
 	/* Reset and Start SMU logging - to monitor the s0i3 stats */
 	amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_RESET, 0);
 	amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_START, 0);
@@ -658,7 +661,7 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
 	}
 
 	/* Dump the IdleMask before we send hint to SMU */
-	amd_pmc_idlemask_read(pdev, dev, NULL);
+	amd_pmc_idlemask_read(pdev, pdev->dev, NULL);
 	msg = amd_pmc_get_os_hint(pdev);
 	rc = amd_pmc_send_cmd(pdev, arg, NULL, msg, 0);
 	if (rc) {
@@ -681,28 +684,28 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
 	return rc;
 }
 
-static int __maybe_unused amd_pmc_resume(struct device *dev)
+static void amd_pmc_resume(void *context)
 {
-	struct amd_pmc_dev *pdev = dev_get_drvdata(dev);
+	struct amd_pmc_dev *pdev = dev_get_drvdata((struct device *)context);
 	int rc;
 	u8 msg;
 
 	msg = amd_pmc_get_os_hint(pdev);
 	rc = amd_pmc_send_cmd(pdev, 0, NULL, msg, 0);
 	if (rc)
-		dev_err(pdev->dev, "resume failed\n");
+		dev_err(pdev->dev, "resume failed: %d\n", rc);
 
 	/* Let SMU know that we are looking for stats */
 	amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_DUMP_DATA, 0);
 
 	/* Dump the IdleMask to see the blockers */
-	amd_pmc_idlemask_read(pdev, dev, NULL);
+	amd_pmc_idlemask_read(pdev, pdev->dev, NULL);
 
 	/* Write data incremented by 1 to distinguish in stb_read */
 	if (enable_stb)
 		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF + 1);
 	if (rc)
-		dev_err(pdev->dev, "error writing to STB\n");
+		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
 
 	/* Restore the QoS request back to defaults if it was set */
 	if (pdev->cpu_id == AMD_CPU_ID_CZN)
@@ -711,15 +714,8 @@ static int __maybe_unused amd_pmc_resume(struct device *dev)
 
 	/* Notify on failed entry */
 	amd_pmc_validate_deepest(pdev);
-
-	return rc;
 }
 
-static const struct dev_pm_ops amd_pmc_pm_ops = {
-	.suspend_noirq = amd_pmc_suspend,
-	.resume_noirq = amd_pmc_resume,
-};
-
 static const struct pci_device_id pmc_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_YC) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_CZN) },
@@ -805,6 +801,11 @@ static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf)
 static int amd_pmc_probe(struct platform_device *pdev)
 {
 	struct amd_pmc_dev *dev = &pmc;
+	struct lps0_callback_handler lps0_handler = {
+		.prepare_late_callback = amd_pmc_suspend,
+		.restore_early_callback = amd_pmc_resume,
+		.context = &pdev->dev,
+	};
 	struct pci_dev *rdev;
 	u32 base_addr_lo, base_addr_hi;
 	u64 base_addr, fch_phys_addr;
@@ -884,6 +885,10 @@ static int amd_pmc_probe(struct platform_device *pdev)
 
 	amd_pmc_get_smu_version(dev);
 	platform_set_drvdata(pdev, dev);
+	err = acpi_register_lps0_callbacks(&lps0_handler);
+	if (err)
+		goto err_pci_dev_put;
+
 	amd_pmc_dbgfs_register(dev);
 	cpu_latency_qos_add_request(&dev->amd_pmc_pm_qos_req, PM_QOS_DEFAULT_VALUE);
 	return 0;
@@ -896,7 +901,13 @@ static int amd_pmc_probe(struct platform_device *pdev)
 static int amd_pmc_remove(struct platform_device *pdev)
 {
 	struct amd_pmc_dev *dev = platform_get_drvdata(pdev);
+	struct lps0_callback_handler lps0_handler = {
+		.prepare_late_callback = amd_pmc_suspend,
+		.restore_early_callback = amd_pmc_resume,
+		.context = &pdev->dev,
+	};
 
+	acpi_unregister_lps0_callbacks(&lps0_handler);
 	amd_pmc_dbgfs_unregister(dev);
 	pci_dev_put(dev->rdev);
 	mutex_destroy(&dev->lock);
@@ -917,7 +928,6 @@ static struct platform_driver amd_pmc_driver = {
 	.driver = {
 		.name = "amd_pmc",
 		.acpi_match_table = amd_pmc_acpi_ids,
-		.pm = &amd_pmc_pm_ops,
 	},
 	.probe = amd_pmc_probe,
 	.remove = amd_pmc_remove,
-- 
2.34.1


  parent reply	other threads:[~2022-03-14  5:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-14  5:03 [PATCH v3 1/5] ACPI / x86: Add support for LPS0 callback handler Mario Limonciello
2022-03-14  5:03 ` [PATCH v3 2/5] ACPI / x86: Pass the constraints checking result to LPS0 callback Mario Limonciello
2022-03-14  5:03 ` [PATCH v3 3/5] ACPI / x86: Check LPI constraints by default Mario Limonciello
2022-03-14  5:03 ` Mario Limonciello [this message]
2022-03-14  5:03 ` [PATCH v3 5/5] platform/x86: amd-pmc: Drop CPU QoS workaround Mario Limonciello
2022-03-14  9:05 ` [PATCH v3 1/5] ACPI / x86: Add support for LPS0 callback handler Hans de Goede
2022-03-14 13:32   ` Limonciello, Mario
2022-03-14 13:37     ` Hans de Goede
2022-03-16 15:02       ` Rafael J. Wysocki
2022-03-16 15:34         ` Rafael J. Wysocki
2022-03-16 15:43           ` Limonciello, Mario
2022-03-16 15:52             ` Rafael J. Wysocki
2022-03-16 16:43               ` Limonciello, Mario
2022-03-16 17:27                 ` Rafael J. Wysocki
2022-03-15  1:01     ` David E. Box
2022-03-14  9:12 ` Lukas Wunner
2022-03-14 13:28   ` Limonciello, Mario

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220314050340.1176-4-mario.limonciello@amd.com \
    --to=mario.limonciello@amd.com \
    --cc=Sanket.Goswami@amd.com \
    --cc=Shyam-sundar.S-k@amd.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=mgross@linux.intel.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.