Re: [PATCH] PM / Domains: allow domain idle states to be disabled

[Lina Iyer <ilina@codeaurora.org>]

On Tue, Dec 22 2020 at 03:16 -0700, Ulf Hansson wrote:
>On Wed, 16 Dec 2020 at 18:51, Lina Iyer <ilina@codeaurora.org> wrote:
>>
>> In order to debug critical domain and device power issues, it may be
>> necessary to disallow certain idle states at runtime. Let the device
>> disallow a domain idle state before suspending.The domain governor shall
>> check for the 'disabled' flag while determining the domain idle state.
>
>For debug purposes, you might as well just set a dev PM Qos latency
>request that corresponds to the state you want to disable. This will
>then prevent the genpd governor from selecting the state.
>
True, but it will also disable idle states deeper as well. Would like to
avoid that.

--Lina

>Kind regards
>Uffe
>
>>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> ---
>>  drivers/base/power/domain.c          | 30 ++++++++++++++++++++++++++++
>>  drivers/base/power/domain_governor.c |  3 +++
>>  include/linux/pm_domain.h            |  7 +++++++
>>  3 files changed, 40 insertions(+)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 191539a8e06d..e4e7ab75bdea 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -1926,6 +1926,36 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
>>  }
>>  EXPORT_SYMBOL_GPL(pm_genpd_remove_subdomain);
>>
>> +/**
>> + * dev_pm_genpd_disable_idle_state - Disallow a PM domain's idle state
>> + *
>> + * @dev: device attached to the PM domain
>> + * @idx: index of the PM domain's idle state to be disabled
>> + * @disable: enable/disable idle state
>> + *
>> + * Allow a PM domain's idle state to be disabled. Disabled idle states will
>> + * be ignored by the domain governor when entering idle. Devices would
>> + * invoke this before calling runtime suspend.
>> + */
>> +int dev_pm_genpd_disable_idle_state(struct device *dev, unsigned int idx, bool disable)
>> +{
>> +       struct generic_pm_domain *genpd;
>> +
>> +       genpd = dev_to_genpd_safe(dev);
>> +       if (!genpd)
>> +               return -ENODEV;
>> +
>> +       if (idx >= genpd->state_count)
>> +               return -EINVAL;
>> +
>> +       genpd_lock(genpd);
>> +       genpd->states[idx].disabled = disable;
>> +       genpd_unlock(genpd);
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_genpd_disable_idle_state);
>> +
>>  static void genpd_free_default_power_state(struct genpd_power_state *states,
>>                                            unsigned int state_count)
>>  {
>> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
>> index 2afb7fa90d5d..8decd17c5a6a 100644
>> --- a/drivers/base/power/domain_governor.c
>> +++ b/drivers/base/power/domain_governor.c
>> @@ -175,6 +175,9 @@ static bool __default_power_down_ok(struct dev_pm_domain *pd,
>>         s64 min_off_time_ns;
>>         s64 off_on_time_ns;
>>
>> +       if (genpd->states[state].disabled)
>> +               return false;
>> +
>>         off_on_time_ns = genpd->states[state].power_off_latency_ns +
>>                 genpd->states[state].power_on_latency_ns;
>>
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index a41aea9d1c06..3d251b5b461a 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -101,6 +101,7 @@ struct genpd_power_state {
>>         struct fwnode_handle *fwnode;
>>         ktime_t idle_time;
>>         void *data;
>> +       bool disabled;
>>  };
>>
>>  struct genpd_lock_ops;
>> @@ -233,6 +234,7 @@ int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block *nb);
>>  int dev_pm_genpd_remove_notifier(struct device *dev);
>>  void genpd_enable_next_wakeup(struct generic_pm_domain *genpd, bool enable);
>>  int dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next);
>> +int dev_pm_genpd_disable_idle_state(struct device *dev, unsigned int idx, bool disable);
>>
>>  extern struct dev_power_governor simple_qos_governor;
>>  extern struct dev_power_governor pm_domain_always_on_gov;
>> @@ -300,6 +302,11 @@ static inline int dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next)
>>         return -EOPNOTSUPP;
>>  }
>>
>> +static inline int dev_pm_genpd_disable_idle_state(struct device *dev, unsigned int idx, bool disable)
>> +{
>> +       return -EOPNOTSUPP;
>> +}
>> +
>>  #define simple_qos_governor            (*(struct dev_power_governor *)(NULL))
>>  #define pm_domain_always_on_gov                (*(struct dev_power_governor *)(NULL))
>>  #endif
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>