| Message ID | 20231223032548.1680738-1-david.e.box@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Intel PMC Core GBE LTR regression fix | expand |
On Fri, 22 Dec 2023, David E. Box wrote: > This patch series addresses the network performance regression caused by > commit 804951203aa5 ("platform/x86:intel/pmc: Combine core_init() and > core_configure()"). > > Unfortunately, the regression is included in the recent Lunar Lake and > Arrow Lake support patches in the review branch. Patches 1 and 2 remove the > LTR ignore without a fix. They may be folded into the respective enabling > patches indicated in the changelog. This is done so that the next patches > fixing the regression can be backported to stable kernels with fewer, if > any, conflicts. > > Patches 3 and 4 provide the support needed for Patch 5 to move the GBE LTR > ignore from probe-time to suspend/resume time. All three carry the same > Fixes tag so that the stable kernels can pick them up without causing a > separate suspend-time PC10 regression. > > Patches 6 and 7 then add the LTR suspend/resume fix for Arrow Lake and > Lunar Lake. Of course, they cannot be folded into the enabling patches > unless the LTR fixes (3-5) are applied before. Sorry about this :(. Wow, this is messy... So the best order would be placing 3-5 before these Arrow Lake and Lunar Lake commits in for-next: 119652b855e6 ("platform/x86/intel/pmc: Add Lunar Lake M support to intel_pmc_core driver") f34dcf397286 ("platform/x86/intel/pmc: Add Arrow Lake S support to intel_pmc_core driver") ? And then folding 1-2 and 6-7 into those respective commits? It makes me wonder though why those two commits couldn't have been delayed slightly to get these fixes included first... :-/
Hi, On 12/27/23 19:14, Ilpo Järvinen wrote: > On Fri, 22 Dec 2023, David E. Box wrote: > >> This patch series addresses the network performance regression caused by >> commit 804951203aa5 ("platform/x86:intel/pmc: Combine core_init() and >> core_configure()"). >> >> Unfortunately, the regression is included in the recent Lunar Lake and >> Arrow Lake support patches in the review branch. Patches 1 and 2 remove the >> LTR ignore without a fix. They may be folded into the respective enabling >> patches indicated in the changelog. This is done so that the next patches >> fixing the regression can be backported to stable kernels with fewer, if >> any, conflicts. >> >> Patches 3 and 4 provide the support needed for Patch 5 to move the GBE LTR >> ignore from probe-time to suspend/resume time. All three carry the same >> Fixes tag so that the stable kernels can pick them up without causing a >> separate suspend-time PC10 regression. >> >> Patches 6 and 7 then add the LTR suspend/resume fix for Arrow Lake and >> Lunar Lake. Of course, they cannot be folded into the enabling patches >> unless the LTR fixes (3-5) are applied before. Sorry about this :(. > > Wow, this is messy... > > So the best order would be placing 3-5 before these Arrow Lake and Lunar > Lake commits in for-next: > 119652b855e6 ("platform/x86/intel/pmc: Add Lunar Lake M support to intel_pmc_core driver") > f34dcf397286 ("platform/x86/intel/pmc: Add Arrow Lake S support to intel_pmc_core driver") > ? And then folding 1-2 and 6-7 into those respective commits? > > It makes me wonder though why those two commits couldn't have been delayed > slightly to get these fixes included first... :-/ To untangle this mess I have squashed patches 1-2 into the original commits in for-next, so that there won't be a conflict between next and fixes when merging patches 3-5 into fixes. Ilpo can you pick-up patches 3-5 for the fixes branch ? And maybe also "platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe" fix ? I know you have a small review comment on this patch, but IMHO waiting for the small unrelated cleanup to be split out is not worth delaying this deadlock fix. As for the missing fixes tag I believe that should be: Fixes: 9745fb07474f ("platform/x86/intel: Add Primary to Sideband (P2SB) bridge support") And then do one more fixes pull-request for the GBT LTR fixes + the P2SB deadlock fix ? I know it is the holiday season, but if you feel up to it, it would be nice to get those fixes on their way to Linus and the stable kernels a bit earlier then before 6.8-rc1 . I'll merge patches 6-8 into for-next then after back-merging the fixes. Regards, Hans
On Thu, 28 Dec 2023, Hans de Goede wrote: > On 12/27/23 19:14, Ilpo Järvinen wrote: > > On Fri, 22 Dec 2023, David E. Box wrote: > > > >> This patch series addresses the network performance regression caused by > >> commit 804951203aa5 ("platform/x86:intel/pmc: Combine core_init() and > >> core_configure()"). > >> > >> Unfortunately, the regression is included in the recent Lunar Lake and > >> Arrow Lake support patches in the review branch. Patches 1 and 2 remove the > >> LTR ignore without a fix. They may be folded into the respective enabling > >> patches indicated in the changelog. This is done so that the next patches > >> fixing the regression can be backported to stable kernels with fewer, if > >> any, conflicts. > >> > >> Patches 3 and 4 provide the support needed for Patch 5 to move the GBE LTR > >> ignore from probe-time to suspend/resume time. All three carry the same > >> Fixes tag so that the stable kernels can pick them up without causing a > >> separate suspend-time PC10 regression. > >> > >> Patches 6 and 7 then add the LTR suspend/resume fix for Arrow Lake and > >> Lunar Lake. Of course, they cannot be folded into the enabling patches > >> unless the LTR fixes (3-5) are applied before. Sorry about this :(. > > > > Wow, this is messy... > > > > So the best order would be placing 3-5 before these Arrow Lake and Lunar > > Lake commits in for-next: > > 119652b855e6 ("platform/x86/intel/pmc: Add Lunar Lake M support to intel_pmc_core driver") > > f34dcf397286 ("platform/x86/intel/pmc: Add Arrow Lake S support to intel_pmc_core driver") > > ? And then folding 1-2 and 6-7 into those respective commits? > > > > It makes me wonder though why those two commits couldn't have been delayed > > slightly to get these fixes included first... :-/ > > To untangle this mess I have squashed patches 1-2 into the original > commits in for-next, so that there won't be a conflict > between next and fixes when merging patches 3-5 into fixes. Dream on, there will be conflicts, rest assured... > Ilpo can you pick-up patches 3-5 for the fixes branch ? I've now done that and resolved a few conflicts while doing so which you'll encounter while back-merging. > And maybe also "platform/x86: p2sb: Allow p2sb_bar() calls during PCI > device probe" fix ? I know you have a small review comment on this patch, > but IMHO waiting for the small unrelated cleanup to be split out is not > worth delaying this deadlock fix. As for the missing fixes tag I believe > that should be: > > Fixes: 9745fb07474f ("platform/x86/intel: Add Primary to Sideband (P2SB) bridge support") > > And then do one more fixes pull-request for the GBT LTR fixes + > the P2SB deadlock fix ? > > I know it is the holiday season, but if you feel up to it, > it would be nice to get those fixes on their way to Linus > and the stable kernels a bit earlier then before 6.8-rc1 . They're in the hands of lkp. > I'll merge patches 6-8 into for-next then after back-merging > the fixes.
This patch series addresses the network performance regression caused by commit 804951203aa5 ("platform/x86:intel/pmc: Combine core_init() and core_configure()"). Unfortunately, the regression is included in the recent Lunar Lake and Arrow Lake support patches in the review branch. Patches 1 and 2 remove the LTR ignore without a fix. They may be folded into the respective enabling patches indicated in the changelog. This is done so that the next patches fixing the regression can be backported to stable kernels with fewer, if any, conflicts. Patches 3 and 4 provide the support needed for Patch 5 to move the GBE LTR ignore from probe-time to suspend/resume time. All three carry the same Fixes tag so that the stable kernels can pick them up without causing a separate suspend-time PC10 regression. Patches 6 and 7 then add the LTR suspend/resume fix for Arrow Lake and Lunar Lake. Of course, they cannot be folded into the enabling patches unless the LTR fixes (3-5) are applied before. Sorry about this :(. Patch 8 finally addresses an unrelated sparse warning for a missing extern introduced in the commit mentioned in that changelog. This could be folded back into that commit if desired. Note that there is no current solution to address the loss of runtime PC10 with these patches. With a network cable attached to the PCH LAN, the best that can be achieved is PC8/9. However, this is unlikely to affect many mobile systems which tend not to use LAN and if they do, not the PCH LAN. David E. Box (8): platform/x86/intel/pmc/arl.c: Remove probe time LTR ignore platform/x86/intel/pmc/lnl.c: Remove probe time LTR ignore platform/x86/intel/pmc: Add suspend callback platform/x86/intel/pmc: Allow renabling LTRs platform/x86/intel/pmc: Move GBE LTR ignore to suspend callback platform/x86/intel/pmc/arl: Add GBE LTR ignore during suspend platform/x86/intel/pmc/lnl: Add GBE LTR ignore during suspend platform/x86/intel/pmc: Add missing extern drivers/platform/x86/intel/pmc/adl.c | 9 +++------ drivers/platform/x86/intel/pmc/arl.c | 9 +++------ drivers/platform/x86/intel/pmc/cnp.c | 26 ++++++++++++++++++++------ drivers/platform/x86/intel/pmc/core.c | 12 +++++++++--- drivers/platform/x86/intel/pmc/core.h | 9 ++++++++- drivers/platform/x86/intel/pmc/lnl.c | 9 +++------ drivers/platform/x86/intel/pmc/mtl.c | 9 +++------ drivers/platform/x86/intel/pmc/tgl.c | 8 +++----- 8 files changed, 52 insertions(+), 39 deletions(-) base-commit: 119652b855e6c96676406ee9a7f535f4db4e8eff