| Message ID | 20201214192935.895174-4-mario.limonciello@dell.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Improve s0ix flows for systems i219LM | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 63 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
Hi, On 12/14/20 8:29 PM, Mario Limonciello wrote: > commit e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME systems") > disabled s0ix flows for systems that have various incarnations of the > i219-LM ethernet controller. This changed caused power consumption regressions > on the following shipping Dell Comet Lake based laptops: > * Latitude 5310 > * Latitude 5410 > * Latitude 5410 > * Latitude 5510 > * Precision 3550 > * Latitude 5411 > * Latitude 5511 > * Precision 3551 > * Precision 7550 > * Precision 7750 > > This commit was introduced because of some regressions on certain Thinkpad > laptops. This comment was potentially caused by an earlier > commit 632fbd5eb5b0e ("e1000e: fix S0ix flows for cable connected case"). > or it was possibly caused by a system not meeting platform architectural > requirements for low power consumption. Other changes made in the driver > with extended timeouts are expected to make the driver more impervious to > platform firmware behavior. > > Fixes: e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME systems") > Reviewed-by: Alexander Duyck <alexander.duyck@gmail.com> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/net/ethernet/intel/e1000e/netdev.c | 45 +--------------------- > 1 file changed, 2 insertions(+), 43 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > index 6588f5d4a2be..b9800ba2006c 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -103,45 +103,6 @@ static const struct e1000_reg_info e1000_reg_info_tbl[] = { > {0, NULL} > }; > > -struct e1000e_me_supported { > - u16 device_id; /* supported device ID */ > -}; > - > -static const struct e1000e_me_supported me_supported[] = { > - {E1000_DEV_ID_PCH_LPT_I217_LM}, > - {E1000_DEV_ID_PCH_LPTLP_I218_LM}, > - {E1000_DEV_ID_PCH_I218_LM2}, > - {E1000_DEV_ID_PCH_I218_LM3}, > - {E1000_DEV_ID_PCH_SPT_I219_LM}, > - {E1000_DEV_ID_PCH_SPT_I219_LM2}, > - {E1000_DEV_ID_PCH_LBG_I219_LM3}, > - {E1000_DEV_ID_PCH_SPT_I219_LM4}, > - {E1000_DEV_ID_PCH_SPT_I219_LM5}, > - {E1000_DEV_ID_PCH_CNP_I219_LM6}, > - {E1000_DEV_ID_PCH_CNP_I219_LM7}, > - {E1000_DEV_ID_PCH_ICP_I219_LM8}, > - {E1000_DEV_ID_PCH_ICP_I219_LM9}, > - {E1000_DEV_ID_PCH_CMP_I219_LM10}, > - {E1000_DEV_ID_PCH_CMP_I219_LM11}, > - {E1000_DEV_ID_PCH_CMP_I219_LM12}, > - {E1000_DEV_ID_PCH_TGP_I219_LM13}, > - {E1000_DEV_ID_PCH_TGP_I219_LM14}, > - {E1000_DEV_ID_PCH_TGP_I219_LM15}, > - {0} > -}; > - > -static bool e1000e_check_me(u16 device_id) > -{ > - struct e1000e_me_supported *id; > - > - for (id = (struct e1000e_me_supported *)me_supported; > - id->device_id; id++) > - if (device_id == id->device_id) > - return true; > - > - return false; > -} > - > /** > * __ew32_prepare - prepare to write to MAC CSR register on certain parts > * @hw: pointer to the HW structure > @@ -6974,8 +6935,7 @@ static __maybe_unused int e1000e_pm_suspend(struct device *dev) > e1000e_pm_thaw(dev); > } else { > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp && > - !e1000e_check_me(hw->adapter->pdev->device)) > + if (hw->mac.type >= e1000_pch_cnp) > e1000e_s0ix_entry_flow(adapter); > } > > @@ -6991,8 +6951,7 @@ static __maybe_unused int e1000e_pm_resume(struct device *dev) > int rc; > > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp && > - !e1000e_check_me(hw->adapter->pdev->device)) > + if (hw->mac.type >= e1000_pch_cnp) > e1000e_s0ix_exit_flow(adapter); > > rc = __e1000_resume(pdev); >
> -----Original Message----- > From: Limonciello, Mario <Mario_Limonciello@Dell.com> > Sent: Tuesday, December 15, 2020 3:30 AM > To: Jeff Kirsher; Tony Nguyen; intel-wired-lan@lists.osuosl.org > Cc: linux-kernel@vger.kernel.org; Netdev; Alexander Duyck; Jakub Kicinski; > Sasha Netfin; Aaron Brown; Stefan Assmann; David Miller; > darcari@redhat.com; Shen, Yijun; Yuan, Perry; > anthony.wong@canonical.com; Hans de Goede; Limonciello, Mario > Subject: [PATCH v5 3/4] Revert "e1000e: disable s0ix entry and exit flows for > ME systems" > > commit e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME > systems") disabled s0ix flows for systems that have various incarnations of > the i219-LM ethernet controller. This changed caused power consumption > regressions on the following shipping Dell Comet Lake based laptops: > * Latitude 5310 > * Latitude 5410 > * Latitude 5410 > * Latitude 5510 > * Precision 3550 > * Latitude 5411 > * Latitude 5511 > * Precision 3551 > * Precision 7550 > * Precision 7750 > > This commit was introduced because of some regressions on certain > Thinkpad laptops. This comment was potentially caused by an earlier > commit 632fbd5eb5b0e ("e1000e: fix S0ix flows for cable connected case"). > or it was possibly caused by a system not meeting platform architectural > requirements for low power consumption. Other changes made in the driver > with extended timeouts are expected to make the driver more impervious to > platform firmware behavior. > > Fixes: e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME > systems") > Reviewed-by: Alexander Duyck <alexander.duyck@gmail.com> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> Verified this series patches with Dell Systems. Tested-By: Yijun Shen <Yijun.shen@dell.com> > --- > drivers/net/ethernet/intel/e1000e/netdev.c | 45 +--------------------- > 1 file changed, 2 insertions(+), 43 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c > b/drivers/net/ethernet/intel/e1000e/netdev.c > index 6588f5d4a2be..b9800ba2006c 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -103,45 +103,6 @@ static const struct e1000_reg_info > e1000_reg_info_tbl[] = { > {0, NULL} > }; > > -struct e1000e_me_supported { > - u16 device_id; /* supported device ID */ > -}; > - > -static const struct e1000e_me_supported me_supported[] = { > - {E1000_DEV_ID_PCH_LPT_I217_LM}, > - {E1000_DEV_ID_PCH_LPTLP_I218_LM}, > - {E1000_DEV_ID_PCH_I218_LM2}, > - {E1000_DEV_ID_PCH_I218_LM3}, > - {E1000_DEV_ID_PCH_SPT_I219_LM}, > - {E1000_DEV_ID_PCH_SPT_I219_LM2}, > - {E1000_DEV_ID_PCH_LBG_I219_LM3}, > - {E1000_DEV_ID_PCH_SPT_I219_LM4}, > - {E1000_DEV_ID_PCH_SPT_I219_LM5}, > - {E1000_DEV_ID_PCH_CNP_I219_LM6}, > - {E1000_DEV_ID_PCH_CNP_I219_LM7}, > - {E1000_DEV_ID_PCH_ICP_I219_LM8}, > - {E1000_DEV_ID_PCH_ICP_I219_LM9}, > - {E1000_DEV_ID_PCH_CMP_I219_LM10}, > - {E1000_DEV_ID_PCH_CMP_I219_LM11}, > - {E1000_DEV_ID_PCH_CMP_I219_LM12}, > - {E1000_DEV_ID_PCH_TGP_I219_LM13}, > - {E1000_DEV_ID_PCH_TGP_I219_LM14}, > - {E1000_DEV_ID_PCH_TGP_I219_LM15}, > - {0} > -}; > - > -static bool e1000e_check_me(u16 device_id) -{ > - struct e1000e_me_supported *id; > - > - for (id = (struct e1000e_me_supported *)me_supported; > - id->device_id; id++) > - if (device_id == id->device_id) > - return true; > - > - return false; > -} > - > /** > * __ew32_prepare - prepare to write to MAC CSR register on certain parts > * @hw: pointer to the HW structure > @@ -6974,8 +6935,7 @@ static __maybe_unused int > e1000e_pm_suspend(struct device *dev) > e1000e_pm_thaw(dev); > } else { > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp && > - !e1000e_check_me(hw->adapter->pdev->device)) > + if (hw->mac.type >= e1000_pch_cnp) > e1000e_s0ix_entry_flow(adapter); > } > > @@ -6991,8 +6951,7 @@ static __maybe_unused int > e1000e_pm_resume(struct device *dev) > int rc; > > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp && > - !e1000e_check_me(hw->adapter->pdev->device)) > + if (hw->mac.type >= e1000_pch_cnp) > e1000e_s0ix_exit_flow(adapter); > > rc = __e1000_resume(pdev); > -- > 2.25.1
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index 6588f5d4a2be..b9800ba2006c 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -103,45 +103,6 @@ static const struct e1000_reg_info e1000_reg_info_tbl[] = { {0, NULL} }; -struct e1000e_me_supported { - u16 device_id; /* supported device ID */ -}; - -static const struct e1000e_me_supported me_supported[] = { - {E1000_DEV_ID_PCH_LPT_I217_LM}, - {E1000_DEV_ID_PCH_LPTLP_I218_LM}, - {E1000_DEV_ID_PCH_I218_LM2}, - {E1000_DEV_ID_PCH_I218_LM3}, - {E1000_DEV_ID_PCH_SPT_I219_LM}, - {E1000_DEV_ID_PCH_SPT_I219_LM2}, - {E1000_DEV_ID_PCH_LBG_I219_LM3}, - {E1000_DEV_ID_PCH_SPT_I219_LM4}, - {E1000_DEV_ID_PCH_SPT_I219_LM5}, - {E1000_DEV_ID_PCH_CNP_I219_LM6}, - {E1000_DEV_ID_PCH_CNP_I219_LM7}, - {E1000_DEV_ID_PCH_ICP_I219_LM8}, - {E1000_DEV_ID_PCH_ICP_I219_LM9}, - {E1000_DEV_ID_PCH_CMP_I219_LM10}, - {E1000_DEV_ID_PCH_CMP_I219_LM11}, - {E1000_DEV_ID_PCH_CMP_I219_LM12}, - {E1000_DEV_ID_PCH_TGP_I219_LM13}, - {E1000_DEV_ID_PCH_TGP_I219_LM14}, - {E1000_DEV_ID_PCH_TGP_I219_LM15}, - {0} -}; - -static bool e1000e_check_me(u16 device_id) -{ - struct e1000e_me_supported *id; - - for (id = (struct e1000e_me_supported *)me_supported; - id->device_id; id++) - if (device_id == id->device_id) - return true; - - return false; -} - /** * __ew32_prepare - prepare to write to MAC CSR register on certain parts * @hw: pointer to the HW structure @@ -6974,8 +6935,7 @@ static __maybe_unused int e1000e_pm_suspend(struct device *dev) e1000e_pm_thaw(dev); } else { /* Introduce S0ix implementation */ - if (hw->mac.type >= e1000_pch_cnp && - !e1000e_check_me(hw->adapter->pdev->device)) + if (hw->mac.type >= e1000_pch_cnp) e1000e_s0ix_entry_flow(adapter); } @@ -6991,8 +6951,7 @@ static __maybe_unused int e1000e_pm_resume(struct device *dev) int rc; /* Introduce S0ix implementation */ - if (hw->mac.type >= e1000_pch_cnp && - !e1000e_check_me(hw->adapter->pdev->device)) + if (hw->mac.type >= e1000_pch_cnp) e1000e_s0ix_exit_flow(adapter); rc = __e1000_resume(pdev);
commit e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME systems") disabled s0ix flows for systems that have various incarnations of the i219-LM ethernet controller. This changed caused power consumption regressions on the following shipping Dell Comet Lake based laptops: * Latitude 5310 * Latitude 5410 * Latitude 5410 * Latitude 5510 * Precision 3550 * Latitude 5411 * Latitude 5511 * Precision 3551 * Precision 7550 * Precision 7750 This commit was introduced because of some regressions on certain Thinkpad laptops. This comment was potentially caused by an earlier commit 632fbd5eb5b0e ("e1000e: fix S0ix flows for cable connected case"). or it was possibly caused by a system not meeting platform architectural requirements for low power consumption. Other changes made in the driver with extended timeouts are expected to make the driver more impervious to platform firmware behavior. Fixes: e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME systems") Reviewed-by: Alexander Duyck <alexander.duyck@gmail.com> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> --- drivers/net/ethernet/intel/e1000e/netdev.c | 45 +--------------------- 1 file changed, 2 insertions(+), 43 deletions(-)