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 ?
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
