| Message ID | 20210430060736.590321-1-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: hp_accel: Avoid invoking _INI to speed up resume | expand |
On 30/04/2021 08:07, Kai-Heng Feng wrote: > hp_accel can take almost two seconds to resume on some HP laptops. > > The bottleneck is on evaluating _INI, which is only needed to run once. > > Resolve the issue by only invoking _INI when it's necessary. Namely, on > probe and on hibernation restore. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Although I'm listed as the maintainer, I don't have the hardware anymore to test. However, the changes seem fine to me. Acked-by: Éric Piel <eric.piel@trempplin-utc.net> Best, Éric > --- > drivers/misc/lis3lv02d/lis3lv02d.h | 1 + > drivers/platform/x86/hp_accel.c | 22 +++++++++++++++++++++- > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h > index c394c0b08519a..7ac788fae1b86 100644 > --- a/drivers/misc/lis3lv02d/lis3lv02d.h > +++ b/drivers/misc/lis3lv02d/lis3lv02d.h > @@ -271,6 +271,7 @@ struct lis3lv02d { > int regs_size; > u8 *reg_cache; > bool regs_stored; > + bool init_required; > u8 odr_mask; /* ODR bit mask */ > u8 whoami; /* indicates measurement precision */ > s16 (*read_data) (struct lis3lv02d *lis3, int reg); > diff --git a/drivers/platform/x86/hp_accel.c b/drivers/platform/x86/hp_accel.c > index 799cbe2ffcf36..8c0867bda8280 100644 > --- a/drivers/platform/x86/hp_accel.c > +++ b/drivers/platform/x86/hp_accel.c > @@ -88,6 +88,9 @@ MODULE_DEVICE_TABLE(acpi, lis3lv02d_device_ids); > static int lis3lv02d_acpi_init(struct lis3lv02d *lis3) > { > struct acpi_device *dev = lis3->bus_priv; > + if (!lis3->init_required) > + return 0; > + > if (acpi_evaluate_object(dev->handle, METHOD_NAME__INI, > NULL, NULL) != AE_OK) > return -EINVAL; > @@ -356,6 +359,7 @@ static int lis3lv02d_add(struct acpi_device *device) > } > > /* call the core layer do its init */ > + lis3_dev.init_required = true; > ret = lis3lv02d_init_device(&lis3_dev); > if (ret) > return ret; > @@ -403,11 +407,27 @@ static int lis3lv02d_suspend(struct device *dev) > > static int lis3lv02d_resume(struct device *dev) > { > + lis3_dev.init_required = false; > + lis3lv02d_poweron(&lis3_dev); > + return 0; > +} > + > +static int lis3lv02d_restore(struct device *dev) > +{ > + lis3_dev.init_required = true; > lis3lv02d_poweron(&lis3_dev); > return 0; > } > > -static SIMPLE_DEV_PM_OPS(hp_accel_pm, lis3lv02d_suspend, lis3lv02d_resume); > +static const struct dev_pm_ops hp_accel_pm = { > + .suspend = lis3lv02d_suspend, > + .resume = lis3lv02d_resume, > + .freeze = lis3lv02d_suspend, > + .thaw = lis3lv02d_resume, > + .poweroff = lis3lv02d_suspend, > + .restore = lis3lv02d_restore, > +}; > + > #define HP_ACCEL_PM (&hp_accel_pm) > #else > #define HP_ACCEL_PM NULL >
Hi, On 4/30/21 8:07 AM, Kai-Heng Feng wrote: > hp_accel can take almost two seconds to resume on some HP laptops. > > The bottleneck is on evaluating _INI, which is only needed to run once. > > Resolve the issue by only invoking _INI when it's necessary. Namely, on > probe and on hibernation restore. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Thank you I've added this to my review-hans branch. I'll also add this to my fixes branch and include it in a future pdx86 fixes pull-req for Linus for 5.13. 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 > --- > drivers/misc/lis3lv02d/lis3lv02d.h | 1 + > drivers/platform/x86/hp_accel.c | 22 +++++++++++++++++++++- > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h > index c394c0b08519a..7ac788fae1b86 100644 > --- a/drivers/misc/lis3lv02d/lis3lv02d.h > +++ b/drivers/misc/lis3lv02d/lis3lv02d.h > @@ -271,6 +271,7 @@ struct lis3lv02d { > int regs_size; > u8 *reg_cache; > bool regs_stored; > + bool init_required; > u8 odr_mask; /* ODR bit mask */ > u8 whoami; /* indicates measurement precision */ > s16 (*read_data) (struct lis3lv02d *lis3, int reg); > diff --git a/drivers/platform/x86/hp_accel.c b/drivers/platform/x86/hp_accel.c > index 799cbe2ffcf36..8c0867bda8280 100644 > --- a/drivers/platform/x86/hp_accel.c > +++ b/drivers/platform/x86/hp_accel.c > @@ -88,6 +88,9 @@ MODULE_DEVICE_TABLE(acpi, lis3lv02d_device_ids); > static int lis3lv02d_acpi_init(struct lis3lv02d *lis3) > { > struct acpi_device *dev = lis3->bus_priv; > + if (!lis3->init_required) > + return 0; > + > if (acpi_evaluate_object(dev->handle, METHOD_NAME__INI, > NULL, NULL) != AE_OK) > return -EINVAL; > @@ -356,6 +359,7 @@ static int lis3lv02d_add(struct acpi_device *device) > } > > /* call the core layer do its init */ > + lis3_dev.init_required = true; > ret = lis3lv02d_init_device(&lis3_dev); > if (ret) > return ret; > @@ -403,11 +407,27 @@ static int lis3lv02d_suspend(struct device *dev) > > static int lis3lv02d_resume(struct device *dev) > { > + lis3_dev.init_required = false; > + lis3lv02d_poweron(&lis3_dev); > + return 0; > +} > + > +static int lis3lv02d_restore(struct device *dev) > +{ > + lis3_dev.init_required = true; > lis3lv02d_poweron(&lis3_dev); > return 0; > } > > -static SIMPLE_DEV_PM_OPS(hp_accel_pm, lis3lv02d_suspend, lis3lv02d_resume); > +static const struct dev_pm_ops hp_accel_pm = { > + .suspend = lis3lv02d_suspend, > + .resume = lis3lv02d_resume, > + .freeze = lis3lv02d_suspend, > + .thaw = lis3lv02d_resume, > + .poweroff = lis3lv02d_suspend, > + .restore = lis3lv02d_restore, > +}; > + > #define HP_ACCEL_PM (&hp_accel_pm) > #else > #define HP_ACCEL_PM NULL >
diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h index c394c0b08519a..7ac788fae1b86 100644 --- a/drivers/misc/lis3lv02d/lis3lv02d.h +++ b/drivers/misc/lis3lv02d/lis3lv02d.h @@ -271,6 +271,7 @@ struct lis3lv02d { int regs_size; u8 *reg_cache; bool regs_stored; + bool init_required; u8 odr_mask; /* ODR bit mask */ u8 whoami; /* indicates measurement precision */ s16 (*read_data) (struct lis3lv02d *lis3, int reg); diff --git a/drivers/platform/x86/hp_accel.c b/drivers/platform/x86/hp_accel.c index 799cbe2ffcf36..8c0867bda8280 100644 --- a/drivers/platform/x86/hp_accel.c +++ b/drivers/platform/x86/hp_accel.c @@ -88,6 +88,9 @@ MODULE_DEVICE_TABLE(acpi, lis3lv02d_device_ids); static int lis3lv02d_acpi_init(struct lis3lv02d *lis3) { struct acpi_device *dev = lis3->bus_priv; + if (!lis3->init_required) + return 0; + if (acpi_evaluate_object(dev->handle, METHOD_NAME__INI, NULL, NULL) != AE_OK) return -EINVAL; @@ -356,6 +359,7 @@ static int lis3lv02d_add(struct acpi_device *device) } /* call the core layer do its init */ + lis3_dev.init_required = true; ret = lis3lv02d_init_device(&lis3_dev); if (ret) return ret; @@ -403,11 +407,27 @@ static int lis3lv02d_suspend(struct device *dev) static int lis3lv02d_resume(struct device *dev) { + lis3_dev.init_required = false; + lis3lv02d_poweron(&lis3_dev); + return 0; +} + +static int lis3lv02d_restore(struct device *dev) +{ + lis3_dev.init_required = true; lis3lv02d_poweron(&lis3_dev); return 0; } -static SIMPLE_DEV_PM_OPS(hp_accel_pm, lis3lv02d_suspend, lis3lv02d_resume); +static const struct dev_pm_ops hp_accel_pm = { + .suspend = lis3lv02d_suspend, + .resume = lis3lv02d_resume, + .freeze = lis3lv02d_suspend, + .thaw = lis3lv02d_resume, + .poweroff = lis3lv02d_suspend, + .restore = lis3lv02d_restore, +}; + #define HP_ACCEL_PM (&hp_accel_pm) #else #define HP_ACCEL_PM NULL
hp_accel can take almost two seconds to resume on some HP laptops. The bottleneck is on evaluating _INI, which is only needed to run once. Resolve the issue by only invoking _INI when it's necessary. Namely, on probe and on hibernation restore. Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/misc/lis3lv02d/lis3lv02d.h | 1 + drivers/platform/x86/hp_accel.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)