On 16/11/18 6:57 PM, Anisse Astier wrote:
> Hi Adrian,
>
> On Fri, Nov 16, 2018 at 04:01:36PM +0200, Adrian Hunter wrote:
>> On 16/11/18 11:50 AM, Adrian Hunter wrote:
>>> On 16/11/18 11:37 AM, Anisse Astier wrote:
>>>> Hi Adrian,
>>>>
>>>> On Thu, Nov 08, 2018 at 05:43:32PM +0000, Hunter, Adrian wrote:
>>>>> [snip]
>>>>> I found some time today. It seems that the ACPI _PS3 method is failing to save the tuning value.
>>>>> That results in a CRC error, but the driver does not recover very gracefully.
>>>>>
>>>>> I will look at making a patch for the driver recovery, but also add some diagnostics prints to the ACPI DSDT to try to figure out what is going wrong there.
>>>>>
>>>>>
>>>>
>>>> Have you had the time to look at what diagnostics could be added ?
>>>
>>> I have a patch, but I am still testing it.
>>>
>>
>> Please try the attached patch
>
>> From 5c33302367e7852aa0a723e311b1749bea59f055 Mon Sep 17 00:00:00 2001
>> From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> Date: Fri, 16 Nov 2018 14:02:12 +0200
>> Subject: [PATCH] mmc: sdhci-pci: Workaround GLK firmware failing to restore
>> the tuning value
>>
>> GLK firmware can indicate that the tuning value will be restored after
>> runtime suspend, but not actually do that. Add a workaround that detects
>> such cases, and lets the driver do re-tuning instead.
>>
>
> This looks like it solves the issue. Please find the boot log here:
>
> https://anisse.astier.eu/static/dmesg-sdhci-glk-workaround.txt.xz
>
> We still have I/O errors I don't understand, but no more long CQE
> timeouts.
Please try this patch instead.
>From ff445c899af814f0623ef191213b9d7ddb51aa69 Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Date: Fri, 16 Nov 2018 14:02:12 +0200
Subject: [PATCH V2] mmc: sdhci-pci: Workaround GLK firmware failing to restore
the tuning value
GLK firmware can indicate that the tuning value will be restored after
runtime suspend, but not actually do that. Add a workaround that detects
such cases, and lets the driver do re-tuning instead.
Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
---
drivers/mmc/host/sdhci-pci-core.c | 79 ++++++++++++++++++++++++++++++-
1 file changed, 77 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 7bfd366d970d..f5176d974a88 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -12,6 +12,7 @@
* - JMicron (hardware and technical support)
*/
+#include <linux/bitfield.h>
#include <linux/string.h>
#include <linux/delay.h>
#include <linux/highmem.h>
@@ -462,6 +463,9 @@ struct intel_host {
u32 dsm_fns;
int drv_strength;
bool d3_retune;
+ bool rpm_retune_ok;
+ u32 glk_rx_ctrl1;
+ u32 glk_tun_val;
};
static const guid_t intel_dsm_guid =
@@ -791,6 +795,77 @@ static int glk_emmc_add_host(struct sdhci_pci_slot *slot)
return ret;
}
+#ifdef CONFIG_PM
+#define GLK_RX_CTRL1 0x834
+#define GLK_TUN_VAL 0x840
+#define GLK_PATH_PLL GENMASK(13, 8)
+#define GLK_DLY GENMASK(6, 0)
+/* Workaround firmware failing to restore the tuning value */
+static void glk_rpm_retune_wa(struct sdhci_pci_chip *chip, bool susp)
+{
+ struct sdhci_pci_slot *slot = chip->slots[0];
+ struct intel_host *intel_host = sdhci_pci_priv(slot);
+ struct sdhci_host *host = slot->host;
+ u32 glk_rx_ctrl1;
+ u32 glk_tun_val;
+ u32 dly;
+
+ if (intel_host->rpm_retune_ok || !mmc_can_retune(host->mmc))
+ return;
+
+ glk_rx_ctrl1 = sdhci_readl(host, GLK_RX_CTRL1);
+ glk_tun_val = sdhci_readl(host, GLK_TUN_VAL);
+
+ if (susp) {
+ intel_host->glk_rx_ctrl1 = glk_rx_ctrl1;
+ intel_host->glk_tun_val = glk_tun_val;
+ return;
+ }
+
+ if (!intel_host->glk_tun_val)
+ return;
+
+ if (glk_rx_ctrl1 != intel_host->glk_rx_ctrl1) {
+ intel_host->rpm_retune_ok = true;
+ return;
+ }
+
+ dly = FIELD_PREP(GLK_DLY, FIELD_GET(GLK_PATH_PLL, glk_rx_ctrl1) +
+ (intel_host->glk_tun_val << 1));
+ if (dly == FIELD_GET(GLK_DLY, glk_rx_ctrl1))
+ return;
+
+ glk_rx_ctrl1 = (glk_rx_ctrl1 & ~GLK_DLY) | dly;
+ sdhci_writel(host, glk_rx_ctrl1, GLK_RX_CTRL1);
+
+ intel_host->rpm_retune_ok = true;
+ chip->rpm_retune = true;
+ mmc_retune_needed(host->mmc);
+ pr_info("%s: Requiring re-tune after rpm resume", mmc_hostname(host->mmc));
+}
+
+static void glk_rpm_retune_chk(struct sdhci_pci_chip *chip, bool susp)
+{
+ if (chip->pdev->device == PCI_DEVICE_ID_INTEL_GLK_EMMC &&
+ !chip->rpm_retune)
+ glk_rpm_retune_wa(chip, susp);
+}
+
+static int glk_runtime_suspend(struct sdhci_pci_chip *chip)
+{
+ glk_rpm_retune_chk(chip, true);
+
+ return sdhci_cqhci_runtime_suspend(chip);
+}
+
+static int glk_runtime_resume(struct sdhci_pci_chip *chip)
+{
+ glk_rpm_retune_chk(chip, false);
+
+ return sdhci_cqhci_runtime_resume(chip);
+}
+#endif
+
#ifdef CONFIG_ACPI
static int ni_set_max_freq(struct sdhci_pci_slot *slot)
{
@@ -879,8 +954,8 @@ static const struct sdhci_pci_fixes sdhci_intel_glk_emmc = {
.resume = sdhci_cqhci_resume,
#endif
#ifdef CONFIG_PM
- .runtime_suspend = sdhci_cqhci_runtime_suspend,
- .runtime_resume = sdhci_cqhci_runtime_resume,
+ .runtime_suspend = glk_runtime_suspend,
+ .runtime_resume = glk_runtime_resume,
#endif
.quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
--
2.17.1
