| Message ID | 1641749107-31979-9-git-send-email-quic_mkshah@quicinc.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Add APSS RSC to Cluster power domain | expand |
On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote: > > The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI) > SoCs when the deepest CPUidle modes results in the SoC also to enter > the low power mode. > > RSC is part of CPU subsystem and APSS rsc device is attached to cluster > power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which > can wakeup the SoC from deepest low power states. The CONTROL_TCS does > this by writing next wakeup in always on domain timer when the SoC is > entering the low power state. > > Store the domain wakeup time from all the CPUs which can be used from > domain power off callback by RSC device. > > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> I need to think a little bit more about this one, so I have to get back with some more detailed comments next week. Kind regards Uffe > --- > drivers/base/power/domain_governor.c | 1 + > include/linux/pm_domain.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c > index cd08c58..a4c7dd8 100644 > --- a/drivers/base/power/domain_governor.c > +++ b/drivers/base/power/domain_governor.c > @@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd) > domain_wakeup = next_hrtimer; > } > } > + genpd->next_hrtimer = domain_wakeup; > > /* The minimum idle duration is from now - until the next wakeup. */ > idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now)); > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index 67017c9..682b372 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -136,6 +136,7 @@ struct generic_pm_domain { > struct gpd_dev_ops dev_ops; > s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ > ktime_t next_wakeup; /* Maintained by the domain governor */ > + ktime_t next_hrtimer; /* Closest hrtimer event of the domain CPUs */ > bool max_off_time_changed; > bool cached_power_down_ok; > bool cached_power_down_state_idx; > -- > 2.7.4 >
On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote: > > The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI) > SoCs when the deepest CPUidle modes results in the SoC also to enter > the low power mode. > > RSC is part of CPU subsystem and APSS rsc device is attached to cluster > power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which > can wakeup the SoC from deepest low power states. The CONTROL_TCS does > this by writing next wakeup in always on domain timer when the SoC is > entering the low power state. > > Store the domain wakeup time from all the CPUs which can be used from > domain power off callback by RSC device. > > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> > --- > drivers/base/power/domain_governor.c | 1 + > include/linux/pm_domain.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c > index cd08c58..a4c7dd8 100644 > --- a/drivers/base/power/domain_governor.c > +++ b/drivers/base/power/domain_governor.c > @@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd) > domain_wakeup = next_hrtimer; > } > } > + genpd->next_hrtimer = domain_wakeup; > > /* The minimum idle duration is from now - until the next wakeup. */ > idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now)); > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index 67017c9..682b372 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -136,6 +136,7 @@ struct generic_pm_domain { > struct gpd_dev_ops dev_ops; > s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ > ktime_t next_wakeup; /* Maintained by the domain governor */ > + ktime_t next_hrtimer; /* Closest hrtimer event of the domain CPUs */ Would you mind clarifying the comment into something along the lines of: "/* Next hrtimer for the CPU PM domain */ > bool max_off_time_changed; > bool cached_power_down_ok; > bool cached_power_down_state_idx; > -- > 2.7.4 > Beside the nitpick above, I have a few additional minor comments. *) Users of genpd->next_hrtimer should not access this member in the struct generic_pm_domain themselves. Instead, I suggest we add a genpd helper function to deal with this. In this regard, we should also add a description to the helper function to explain under what *specific* conditions it's allowed to be called for. **) We should assign genpd->next_hrtimer a default value in pm_genpd_init(). Perhaps that can also be used in a way to make sure the helper function always returns a valid value!? Other than this, I think the approach looks sane to me! Kind regards Uffe
diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index cd08c58..a4c7dd8 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd) domain_wakeup = next_hrtimer; } } + genpd->next_hrtimer = domain_wakeup; /* The minimum idle duration is from now - until the next wakeup. */ idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now)); diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 67017c9..682b372 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -136,6 +136,7 @@ struct generic_pm_domain { struct gpd_dev_ops dev_ops; s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ ktime_t next_wakeup; /* Maintained by the domain governor */ + ktime_t next_hrtimer; /* Closest hrtimer event of the domain CPUs */ bool max_off_time_changed; bool cached_power_down_ok; bool cached_power_down_state_idx;
The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI) SoCs when the deepest CPUidle modes results in the SoC also to enter the low power mode. RSC is part of CPU subsystem and APSS rsc device is attached to cluster power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which can wakeup the SoC from deepest low power states. The CONTROL_TCS does this by writing next wakeup in always on domain timer when the SoC is entering the low power state. Store the domain wakeup time from all the CPUs which can be used from domain power off callback by RSC device. Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> --- drivers/base/power/domain_governor.c | 1 + include/linux/pm_domain.h | 1 + 2 files changed, 2 insertions(+)