Message ID | 20231207182311.2080972-1-david.e.box@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp4978265vqy; Thu, 7 Dec 2023 10:23:19 -0800 (PST) X-Google-Smtp-Source: AGHT+IGp2sHmKwFVSYnX/ITOugrUcZ2NsQQdQxe85tuqToPphbe2Rqv9+6R/913uDWgeIkp8NLOP X-Received: by 2002:a05:6a00:99d:b0:6bd:3157:2dfe with SMTP id u29-20020a056a00099d00b006bd31572dfemr2782510pfg.7.1701973398841; Thu, 07 Dec 2023 10:23:18 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701973398; cv=none; d=google.com; s=arc-20160816; b=lSA4/7wRXYoxwRAj2Vijop3PyznW1757mXKyYF16vuut0ViTGuyTJ8uO6HpCHyJM11 VtN01OeNXXx9RRR8DGBOk3F+hyc0vC1BncIdNmmL8yYl6GfSUz6ql0qe2JU+YMWKyxfk TCKX6evlOEbVwgRyeBMfj6KOt/csGEeYr8BWLpMr+3FLdxB+j0Xb7StgjAuSg1aSgAKP 2IR3cyy9hNZjk+9N3o1m0+jrKZkFF+fr1V5QMjJKBxX+uxLXG2ylI9abfZcRLrb4KKxj e5lmcpwbh4REy0Q0SdEHbRVOFziJkQUsP+9BFzFDdJX6QmA4oQY+EL8FWsZLoagiJPW0 EeNA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:to:from:dkim-signature; bh=omZuTCrgqI7Lr9rGj14NPQYrrRZq1oFixp+HIWCo3Rk=; fh=YhuVP4oE7YLBUJg9yCv2SmdJ2Iyo5HLfmdJ4clQI4MU=; b=lowPBtZG/soKBEB8j9kD9xsc4vFl/6msQW2T5LEIeFYrcby5ouAjHUWL/T6eM3IwlF uE0rfakQ0pMBTy10T7Fz+ZdvvAbUBEUE+bXjdbnyIMC9GYrZiGraOrFIceP6AyCtIDsz NPXoYw89fA2tySEpduzlEGIUBcl1FXcT1dDFtOBNyJfxhSeoOWNofkdN8ewZhU3DKgrP jhW6W5hRfWJzLhHcRa/9QgIU93Nmzz1Un/GCh0RsGizZMvMJ6qFCP+112qDFYllic58X pkoYDccxFvW1Gdt284GfLMu/HiaUhOrqcEmbFgJvq80tTbjSfhlv+JmIC18c34T8ue30 BHtw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=It8sKdzT; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id l190-20020a6325c7000000b005c219415020si93793pgl.262.2023.12.07.10.23.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 10:23:18 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=It8sKdzT; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 928D4802D233; Thu, 7 Dec 2023 10:23:17 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1443720AbjLGSXJ (ORCPT <rfc822;chrisfriedt@gmail.com> + 99 others); Thu, 7 Dec 2023 13:23:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58936 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1443711AbjLGSXI (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 7 Dec 2023 13:23:08 -0500 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.11]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A9D1A1711; Thu, 7 Dec 2023 10:23:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701973392; x=1733509392; h=from:to:subject:date:message-id:mime-version: content-transfer-encoding; bh=CMmR5sSp9pmlIzsd9ubZrV13Ldr8URHh9yutCNFrV4I=; b=It8sKdzTjOFtBDBjFbWGlOVN0hd/v5AibVMWQowOW4N7k7NpSHr/5mIp W5C3T5dXnr+iqf6V30lHpOHWUfrYSb42e13k/ieIXjkWCB7ASSkNcjnMP b9/HUeAjwozj9MBIhdvuJ/7rtfFv5UftI9nA2oi6qvx6bRaDw37s1i2ns APmaBsWiv0ZW5i2qsHbmo3EXUjQGp+6jTKDokoCRbPSuoZvR1AkRXWjK0 fY7XXA+RY37t8/2L15hZjwHjZLy3CwkHmvBRnHAMB0Ex8qVbQINlxT0qB 9bgWnJbYe9iBmUYpO7IIuXIA8za1RqXA/KmRA1Ua9EsILRBzc5e+y3nY4 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10917"; a="1104825" X-IronPort-AV: E=Sophos;i="6.04,258,1695711600"; d="scan'208";a="1104825" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmvoesa105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Dec 2023 10:23:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10917"; a="800811390" X-IronPort-AV: E=Sophos;i="6.04,258,1695711600"; d="scan'208";a="800811390" Received: from linux.intel.com ([10.54.29.200]) by orsmga008.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Dec 2023 10:23:11 -0800 Received: from debox1-desk2.jf.intel.com (debox1-desk2.jf.intel.com [10.54.75.33]) by linux.intel.com (Postfix) with ESMTP id A131D580D4F; Thu, 7 Dec 2023 10:23:11 -0800 (PST) From: "David E. Box" <david.e.box@linux.intel.com> To: jahutchinson99@googlemail.com, david.e.box@linux.intel.com, hdegoede@redhat.com, ilpo.jarvinen@linux.intel.com, xi.pardee@intel.com, platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, rajvi.jingar@linux.intel.com Subject: [PATCH] platform/x86/intel/pmc: Remove GBE LTR ignore Date: Thu, 7 Dec 2023 10:23:11 -0800 Message-Id: <20231207182311.2080972-1-david.e.box@linux.intel.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 07 Dec 2023 10:23:17 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784648458475251695 X-GMAIL-MSGID: 1784648458475251695 |
Series |
platform/x86/intel/pmc: Remove GBE LTR ignore
|
|
Commit Message
David E. Box
Dec. 7, 2023, 6:23 p.m. UTC
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@googlemail.com> 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@linux.intel.com> Tested-by: James Hutchinson <jahutchinson99@googlemail.com> --- 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(-) base-commit: 35ddd61cf023b5deb2b7e9f1627abef053281c0a
Comments
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@googlemail.com> > 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@linux.intel.com> > Tested-by: James Hutchinson <jahutchinson99@googlemail.com> 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
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@googlemail.com> >> 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@linux.intel.com> >> Tested-by: James Hutchinson <jahutchinson99@googlemail.com> > > 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 > >
On Thu, 2023-12-07 at 13:18 -0600, Mario Limonciello wrote: > 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@googlemail.com> > > > 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@linux.intel.com> > > > Tested-by: James Hutchinson <jahutchinson99@googlemail.com> > > > > 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 ? We don't have many systems attached to onboard LAN. I initially tested on an Alder Lake and D3 during suspend was enough to allow PC10 and s0ix without ignoring the LTR. But with your comment I tried on a Tiger Lake and indeed it could not get to PC10 during suspend with D3 only. So I'll send a V2 with those patches. Thanks. > > In addition to system suspend on these system does GBE support runtime PM? LTR is the runtime pm mechanism for our GBE. The problem is the hardware. GBE wasn't provided an LTR register large enough to write a value to get to PC10. As is the maximum LTR value it can write will only get to PC8 or PC9 depending on the platform. David > > 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 > > > > >
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; }