Re: [PATCH] platform/x86/intel/pmc: Remove GBE LTR ignore
From: Mario Limonciello
Date: Thu Dec 07 2023 - 14:18:30 EST
On 12/7/2023 13:02, Hans de Goede wrote:
Hi David,
On 12/7/23 19:23, David E. Box wrote:
Commit 804951203aa5 ("platform/x86:intel/pmc: Combine core_init() and
core_configure()") moved the GBE LTR ignore workaround from core.c to PCH
code and added it new for Cannon Lake PCH in cnp.c. This introduced a
network performance regression on a CNP PCH system [1] which has been
observed on other PCH architectures during testing. Remove the probe-time
GBE LTR ignore for all platforms. While this will prevent performance
degradation, it will also limit the deepest SoC Package C state that can be
entered at runtime while a LAN cable is attached.
Reported-by: James Hutchinson <jahutchinson99@xxxxxxxxxxxxxx>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218143 [1]
Fixes: 804951203aa5 ("platform/x86:intel/pmc: Combine core_init() and core_configure()")
Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
Tested-by: James Hutchinson <jahutchinson99@xxxxxxxxxxxxxx>
So it seems to me that to fix 804951203aa5 the only thing
which needs to be done is remove the pmc_core_send_ltr_ignore()
call from cnp_core_init(), the other changes here are NOT
related to fixing the regression.
So IMHO it seems better to split this into 2 patches?
Also if this block the system from reaching PC10 should
this then not at least be done at suspend time and
undone at resume ? I'm not seeing anything in the current
code which does an equivalent on suspend/resume, so it
seems to me like this will cause a significant increase
on suspended power-usage if an ethernet cable is attached ?
In addition to system suspend on these system does GBE support runtime PM?
If so, would it make sense to also have a Linux communication path from
the GBE driver to this driver as part of the runtime PM callbacks?
Then if the the GBE goes into runtime PM it could tell this driver to
waive LTR and if exits runtime PM it could tell this driver to stop
waiving LTR.
Regards,
Hans
---
drivers/platform/x86/intel/pmc/adl.c | 6 ------
drivers/platform/x86/intel/pmc/cnp.c | 6 ------
drivers/platform/x86/intel/pmc/core.c | 2 +-
drivers/platform/x86/intel/pmc/core.h | 1 -
drivers/platform/x86/intel/pmc/mtl.c | 6 ------
drivers/platform/x86/intel/pmc/tgl.c | 5 -----
6 files changed, 1 insertion(+), 25 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c
index 64c492391ede..e4a421ca64be 100644
--- a/drivers/platform/x86/intel/pmc/adl.c
+++ b/drivers/platform/x86/intel/pmc/adl.c
@@ -321,11 +321,5 @@ int adl_core_init(struct pmc_dev *pmcdev)
pmc_core_get_low_power_modes(pmcdev);
- /* Due to a hardware limitation, the GBE LTR blocks PC10
- * when a cable is attached. Tell the PMC to ignore it.
- */
- dev_dbg(&pmcdev->pdev->dev, "ignoring GBE LTR\n");
- pmc_core_send_ltr_ignore(pmcdev, 3);
-
return 0;
}
diff --git a/drivers/platform/x86/intel/pmc/cnp.c b/drivers/platform/x86/intel/pmc/cnp.c
index 59298f184d0e..416d3a0c3615 100644
--- a/drivers/platform/x86/intel/pmc/cnp.c
+++ b/drivers/platform/x86/intel/pmc/cnp.c
@@ -216,11 +216,5 @@ int cnp_core_init(struct pmc_dev *pmcdev)
pmc_core_get_low_power_modes(pmcdev);
- /* Due to a hardware limitation, the GBE LTR blocks PC10
- * when a cable is attached. Tell the PMC to ignore it.
- */
- dev_dbg(&pmcdev->pdev->dev, "ignoring GBE LTR\n");
- pmc_core_send_ltr_ignore(pmcdev, 3);
-
return 0;
}
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 983e3a8f4910..7c6a74957d57 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -462,7 +462,7 @@ static int pmc_core_pll_show(struct seq_file *s, void *unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
-int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value)
+static int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value)
{
struct pmc *pmc;
const struct pmc_reg_map *map;
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index 6d7673145f90..3bbdb41a754f 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -493,7 +493,6 @@ extern const struct pmc_reg_map mtl_ioem_reg_map;
extern void pmc_core_get_tgl_lpm_reqs(struct platform_device *pdev);
extern int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev);
-extern int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value);
int pmc_core_resume_common(struct pmc_dev *pmcdev);
int get_primary_reg_base(struct pmc *pmc);
diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
index 38c2f946ec23..33d32a76c43a 100644
--- a/drivers/platform/x86/intel/pmc/mtl.c
+++ b/drivers/platform/x86/intel/pmc/mtl.c
@@ -1065,11 +1065,5 @@ int mtl_core_init(struct pmc_dev *pmcdev)
pmc_core_get_low_power_modes(pmcdev);
mtl_punit_pmt_init(pmcdev);
- /* Due to a hardware limitation, the GBE LTR blocks PC10
- * when a cable is attached. Tell the PMC to ignore it.
- */
- dev_dbg(&pmcdev->pdev->dev, "ignoring GBE LTR\n");
- pmc_core_send_ltr_ignore(pmcdev, 3);
-
return pmc_core_ssram_get_lpm_reqs(pmcdev);
}
diff --git a/drivers/platform/x86/intel/pmc/tgl.c b/drivers/platform/x86/intel/pmc/tgl.c
index d5f1d2223c5a..7e6f5739a197 100644
--- a/drivers/platform/x86/intel/pmc/tgl.c
+++ b/drivers/platform/x86/intel/pmc/tgl.c
@@ -265,11 +265,6 @@ int tgl_core_init(struct pmc_dev *pmcdev)
pmc_core_get_low_power_modes(pmcdev);
pmc_core_get_tgl_lpm_reqs(pmcdev->pdev);
- /* Due to a hardware limitation, the GBE LTR blocks PC10
- * when a cable is attached. Tell the PMC to ignore it.
- */
- dev_dbg(&pmcdev->pdev->dev, "ignoring GBE LTR\n");
- pmc_core_send_ltr_ignore(pmcdev, 3);
return 0;
}
base-commit: 35ddd61cf023b5deb2b7e9f1627abef053281c0a