Message ID | 20240906184016.268153-1-xi.pardee@linux.intel.com (mailing list archive) |
---|---|
State | Accepted, archived |
Headers | show |
Series | [v2] platform/x86:intel/pmc: Ignore all LTRs during suspend | expand |
On Fri, 6 Sep 2024, Xi Pardee wrote: > From: Xi Pardee <xi.pardee@intel.com> > > Add support to ignore all LTRs before suspend and restore the previous > LTR values after suspend. This feature could be turned off with module > parameter ltr_ignore_all_suspend. > > LTR value is a mechanism for a device to indicate tolerance to access > the corresponding resource. When system suspends, the resource is not > available and therefore the LTR value could be ignored. Ignoring all > LTR values prevents problematic device from blocking the system to get > to the deepest package state during suspend. > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Signed-off-by: Xi Pardee <xi.pardee@intel.com> > > v2: > - Add more details to commit message > - Fix format: ltr->LTR, S0IX->S0ix, space between name and email > > --- > drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++ > drivers/platform/x86/intel/pmc/core.h | 2 + > 2 files changed, 55 insertions(+) > > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c > index 01ae71c6df59..0ec703af16a4 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused) > } > DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker); > > +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { > + struct pmc *pmc; > + u32 ltr_ign; > + > + pmc = pmcdev->pmcs[i]; > + if (!pmc) > + continue; > + > + guard(mutex)(&pmcdev->lock); > + pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset); > + > + /* ltr_ignore_max is the max index value for LTR ignore register */ > + ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0); > + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign); > + } > + > + /* > + * Ignoring ME during suspend is blocking platforms with ADL PCH to get to > + * deeper S0ix substate. > + */ > + pmc_core_send_ltr_ignore(pmcdev, 6, 0); > +} > + > +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { > + struct pmc *pmc; > + > + pmc = pmcdev->pmcs[i]; > + if (!pmc) > + continue; > + > + guard(mutex)(&pmcdev->lock); > + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign); > + } > +} > + > static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset, > const int lpm_adj_x2) > { > @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures; > module_param(warn_on_s0ix_failures, bool, 0644); > MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures"); > > +static bool ltr_ignore_all_suspend = true; > +module_param(ltr_ignore_all_suspend, bool, 0644); > +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend"); > + > static __maybe_unused int pmc_core_suspend(struct device *dev) > { > struct pmc_dev *pmcdev = dev_get_drvdata(dev); > @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) > if (pmcdev->suspend) > pmcdev->suspend(pmcdev); > > + if (ltr_ignore_all_suspend) > + pmc_core_ltr_ignore_all(pmcdev); > + > /* Check if the syspend will actually use S0ix */ > if (pm_suspend_via_firmware()) > return 0; > @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev) > { > struct pmc_dev *pmcdev = dev_get_drvdata(dev); > > + if (ltr_ignore_all_suspend) > + pmc_core_ltr_restore_all(pmcdev); > + > if (pmcdev->resume) > return pmcdev->resume(pmcdev); > > diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h > index ea04de7eb9e8..e862ea88b816 100644 > --- a/drivers/platform/x86/intel/pmc/core.h > +++ b/drivers/platform/x86/intel/pmc/core.h > @@ -372,6 +372,7 @@ struct pmc_info { > * @map: pointer to pmc_reg_map struct that contains platform > * specific attributes > * @lpm_req_regs: List of substate requirements > + * @ltr_ign: Holds LTR ignore data while suspended > * > * pmc contains info about one power management controller device. > */ > @@ -380,6 +381,7 @@ struct pmc { > void __iomem *regbase; > const struct pmc_reg_map *map; > u32 *lpm_req_regs; > + u32 ltr_ign; > }; > > /** > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Hi, On 9/9/2024 1:07 AM, Ilpo Järvinen wrote: > On Fri, 6 Sep 2024, Xi Pardee wrote: > >> From: Xi Pardee <xi.pardee@intel.com> >> >> Add support to ignore all LTRs before suspend and restore the previous >> LTR values after suspend. This feature could be turned off with module >> parameter ltr_ignore_all_suspend. >> >> LTR value is a mechanism for a device to indicate tolerance to access >> the corresponding resource. When system suspends, the resource is not >> available and therefore the LTR value could be ignored. Ignoring all >> LTR values prevents problematic device from blocking the system to get >> to the deepest package state during suspend. >> >> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Signed-off-by: Xi Pardee <xi.pardee@intel.com> >> >> v2: >> - Add more details to commit message >> - Fix format: ltr->LTR, S0IX->S0ix, space between name and email >> >> --- >> drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++ >> drivers/platform/x86/intel/pmc/core.h | 2 + >> 2 files changed, 55 insertions(+) >> >> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c >> index 01ae71c6df59..0ec703af16a4 100644 >> --- a/drivers/platform/x86/intel/pmc/core.c >> +++ b/drivers/platform/x86/intel/pmc/core.c >> @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused) >> } >> DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker); >> >> +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev) >> +{ >> + unsigned int i; >> + >> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { >> + struct pmc *pmc; >> + u32 ltr_ign; >> + >> + pmc = pmcdev->pmcs[i]; >> + if (!pmc) >> + continue; >> + >> + guard(mutex)(&pmcdev->lock); >> + pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset); >> + >> + /* ltr_ignore_max is the max index value for LTR ignore register */ >> + ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0); >> + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign); >> + } >> + >> + /* >> + * Ignoring ME during suspend is blocking platforms with ADL PCH to get to >> + * deeper S0ix substate. >> + */ >> + pmc_core_send_ltr_ignore(pmcdev, 6, 0); >> +} >> + >> +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev) >> +{ >> + unsigned int i; >> + >> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { >> + struct pmc *pmc; >> + >> + pmc = pmcdev->pmcs[i]; >> + if (!pmc) >> + continue; >> + >> + guard(mutex)(&pmcdev->lock); >> + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign); >> + } >> +} >> + >> static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset, >> const int lpm_adj_x2) >> { >> @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures; >> module_param(warn_on_s0ix_failures, bool, 0644); >> MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures"); >> >> +static bool ltr_ignore_all_suspend = true; >> +module_param(ltr_ignore_all_suspend, bool, 0644); >> +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend"); >> + >> static __maybe_unused int pmc_core_suspend(struct device *dev) >> { >> struct pmc_dev *pmcdev = dev_get_drvdata(dev); >> @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) >> if (pmcdev->suspend) >> pmcdev->suspend(pmcdev); >> >> + if (ltr_ignore_all_suspend) >> + pmc_core_ltr_ignore_all(pmcdev); >> + >> /* Check if the syspend will actually use S0ix */ >> if (pm_suspend_via_firmware()) >> return 0; >> @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev) >> { >> struct pmc_dev *pmcdev = dev_get_drvdata(dev); >> >> + if (ltr_ignore_all_suspend) >> + pmc_core_ltr_restore_all(pmcdev); >> + >> if (pmcdev->resume) >> return pmcdev->resume(pmcdev); >> >> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h >> index ea04de7eb9e8..e862ea88b816 100644 >> --- a/drivers/platform/x86/intel/pmc/core.h >> +++ b/drivers/platform/x86/intel/pmc/core.h >> @@ -372,6 +372,7 @@ struct pmc_info { >> * @map: pointer to pmc_reg_map struct that contains platform >> * specific attributes >> * @lpm_req_regs: List of substate requirements >> + * @ltr_ign: Holds LTR ignore data while suspended >> * >> * pmc contains info about one power management controller device. >> */ >> @@ -380,6 +381,7 @@ struct pmc { >> void __iomem *regbase; >> const struct pmc_reg_map *map; >> u32 *lpm_req_regs; >> + u32 ltr_ign; >> }; >> >> /** >> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Thanks for the Reviewed-by tag! I wonder if I need to send another version with the Reviewed-by tag for this patch to be accepted. Thanks! Xi
Hi Xi, On 9/10/24 2:43 AM, Xi Pardee wrote: > Hi, > > On 9/9/2024 1:07 AM, Ilpo Järvinen wrote: >> On Fri, 6 Sep 2024, Xi Pardee wrote: >> >>> From: Xi Pardee <xi.pardee@intel.com> >>> >>> Add support to ignore all LTRs before suspend and restore the previous >>> LTR values after suspend. This feature could be turned off with module >>> parameter ltr_ignore_all_suspend. >>> >>> LTR value is a mechanism for a device to indicate tolerance to access >>> the corresponding resource. When system suspends, the resource is not >>> available and therefore the LTR value could be ignored. Ignoring all >>> LTR values prevents problematic device from blocking the system to get >>> to the deepest package state during suspend. >>> >>> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> Signed-off-by: Xi Pardee <xi.pardee@intel.com> >>> >>> v2: >>> - Add more details to commit message >>> - Fix format: ltr->LTR, S0IX->S0ix, space between name and email >>> >>> --- >>> drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++ >>> drivers/platform/x86/intel/pmc/core.h | 2 + >>> 2 files changed, 55 insertions(+) >>> >>> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c >>> index 01ae71c6df59..0ec703af16a4 100644 >>> --- a/drivers/platform/x86/intel/pmc/core.c >>> +++ b/drivers/platform/x86/intel/pmc/core.c >>> @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused) >>> } >>> DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker); >>> +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev) >>> +{ >>> + unsigned int i; >>> + >>> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { >>> + struct pmc *pmc; >>> + u32 ltr_ign; >>> + >>> + pmc = pmcdev->pmcs[i]; >>> + if (!pmc) >>> + continue; >>> + >>> + guard(mutex)(&pmcdev->lock); >>> + pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset); >>> + >>> + /* ltr_ignore_max is the max index value for LTR ignore register */ >>> + ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0); >>> + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign); >>> + } >>> + >>> + /* >>> + * Ignoring ME during suspend is blocking platforms with ADL PCH to get to >>> + * deeper S0ix substate. >>> + */ >>> + pmc_core_send_ltr_ignore(pmcdev, 6, 0); >>> +} >>> + >>> +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev) >>> +{ >>> + unsigned int i; >>> + >>> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { >>> + struct pmc *pmc; >>> + >>> + pmc = pmcdev->pmcs[i]; >>> + if (!pmc) >>> + continue; >>> + >>> + guard(mutex)(&pmcdev->lock); >>> + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign); >>> + } >>> +} >>> + >>> static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset, >>> const int lpm_adj_x2) >>> { >>> @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures; >>> module_param(warn_on_s0ix_failures, bool, 0644); >>> MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures"); >>> +static bool ltr_ignore_all_suspend = true; >>> +module_param(ltr_ignore_all_suspend, bool, 0644); >>> +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend"); >>> + >>> static __maybe_unused int pmc_core_suspend(struct device *dev) >>> { >>> struct pmc_dev *pmcdev = dev_get_drvdata(dev); >>> @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) >>> if (pmcdev->suspend) >>> pmcdev->suspend(pmcdev); >>> + if (ltr_ignore_all_suspend) >>> + pmc_core_ltr_ignore_all(pmcdev); >>> + >>> /* Check if the syspend will actually use S0ix */ >>> if (pm_suspend_via_firmware()) >>> return 0; >>> @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev) >>> { >>> struct pmc_dev *pmcdev = dev_get_drvdata(dev); >>> + if (ltr_ignore_all_suspend) >>> + pmc_core_ltr_restore_all(pmcdev); >>> + >>> if (pmcdev->resume) >>> return pmcdev->resume(pmcdev); >>> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h >>> index ea04de7eb9e8..e862ea88b816 100644 >>> --- a/drivers/platform/x86/intel/pmc/core.h >>> +++ b/drivers/platform/x86/intel/pmc/core.h >>> @@ -372,6 +372,7 @@ struct pmc_info { >>> * @map: pointer to pmc_reg_map struct that contains platform >>> * specific attributes >>> * @lpm_req_regs: List of substate requirements >>> + * @ltr_ign: Holds LTR ignore data while suspended >>> * >>> * pmc contains info about one power management controller device. >>> */ >>> @@ -380,6 +381,7 @@ struct pmc { >>> void __iomem *regbase; >>> const struct pmc_reg_map *map; >>> u32 *lpm_req_regs; >>> + u32 ltr_ign; >>> }; >>> /** >>> >> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > Thanks for the Reviewed-by tag! I wonder if I need to send another version with the Reviewed-by tag for this patch to be accepted. There is no need for a v3. I'll merge this patch into my review-hans branch (and from there on it will move to for-next) soon. Regards, Hans
Hi, On 9/6/24 8:40 PM, Xi Pardee wrote: > From: Xi Pardee <xi.pardee@intel.com> > > Add support to ignore all LTRs before suspend and restore the previous > LTR values after suspend. This feature could be turned off with module > parameter ltr_ignore_all_suspend. > > LTR value is a mechanism for a device to indicate tolerance to access > the corresponding resource. When system suspends, the resource is not > available and therefore the LTR value could be ignored. Ignoring all > LTR values prevents problematic device from blocking the system to get > to the deepest package state during suspend. > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Signed-off-by: Xi Pardee <xi.pardee@intel.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > > v2: > - Add more details to commit message > - Fix format: ltr->LTR, S0IX->S0ix, space between name and email > > --- > drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++ > drivers/platform/x86/intel/pmc/core.h | 2 + > 2 files changed, 55 insertions(+) > > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c > index 01ae71c6df59..0ec703af16a4 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused) > } > DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker); > > +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { > + struct pmc *pmc; > + u32 ltr_ign; > + > + pmc = pmcdev->pmcs[i]; > + if (!pmc) > + continue; > + > + guard(mutex)(&pmcdev->lock); > + pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset); > + > + /* ltr_ignore_max is the max index value for LTR ignore register */ > + ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0); > + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign); > + } > + > + /* > + * Ignoring ME during suspend is blocking platforms with ADL PCH to get to > + * deeper S0ix substate. > + */ > + pmc_core_send_ltr_ignore(pmcdev, 6, 0); > +} > + > +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { > + struct pmc *pmc; > + > + pmc = pmcdev->pmcs[i]; > + if (!pmc) > + continue; > + > + guard(mutex)(&pmcdev->lock); > + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign); > + } > +} > + > static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset, > const int lpm_adj_x2) > { > @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures; > module_param(warn_on_s0ix_failures, bool, 0644); > MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures"); > > +static bool ltr_ignore_all_suspend = true; > +module_param(ltr_ignore_all_suspend, bool, 0644); > +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend"); > + > static __maybe_unused int pmc_core_suspend(struct device *dev) > { > struct pmc_dev *pmcdev = dev_get_drvdata(dev); > @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) > if (pmcdev->suspend) > pmcdev->suspend(pmcdev); > > + if (ltr_ignore_all_suspend) > + pmc_core_ltr_ignore_all(pmcdev); > + > /* Check if the syspend will actually use S0ix */ > if (pm_suspend_via_firmware()) > return 0; > @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev) > { > struct pmc_dev *pmcdev = dev_get_drvdata(dev); > > + if (ltr_ignore_all_suspend) > + pmc_core_ltr_restore_all(pmcdev); > + > if (pmcdev->resume) > return pmcdev->resume(pmcdev); > > diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h > index ea04de7eb9e8..e862ea88b816 100644 > --- a/drivers/platform/x86/intel/pmc/core.h > +++ b/drivers/platform/x86/intel/pmc/core.h > @@ -372,6 +372,7 @@ struct pmc_info { > * @map: pointer to pmc_reg_map struct that contains platform > * specific attributes > * @lpm_req_regs: List of substate requirements > + * @ltr_ign: Holds LTR ignore data while suspended > * > * pmc contains info about one power management controller device. > */ > @@ -380,6 +381,7 @@ struct pmc { > void __iomem *regbase; > const struct pmc_reg_map *map; > u32 *lpm_req_regs; > + u32 ltr_ign; > }; > > /**
Hi, On 9/11/24 2:16 PM, Hans de Goede wrote: > Hi, > > On 9/6/24 8:40 PM, Xi Pardee wrote: >> From: Xi Pardee <xi.pardee@intel.com> >> >> Add support to ignore all LTRs before suspend and restore the previous >> LTR values after suspend. This feature could be turned off with module >> parameter ltr_ignore_all_suspend. >> >> LTR value is a mechanism for a device to indicate tolerance to access >> the corresponding resource. When system suspends, the resource is not >> available and therefore the LTR value could be ignored. Ignoring all >> LTR values prevents problematic device from blocking the system to get >> to the deepest package state during suspend. >> >> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Signed-off-by: Xi Pardee <xi.pardee@intel.com> > > Thanks, patch looks good to me: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> Oops, wrong auto-reply, what I meant is: Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans >> v2: >> - Add more details to commit message >> - Fix format: ltr->LTR, S0IX->S0ix, space between name and email >> >> --- >> drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++ >> drivers/platform/x86/intel/pmc/core.h | 2 + >> 2 files changed, 55 insertions(+) >> >> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c >> index 01ae71c6df59..0ec703af16a4 100644 >> --- a/drivers/platform/x86/intel/pmc/core.c >> +++ b/drivers/platform/x86/intel/pmc/core.c >> @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused) >> } >> DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker); >> >> +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev) >> +{ >> + unsigned int i; >> + >> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { >> + struct pmc *pmc; >> + u32 ltr_ign; >> + >> + pmc = pmcdev->pmcs[i]; >> + if (!pmc) >> + continue; >> + >> + guard(mutex)(&pmcdev->lock); >> + pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset); >> + >> + /* ltr_ignore_max is the max index value for LTR ignore register */ >> + ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0); >> + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign); >> + } >> + >> + /* >> + * Ignoring ME during suspend is blocking platforms with ADL PCH to get to >> + * deeper S0ix substate. >> + */ >> + pmc_core_send_ltr_ignore(pmcdev, 6, 0); >> +} >> + >> +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev) >> +{ >> + unsigned int i; >> + >> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { >> + struct pmc *pmc; >> + >> + pmc = pmcdev->pmcs[i]; >> + if (!pmc) >> + continue; >> + >> + guard(mutex)(&pmcdev->lock); >> + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign); >> + } >> +} >> + >> static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset, >> const int lpm_adj_x2) >> { >> @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures; >> module_param(warn_on_s0ix_failures, bool, 0644); >> MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures"); >> >> +static bool ltr_ignore_all_suspend = true; >> +module_param(ltr_ignore_all_suspend, bool, 0644); >> +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend"); >> + >> static __maybe_unused int pmc_core_suspend(struct device *dev) >> { >> struct pmc_dev *pmcdev = dev_get_drvdata(dev); >> @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) >> if (pmcdev->suspend) >> pmcdev->suspend(pmcdev); >> >> + if (ltr_ignore_all_suspend) >> + pmc_core_ltr_ignore_all(pmcdev); >> + >> /* Check if the syspend will actually use S0ix */ >> if (pm_suspend_via_firmware()) >> return 0; >> @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev) >> { >> struct pmc_dev *pmcdev = dev_get_drvdata(dev); >> >> + if (ltr_ignore_all_suspend) >> + pmc_core_ltr_restore_all(pmcdev); >> + >> if (pmcdev->resume) >> return pmcdev->resume(pmcdev); >> >> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h >> index ea04de7eb9e8..e862ea88b816 100644 >> --- a/drivers/platform/x86/intel/pmc/core.h >> +++ b/drivers/platform/x86/intel/pmc/core.h >> @@ -372,6 +372,7 @@ struct pmc_info { >> * @map: pointer to pmc_reg_map struct that contains platform >> * specific attributes >> * @lpm_req_regs: List of substate requirements >> + * @ltr_ign: Holds LTR ignore data while suspended >> * >> * pmc contains info about one power management controller device. >> */ >> @@ -380,6 +381,7 @@ struct pmc { >> void __iomem *regbase; >> const struct pmc_reg_map *map; >> u32 *lpm_req_regs; >> + u32 ltr_ign; >> }; >> >> /** >
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c index 01ae71c6df59..0ec703af16a4 100644 --- a/drivers/platform/x86/intel/pmc/core.c +++ b/drivers/platform/x86/intel/pmc/core.c @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused) } DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker); +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { + struct pmc *pmc; + u32 ltr_ign; + + pmc = pmcdev->pmcs[i]; + if (!pmc) + continue; + + guard(mutex)(&pmcdev->lock); + pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset); + + /* ltr_ignore_max is the max index value for LTR ignore register */ + ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0); + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign); + } + + /* + * Ignoring ME during suspend is blocking platforms with ADL PCH to get to + * deeper S0ix substate. + */ + pmc_core_send_ltr_ignore(pmcdev, 6, 0); +} + +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) { + struct pmc *pmc; + + pmc = pmcdev->pmcs[i]; + if (!pmc) + continue; + + guard(mutex)(&pmcdev->lock); + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign); + } +} + static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset, const int lpm_adj_x2) { @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures; module_param(warn_on_s0ix_failures, bool, 0644); MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures"); +static bool ltr_ignore_all_suspend = true; +module_param(ltr_ignore_all_suspend, bool, 0644); +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend"); + static __maybe_unused int pmc_core_suspend(struct device *dev) { struct pmc_dev *pmcdev = dev_get_drvdata(dev); @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) if (pmcdev->suspend) pmcdev->suspend(pmcdev); + if (ltr_ignore_all_suspend) + pmc_core_ltr_ignore_all(pmcdev); + /* Check if the syspend will actually use S0ix */ if (pm_suspend_via_firmware()) return 0; @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev) { struct pmc_dev *pmcdev = dev_get_drvdata(dev); + if (ltr_ignore_all_suspend) + pmc_core_ltr_restore_all(pmcdev); + if (pmcdev->resume) return pmcdev->resume(pmcdev); diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h index ea04de7eb9e8..e862ea88b816 100644 --- a/drivers/platform/x86/intel/pmc/core.h +++ b/drivers/platform/x86/intel/pmc/core.h @@ -372,6 +372,7 @@ struct pmc_info { * @map: pointer to pmc_reg_map struct that contains platform * specific attributes * @lpm_req_regs: List of substate requirements + * @ltr_ign: Holds LTR ignore data while suspended * * pmc contains info about one power management controller device. */ @@ -380,6 +381,7 @@ struct pmc { void __iomem *regbase; const struct pmc_reg_map *map; u32 *lpm_req_regs; + u32 ltr_ign; }; /**