| Message ID | 20181106105314.30069-1-acelan.kao@canonical.com |
|---|---|
| Headers | show |
| Series | Power consumption during s2idle is higher than long idle(sk hynix) | expand |
On 11/06/18 11:53, AceLan Kao wrote: > BugLink: https://bugs.launchpad.net/bugs/1801875 > > [Impact] > On some Dell XPS models, we observed the power consumption raises higher > than long idle does during s2idle with sk hynix nvme. > > C2: > Short idle: 4 > Long idle: 1 > S2I: 3.7 > S5: 0.19 > > C3: > Short idle: 7.2 > Long idle: 4.5 > S2I: 6.22 > S5: 0.18 > > C5: > Short idle: 6.5 > Long idle: 1 > S2I: 2.88 > S5: 0.18 > > [Fix] > From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own > APST feature to do the power management. To leverage its APST feature > during s2idle, we can't disable nvme device while suspending, too. > So, here is what we did on the driver, 1. prevent nvme from entering > D3, 2. prevent nvme from being disabled when suspending. > > [Test] > Verified on different XPS machines with different sk hynix nvme disks, > it fixes the power consumption issue with no regression. And the power > consumption drops to 0.77W during s2idle. > > [Regression Potential] > Low, the patches only applied to specific nvme module, and from our > test, the system is still stable. > > [Misc] > This issue should be fixed by the firmware, and we're pushing sk hynix > to fix it. But before sk hynix find out how to solve it, we have to > preserve these commits in our kernel for a while. > BTW, the patches require pm_suspend_via_s2idle() function from below > commit which is still under reviewing. > https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html > > AceLan Kao (2): > pci: prevent sk hynix nvme from entering D3 > nvme: add quirk to not call disable function when suspending > > drivers/nvme/host/nvme.h | 5 +++++ > drivers/nvme/host/pci.c | 8 +++++++- > drivers/pci/quirks.c | 1 + > include/linux/pci_ids.h | 2 ++ > 4 files changed, 15 insertions(+), 1 deletion(-) > For Bionic and Cosmic: Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
On 06.11.18 11:53, AceLan Kao wrote: > BugLink: https://bugs.launchpad.net/bugs/1801875 > > [Impact] > On some Dell XPS models, we observed the power consumption raises higher > than long idle does during s2idle with sk hynix nvme. > > C2: > Short idle: 4 > Long idle: 1 > S2I: 3.7 > S5: 0.19 > > C3: > Short idle: 7.2 > Long idle: 4.5 > S2I: 6.22 > S5: 0.18 > > C5: > Short idle: 6.5 > Long idle: 1 > S2I: 2.88 > S5: 0.18 > > [Fix] > From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own > APST feature to do the power management. To leverage its APST feature > during s2idle, we can't disable nvme device while suspending, too. > So, here is what we did on the driver, 1. prevent nvme from entering > D3, 2. prevent nvme from being disabled when suspending. > > [Test] > Verified on different XPS machines with different sk hynix nvme disks, > it fixes the power consumption issue with no regression. And the power > consumption drops to 0.77W during s2idle. > > [Regression Potential] > Low, the patches only applied to specific nvme module, and from our > test, the system is still stable. > > [Misc] > This issue should be fixed by the firmware, and we're pushing sk hynix > to fix it. But before sk hynix find out how to solve it, we have to > preserve these commits in our kernel for a while. > BTW, the patches require pm_suspend_via_s2idle() function from below > commit which is still under reviewing. > https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html > > AceLan Kao (2): > pci: prevent sk hynix nvme from entering D3 > nvme: add quirk to not call disable function when suspending > > drivers/nvme/host/nvme.h | 5 +++++ > drivers/nvme/host/pci.c | 8 +++++++- > drivers/pci/quirks.c | 1 + > include/linux/pci_ids.h | 2 ++ > 4 files changed, 15 insertions(+), 1 deletion(-) > Patches look reasonable and bug report has expected tasks. Just cover email is a bit confusing because it does not list Cosmic at all. Acked-by: Stefan Bader <stefan.bader@canonical.com>
On Tue, Nov 06, 2018 at 06:53:10PM +0800, AceLan Kao wrote: > BugLink: https://bugs.launchpad.net/bugs/1801875 > > [Impact] > On some Dell XPS models, we observed the power consumption raises higher > than long idle does during s2idle with sk hynix nvme. > > C2: > Short idle: 4 > Long idle: 1 > S2I: 3.7 > S5: 0.19 > > C3: > Short idle: 7.2 > Long idle: 4.5 > S2I: 6.22 > S5: 0.18 > > C5: > Short idle: 6.5 > Long idle: 1 > S2I: 2.88 > S5: 0.18 > > [Fix] > From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own > APST feature to do the power management. To leverage its APST feature > during s2idle, we can't disable nvme device while suspending, too. > So, here is what we did on the driver, 1. prevent nvme from entering > D3, 2. prevent nvme from being disabled when suspending. > > [Test] > Verified on different XPS machines with different sk hynix nvme disks, > it fixes the power consumption issue with no regression. And the power > consumption drops to 0.77W during s2idle. > > [Regression Potential] > Low, the patches only applied to specific nvme module, and from our > test, the system is still stable. > > [Misc] > This issue should be fixed by the firmware, and we're pushing sk hynix > to fix it. But before sk hynix find out how to solve it, we have to > preserve these commits in our kernel for a while. > BTW, the patches require pm_suspend_via_s2idle() function from below > commit which is still under reviewing. > https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html Applied to unstable/master, thanks!
Does this need to be applied to bionic-oem specifically? It's been applied only to bionic/master-next (and cosmic/master-next). On 2018-11-06 18:53:10 , AceLan Kao wrote: > BugLink: https://bugs.launchpad.net/bugs/1801875 > > [Impact] > On some Dell XPS models, we observed the power consumption raises higher > than long idle does during s2idle with sk hynix nvme. > > C2: > Short idle: 4 > Long idle: 1 > S2I: 3.7 > S5: 0.19 > > C3: > Short idle: 7.2 > Long idle: 4.5 > S2I: 6.22 > S5: 0.18 > > C5: > Short idle: 6.5 > Long idle: 1 > S2I: 2.88 > S5: 0.18 > > [Fix] > From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own > APST feature to do the power management. To leverage its APST feature > during s2idle, we can't disable nvme device while suspending, too. > So, here is what we did on the driver, 1. prevent nvme from entering > D3, 2. prevent nvme from being disabled when suspending. > > [Test] > Verified on different XPS machines with different sk hynix nvme disks, > it fixes the power consumption issue with no regression. And the power > consumption drops to 0.77W during s2idle. > > [Regression Potential] > Low, the patches only applied to specific nvme module, and from our > test, the system is still stable. > > [Misc] > This issue should be fixed by the firmware, and we're pushing sk hynix > to fix it. But before sk hynix find out how to solve it, we have to > preserve these commits in our kernel for a while. > BTW, the patches require pm_suspend_via_s2idle() function from below > commit which is still under reviewing. > https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html > > AceLan Kao (2): > pci: prevent sk hynix nvme from entering D3 > nvme: add quirk to not call disable function when suspending > > drivers/nvme/host/nvme.h | 5 +++++ > drivers/nvme/host/pci.c | 8 +++++++- > drivers/pci/quirks.c | 1 + > include/linux/pci_ids.h | 2 ++ > 4 files changed, 15 insertions(+), 1 deletion(-) > > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
Yes, these commits should apply to bionic-oem, too. I think bionic-oem will rebase on generic kernel, so I didn't specify bionic-oem explicitly. Khaled Elmously <khalid.elmously@canonical.com> 於 2018年11月8日 週四 下午1:06寫道： > > Does this need to be applied to bionic-oem specifically? It's been applied only to bionic/master-next (and cosmic/master-next). > > > On 2018-11-06 18:53:10 , AceLan Kao wrote: > > BugLink: https://bugs.launchpad.net/bugs/1801875 > > > > [Impact] > > On some Dell XPS models, we observed the power consumption raises higher > > than long idle does during s2idle with sk hynix nvme. > > > > C2: > > Short idle: 4 > > Long idle: 1 > > S2I: 3.7 > > S5: 0.19 > > > > C3: > > Short idle: 7.2 > > Long idle: 4.5 > > S2I: 6.22 > > S5: 0.18 > > > > C5: > > Short idle: 6.5 > > Long idle: 1 > > S2I: 2.88 > > S5: 0.18 > > > > [Fix] > > From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own > > APST feature to do the power management. To leverage its APST feature > > during s2idle, we can't disable nvme device while suspending, too. > > So, here is what we did on the driver, 1. prevent nvme from entering > > D3, 2. prevent nvme from being disabled when suspending. > > > > [Test] > > Verified on different XPS machines with different sk hynix nvme disks, > > it fixes the power consumption issue with no regression. And the power > > consumption drops to 0.77W during s2idle. > > > > [Regression Potential] > > Low, the patches only applied to specific nvme module, and from our > > test, the system is still stable. > > > > [Misc] > > This issue should be fixed by the firmware, and we're pushing sk hynix > > to fix it. But before sk hynix find out how to solve it, we have to > > preserve these commits in our kernel for a while. > > BTW, the patches require pm_suspend_via_s2idle() function from below > > commit which is still under reviewing. > > https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html > > > > AceLan Kao (2): > > pci: prevent sk hynix nvme from entering D3 > > nvme: add quirk to not call disable function when suspending > > > > drivers/nvme/host/nvme.h | 5 +++++ > > drivers/nvme/host/pci.c | 8 +++++++- > > drivers/pci/quirks.c | 1 + > > include/linux/pci_ids.h | 2 ++ > > 4 files changed, 15 insertions(+), 1 deletion(-) > > > > -- > > 2.17.1 > > > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
BTW, my patches use a function, pm_suspend_via_s2idle(), from below commit which seems doesn't be applied to bionic/cosmic yet. I've give it one ack, please consider apply this commit before mine. Thanks https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html AceLan Kao <acelan.kao@canonical.com> 於 2018年11月8日 週四 下午4:32寫道： > > Yes, these commits should apply to bionic-oem, too. > I think bionic-oem will rebase on generic kernel, so I didn't specify > bionic-oem explicitly. > Khaled Elmously <khalid.elmously@canonical.com> 於 2018年11月8日 週四 下午1:06寫道： > > > > Does this need to be applied to bionic-oem specifically? It's been applied only to bionic/master-next (and cosmic/master-next). > > > > > > On 2018-11-06 18:53:10 , AceLan Kao wrote: > > > BugLink: https://bugs.launchpad.net/bugs/1801875 > > > > > > [Impact] > > > On some Dell XPS models, we observed the power consumption raises higher > > > than long idle does during s2idle with sk hynix nvme. > > > > > > C2: > > > Short idle: 4 > > > Long idle: 1 > > > S2I: 3.7 > > > S5: 0.19 > > > > > > C3: > > > Short idle: 7.2 > > > Long idle: 4.5 > > > S2I: 6.22 > > > S5: 0.18 > > > > > > C5: > > > Short idle: 6.5 > > > Long idle: 1 > > > S2I: 2.88 > > > S5: 0.18 > > > > > > [Fix] > > > From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own > > > APST feature to do the power management. To leverage its APST feature > > > during s2idle, we can't disable nvme device while suspending, too. > > > So, here is what we did on the driver, 1. prevent nvme from entering > > > D3, 2. prevent nvme from being disabled when suspending. > > > > > > [Test] > > > Verified on different XPS machines with different sk hynix nvme disks, > > > it fixes the power consumption issue with no regression. And the power > > > consumption drops to 0.77W during s2idle. > > > > > > [Regression Potential] > > > Low, the patches only applied to specific nvme module, and from our > > > test, the system is still stable. > > > > > > [Misc] > > > This issue should be fixed by the firmware, and we're pushing sk hynix > > > to fix it. But before sk hynix find out how to solve it, we have to > > > preserve these commits in our kernel for a while. > > > BTW, the patches require pm_suspend_via_s2idle() function from below > > > commit which is still under reviewing. > > > https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html > > > > > > AceLan Kao (2): > > > pci: prevent sk hynix nvme from entering D3 > > > nvme: add quirk to not call disable function when suspending > > > > > > drivers/nvme/host/nvme.h | 5 +++++ > > > drivers/nvme/host/pci.c | 8 +++++++- > > > drivers/pci/quirks.c | 1 + > > > include/linux/pci_ids.h | 2 ++ > > > 4 files changed, 15 insertions(+), 1 deletion(-) > > > > > > -- > > > 2.17.1 > > > > > > > > > -- > > > kernel-team mailing list > > > kernel-team@lists.ubuntu.com > > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 08.11.18 09:39, AceLan Kao wrote: > BTW, my patches use a function, pm_suspend_via_s2idle(), > from below commit which seems doesn't be applied to bionic/cosmic yet. > I've give it one ack, please consider apply this commit before mine. > Thanks > Then why was this not clearly documented in the patch submission? We cannot keep track of those dependencies and if things like that cause us grieve we might less easy going with future requests. > https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html > AceLan Kao <acelan.kao@canonical.com> 於 2018年11月8日 週四 下午4:32寫道： >> >> Yes, these commits should apply to bionic-oem, too. >> I think bionic-oem will rebase on generic kernel, so I didn't specify >> bionic-oem explicitly. >> Khaled Elmously <khalid.elmously@canonical.com> 於 2018年11月8日 週四 下午1:06寫道： >>> >>> Does this need to be applied to bionic-oem specifically? It's been applied only to bionic/master-next (and cosmic/master-next). Usually saying OEM is saying it needs to go there faster than via the main kernel. And usually AceLan or Timo will take of those things (at least that is my expectation). So from our stable point of view I blank any OEM-? targets. -Stefan >>> >>> >>> On 2018-11-06 18:53:10 , AceLan Kao wrote: >>>> BugLink: https://bugs.launchpad.net/bugs/1801875 >>>> >>>> [Impact] >>>> On some Dell XPS models, we observed the power consumption raises higher >>>> than long idle does during s2idle with sk hynix nvme. >>>> >>>> C2: >>>> Short idle: 4 >>>> Long idle: 1 >>>> S2I: 3.7 >>>> S5: 0.19 >>>> >>>> C3: >>>> Short idle: 7.2 >>>> Long idle: 4.5 >>>> S2I: 6.22 >>>> S5: 0.18 >>>> >>>> C5: >>>> Short idle: 6.5 >>>> Long idle: 1 >>>> S2I: 2.88 >>>> S5: 0.18 >>>> >>>> [Fix] >>>> From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own >>>> APST feature to do the power management. To leverage its APST feature >>>> during s2idle, we can't disable nvme device while suspending, too. >>>> So, here is what we did on the driver, 1. prevent nvme from entering >>>> D3, 2. prevent nvme from being disabled when suspending. >>>> >>>> [Test] >>>> Verified on different XPS machines with different sk hynix nvme disks, >>>> it fixes the power consumption issue with no regression. And the power >>>> consumption drops to 0.77W during s2idle. >>>> >>>> [Regression Potential] >>>> Low, the patches only applied to specific nvme module, and from our >>>> test, the system is still stable. >>>> >>>> [Misc] >>>> This issue should be fixed by the firmware, and we're pushing sk hynix >>>> to fix it. But before sk hynix find out how to solve it, we have to >>>> preserve these commits in our kernel for a while. >>>> BTW, the patches require pm_suspend_via_s2idle() function from below >>>> commit which is still under reviewing. >>>> https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html >>>> >>>> AceLan Kao (2): >>>> pci: prevent sk hynix nvme from entering D3 >>>> nvme: add quirk to not call disable function when suspending >>>> >>>> drivers/nvme/host/nvme.h | 5 +++++ >>>> drivers/nvme/host/pci.c | 8 +++++++- >>>> drivers/pci/quirks.c | 1 + >>>> include/linux/pci_ids.h | 2 ++ >>>> 4 files changed, 15 insertions(+), 1 deletion(-) >>>> >>>> -- >>>> 2.17.1 >>>> >>>> >>>> -- >>>> kernel-team mailing list >>>> kernel-team@lists.ubuntu.com >>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team >
Yes, i've written some words about the dependency in cover letter. I'll try to point it out more clearly next time. <quote> [Misc] This issue should be fixed by the firmware, and we're pushing sk hynix to fix it. But before sk hynix find out how to solve it, we have to preserve these commits in our kernel for a while. BTW, the patches require pm_suspend_via_s2idle() function from below commit which is still under reviewing. https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html </quote> Stefan Bader <stefan.bader@canonical.com> 於 2018年11月8日 週四 下午6:21寫道： > > On 08.11.18 09:39, AceLan Kao wrote: > > BTW, my patches use a function, pm_suspend_via_s2idle(), > > from below commit which seems doesn't be applied to bionic/cosmic yet. > > I've give it one ack, please consider apply this commit before mine. > > Thanks > > > Then why was this not clearly documented in the patch submission? We cannot keep > track of those dependencies and if things like that cause us grieve we might > less easy going with future requests. > > > https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html > > AceLan Kao <acelan.kao@canonical.com> 於 2018年11月8日 週四 下午4:32寫道： > >> > >> Yes, these commits should apply to bionic-oem, too. > >> I think bionic-oem will rebase on generic kernel, so I didn't specify > >> bionic-oem explicitly. > >> Khaled Elmously <khalid.elmously@canonical.com> 於 2018年11月8日 週四 下午1:06寫道： > >>> > >>> Does this need to be applied to bionic-oem specifically? It's been applied only to bionic/master-next (and cosmic/master-next). > > Usually saying OEM is saying it needs to go there faster than via the main > kernel. And usually AceLan or Timo will take of those things (at least that is > my expectation). So from our stable point of view I blank any OEM-? targets. > > -Stefan > >>> > >>> > >>> On 2018-11-06 18:53:10 , AceLan Kao wrote: > >>>> BugLink: https://bugs.launchpad.net/bugs/1801875 > >>>> > >>>> [Impact] > >>>> On some Dell XPS models, we observed the power consumption raises higher > >>>> than long idle does during s2idle with sk hynix nvme. > >>>> > >>>> C2: > >>>> Short idle: 4 > >>>> Long idle: 1 > >>>> S2I: 3.7 > >>>> S5: 0.19 > >>>> > >>>> C3: > >>>> Short idle: 7.2 > >>>> Long idle: 4.5 > >>>> S2I: 6.22 > >>>> S5: 0.18 > >>>> > >>>> C5: > >>>> Short idle: 6.5 > >>>> Long idle: 1 > >>>> S2I: 2.88 > >>>> S5: 0.18 > >>>> > >>>> [Fix] > >>>> From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own > >>>> APST feature to do the power management. To leverage its APST feature > >>>> during s2idle, we can't disable nvme device while suspending, too. > >>>> So, here is what we did on the driver, 1. prevent nvme from entering > >>>> D3, 2. prevent nvme from being disabled when suspending. > >>>> > >>>> [Test] > >>>> Verified on different XPS machines with different sk hynix nvme disks, > >>>> it fixes the power consumption issue with no regression. And the power > >>>> consumption drops to 0.77W during s2idle. > >>>> > >>>> [Regression Potential] > >>>> Low, the patches only applied to specific nvme module, and from our > >>>> test, the system is still stable. > >>>> > >>>> [Misc] > >>>> This issue should be fixed by the firmware, and we're pushing sk hynix > >>>> to fix it. But before sk hynix find out how to solve it, we have to > >>>> preserve these commits in our kernel for a while. > >>>> BTW, the patches require pm_suspend_via_s2idle() function from below > >>>> commit which is still under reviewing. > >>>> https://lists.ubuntu.com/archives/kernel-team/2018-October/096141.html > >>>> > >>>> AceLan Kao (2): > >>>> pci: prevent sk hynix nvme from entering D3 > >>>> nvme: add quirk to not call disable function when suspending > >>>> > >>>> drivers/nvme/host/nvme.h | 5 +++++ > >>>> drivers/nvme/host/pci.c | 8 +++++++- > >>>> drivers/pci/quirks.c | 1 + > >>>> include/linux/pci_ids.h | 2 ++ > >>>> 4 files changed, 15 insertions(+), 1 deletion(-) > >>>> > >>>> -- > >>>> 2.17.1 > >>>> > >>>> > >>>> -- > >>>> kernel-team mailing list > >>>> kernel-team@lists.ubuntu.com > >>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team > > > >