Re: [PATCH v2 3/3] pmdommain: add support system-wide resume latency constraints

[Kevin Hilman]

Ulf Hansson <ulf.hansson@xxxxxxxxxx> writes:

> On Fri, 6 Feb 2026 at 01:30, Kevin Hilman (TI) <khilman@xxxxxxxxxxxx> wrote:
>>
>> In addition to checking for CPU latency constraints when checking if
>> OK to power down a domain, also check for QoS latency constraints in
>> all devices of a domain and use that in determining the final latency
>> constraint to use for the domain.
>>
>> Since cpu_system_power_down_ok() is used for system-wide suspend, the
>> per-device constratints are only relevant if the LATENCY_SYS QoS flag
>> is set.
>>
>> Signed-off-by: Kevin Hilman (TI) <khilman@xxxxxxxxxxxx>
>> ---
>>  drivers/pmdomain/governor.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>>
>> diff --git a/drivers/pmdomain/governor.c b/drivers/pmdomain/governor.c
>> index 96737abbb496..bbf59b93c8b6 100644
>> --- a/drivers/pmdomain/governor.c
>> +++ b/drivers/pmdomain/governor.c
>> @@ -13,6 +13,8 @@
>>  #include <linux/cpumask.h>
>>  #include <linux/ktime.h>
>>
>> +#include "core.h"
>> +
>>  static int dev_update_qos_constraint(struct device *dev, void *data)
>>  {
>>         s64 *constraint_ns_p = data;
>> @@ -425,17 +427,60 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
>>         return true;
>>  }
>>
>> +/**
>> + * check_device_qos_latency - Callback to check device QoS latency constraints
>> + * @dev: Device to check
>> + * @data: Pointer to s32 variable holding minimum latency found so far
>> + *
>> + * This callback checks if the device has a system-wide resume latency QoS
>> + * constraint and updates the minimum latency if this device has a stricter
>> + * constraint.
>> + *
>> + * Returns: 0 to continue iteration.
>> + */
>> +static int check_device_qos_latency(struct device *dev, void *data)
>> +{
>> +       s32 *min_dev_latency = data;
>> +       enum pm_qos_flags_status flag_status;
>> +       s32 dev_latency;
>> +
>> +       dev_latency = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
>
> ->cpu_system_power_down_ok() executes in atomic context on a
> PREEMPT_RT configured system.

Oh, right.  Good point.  Thanks for pointing that out!

> dev_pm_qos_read_value() uses spinlocks, which sleep in that configuration.
>
> I don't know the best approach to fix this. Perhaps add a specific
> dev_pm_qos interface that can be used here? Or re-work the code so it
> only applies for !PREEMPT_RT?

There's already a "raw" lockless helper to read the resume latench
(dev_pm_qos_raw_resume_latency), so I'll use that, as well as add a
similar lockless read for the flags.

Thanks for the review,

Kevin