LinuxLists.cc - [PATCH v1 1/2] PM: QoS: Rename freq to range constraint

2023-12-13 17:59:25

Subject: [PATCH v1 1/2] PM: QoS: Rename freq to range constraint

The frequency pm_qos relies on a couple of values, the min and max
frequencies. However more pm_qos will be added with the same logic of
a couple of min and max. Instead of writing new set of constraints as
well as type, etc... let's rename freq_* to a more generic name
range_*

That way, new qos range based can be added easily.

No functional changes intended.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/base/power/qos-test.c | 14 +++---
drivers/base/power/qos.c | 4 +-
drivers/cpufreq/intel_pstate.c | 8 ++--
drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 6 +--
drivers/macintosh/windfarm_cpufreq_clamp.c | 2 +-
drivers/powercap/dtpm_cpu.c | 2 +-
drivers/thermal/cpufreq_cooling.c | 2 +-
include/acpi/processor.h | 4 +-
include/linux/amd-pstate.h | 2 +-
include/linux/cpufreq.h | 6 +--
include/linux/pm_qos.h | 55 +++++++++++-----------
kernel/power/qos.c | 53 ++++++++++-----------
12 files changed, 79 insertions(+), 79 deletions(-)

diff --git a/drivers/base/power/qos-test.c b/drivers/base/power/qos-test.c
index 79fc6c4418da..56f33acb9e3c 100644
--- a/drivers/base/power/qos-test.c
+++ b/drivers/base/power/qos-test.c
@@ -8,8 +8,8 @@
/* Basic test for aggregating two "min" requests */
static void freq_qos_test_min(struct kunit *test)
{
- struct freq_constraints qos;
- struct freq_qos_request req1, req2;
+ struct range_constraints qos;
+ struct range_qos_request req1, req2;
int ret;

freq_constraints_init(&qos);
@@ -36,8 +36,8 @@ static void freq_qos_test_min(struct kunit *test)
/* Test that requests for MAX_DEFAULT_VALUE have no effect */
static void freq_qos_test_maxdef(struct kunit *test)
{
- struct freq_constraints qos;
- struct freq_qos_request req1, req2;
+ struct range_constraints qos;
+ struct range_qos_request req1, req2;
int ret;

freq_constraints_init(&qos);
@@ -70,15 +70,15 @@ static void freq_qos_test_maxdef(struct kunit *test)
}

/*
- * Test that a freq_qos_request can be added again after removal
+ * Test that a range_qos_request can be added again after removal
*
* This issue was solved by commit 05ff1ba412fd ("PM: QoS: Invalidate frequency
* QoS requests after removal")
*/
static void freq_qos_test_readd(struct kunit *test)
{
- struct freq_constraints qos;
- struct freq_qos_request req;
+ struct range_constraints qos;
+ struct range_qos_request req;
int ret;

freq_constraints_init(&qos);
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 8e93167f1783..ae0b9d2573ec 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -285,14 +285,14 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
memset(req, 0, sizeof(*req));
}

- c = &qos->freq.min_freq;
+ c = &qos->freq.lower_bound;
plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
apply_constraint(req, PM_QOS_REMOVE_REQ,
PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE);
memset(req, 0, sizeof(*req));
}

- c = &qos->freq.max_freq;
+ c = &qos->freq.upper_bound;
plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
apply_constraint(req, PM_QOS_REMOVE_REQ,
PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index dd6d23e389f1..eafe39f37c9a 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1320,9 +1320,9 @@ static ssize_t store_no_turbo(struct kobject *a, struct kobj_attribute *b,
return count;
}

-static void update_qos_request(enum freq_qos_req_type type)
+static void update_qos_request(enum range_qos_req_type type)
{
- struct freq_qos_request *req;
+ struct range_qos_request *req;
struct cpufreq_policy *policy;
int i;

@@ -2984,7 +2984,7 @@ static void intel_cpufreq_adjust_perf(unsigned int cpunum,

static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
{
- struct freq_qos_request *req;
+ struct range_qos_request *req;
struct cpudata *cpu;
struct device *dev;
int ret, freq;
@@ -3058,7 +3058,7 @@ static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)

static int intel_cpufreq_cpu_exit(struct cpufreq_policy *policy)
{
- struct freq_qos_request *req;
+ struct range_qos_request *req;

req = policy->driver_data;

diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
index 6f0c32592416..faa7f19a1702 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
@@ -64,7 +64,7 @@ EXPORT_SYMBOL_GPL(cbe_cpufreq_set_pmode_pmi);
static void cbe_cpufreq_handle_pmi(pmi_message_t pmi_msg)
{
struct cpufreq_policy *policy;
- struct freq_qos_request *req;
+ struct range_qos_request *req;
u8 node, slow_mode;
int cpu, ret;

@@ -102,7 +102,7 @@ static struct pmi_handler cbe_pmi_handler = {

void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy)
{
- struct freq_qos_request *req;
+ struct range_qos_request *req;
int ret;

if (!cbe_cpufreq_has_pmi)
@@ -126,7 +126,7 @@ EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_init);

void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy)
{
- struct freq_qos_request *req = policy->driver_data;
+ struct range_qos_request *req = policy->driver_data;

if (cbe_cpufreq_has_pmi) {
freq_qos_remove_request(req);
diff --git a/drivers/macintosh/windfarm_cpufreq_clamp.c b/drivers/macintosh/windfarm_cpufreq_clamp.c
index 28d18ef22bbb..a24ddf1072f3 100644
--- a/drivers/macintosh/windfarm_cpufreq_clamp.c
+++ b/drivers/macintosh/windfarm_cpufreq_clamp.c
@@ -16,7 +16,7 @@

static int clamped;
static struct wf_control *clamp_control;
-static struct freq_qos_request qos_req;
+static struct range_qos_request qos_req;
static unsigned int min_freq, max_freq;

static int clamp_set(struct wf_control *ct, s32 value)
diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c
index 9193c3b8edeb..408dbc842e5b 100644
--- a/drivers/powercap/dtpm_cpu.c
+++ b/drivers/powercap/dtpm_cpu.c
@@ -27,7 +27,7 @@

struct dtpm_cpu {
struct dtpm dtpm;
- struct freq_qos_request qos_req;
+ struct range_qos_request qos_req;
int cpu;
};

diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
index e2cc7bd30862..77f807cf7077 100644
--- a/drivers/thermal/cpufreq_cooling.c
+++ b/drivers/thermal/cpufreq_cooling.c
@@ -77,7 +77,7 @@ struct cpufreq_cooling_device {
#ifndef CONFIG_SMP
struct time_in_idle *idle_time;
#endif
- struct freq_qos_request qos_req;
+ struct range_qos_request qos_req;
};

#ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 3f34ebb27525..67092ba80615 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -237,8 +237,8 @@ struct acpi_processor {
struct acpi_processor_limit limit;
struct thermal_cooling_device *cdev;
struct device *dev; /* Processor device. */
- struct freq_qos_request perflib_req;
- struct freq_qos_request thermal_req;
+ struct range_qos_request perflib_req;
+ struct range_qos_request thermal_req;
};

struct acpi_processor_errata {
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
index 6ad02ad9c7b4..55c3fd4797e6 100644
--- a/include/linux/amd-pstate.h
+++ b/include/linux/amd-pstate.h
@@ -63,7 +63,7 @@ struct amd_aperf_mperf {
struct amd_cpudata {
int cpu;

- struct freq_qos_request req[2];
+ struct range_qos_request req[2];
u64 cppc_req_cached;

u32 highest_perf;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 1c5ca92a0555..08a8ba4bfd2d 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -80,9 +80,9 @@ struct cpufreq_policy {
struct work_struct update; /* if update_policy() needs to be
* called, but you're in IRQ context */

- struct freq_constraints constraints;
- struct freq_qos_request *min_freq_req;
- struct freq_qos_request *max_freq_req;
+ struct range_constraints constraints;
+ struct range_qos_request *min_freq_req;
+ struct range_qos_request *max_freq_req;

struct cpufreq_frequency_table *freq_table;
enum cpufreq_table_sorting freq_table_sorted;
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 4a69d4af3ff8..5f5d967ede32 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -77,25 +77,26 @@ struct pm_qos_flags {
#define FREQ_QOS_MIN_DEFAULT_VALUE 0
#define FREQ_QOS_MAX_DEFAULT_VALUE S32_MAX

-enum freq_qos_req_type {
- FREQ_QOS_MIN = 1,
+enum range_qos_req_type {
+ RANGE_QOS_MIN = 1,
+ RANGE_QOS_MAX,
+ FREQ_QOS_MIN,
FREQ_QOS_MAX,
};

-struct freq_constraints {
- struct pm_qos_constraints min_freq;
- struct blocking_notifier_head min_freq_notifiers;
- struct pm_qos_constraints max_freq;
- struct blocking_notifier_head max_freq_notifiers;
+struct range_constraints {
+ struct pm_qos_constraints lower_bound;
+ struct blocking_notifier_head lower_bound_notifiers;
+ struct pm_qos_constraints upper_bound;
+ struct blocking_notifier_head upper_bound_notifiers;
};

-struct freq_qos_request {
- enum freq_qos_req_type type;
+struct range_qos_request {
+ enum range_qos_req_type type;
struct plist_node pnode;
- struct freq_constraints *qos;
+ struct range_constraints *qos;
};

-
enum dev_pm_qos_req_type {
DEV_PM_QOS_RESUME_LATENCY = 1,
DEV_PM_QOS_LATENCY_TOLERANCE,
@@ -109,7 +110,7 @@ struct dev_pm_qos_request {
union {
struct plist_node pnode;
struct pm_qos_flags_request flr;
- struct freq_qos_request freq;
+ struct range_qos_request freq;
} data;
struct device *dev;
};
@@ -117,7 +118,7 @@ struct dev_pm_qos_request {
struct dev_pm_qos {
struct pm_qos_constraints resume_latency;
struct pm_qos_constraints latency_tolerance;
- struct freq_constraints freq;
+ struct range_constraints freq;
struct pm_qos_flags flags;
struct dev_pm_qos_request *resume_latency_req;
struct dev_pm_qos_request *latency_tolerance_req;
@@ -291,29 +292,29 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
}
#endif

-static inline int freq_qos_request_active(struct freq_qos_request *req)
+static inline int freq_qos_request_active(struct range_qos_request *req)
{
return !IS_ERR_OR_NULL(req->qos);
}

-void freq_constraints_init(struct freq_constraints *qos);
+void freq_constraints_init(struct range_constraints *qos);

-s32 freq_qos_read_value(struct freq_constraints *qos,
- enum freq_qos_req_type type);
+s32 freq_qos_read_value(struct range_constraints *qos,
+ enum range_qos_req_type type);

-int freq_qos_add_request(struct freq_constraints *qos,
- struct freq_qos_request *req,
- enum freq_qos_req_type type, s32 value);
-int freq_qos_update_request(struct freq_qos_request *req, s32 new_value);
-int freq_qos_remove_request(struct freq_qos_request *req);
-int freq_qos_apply(struct freq_qos_request *req,
+int freq_qos_add_request(struct range_constraints *qos,
+ struct range_qos_request *req,
+ enum range_qos_req_type type, s32 value);
+int freq_qos_update_request(struct range_qos_request *req, s32 new_value);
+int freq_qos_remove_request(struct range_qos_request *req);
+int freq_qos_apply(struct range_qos_request *req,
enum pm_qos_req_action action, s32 value);

-int freq_qos_add_notifier(struct freq_constraints *qos,
- enum freq_qos_req_type type,
+int freq_qos_add_notifier(struct range_constraints *qos,
+ enum range_qos_req_type type,
struct notifier_block *notifier);
-int freq_qos_remove_notifier(struct freq_constraints *qos,
- enum freq_qos_req_type type,
+int freq_qos_remove_notifier(struct range_constraints *qos,
+ enum range_qos_req_type type,
struct notifier_block *notifier);

#endif
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 4244b069442e..39919a2eed73 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -440,26 +440,26 @@ static inline bool freq_qos_value_invalid(s32 value)
* freq_constraints_init - Initialize frequency QoS constraints.
* @qos: Frequency QoS constraints to initialize.
*/
-void freq_constraints_init(struct freq_constraints *qos)
+void freq_constraints_init(struct range_constraints *qos)
{
struct pm_qos_constraints *c;

- c = &qos->min_freq;
+ c = &qos->lower_bound;
plist_head_init(&c->list);
c->target_value = FREQ_QOS_MIN_DEFAULT_VALUE;
c->default_value = FREQ_QOS_MIN_DEFAULT_VALUE;
c->no_constraint_value = FREQ_QOS_MIN_DEFAULT_VALUE;
c->type = PM_QOS_MAX;
- c->notifiers = &qos->min_freq_notifiers;
+ c->notifiers = &qos->lower_bound_notifiers;
BLOCKING_INIT_NOTIFIER_HEAD(c->notifiers);

- c = &qos->max_freq;
+ c = &qos->upper_bound;
plist_head_init(&c->list);
c->target_value = FREQ_QOS_MAX_DEFAULT_VALUE;
c->default_value = FREQ_QOS_MAX_DEFAULT_VALUE;
c->no_constraint_value = FREQ_QOS_MAX_DEFAULT_VALUE;
c->type = PM_QOS_MIN;
- c->notifiers = &qos->max_freq_notifiers;
+ c->notifiers = &qos->upper_bound_notifiers;
BLOCKING_INIT_NOTIFIER_HEAD(c->notifiers);
}

@@ -468,8 +468,8 @@ void freq_constraints_init(struct freq_constraints *qos)
* @qos: Constraints to evaluate.
* @type: QoS request type.
*/
-s32 freq_qos_read_value(struct freq_constraints *qos,
- enum freq_qos_req_type type)
+s32 freq_qos_read_value(struct range_constraints *qos,
+ enum range_qos_req_type type)
{
s32 ret;

@@ -477,15 +477,14 @@ s32 freq_qos_read_value(struct freq_constraints *qos,
case FREQ_QOS_MIN:
ret = IS_ERR_OR_NULL(qos) ?
FREQ_QOS_MIN_DEFAULT_VALUE :
- pm_qos_read_value(&qos->min_freq);
+ pm_qos_read_value(&qos->lower_bound);
break;
case FREQ_QOS_MAX:
ret = IS_ERR_OR_NULL(qos) ?
FREQ_QOS_MAX_DEFAULT_VALUE :
- pm_qos_read_value(&qos->max_freq);
+ pm_qos_read_value(&qos->upper_bound);
break;
default:
- WARN_ON(1);
ret = 0;
}

@@ -500,18 +499,18 @@ s32 freq_qos_read_value(struct freq_constraints *qos,
*
* This is only meant to be called from inside pm_qos, not drivers.
*/
-int freq_qos_apply(struct freq_qos_request *req,
- enum pm_qos_req_action action, s32 value)
+int freq_qos_apply(struct range_qos_request *req,
+ enum pm_qos_req_action action, s32 value)
{
int ret;

switch(req->type) {
case FREQ_QOS_MIN:
- ret = pm_qos_update_target(&req->qos->min_freq, &req->pnode,
+ ret = pm_qos_update_target(&req->qos->lower_bound, &req->pnode,
action, value);
break;
case FREQ_QOS_MAX:
- ret = pm_qos_update_target(&req->qos->max_freq, &req->pnode,
+ ret = pm_qos_update_target(&req->qos->upper_bound, &req->pnode,
action, value);
break;
default:
@@ -535,9 +534,9 @@ int freq_qos_apply(struct freq_qos_request *req,
* Return 1 if the effective constraint value has changed, 0 if the effective
* constraint value has not changed, or a negative error code on failures.
*/
-int freq_qos_add_request(struct freq_constraints *qos,
- struct freq_qos_request *req,
- enum freq_qos_req_type type, s32 value)
+int freq_qos_add_request(struct range_constraints *qos,
+ struct range_qos_request *req,
+ enum range_qos_req_type type, s32 value)
{
int ret;

@@ -571,7 +570,7 @@ EXPORT_SYMBOL_GPL(freq_qos_add_request);
* Return 1 if the effective constraint value has changed, 0 if the effective
* constraint value has not changed, or a negative error code on failures.
*/
-int freq_qos_update_request(struct freq_qos_request *req, s32 new_value)
+int freq_qos_update_request(struct range_qos_request *req, s32 new_value)
{
if (!req || freq_qos_value_invalid(new_value))
return -EINVAL;
@@ -597,7 +596,7 @@ EXPORT_SYMBOL_GPL(freq_qos_update_request);
* Return 1 if the effective constraint value has changed, 0 if the effective
* constraint value has not changed, or a negative error code on failures.
*/
-int freq_qos_remove_request(struct freq_qos_request *req)
+int freq_qos_remove_request(struct range_qos_request *req)
{
int ret;

@@ -622,8 +621,8 @@ EXPORT_SYMBOL_GPL(freq_qos_remove_request);
* @type: Request type.
* @notifier: Notifier block to add.
*/
-int freq_qos_add_notifier(struct freq_constraints *qos,
- enum freq_qos_req_type type,
+int freq_qos_add_notifier(struct range_constraints *qos,
+ enum range_qos_req_type type,
struct notifier_block *notifier)
{
int ret;
@@ -633,11 +632,11 @@ int freq_qos_add_notifier(struct freq_constraints *qos,

switch (type) {
case FREQ_QOS_MIN:
- ret = blocking_notifier_chain_register(qos->min_freq.notifiers,
+ ret = blocking_notifier_chain_register(qos->lower_bound.notifiers,
notifier);
break;
case FREQ_QOS_MAX:
- ret = blocking_notifier_chain_register(qos->max_freq.notifiers,
+ ret = blocking_notifier_chain_register(qos->upper_bound.notifiers,
notifier);
break;
default:
@@ -655,8 +654,8 @@ EXPORT_SYMBOL_GPL(freq_qos_add_notifier);
* @type: Request type.
* @notifier: Notifier block to remove.
*/
-int freq_qos_remove_notifier(struct freq_constraints *qos,
- enum freq_qos_req_type type,
+int freq_qos_remove_notifier(struct range_constraints *qos,
+ enum range_qos_req_type type,
struct notifier_block *notifier)
{
int ret;
@@ -666,11 +665,11 @@ int freq_qos_remove_notifier(struct freq_constraints *qos,

switch (type) {
case FREQ_QOS_MIN:
- ret = blocking_notifier_chain_unregister(qos->min_freq.notifiers,
+ ret = blocking_notifier_chain_unregister(qos->lower_bound.notifiers,
notifier);
break;
case FREQ_QOS_MAX:
- ret = blocking_notifier_chain_unregister(qos->max_freq.notifiers,
+ ret = blocking_notifier_chain_unregister(qos->upper_bound.notifiers,
notifier);
break;
default:
--
2.34.1

2023-12-13 17:59:32

by Daniel Lezcano

[permalink] [raw]

Subject: [PATCH v1 2/2] PM: QoS: Add a performance QoS

Currently cpufreq and devfreq are using the freq QoS to aggregate the
requests for frequency ranges.

However, there are new devices wanting to act not on a frequency range
but on a performance index range. Those need also to export to
userspace the knob to act on their performance limits.

This change provides a performance limiter QoS based on a minimum /
maximum performance values. At init time, the limits of the interval
are 0 / 1024. It is up to the backend to convert the 1024 to the
maximum performance state. So if the performance must be limited to
50%, it should set to maximum limit to 512 where the backend will end
up by converting (max performance index / 2). The same applies for the
minimum. Obviously, the min can not be greater than the max.

1. With the example above, if there is a odd number like 5 for the
number of performance indexes and we ask for 512 (so 50%), what would
be the performance index computed? (5/2=2 or 5/2=3)? (I would say the
minimum otherwise we end up with a performance limit greater than
what we actually asked for).

2. The conversion from 1024 to a performance index will inevatibly
end up to a state above or below the percentage given. Shall it be
reflected in the value set? eg. We want to apply a performance limit
to be 33% maximum. So it is, 1024 x 0.333333 = 314. If there are 20
performance indexes, that will be (20 x 314) / 1024 = 6.13, so index
6. Shall we convert this index back to the requested performance
limit to (6.13 x 1024) / 20 = 307 ? (So requested is 314 but it is
actually 307).

The end goal is to make the freq QoS and perf QoS to co-exist together
in the next changes in the different backends. A change of one of the
QoS impacts the other. For instance if there are 5 performance states
and we set a performance limit to 80%, then the maximum state will 4.

For the long term, when those can co-exist, then we can implement a
cooling device based on the performance Qos which will be generic for
all devices using this QoS. That will imply the CPUs, the GPUs and any
devfreq devices. So devfreq and cpufreq cooling devices can be merged
into a single performance cooling device which will be generic for all
devices with a performance limit QoS.

In a similar way, in the future, a power QoS could be added also and a
power based cooling device. So any device with the energy model and a
power capping feature can become a cooling device and the power
computation part in the cooling devices will move to the back ends. We
will end up with a generic power cooling device compatible with all
power capable devices.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/base/power/power.h | 2 +
drivers/base/power/qos.c | 158 +++++++++++++++++++++++++-
drivers/base/power/sysfs.c | 92 +++++++++++++++
include/linux/cpufreq.h | 2 +
include/linux/pm_qos.h | 42 +++++++
kernel/power/qos.c | 225 +++++++++++++++++++++++++++++++++++++
6 files changed, 517 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index 922ed457db19..eb1a77a7a0f4 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -78,6 +78,8 @@ extern int pm_qos_sysfs_add_flags(struct device *dev);
extern void pm_qos_sysfs_remove_flags(struct device *dev);
extern int pm_qos_sysfs_add_latency_tolerance(struct device *dev);
extern void pm_qos_sysfs_remove_latency_tolerance(struct device *dev);
+extern int pm_qos_sysfs_add_perf_limit(struct device *dev);
+extern void pm_qos_sysfs_remove_perf_limit(struct device *dev);
extern int dpm_sysfs_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid);

#else /* CONFIG_PM */
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index ae0b9d2573ec..a71cff1f8048 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -128,6 +128,14 @@ s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE
: freq_qos_read_value(&qos->freq, FREQ_QOS_MAX);
break;
+ case DEV_PM_QOS_MIN_PERF:
+ ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MIN_PERF_DEFAULT_VALUE
+ : perf_qos_read_value(&qos->perf, RANGE_QOS_MIN);
+ break;
+ case DEV_PM_QOS_MAX_PERF:
+ ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_PERF_DEFAULT_VALUE
+ : perf_qos_read_value(&qos->perf, RANGE_QOS_MAX);
+ break;
default:
WARN_ON(1);
ret = 0;
@@ -177,6 +185,10 @@ static int apply_constraint(struct dev_pm_qos_request *req,
ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
action, value);
break;
+ case DEV_PM_QOS_MIN_PERF:
+ case DEV_PM_QOS_MAX_PERF:
+ ret = perf_qos_apply(&req->data.perf, action, value);
+ break;
default:
ret = -EINVAL;
}
@@ -223,6 +235,20 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
c->type = PM_QOS_MIN;

+ c = &qos->perf.lower_bound;
+ plist_head_init(&c->list);
+ c->target_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
+ c->default_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
+ c->no_constraint_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
+ c->type = PM_QOS_MAX;
+
+ c = &qos->perf.upper_bound;
+ plist_head_init(&c->list);
+ c->target_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
+ c->default_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
+ c->no_constraint_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
+ c->type = PM_QOS_MIN;
+
freq_constraints_init(&qos->freq);

INIT_LIST_HEAD(&qos->flags.list);
@@ -299,6 +325,20 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
memset(req, 0, sizeof(*req));
}

+ c = &qos->perf.lower_bound;
+ plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
+ apply_constraint(req, PM_QOS_REMOVE_REQ,
+ PM_QOS_MIN_PERF_DEFAULT_VALUE);
+ memset(req, 0, sizeof(*req));
+ }
+
+ c = &qos->perf.upper_bound;
+ plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
+ apply_constraint(req, PM_QOS_REMOVE_REQ,
+ PM_QOS_MAX_PERF_DEFAULT_VALUE);
+ memset(req, 0, sizeof(*req));
+ }
+
f = &qos->flags;
list_for_each_entry_safe(req, tmp, &f->list, data.flr.node) {
apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
@@ -349,17 +389,32 @@ static int __dev_pm_qos_add_request(struct device *dev,

req->dev = dev;
req->type = type;
- if (req->type == DEV_PM_QOS_MIN_FREQUENCY)
+
+ switch (type) {
+ case DEV_PM_QOS_MIN_FREQUENCY:
ret = freq_qos_add_request(&dev->power.qos->freq,
&req->data.freq,
FREQ_QOS_MIN, value);
- else if (req->type == DEV_PM_QOS_MAX_FREQUENCY)
+ break;
+ case DEV_PM_QOS_MAX_FREQUENCY:
ret = freq_qos_add_request(&dev->power.qos->freq,
&req->data.freq,
FREQ_QOS_MAX, value);
- else
+ break;
+ case DEV_PM_QOS_MIN_PERF:
+ ret = perf_qos_add_request(&dev->power.qos->perf,
+ &req->data.perf,
+ RANGE_QOS_MIN, value);
+ break;
+ case DEV_PM_QOS_MAX_PERF:
+ ret = perf_qos_add_request(&dev->power.qos->perf,
+ &req->data.perf,
+ RANGE_QOS_MAX, value);
+ break;
+ default:
ret = apply_constraint(req, PM_QOS_ADD_REQ, value);
-
+ break;
+ }
return ret;
}

@@ -427,6 +482,10 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
case DEV_PM_QOS_MAX_FREQUENCY:
curr_value = req->data.freq.pnode.prio;
break;
+ case DEV_PM_QOS_MIN_PERF:
+ case DEV_PM_QOS_MAX_PERF:
+ curr_value = req->data.perf.pnode.prio;
+ break;
case DEV_PM_QOS_FLAGS:
curr_value = req->data.flr.flags;
break;
@@ -674,6 +733,14 @@ static void __dev_pm_qos_drop_user_request(struct device *dev,
req = dev->power.qos->flags_req;
dev->power.qos->flags_req = NULL;
break;
+ case DEV_PM_QOS_MIN_PERF:
+ req = dev->power.qos->perf_min_req;
+ dev->power.qos->perf_min_req = NULL;
+ break;
+ case DEV_PM_QOS_MAX_PERF:
+ req = dev->power.qos->perf_max_req;
+ dev->power.qos->perf_max_req = NULL;
+ break;
default:
WARN_ON(1);
return;
@@ -980,3 +1047,86 @@ void dev_pm_qos_hide_latency_tolerance(struct device *dev)
pm_runtime_put(dev);
}
EXPORT_SYMBOL_GPL(dev_pm_qos_hide_latency_tolerance);
+
+int dev_pm_qos_expose_perf_limit(struct device *dev)
+{
+ struct dev_pm_qos_request *req_min;
+ struct dev_pm_qos_request *req_max;
+ int ret;
+
+ if (!device_is_registered(dev))
+ return -EINVAL;
+
+ req_min = kzalloc(sizeof(*req_min), GFP_KERNEL);
+ if (!req_min)
+ return -ENOMEM;
+
+ req_max = kzalloc(sizeof(*req_max), GFP_KERNEL);
+ if (!req_max) {
+ kfree(req_min);
+ return -ENOMEM;
+ }
+
+ ret = dev_pm_qos_add_request(dev, req_min, DEV_PM_QOS_MIN_PERF,
+ PM_QOS_MIN_PERF_DEFAULT_VALUE);
+ if (ret < 0) {
+ kfree(req_min);
+ kfree(req_max);
+ return ret;
+ }
+
+ ret = dev_pm_qos_add_request(dev, req_max, DEV_PM_QOS_MAX_PERF,
+ PM_QOS_MAX_PERF_DEFAULT_VALUE);
+ if (ret < 0) {
+ dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
+ return ret;
+ }
+
+ mutex_lock(&dev_pm_qos_sysfs_mtx);
+
+ mutex_lock(&dev_pm_qos_mtx);
+
+ if (IS_ERR_OR_NULL(dev->power.qos))
+ ret = -ENODEV;
+ else if (dev->power.qos->perf_min_req || dev->power.qos->perf_max_req)
+ ret = -EEXIST;
+
+ if (ret < 0) {
+ __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
+ __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
+ mutex_unlock(&dev_pm_qos_mtx);
+ goto out;
+ }
+
+ dev->power.qos->perf_min_req = req_min;
+ dev->power.qos->perf_max_req = req_max;
+
+ mutex_unlock(&dev_pm_qos_mtx);
+
+ ret = pm_qos_sysfs_add_perf_limit(dev);
+ if (ret) {
+ dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
+ dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
+ }
+out:
+ mutex_unlock(&dev_pm_qos_sysfs_mtx);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_qos_expose_perf_limit);
+
+void dev_pm_qos_hide_perf_limit(struct device *dev)
+{
+ mutex_lock(&dev_pm_qos_sysfs_mtx);
+
+ pm_qos_sysfs_remove_perf_limit(dev);
+
+ mutex_lock(&dev_pm_qos_mtx);
+
+ __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
+ __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
+
+ mutex_unlock(&dev_pm_qos_mtx);
+
+ mutex_unlock(&dev_pm_qos_sysfs_mtx);
+}
+EXPORT_SYMBOL_GPL(dev_pm_qos_hide_perf_limit);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index a1474fb67db9..5a45191006c1 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -317,6 +317,76 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,

static DEVICE_ATTR_RW(pm_qos_no_power_off);

+
+static ssize_t pm_qos_perf_limit_min_max_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf, bool max)
+{
+ s32 value = dev_pm_qos_read_value(dev, max ? DEV_PM_QOS_MAX_PERF :
+ DEV_PM_QOS_MIN_PERF);
+
+ return sysfs_emit(buf, "%d\n", value);
+}
+
+static ssize_t pm_qos_perf_limit_min_max_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t n, bool max)
+{
+ int ret;
+ s32 min_value = dev_pm_qos_read_value(dev, DEV_PM_QOS_MIN_PERF);
+ s32 max_value = dev_pm_qos_read_value(dev, DEV_PM_QOS_MAX_PERF);
+ s32 new_value;
+
+ if (kstrtoint(buf, 0, &new_value))
+ return -EINVAL;
+
+ if (new_value < PM_QOS_MIN_PERF_DEFAULT_VALUE ||
+ new_value > PM_QOS_MAX_PERF_DEFAULT_VALUE)
+ return -EINVAL;
+
+ if (max && (new_value < min_value))
+ return -EINVAL;
+
+ if (!max && (new_value > max_value))
+ return -EINVAL;
+
+ ret = dev_pm_qos_update_request(max ? dev->power.qos->perf_max_req :
+ dev->power.qos->perf_min_req, new_value);
+
+ return ret < 0 ? ret : n;
+}
+
+static ssize_t pm_qos_perf_limit_min_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return pm_qos_perf_limit_min_max_show(dev, attr, buf, false);
+}
+
+static ssize_t pm_qos_perf_limit_min_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t n)
+{
+ return pm_qos_perf_limit_min_max_store(dev, attr, buf, n, false);
+}
+
+static ssize_t pm_qos_perf_limit_max_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return pm_qos_perf_limit_min_max_show(dev, attr, buf, true);
+}
+
+static ssize_t pm_qos_perf_limit_max_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t n)
+{
+ return pm_qos_perf_limit_min_max_store(dev, attr, buf, n, true);
+}
+
+static DEVICE_ATTR_RW(pm_qos_perf_limit_min);
+static DEVICE_ATTR_RW(pm_qos_perf_limit_max);
+
#ifdef CONFIG_PM_SLEEP
static const char _enabled[] = "enabled";
static const char _disabled[] = "disabled";
@@ -686,6 +756,17 @@ static struct attribute *pm_qos_flags_attrs[] = {
&dev_attr_pm_qos_no_power_off.attr,
NULL,
};
+
+static struct attribute *pm_qos_perf_limit_attrs[] = {
+ &dev_attr_pm_qos_perf_limit_min.attr,
+ &dev_attr_pm_qos_perf_limit_max.attr,
+ NULL,
+};
+static const struct attribute_group pm_qos_perf_limit_attr_group = {
+ .name = power_group_name,
+ .attrs = pm_qos_perf_limit_attrs,
+};
+
static const struct attribute_group pm_qos_flags_attr_group = {
.name = power_group_name,
.attrs = pm_qos_flags_attrs,
@@ -821,6 +902,17 @@ void pm_qos_sysfs_remove_latency_tolerance(struct device *dev)
sysfs_unmerge_group(&dev->kobj, &pm_qos_latency_tolerance_attr_group);
}

+int pm_qos_sysfs_add_perf_limit(struct device *dev)
+{
+ return sysfs_merge_group(&dev->kobj,
+ &pm_qos_perf_limit_attr_group);
+}
+
+void pm_qos_sysfs_remove_perf_limit(struct device *dev)
+{
+ sysfs_unmerge_group(&dev->kobj, &pm_qos_perf_limit_attr_group);
+}
+
void rpm_sysfs_remove(struct device *dev)
{
sysfs_unmerge_group(&dev->kobj, &pm_runtime_attr_group);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 08a8ba4bfd2d..b33fc4db4277 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -83,6 +83,8 @@ struct cpufreq_policy {
struct range_constraints constraints;
struct range_qos_request *min_freq_req;
struct range_qos_request *max_freq_req;
+ struct range_qos_request *min_perf_req;
+ struct range_qos_request *max_perf_req;

struct cpufreq_frequency_table *freq_table;
enum cpufreq_table_sorting freq_table_sorted;
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 5f5d967ede32..ab4412877b59 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -34,6 +34,8 @@ enum pm_qos_flags_status {
#define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0
#define PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE 0
#define PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE FREQ_QOS_MAX_DEFAULT_VALUE
+#define PM_QOS_MIN_PERF_DEFAULT_VALUE 0
+#define PM_QOS_MAX_PERF_DEFAULT_VALUE 1024
#define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1)

#define PM_QOS_FLAG_NO_POWER_OFF (1 << 0)
@@ -102,6 +104,8 @@ enum dev_pm_qos_req_type {
DEV_PM_QOS_LATENCY_TOLERANCE,
DEV_PM_QOS_MIN_FREQUENCY,
DEV_PM_QOS_MAX_FREQUENCY,
+ DEV_PM_QOS_MIN_PERF,
+ DEV_PM_QOS_MAX_PERF,
DEV_PM_QOS_FLAGS,
};

@@ -111,6 +115,7 @@ struct dev_pm_qos_request {
struct plist_node pnode;
struct pm_qos_flags_request flr;
struct range_qos_request freq;
+ struct range_qos_request perf;
} data;
struct device *dev;
};
@@ -119,10 +124,13 @@ struct dev_pm_qos {
struct pm_qos_constraints resume_latency;
struct pm_qos_constraints latency_tolerance;
struct range_constraints freq;
+ struct range_constraints perf;
struct pm_qos_flags flags;
struct dev_pm_qos_request *resume_latency_req;
struct dev_pm_qos_request *latency_tolerance_req;
struct dev_pm_qos_request *flags_req;
+ struct dev_pm_qos_request *perf_min_req;
+ struct dev_pm_qos_request *perf_max_req;
};

/* Action requested to pm_qos_update_target */
@@ -192,6 +200,8 @@ s32 dev_pm_qos_get_user_latency_tolerance(struct device *dev);
int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val);
int dev_pm_qos_expose_latency_tolerance(struct device *dev);
void dev_pm_qos_hide_latency_tolerance(struct device *dev);
+int dev_pm_qos_expose_perf_limit(struct device *dev);
+void dev_pm_qos_hide_perf_limit(struct device *dev);

static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
{
@@ -228,6 +238,10 @@ static inline s32 dev_pm_qos_read_value(struct device *dev,
return PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
case DEV_PM_QOS_MAX_FREQUENCY:
return PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
+ case DEV_PM_QOS_MIN_PERF:
+ return PM_QOS_MIN_PERF_DEFAULT_VALUE;
+ case DEV_PM_QOS_MAX_PERF:
+ return PM_QOS_MAX_PERF_DEFAULT_VALUE;
default:
WARN_ON(1);
return 0;
@@ -281,6 +295,10 @@ static inline int dev_pm_qos_expose_latency_tolerance(struct device *dev)
{ return 0; }
static inline void dev_pm_qos_hide_latency_tolerance(struct device *dev) {}

+static inline int dev_pm_qos_expose_perf_limit(struct device *dev)
+ { return 0; }
+void dev_pm_qos_hide_perf_limit(struct device *dev) {}
+
static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
{
return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
@@ -317,4 +335,28 @@ int freq_qos_remove_notifier(struct range_constraints *qos,
enum range_qos_req_type type,
struct notifier_block *notifier);

+static inline int perf_qos_request_active(struct range_qos_request *req)
+{
+ return !IS_ERR_OR_NULL(req->qos);
+}
+
+s32 perf_qos_read_value(struct range_constraints *qos,
+ enum range_qos_req_type type);
+
+int perf_qos_apply(struct range_qos_request *req,
+ enum pm_qos_req_action action, s32 value);
+
+int perf_qos_add_request(struct range_constraints *qos,
+ struct range_qos_request *req,
+ enum range_qos_req_type type, s32 value);
+int perf_qos_update_request(struct range_qos_request *req, s32 new_value);
+int perf_qos_remove_request(struct range_qos_request *req);
+
+int perf_qos_add_notifier(struct range_constraints *qos,
+ enum range_qos_req_type type,
+ struct notifier_block *notifier);
+int perf_qos_remove_notifier(struct range_constraints *qos,
+ enum range_qos_req_type type,
+ struct notifier_block *notifier);
+
#endif
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 39919a2eed73..2787473e6048 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -680,3 +680,228 @@ int freq_qos_remove_notifier(struct range_constraints *qos,
return ret;
}
EXPORT_SYMBOL_GPL(freq_qos_remove_notifier);
+
+static inline bool perf_qos_value_invalid(s32 value)
+{
+ return value < 0 && value != PM_QOS_DEFAULT_VALUE;
+}
+
+/**
+ * perf_qos_apply - Add/modify/remove performance QoS request.
+ * @req: Constraint request to apply.
+ * @action: Action to perform (add/update/remove).
+ * @value: Value to assign to the QoS request.
+ *
+ * This is only meant to be called from inside pm_qos, not drivers.
+ */
+int perf_qos_apply(struct range_qos_request *req,
+ enum pm_qos_req_action action, s32 value)
+{
+ int ret;
+
+ switch(req->type) {
+ case RANGE_QOS_MIN:
+ ret = pm_qos_update_target(&req->qos->lower_bound, &req->pnode,
+ action, value);
+ break;
+ case RANGE_QOS_MAX:
+ ret = pm_qos_update_target(&req->qos->upper_bound, &req->pnode,
+ action, value);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+/**
+ * perf_qos_read_value - Get performance QoS constraint for a given list.
+ * @qos: Constraints to evaluate.
+ * @type: QoS request type.
+ */
+s32 perf_qos_read_value(struct range_constraints *qos,
+ enum range_qos_req_type type)
+{
+ s32 ret;
+
+ switch (type) {
+ case RANGE_QOS_MIN:
+ ret = IS_ERR_OR_NULL(qos) ?
+ PM_QOS_MIN_PERF_DEFAULT_VALUE :
+ pm_qos_read_value(&qos->lower_bound);
+ break;
+ case RANGE_QOS_MAX:
+ ret = IS_ERR_OR_NULL(qos) ?
+ PM_QOS_MAX_PERF_DEFAULT_VALUE :
+ pm_qos_read_value(&qos->upper_bound);
+ break;
+ default:
+ ret = 0;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(perf_qos_read_value);
+
+/**
+ * perf_qos_add_request - Insert new performance QoS request into a given list.
+ * @qos: Constraints to update.
+ * @req: Preallocated request object.
+ * @type: Request type.
+ * @value: Request value.
+ *
+ * Insert a new entry into the @qos list of requests, recompute the effective
+ * QoS constraint value for that list and initialize the @req object. The
+ * caller needs to save that object for later use in updates and removal.
+ *
+ * Return 1 if the effective constraint value has changed, 0 if the effective
+ * constraint value has not changed, or a negative error code on failures.
+ */
+int perf_qos_add_request(struct range_constraints *qos,
+ struct range_qos_request *req,
+ enum range_qos_req_type type, s32 value)
+{
+ int ret;
+
+ if (IS_ERR_OR_NULL(qos) || !req || perf_qos_value_invalid(value))
+ return -EINVAL;
+
+ if (WARN(perf_qos_request_active(req),
+ "%s() called for active request\n", __func__))
+ return -EINVAL;
+
+ req->qos = qos;
+ req->type = type;
+ ret = perf_qos_apply(req, PM_QOS_ADD_REQ, value);
+ if (ret < 0) {
+ req->qos = NULL;
+ req->type = 0;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(perf_qos_add_request);
+
+/**
+ * perf_qos_update_request - Modify existing performance QoS request.
+ * @req: Request to modify.
+ * @new_value: New request value.
+ *
+ * Update an existing performance QoS request along with the effective
+ * constraint value for the list of requests it belongs to.
+ *
+ * Return 1 if the effective constraint value has changed, 0 if the effective
+ * constraint value has not changed, or a negative error code on failures.
+ */
+int perf_qos_update_request(struct range_qos_request *req, s32 new_value)
+{
+ if (!req || perf_qos_value_invalid(new_value))
+ return -EINVAL;
+
+ if (WARN(!perf_qos_request_active(req),
+ "%s() called for unknown object\n", __func__))
+ return -EINVAL;
+
+ if (req->pnode.prio == new_value)
+ return 0;
+
+ return perf_qos_apply(req, PM_QOS_UPDATE_REQ, new_value);
+}
+EXPORT_SYMBOL_GPL(perf_qos_update_request);
+
+/**
+ * perf_qos_remove_request - Remove performance QoS request from its list.
+ * @req: Request to remove.
+ *
+ * Remove the given performance QoS request from the list of
+ * constraints it belongs to and recompute the effective constraint
+ * value for that list.
+ *
+ * Return 1 if the effective constraint value has changed, 0 if the effective
+ * constraint value has not changed, or a negative error code on failures.
+ */
+int perf_qos_remove_request(struct range_qos_request *req)
+{
+ int ret;
+
+ if (!req)
+ return -EINVAL;
+
+ if (WARN(!perf_qos_request_active(req),
+ "%s() called for unknown object\n", __func__))
+ return -EINVAL;
+
+ ret = perf_qos_apply(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
+ req->qos = NULL;
+ req->type = 0;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(perf_qos_remove_request);
+
+/**
+ * perf_qos_add_notifier - Add performance QoS change notifier.
+ * @qos: List of requests to add the notifier to.
+ * @type: Request type.
+ * @notifier: Notifier block to add.
+ */
+int perf_qos_add_notifier(struct range_constraints *qos,
+ enum range_qos_req_type type,
+ struct notifier_block *notifier)
+{
+ int ret;
+
+ if (IS_ERR_OR_NULL(qos) || !notifier)
+ return -EINVAL;
+
+ switch (type) {
+ case RANGE_QOS_MIN:
+ ret = blocking_notifier_chain_register(qos->lower_bound.notifiers,
+ notifier);
+ break;
+ case RANGE_QOS_MAX:
+ ret = blocking_notifier_chain_register(qos->upper_bound.notifiers,
+ notifier);
+ break;
+ default:
+ WARN_ON(1);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(perf_qos_add_notifier);
+
+/**
+ * perf_qos_remove_notifier - Remove performance QoS change notifier.
+ * @qos: List of requests to remove the notifier from.
+ * @type: Request type.
+ * @notifier: Notifier block to remove.
+ */
+int perf_qos_remove_notifier(struct range_constraints *qos,
+ enum range_qos_req_type type,
+ struct notifier_block *notifier)
+{
+ int ret;
+
+ if (IS_ERR_OR_NULL(qos) || !notifier)
+ return -EINVAL;
+
+ switch (type) {
+ case RANGE_QOS_MIN:
+ ret = blocking_notifier_chain_unregister(qos->lower_bound.notifiers,
+ notifier);
+ break;
+ case RANGE_QOS_MAX:
+ ret = blocking_notifier_chain_unregister(qos->upper_bound.notifiers,
+ notifier);
+ break;
+ default:
+ WARN_ON(1);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(perf_qos_remove_notifier);
--
2.34.1

2023-12-13 18:36:06

by Caleb Connolly

[permalink] [raw]

Subject: Re: [PATCH v1 2/2] PM: QoS: Add a performance QoS

Hi Daniel,

On 13/12/2023 17:58, Daniel Lezcano wrote:
> Currently cpufreq and devfreq are using the freq QoS to aggregate the
> requests for frequency ranges.
>
> However, there are new devices wanting to act not on a frequency range
> but on a performance index range. Those need also to export to
> userspace the knob to act on their performance limits.
>
> This change provides a performance limiter QoS based on a minimum /
> maximum performance values. At init time, the limits of the interval
> are 0 / 1024. It is up to the backend to convert the 1024 to the
> maximum performance state. So if the performance must be limited to
> 50%, it should set to maximum limit to 512 where the backend will end
> up by converting (max performance index / 2). The same applies for the
> minimum. Obviously, the min can not be greater than the max.

I really feel like it should be possible to have arbitrary min/max
performance values. As is the case with latency and frequency.
>
> 1. With the example above, if there is a odd number like 5 for the
> number of performance indexes and we ask for 512 (so 50%), what would
> be the performance index computed? (5/2=2 or 5/2=3)? (I would say the
> minimum otherwise we end up with a performance limit greater than
> what we actually asked for).

For a device with just a handful of performance indices this is quite a
large margin for error. If there are just 3 for example, and some
algorithm is decreasing the performance level over time (e.g. due to
some thermal condition), the algorithm cannot determine at what point
the devices performance level has actually changed, making debugging and
tuning of behaviour needlessly difficult.

This also leaves it up to the backend driver to decide if it should
round up or down, something that should definitely be handled by the
framework.

Maybe I missed some previous discussion, but isn't this what
operating-points is designed for?

It has an `opp-level` property, but that is meant to be device-specific.
With the `opp-hz` property being the "normalised" values that the
framework deals with.

We would just want some way to defined an `opp-level` as a percentage
(or whatever), with an arbitrary `opp-performance-index` being the
device-specific property.

This also gracefully handles non-linear performance scaling.
>
> 2. The conversion from 1024 to a performance index will inevatibly
> end up to a state above or below the percentage given. Shall it be
> reflected in the value set? eg. We want to apply a performance limit
> to be 33% maximum. So it is, 1024 x 0.333333 = 314. If there are 20
> performance indexes, that will be (20 x 314) / 1024 = 6.13, so index
> 6. Shall we convert this index back to the requested performance
> limit to (6.13 x 1024) / 20 = 307 ? (So requested is 314 but it is
> actually 307).>
> The end goal is to make the freq QoS and perf QoS to co-exist together
> in the next changes in the different backends. A change of one of the
> QoS impacts the other. For instance if there are 5 performance states
> and we set a performance limit to 80%, then the maximum state will 4.
>
> For the long term, when those can co-exist, then we can implement a
> cooling device based on the performance Qos which will be generic for
> all devices using this QoS. That will imply the CPUs, the GPUs and any
> devfreq devices. So devfreq and cpufreq cooling devices can be merged
> into a single performance cooling device which will be generic for all
> devices with a performance limit QoS.
>
> In a similar way, in the future, a power QoS could be added also and a
> power based cooling device. So any device with the energy model and a
> power capping feature can become a cooling device and the power
> computation part in the cooling devices will move to the back ends. We
> will end up with a generic power cooling device compatible with all
> power capable devices.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
> drivers/base/power/power.h | 2 +
> drivers/base/power/qos.c | 158 +++++++++++++++++++++++++-
> drivers/base/power/sysfs.c | 92 +++++++++++++++
> include/linux/cpufreq.h | 2 +
> include/linux/pm_qos.h | 42 +++++++
> kernel/power/qos.c | 225 +++++++++++++++++++++++++++++++++++++
> 6 files changed, 517 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index 922ed457db19..eb1a77a7a0f4 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
> @@ -78,6 +78,8 @@ extern int pm_qos_sysfs_add_flags(struct device *dev);
> extern void pm_qos_sysfs_remove_flags(struct device *dev);
> extern int pm_qos_sysfs_add_latency_tolerance(struct device *dev);
> extern void pm_qos_sysfs_remove_latency_tolerance(struct device *dev);
> +extern int pm_qos_sysfs_add_perf_limit(struct device *dev);
> +extern void pm_qos_sysfs_remove_perf_limit(struct device *dev);
> extern int dpm_sysfs_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid);
>
> #else /* CONFIG_PM */
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index ae0b9d2573ec..a71cff1f8048 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -128,6 +128,14 @@ s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
> ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE
> : freq_qos_read_value(&qos->freq, FREQ_QOS_MAX);
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MIN_PERF_DEFAULT_VALUE
> + : perf_qos_read_value(&qos->perf, RANGE_QOS_MIN);
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_PERF_DEFAULT_VALUE
> + : perf_qos_read_value(&qos->perf, RANGE_QOS_MAX);
> + break;
> default:
> WARN_ON(1);
> ret = 0;
> @@ -177,6 +185,10 @@ static int apply_constraint(struct dev_pm_qos_request *req,
> ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
> action, value);
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + case DEV_PM_QOS_MAX_PERF:
> + ret = perf_qos_apply(&req->data.perf, action, value);
> + break;
> default:
> ret = -EINVAL;
> }
> @@ -223,6 +235,20 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
> c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
> c->type = PM_QOS_MIN;
>
> + c = &qos->perf.lower_bound;
> + plist_head_init(&c->list);
> + c->target_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->default_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->no_constraint_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->type = PM_QOS_MAX;
> +
> + c = &qos->perf.upper_bound;
> + plist_head_init(&c->list);
> + c->target_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->default_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->no_constraint_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->type = PM_QOS_MIN;
> +
> freq_constraints_init(&qos->freq);
>
> INIT_LIST_HEAD(&qos->flags.list);
> @@ -299,6 +325,20 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
> memset(req, 0, sizeof(*req));
> }
>
> + c = &qos->perf.lower_bound;
> + plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
> + apply_constraint(req, PM_QOS_REMOVE_REQ,
> + PM_QOS_MIN_PERF_DEFAULT_VALUE);
> + memset(req, 0, sizeof(*req));
> + }
> +
> + c = &qos->perf.upper_bound;
> + plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
> + apply_constraint(req, PM_QOS_REMOVE_REQ,
> + PM_QOS_MAX_PERF_DEFAULT_VALUE);
> + memset(req, 0, sizeof(*req));
> + }
> +
> f = &qos->flags;
> list_for_each_entry_safe(req, tmp, &f->list, data.flr.node) {
> apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
> @@ -349,17 +389,32 @@ static int __dev_pm_qos_add_request(struct device *dev,
>
> req->dev = dev;
> req->type = type;
> - if (req->type == DEV_PM_QOS_MIN_FREQUENCY)
> +
> + switch (type) {
> + case DEV_PM_QOS_MIN_FREQUENCY:
> ret = freq_qos_add_request(&dev->power.qos->freq,
> &req->data.freq,
> FREQ_QOS_MIN, value);
> - else if (req->type == DEV_PM_QOS_MAX_FREQUENCY)
> + break;
> + case DEV_PM_QOS_MAX_FREQUENCY:
> ret = freq_qos_add_request(&dev->power.qos->freq,
> &req->data.freq,
> FREQ_QOS_MAX, value);
> - else
> + break;
> + case DEV_PM_QOS_MIN_PERF:
> + ret = perf_qos_add_request(&dev->power.qos->perf,
> + &req->data.perf,
> + RANGE_QOS_MIN, value);
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + ret = perf_qos_add_request(&dev->power.qos->perf,
> + &req->data.perf,
> + RANGE_QOS_MAX, value);
> + break;
> + default:
> ret = apply_constraint(req, PM_QOS_ADD_REQ, value);
> -
> + break;
> + }
> return ret;
> }
>
> @@ -427,6 +482,10 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
> case DEV_PM_QOS_MAX_FREQUENCY:
> curr_value = req->data.freq.pnode.prio;
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + case DEV_PM_QOS_MAX_PERF:
> + curr_value = req->data.perf.pnode.prio;
> + break;
> case DEV_PM_QOS_FLAGS:
> curr_value = req->data.flr.flags;
> break;
> @@ -674,6 +733,14 @@ static void __dev_pm_qos_drop_user_request(struct device *dev,
> req = dev->power.qos->flags_req;
> dev->power.qos->flags_req = NULL;
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + req = dev->power.qos->perf_min_req;
> + dev->power.qos->perf_min_req = NULL;
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + req = dev->power.qos->perf_max_req;
> + dev->power.qos->perf_max_req = NULL;
> + break;
> default:
> WARN_ON(1);
> return;
> @@ -980,3 +1047,86 @@ void dev_pm_qos_hide_latency_tolerance(struct device *dev)
> pm_runtime_put(dev);
> }
> EXPORT_SYMBOL_GPL(dev_pm_qos_hide_latency_tolerance);
> +
> +int dev_pm_qos_expose_perf_limit(struct device *dev)
> +{
> + struct dev_pm_qos_request *req_min;
> + struct dev_pm_qos_request *req_max;
> + int ret;
> +
> + if (!device_is_registered(dev))
> + return -EINVAL;
> +
> + req_min = kzalloc(sizeof(*req_min), GFP_KERNEL);
> + if (!req_min)
> + return -ENOMEM;
> +
> + req_max = kzalloc(sizeof(*req_max), GFP_KERNEL);
> + if (!req_max) {
> + kfree(req_min);
> + return -ENOMEM;
> + }
> +
> + ret = dev_pm_qos_add_request(dev, req_min, DEV_PM_QOS_MIN_PERF,
> + PM_QOS_MIN_PERF_DEFAULT_VALUE);
> + if (ret < 0) {
> + kfree(req_min);
> + kfree(req_max);
> + return ret;
> + }
> +
> + ret = dev_pm_qos_add_request(dev, req_max, DEV_PM_QOS_MAX_PERF,
> + PM_QOS_MAX_PERF_DEFAULT_VALUE);
> + if (ret < 0) {
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + return ret;
> + }
> +
> + mutex_lock(&dev_pm_qos_sysfs_mtx);
> +
> + mutex_lock(&dev_pm_qos_mtx);
> +
> + if (IS_ERR_OR_NULL(dev->power.qos))
> + ret = -ENODEV;
> + else if (dev->power.qos->perf_min_req || dev->power.qos->perf_max_req)
> + ret = -EEXIST;
> +
> + if (ret < 0) {
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> + mutex_unlock(&dev_pm_qos_mtx);
> + goto out;
> + }
> +
> + dev->power.qos->perf_min_req = req_min;
> + dev->power.qos->perf_max_req = req_max;
> +
> + mutex_unlock(&dev_pm_qos_mtx);
> +
> + ret = pm_qos_sysfs_add_perf_limit(dev);
> + if (ret) {
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> + }
> +out:
> + mutex_unlock(&dev_pm_qos_sysfs_mtx);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_qos_expose_perf_limit);
> +
> +void dev_pm_qos_hide_perf_limit(struct device *dev)
> +{
> + mutex_lock(&dev_pm_qos_sysfs_mtx);
> +
> + pm_qos_sysfs_remove_perf_limit(dev);
> +
> + mutex_lock(&dev_pm_qos_mtx);
> +
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> +
> + mutex_unlock(&dev_pm_qos_mtx);
> +
> + mutex_unlock(&dev_pm_qos_sysfs_mtx);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_qos_hide_perf_limit);
> diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
> index a1474fb67db9..5a45191006c1 100644
> --- a/drivers/base/power/sysfs.c
> +++ b/drivers/base/power/sysfs.c
> @@ -317,6 +317,76 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,
>
> static DEVICE_ATTR_RW(pm_qos_no_power_off);
>
> +
> +static ssize_t pm_qos_perf_limit_min_max_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf, bool max)
> +{
> + s32 value = dev_pm_qos_read_value(dev, max ? DEV_PM_QOS_MAX_PERF :
> + DEV_PM_QOS_MIN_PERF);
> +
> + return sysfs_emit(buf, "%d\n", value);
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_max_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n, bool max)
> +{
> + int ret;
> + s32 min_value = dev_pm_qos_read_value(dev, DEV_PM_QOS_MIN_PERF);
> + s32 max_value = dev_pm_qos_read_value(dev, DEV_PM_QOS_MAX_PERF);
> + s32 new_value;
> +
> + if (kstrtoint(buf, 0, &new_value))
> + return -EINVAL;
> +
> + if (new_value < PM_QOS_MIN_PERF_DEFAULT_VALUE ||
> + new_value > PM_QOS_MAX_PERF_DEFAULT_VALUE)
> + return -EINVAL;
> +
> + if (max && (new_value < min_value))
> + return -EINVAL;
> +
> + if (!max && (new_value > max_value))
> + return -EINVAL;
> +
> + ret = dev_pm_qos_update_request(max ? dev->power.qos->perf_max_req :
> + dev->power.qos->perf_min_req, new_value);
> +
> + return ret < 0 ? ret : n;
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return pm_qos_perf_limit_min_max_show(dev, attr, buf, false);
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + return pm_qos_perf_limit_min_max_store(dev, attr, buf, n, false);
> +}
> +
> +static ssize_t pm_qos_perf_limit_max_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return pm_qos_perf_limit_min_max_show(dev, attr, buf, true);
> +}
> +
> +static ssize_t pm_qos_perf_limit_max_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + return pm_qos_perf_limit_min_max_store(dev, attr, buf, n, true);
> +}
> +
> +static DEVICE_ATTR_RW(pm_qos_perf_limit_min);
> +static DEVICE_ATTR_RW(pm_qos_perf_limit_max);
> +
> #ifdef CONFIG_PM_SLEEP
> static const char _enabled[] = "enabled";
> static const char _disabled[] = "disabled";
> @@ -686,6 +756,17 @@ static struct attribute *pm_qos_flags_attrs[] = {
> &dev_attr_pm_qos_no_power_off.attr,
> NULL,
> };
> +
> +static struct attribute *pm_qos_perf_limit_attrs[] = {
> + &dev_attr_pm_qos_perf_limit_min.attr,
> + &dev_attr_pm_qos_perf_limit_max.attr,
> + NULL,
> +};
> +static const struct attribute_group pm_qos_perf_limit_attr_group = {
> + .name = power_group_name,
> + .attrs = pm_qos_perf_limit_attrs,
> +};
> +
> static const struct attribute_group pm_qos_flags_attr_group = {
> .name = power_group_name,
> .attrs = pm_qos_flags_attrs,
> @@ -821,6 +902,17 @@ void pm_qos_sysfs_remove_latency_tolerance(struct device *dev)
> sysfs_unmerge_group(&dev->kobj, &pm_qos_latency_tolerance_attr_group);
> }
>
> +int pm_qos_sysfs_add_perf_limit(struct device *dev)
> +{
> + return sysfs_merge_group(&dev->kobj,
> + &pm_qos_perf_limit_attr_group);
> +}
> +
> +void pm_qos_sysfs_remove_perf_limit(struct device *dev)
> +{
> + sysfs_unmerge_group(&dev->kobj, &pm_qos_perf_limit_attr_group);
> +}
> +
> void rpm_sysfs_remove(struct device *dev)
> {
> sysfs_unmerge_group(&dev->kobj, &pm_runtime_attr_group);
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 08a8ba4bfd2d..b33fc4db4277 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -83,6 +83,8 @@ struct cpufreq_policy {
> struct range_constraints constraints;
> struct range_qos_request *min_freq_req;
> struct range_qos_request *max_freq_req;
> + struct range_qos_request *min_perf_req;
> + struct range_qos_request *max_perf_req;
>
> struct cpufreq_frequency_table *freq_table;
> enum cpufreq_table_sorting freq_table_sorted;
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 5f5d967ede32..ab4412877b59 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -34,6 +34,8 @@ enum pm_qos_flags_status {
> #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0
> #define PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE 0
> #define PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE FREQ_QOS_MAX_DEFAULT_VALUE
> +#define PM_QOS_MIN_PERF_DEFAULT_VALUE 0
> +#define PM_QOS_MAX_PERF_DEFAULT_VALUE 1024
> #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1)
>
> #define PM_QOS_FLAG_NO_POWER_OFF (1 << 0)
> @@ -102,6 +104,8 @@ enum dev_pm_qos_req_type {
> DEV_PM_QOS_LATENCY_TOLERANCE,
> DEV_PM_QOS_MIN_FREQUENCY,
> DEV_PM_QOS_MAX_FREQUENCY,
> + DEV_PM_QOS_MIN_PERF,
> + DEV_PM_QOS_MAX_PERF,
> DEV_PM_QOS_FLAGS,
> };
>
> @@ -111,6 +115,7 @@ struct dev_pm_qos_request {
> struct plist_node pnode;
> struct pm_qos_flags_request flr;
> struct range_qos_request freq;
> + struct range_qos_request perf;
> } data;
> struct device *dev;
> };
> @@ -119,10 +124,13 @@ struct dev_pm_qos {
> struct pm_qos_constraints resume_latency;
> struct pm_qos_constraints latency_tolerance;
> struct range_constraints freq;
> + struct range_constraints perf;
> struct pm_qos_flags flags;
> struct dev_pm_qos_request *resume_latency_req;
> struct dev_pm_qos_request *latency_tolerance_req;
> struct dev_pm_qos_request *flags_req;
> + struct dev_pm_qos_request *perf_min_req;
> + struct dev_pm_qos_request *perf_max_req;
> };
>
> /* Action requested to pm_qos_update_target */
> @@ -192,6 +200,8 @@ s32 dev_pm_qos_get_user_latency_tolerance(struct device *dev);
> int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val);
> int dev_pm_qos_expose_latency_tolerance(struct device *dev);
> void dev_pm_qos_hide_latency_tolerance(struct device *dev);
> +int dev_pm_qos_expose_perf_limit(struct device *dev);
> +void dev_pm_qos_hide_perf_limit(struct device *dev);
>
> static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
> {
> @@ -228,6 +238,10 @@ static inline s32 dev_pm_qos_read_value(struct device *dev,
> return PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
> case DEV_PM_QOS_MAX_FREQUENCY:
> return PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
> + case DEV_PM_QOS_MIN_PERF:
> + return PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + case DEV_PM_QOS_MAX_PERF:
> + return PM_QOS_MAX_PERF_DEFAULT_VALUE;
> default:
> WARN_ON(1);
> return 0;
> @@ -281,6 +295,10 @@ static inline int dev_pm_qos_expose_latency_tolerance(struct device *dev)
> { return 0; }
> static inline void dev_pm_qos_hide_latency_tolerance(struct device *dev) {}
>
> +static inline int dev_pm_qos_expose_perf_limit(struct device *dev)
> + { return 0; }
> +void dev_pm_qos_hide_perf_limit(struct device *dev) {}
> +
> static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
> {
> return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> @@ -317,4 +335,28 @@ int freq_qos_remove_notifier(struct range_constraints *qos,
> enum range_qos_req_type type,
> struct notifier_block *notifier);
>
> +static inline int perf_qos_request_active(struct range_qos_request *req)
> +{
> + return !IS_ERR_OR_NULL(req->qos);
> +}
> +
> +s32 perf_qos_read_value(struct range_constraints *qos,
> + enum range_qos_req_type type);
> +
> +int perf_qos_apply(struct range_qos_request *req,
> + enum pm_qos_req_action action, s32 value);
> +
> +int perf_qos_add_request(struct range_constraints *qos,
> + struct range_qos_request *req,
> + enum range_qos_req_type type, s32 value);
> +int perf_qos_update_request(struct range_qos_request *req, s32 new_value);
> +int perf_qos_remove_request(struct range_qos_request *req);
> +
> +int perf_qos_add_notifier(struct range_constraints *qos,
> + enum range_qos_req_type type,
> + struct notifier_block *notifier);
> +int perf_qos_remove_notifier(struct range_constraints *qos,
> + enum range_qos_req_type type,
> + struct notifier_block *notifier);
> +
> #endif
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 39919a2eed73..2787473e6048 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -680,3 +680,228 @@ int freq_qos_remove_notifier(struct range_constraints *qos,
> return ret;
> }
> EXPORT_SYMBOL_GPL(freq_qos_remove_notifier);
> +
> +static inline bool perf_qos_value_invalid(s32 value)
> +{
> + return value < 0 && value != PM_QOS_DEFAULT_VALUE;
> +}
> +
> +/**
> + * perf_qos_apply - Add/modify/remove performance QoS request.
> + * @req: Constraint request to apply.
> + * @action: Action to perform (add/update/remove).
> + * @value: Value to assign to the QoS request.
> + *
> + * This is only meant to be called from inside pm_qos, not drivers.
> + */
> +int perf_qos_apply(struct range_qos_request *req,
> + enum pm_qos_req_action action, s32 value)
> +{
> + int ret;
> +
> + switch(req->type) {
> + case RANGE_QOS_MIN:
> + ret = pm_qos_update_target(&req->qos->lower_bound, &req->pnode,
> + action, value);
> + break;
> + case RANGE_QOS_MAX:
> + ret = pm_qos_update_target(&req->qos->upper_bound, &req->pnode,
> + action, value);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * perf_qos_read_value - Get performance QoS constraint for a given list.
> + * @qos: Constraints to evaluate.
> + * @type: QoS request type.
> + */
> +s32 perf_qos_read_value(struct range_constraints *qos,
> + enum range_qos_req_type type)
> +{
> + s32 ret;
> +
> + switch (type) {
> + case RANGE_QOS_MIN:
> + ret = IS_ERR_OR_NULL(qos) ?
> + PM_QOS_MIN_PERF_DEFAULT_VALUE :
> + pm_qos_read_value(&qos->lower_bound);
> + break;
> + case RANGE_QOS_MAX:
> + ret = IS_ERR_OR_NULL(qos) ?
> + PM_QOS_MAX_PERF_DEFAULT_VALUE :
> + pm_qos_read_value(&qos->upper_bound);
> + break;
> + default:
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_read_value);
> +
> +/**
> + * perf_qos_add_request - Insert new performance QoS request into a given list.
> + * @qos: Constraints to update.
> + * @req: Preallocated request object.
> + * @type: Request type.
> + * @value: Request value.
> + *
> + * Insert a new entry into the @qos list of requests, recompute the effective
> + * QoS constraint value for that list and initialize the @req object. The
> + * caller needs to save that object for later use in updates and removal.
> + *
> + * Return 1 if the effective constraint value has changed, 0 if the effective
> + * constraint value has not changed, or a negative error code on failures.
> + */
> +int perf_qos_add_request(struct range_constraints *qos,
> + struct range_qos_request *req,
> + enum range_qos_req_type type, s32 value)
> +{
> + int ret;
> +
> + if (IS_ERR_OR_NULL(qos) || !req || perf_qos_value_invalid(value))
> + return -EINVAL;
> +
> + if (WARN(perf_qos_request_active(req),
> + "%s() called for active request\n", __func__))
> + return -EINVAL;
> +
> + req->qos = qos;
> + req->type = type;
> + ret = perf_qos_apply(req, PM_QOS_ADD_REQ, value);
> + if (ret < 0) {
> + req->qos = NULL;
> + req->type = 0;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_add_request);
> +
> +/**
> + * perf_qos_update_request - Modify existing performance QoS request.
> + * @req: Request to modify.
> + * @new_value: New request value.
> + *
> + * Update an existing performance QoS request along with the effective
> + * constraint value for the list of requests it belongs to.
> + *
> + * Return 1 if the effective constraint value has changed, 0 if the effective
> + * constraint value has not changed, or a negative error code on failures.
> + */
> +int perf_qos_update_request(struct range_qos_request *req, s32 new_value)
> +{
> + if (!req || perf_qos_value_invalid(new_value))
> + return -EINVAL;
> +
> + if (WARN(!perf_qos_request_active(req),
> + "%s() called for unknown object\n", __func__))
> + return -EINVAL;
> +
> + if (req->pnode.prio == new_value)
> + return 0;
> +
> + return perf_qos_apply(req, PM_QOS_UPDATE_REQ, new_value);
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_update_request);
> +
> +/**
> + * perf_qos_remove_request - Remove performance QoS request from its list.
> + * @req: Request to remove.
> + *
> + * Remove the given performance QoS request from the list of
> + * constraints it belongs to and recompute the effective constraint
> + * value for that list.
> + *
> + * Return 1 if the effective constraint value has changed, 0 if the effective
> + * constraint value has not changed, or a negative error code on failures.
> + */
> +int perf_qos_remove_request(struct range_qos_request *req)
> +{
> + int ret;
> +
> + if (!req)
> + return -EINVAL;
> +
> + if (WARN(!perf_qos_request_active(req),
> + "%s() called for unknown object\n", __func__))
> + return -EINVAL;
> +
> + ret = perf_qos_apply(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
> + req->qos = NULL;
> + req->type = 0;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_remove_request);
> +
> +/**
> + * perf_qos_add_notifier - Add performance QoS change notifier.
> + * @qos: List of requests to add the notifier to.
> + * @type: Request type.
> + * @notifier: Notifier block to add.
> + */
> +int perf_qos_add_notifier(struct range_constraints *qos,
> + enum range_qos_req_type type,
> + struct notifier_block *notifier)
> +{
> + int ret;
> +
> + if (IS_ERR_OR_NULL(qos) || !notifier)
> + return -EINVAL;
> +
> + switch (type) {
> + case RANGE_QOS_MIN:
> + ret = blocking_notifier_chain_register(qos->lower_bound.notifiers,
> + notifier);
> + break;
> + case RANGE_QOS_MAX:
> + ret = blocking_notifier_chain_register(qos->upper_bound.notifiers,
> + notifier);
> + break;
> + default:
> + WARN_ON(1);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_add_notifier);
> +
> +/**
> + * perf_qos_remove_notifier - Remove performance QoS change notifier.
> + * @qos: List of requests to remove the notifier from.
> + * @type: Request type.
> + * @notifier: Notifier block to remove.
> + */
> +int perf_qos_remove_notifier(struct range_constraints *qos,
> + enum range_qos_req_type type,
> + struct notifier_block *notifier)
> +{
> + int ret;
> +
> + if (IS_ERR_OR_NULL(qos) || !notifier)
> + return -EINVAL;
> +
> + switch (type) {
> + case RANGE_QOS_MIN:
> + ret = blocking_notifier_chain_unregister(qos->lower_bound.notifiers,
> + notifier);
> + break;
> + case RANGE_QOS_MAX:
> + ret = blocking_notifier_chain_unregister(qos->upper_bound.notifiers,
> + notifier);
> + break;
> + default:
> + WARN_ON(1);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_remove_notifier);

--
// Caleb (they/them)

2023-12-14 07:24:22

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v1 2/2] PM: QoS: Add a performance QoS

Hi Daniel,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on rafael-pm/acpi-bus rafael-pm/thermal linus/master rafael-pm/devprop v6.7-rc5 next-20231213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Lezcano/PM-QoS-Add-a-performance-QoS/20231214-020026
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20231213175818.2826876-2-daniel.lezcano%40linaro.org
patch subject: [PATCH v1 2/2] PM: QoS: Add a performance QoS
config: x86_64-randconfig-012-20231214 (https://download.01.org/0day-ci/archive/20231214/[email protected]/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

ld: sound/core/oss/pcm_plugin.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/core/oss/pcm_oss.o:include/linux/pm_qos.h:300: first defined here
ld: sound/core/oss/io.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/core/oss/pcm_oss.o:include/linux/pm_qos.h:300: first defined here
ld: sound/core/oss/copy.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/core/oss/pcm_oss.o:include/linux/pm_qos.h:300: first defined here
ld: sound/core/oss/linear.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/core/oss/pcm_oss.o:include/linux/pm_qos.h:300: first defined here
ld: sound/core/oss/mulaw.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/core/oss/pcm_oss.o:include/linux/pm_qos.h:300: first defined here
ld: sound/core/oss/route.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/core/oss/pcm_oss.o:include/linux/pm_qos.h:300: first defined here
ld: sound/core/oss/rate.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/core/oss/pcm_oss.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/virtio/virtio_chmap.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/virtio/virtio_card.o:include/linux/pm_qos.h:300: first defined here
ld: sound/virtio/virtio_ctl_msg.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/virtio/virtio_card.o:include/linux/pm_qos.h:300: first defined here
ld: sound/virtio/virtio_jack.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/virtio/virtio_card.o:include/linux/pm_qos.h:300: first defined here
ld: sound/virtio/virtio_pcm.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/virtio/virtio_card.o:include/linux/pm_qos.h:300: first defined here
ld: sound/virtio/virtio_pcm_msg.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/virtio/virtio_card.o:include/linux/pm_qos.h:300: first defined here
ld: sound/virtio/virtio_pcm_ops.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/virtio/virtio_card.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/soc/soc-dapm.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-jack.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-utils.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-dai.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-component.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-pcm.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-devres.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-ops.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-link.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-card.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-generic-dmaengine-pcm.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-ac97.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
ld: sound/soc/soc-compress.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/soc-core.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/soc/codecs/cs35l45-tables.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/codecs/cs35l45.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/soc/codecs/rt715-sdw.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/codecs/rt715.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/soc/codecs/rt712-sdca-sdw.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/codecs/rt712-sdca.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/soc/codecs/rt711-sdw.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/codecs/rt711.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/soc/codecs/rt700-sdw.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/codecs/rt700.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/soc/codecs/rt722-sdca-sdw.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/codecs/rt722-sdca.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/soc/fsl/fsl_asrc_dma.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/soc/fsl/fsl_asrc.o:include/linux/pm_qos.h:300: first defined here
--
ld: sound/pci/ac97/ac97_pcm.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/pci/ac97/ac97_codec.o:include/linux/pm_qos.h:300: first defined here
ld: sound/pci/ac97/ac97_proc.o: in function `dev_pm_qos_hide_perf_limit':
>> include/linux/pm_qos.h:300: multiple definition of `dev_pm_qos_hide_perf_limit'; sound/pci/ac97/ac97_codec.o:include/linux/pm_qos.h:300: first defined here
..

vim +300 include/linux/pm_qos.h

297
298 static inline int dev_pm_qos_expose_perf_limit(struct device *dev)
299 { return 0; }
> 300 void dev_pm_qos_hide_perf_limit(struct device *dev) {}
301

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-14 07:36:34

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v1 2/2] PM: QoS: Add a performance QoS

2023-12-14 09:51:20

by Rafael J. Wysocki

[permalink] [raw]

Subject: Re: [PATCH v1 1/2] PM: QoS: Rename freq to range constraint

On Wed, Dec 13, 2023 at 6:58 PM Daniel Lezcano
<[email protected]> wrote:
>
> The frequency pm_qos relies on a couple of values, the min and max
> frequencies. However more pm_qos will be added with the same logic of
> a couple of min and max. Instead of writing new set of constraints as
> well as type, etc... let's rename freq_* to a more generic name
> range_*
>
> That way, new qos range based can be added easily.
>
> No functional changes intended.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---

[cut]

> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -77,25 +77,26 @@ struct pm_qos_flags {
> #define FREQ_QOS_MIN_DEFAULT_VALUE 0
> #define FREQ_QOS_MAX_DEFAULT_VALUE S32_MAX
>
> -enum freq_qos_req_type {
> - FREQ_QOS_MIN = 1,
> +enum range_qos_req_type {
> + RANGE_QOS_MIN = 1,
> + RANGE_QOS_MAX,
> + FREQ_QOS_MIN,
> FREQ_QOS_MAX,
> };

I'd rather do:

+enum range_qos_req_type {
+ RANGE_QOS_MIN = 1
+ RANGE_QOS_MAX,
};

+#define FREQ_QOS_MIN RANGE_QOS_MIN
+#define FREQ_QOS_MAX RANGE_QOS_MAX

and they would map exactly.

2023-12-19 12:33:39

by Daniel Lezcano

[permalink] [raw]

Subject: Re: [PATCH v1 2/2] PM: QoS: Add a performance QoS

Hi Caleb,

[Cc'ed Viresh]

On 13/12/2023 19:35, Caleb Connolly wrote:
> Hi Daniel,
>
> On 13/12/2023 17:58, Daniel Lezcano wrote:
>> Currently cpufreq and devfreq are using the freq QoS to aggregate the
>> requests for frequency ranges.
>>
>> However, there are new devices wanting to act not on a frequency range
>> but on a performance index range. Those need also to export to
>> userspace the knob to act on their performance limits.
>>
>> This change provides a performance limiter QoS based on a minimum /
>> maximum performance values. At init time, the limits of the interval
>> are 0 / 1024. It is up to the backend to convert the 1024 to the
>> maximum performance state. So if the performance must be limited to
>> 50%, it should set to maximum limit to 512 where the backend will end
>> up by converting (max performance index / 2). The same applies for the
>> minimum. Obviously, the min can not be greater than the max.
>
> I really feel like it should be possible to have arbitrary min/max
> performance values. As is the case with latency and frequency.

We had an initial discussion about the performance QoS some weeks ago.
Rafael is reluctant to have arbitrary values. So it was proposed a 1024
based approach and let the back end to convert the value to its index.

If we go for a similar approach to the frequencies, then we should have
more files to describe the different states. At least one defining the
current state, the min and the max.

>> 1. With the example above, if there is a odd number like 5 for the
>> number of performance indexes and we ask for 512 (so 50%), what would
>> be the performance index computed? (5/2=2 or 5/2=3)? (I would say the
>> minimum otherwise we end up with a performance limit greater than
>> what we actually asked for).
>
> For a device with just a handful of performance indices this is quite a
> large margin for error. If there are just 3 for example, and some
> algorithm is decreasing the performance level over time (e.g. due to
> some thermal condition), the algorithm cannot determine at what point
> the devices performance level has actually changed, making debugging and
> tuning of behaviour needlessly difficult.

Yes, it is a valid point. May be we can find an intermediate approach.

If we define an additional information, let's call it "granularity" for
example and keep the 0-1023, then the userspace can rely on this
information to build the steps.

If we take your example with a 3 performance states device, then the
granularity would be:

1024 / 3 = 341.3

As floating does not exist in the kernel, then it would be 342.

State 0 = 0 x 342 = 0
State 1 = 1 x 342 = 342
State 2 = 2 x 342 = 684
State 3 = 3 x 342 = 1026 (floored to 1024)

So we end up with a fixed range, a way to quickly escalate the stairs
and three files in the device's power sysfs entry.

> This also leaves it up to the backend driver to decide if it should
> round up or down, something that should definitely be handled by the
> framework.

> Maybe I missed some previous discussion, but isn't this what
> operating-points is designed for?
>
> It has an `opp-level` property, but that is meant to be device-specific.
> With the `opp-hz` property being the "normalised" values that the
> framework deals with.
>
> We would just want some way to defined an `opp-level` as a percentage
> (or whatever), with an arbitrary `opp-performance-index` being the
> device-specific property.
>
> This also gracefully handles non-linear performance scaling.

I think it is a different subject, we are talking about how to describe
the hardware and these performance states. But I agree, it is worth to
keep the opp description in mind.

[ ... ]

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2024-02-12 17:35:48

by Rafael J. Wysocki

[permalink] [raw]

Subject: Re: [PATCH v1 2/2] PM: QoS: Add a performance QoS

On Wed, Dec 13, 2023 at 6:58 PM Daniel Lezcano
<[email protected]> wrote:
>
> Currently cpufreq and devfreq are using the freq QoS to aggregate the
> requests for frequency ranges.
>
> However, there are new devices wanting to act not on a frequency range
> but on a performance index range. Those need also to export to
> userspace the knob to act on their performance limits.
>
> This change provides a performance limiter QoS based on a minimum /
> maximum performance values. At init time, the limits of the interval
> are 0 / 1024. It is up to the backend to convert the 1024 to the
> maximum performance state. So if the performance must be limited to
> 50%, it should set to maximum limit to 512 where the backend will end
> up by converting (max performance index / 2). The same applies for the
> minimum. Obviously, the min can not be greater than the max.
>
> 1. With the example above, if there is a odd number like 5 for the
> number of performance indexes and we ask for 512 (so 50%), what would
> be the performance index computed? (5/2=2 or 5/2=3)? (I would say the
> minimum otherwise we end up with a performance limit greater than
> what we actually asked for).
>
> 2. The conversion from 1024 to a performance index will inevatibly
> end up to a state above or below the percentage given. Shall it be
> reflected in the value set? eg. We want to apply a performance limit
> to be 33% maximum. So it is, 1024 x 0.333333 = 314. If there are 20
> performance indexes, that will be (20 x 314) / 1024 = 6.13, so index
> 6. Shall we convert this index back to the requested performance
> limit to (6.13 x 1024) / 20 = 307 ? (So requested is 314 but it is
> actually 307).
>
> The end goal is to make the freq QoS and perf QoS to co-exist together
> in the next changes in the different backends. A change of one of the
> QoS impacts the other. For instance if there are 5 performance states
> and we set a performance limit to 80%, then the maximum state will 4.
>
> For the long term, when those can co-exist, then we can implement a
> cooling device based on the performance Qos which will be generic for
> all devices using this QoS. That will imply the CPUs, the GPUs and any
> devfreq devices. So devfreq and cpufreq cooling devices can be merged
> into a single performance cooling device which will be generic for all
> devices with a performance limit QoS.
>
> In a similar way, in the future, a power QoS could be added also and a
> power based cooling device. So any device with the energy model and a
> power capping feature can become a cooling device and the power
> computation part in the cooling devices will move to the back ends. We
> will end up with a generic power cooling device compatible with all
> power capable devices.
>
> Signed-off-by: Daniel Lezcano <[email protected]>

I sent a comment on the first patch in the series and please note that
0-day has reported build issues with this one. I guess it needs to be
refreshed.

At the general level, I think that it would be good to add at least
one user of this framework along with it, as a matter of illustration
how it is going to (or at least how it is intended to) be used. For
example, what is going to happen when user space updates one of the
limits via sysfs for a given device?

I also have a couple of technical comments below.

> ---
> drivers/base/power/power.h | 2 +
> drivers/base/power/qos.c | 158 +++++++++++++++++++++++++-
> drivers/base/power/sysfs.c | 92 +++++++++++++++
> include/linux/cpufreq.h | 2 +
> include/linux/pm_qos.h | 42 +++++++
> kernel/power/qos.c | 225 +++++++++++++++++++++++++++++++++++++
> 6 files changed, 517 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index 922ed457db19..eb1a77a7a0f4 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
> @@ -78,6 +78,8 @@ extern int pm_qos_sysfs_add_flags(struct device *dev);
> extern void pm_qos_sysfs_remove_flags(struct device *dev);
> extern int pm_qos_sysfs_add_latency_tolerance(struct device *dev);
> extern void pm_qos_sysfs_remove_latency_tolerance(struct device *dev);
> +extern int pm_qos_sysfs_add_perf_limit(struct device *dev);
> +extern void pm_qos_sysfs_remove_perf_limit(struct device *dev);
> extern int dpm_sysfs_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid);
>
> #else /* CONFIG_PM */
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index ae0b9d2573ec..a71cff1f8048 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -128,6 +128,14 @@ s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
> ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE
> : freq_qos_read_value(&qos->freq, FREQ_QOS_MAX);
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MIN_PERF_DEFAULT_VALUE
> + : perf_qos_read_value(&qos->perf, RANGE_QOS_MIN);
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_PERF_DEFAULT_VALUE
> + : perf_qos_read_value(&qos->perf, RANGE_QOS_MAX);
> + break;
> default:
> WARN_ON(1);
> ret = 0;
> @@ -177,6 +185,10 @@ static int apply_constraint(struct dev_pm_qos_request *req,
> ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
> action, value);
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + case DEV_PM_QOS_MAX_PERF:
> + ret = perf_qos_apply(&req->data.perf, action, value);
> + break;
> default:
> ret = -EINVAL;
> }
> @@ -223,6 +235,20 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
> c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
> c->type = PM_QOS_MIN;
>
> + c = &qos->perf.lower_bound;
> + plist_head_init(&c->list);
> + c->target_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->default_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->no_constraint_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->type = PM_QOS_MAX;
> +
> + c = &qos->perf.upper_bound;
> + plist_head_init(&c->list);
> + c->target_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->default_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->no_constraint_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->type = PM_QOS_MIN;
> +
> freq_constraints_init(&qos->freq);
>
> INIT_LIST_HEAD(&qos->flags.list);
> @@ -299,6 +325,20 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
> memset(req, 0, sizeof(*req));
> }
>
> + c = &qos->perf.lower_bound;
> + plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
> + apply_constraint(req, PM_QOS_REMOVE_REQ,
> + PM_QOS_MIN_PERF_DEFAULT_VALUE);
> + memset(req, 0, sizeof(*req));
> + }
> +
> + c = &qos->perf.upper_bound;
> + plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
> + apply_constraint(req, PM_QOS_REMOVE_REQ,
> + PM_QOS_MAX_PERF_DEFAULT_VALUE);
> + memset(req, 0, sizeof(*req));
> + }
> +
> f = &qos->flags;
> list_for_each_entry_safe(req, tmp, &f->list, data.flr.node) {
> apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
> @@ -349,17 +389,32 @@ static int __dev_pm_qos_add_request(struct device *dev,
>
> req->dev = dev;
> req->type = type;
> - if (req->type == DEV_PM_QOS_MIN_FREQUENCY)
> +
> + switch (type) {
> + case DEV_PM_QOS_MIN_FREQUENCY:
> ret = freq_qos_add_request(&dev->power.qos->freq,
> &req->data.freq,
> FREQ_QOS_MIN, value);
> - else if (req->type == DEV_PM_QOS_MAX_FREQUENCY)
> + break;
> + case DEV_PM_QOS_MAX_FREQUENCY:
> ret = freq_qos_add_request(&dev->power.qos->freq,
> &req->data.freq,
> FREQ_QOS_MAX, value);
> - else
> + break;
> + case DEV_PM_QOS_MIN_PERF:
> + ret = perf_qos_add_request(&dev->power.qos->perf,
> + &req->data.perf,
> + RANGE_QOS_MIN, value);
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + ret = perf_qos_add_request(&dev->power.qos->perf,
> + &req->data.perf,
> + RANGE_QOS_MAX, value);
> + break;
> + default:
> ret = apply_constraint(req, PM_QOS_ADD_REQ, value);
> -
> + break;
> + }
> return ret;
> }
>
> @@ -427,6 +482,10 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
> case DEV_PM_QOS_MAX_FREQUENCY:
> curr_value = req->data.freq.pnode.prio;
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + case DEV_PM_QOS_MAX_PERF:
> + curr_value = req->data.perf.pnode.prio;
> + break;
> case DEV_PM_QOS_FLAGS:
> curr_value = req->data.flr.flags;
> break;
> @@ -674,6 +733,14 @@ static void __dev_pm_qos_drop_user_request(struct device *dev,
> req = dev->power.qos->flags_req;
> dev->power.qos->flags_req = NULL;
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + req = dev->power.qos->perf_min_req;
> + dev->power.qos->perf_min_req = NULL;
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + req = dev->power.qos->perf_max_req;
> + dev->power.qos->perf_max_req = NULL;
> + break;
> default:
> WARN_ON(1);
> return;
> @@ -980,3 +1047,86 @@ void dev_pm_qos_hide_latency_tolerance(struct device *dev)
> pm_runtime_put(dev);
> }
> EXPORT_SYMBOL_GPL(dev_pm_qos_hide_latency_tolerance);
> +
> +int dev_pm_qos_expose_perf_limit(struct device *dev)
> +{
> + struct dev_pm_qos_request *req_min;
> + struct dev_pm_qos_request *req_max;
> + int ret;
> +
> + if (!device_is_registered(dev))
> + return -EINVAL;
> +
> + req_min = kzalloc(sizeof(*req_min), GFP_KERNEL);
> + if (!req_min)
> + return -ENOMEM;
> +
> + req_max = kzalloc(sizeof(*req_max), GFP_KERNEL);
> + if (!req_max) {
> + kfree(req_min);
> + return -ENOMEM;
> + }
> +
> + ret = dev_pm_qos_add_request(dev, req_min, DEV_PM_QOS_MIN_PERF,
> + PM_QOS_MIN_PERF_DEFAULT_VALUE);
> + if (ret < 0) {
> + kfree(req_min);
> + kfree(req_max);
> + return ret;
> + }
> +
> + ret = dev_pm_qos_add_request(dev, req_max, DEV_PM_QOS_MAX_PERF,
> + PM_QOS_MAX_PERF_DEFAULT_VALUE);
> + if (ret < 0) {
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + return ret;
> + }
> +
> + mutex_lock(&dev_pm_qos_sysfs_mtx);
> +
> + mutex_lock(&dev_pm_qos_mtx);
> +
> + if (IS_ERR_OR_NULL(dev->power.qos))
> + ret = -ENODEV;
> + else if (dev->power.qos->perf_min_req || dev->power.qos->perf_max_req)
> + ret = -EEXIST;
> +
> + if (ret < 0) {
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> + mutex_unlock(&dev_pm_qos_mtx);
> + goto out;
> + }
> +
> + dev->power.qos->perf_min_req = req_min;
> + dev->power.qos->perf_max_req = req_max;
> +
> + mutex_unlock(&dev_pm_qos_mtx);
> +
> + ret = pm_qos_sysfs_add_perf_limit(dev);
> + if (ret) {
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> + }
> +out:
> + mutex_unlock(&dev_pm_qos_sysfs_mtx);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_qos_expose_perf_limit);
> +
> +void dev_pm_qos_hide_perf_limit(struct device *dev)
> +{
> + mutex_lock(&dev_pm_qos_sysfs_mtx);
> +
> + pm_qos_sysfs_remove_perf_limit(dev);
> +
> + mutex_lock(&dev_pm_qos_mtx);
> +
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> +
> + mutex_unlock(&dev_pm_qos_mtx);
> +
> + mutex_unlock(&dev_pm_qos_sysfs_mtx);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_qos_hide_perf_limit);
> diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
> index a1474fb67db9..5a45191006c1 100644
> --- a/drivers/base/power/sysfs.c
> +++ b/drivers/base/power/sysfs.c
> @@ -317,6 +317,76 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,
>
> static DEVICE_ATTR_RW(pm_qos_no_power_off);
>
> +
> +static ssize_t pm_qos_perf_limit_min_max_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf, bool max)

I'm not sure why bool is passed as the last arg. I'd pass
DEV_PM_QOS_MAX_PERF or DEV_PM_QOS_MIN_PERF directly and then do

return sysfs_emit(buf, "%d\n", dev_pm_qos_read_value(dev, req_type));

> +{
> + s32 value = dev_pm_qos_read_value(dev, max ? DEV_PM_QOS_MAX_PERF :
> + DEV_PM_QOS_MIN_PERF);
> +
> + return sysfs_emit(buf, "%d\n", value);
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_max_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n, bool max)

And analogously here, so it is not necessary to use the ternary operator below.

> +{
> + int ret;
> + s32 min_value = dev_pm_qos_read_value(dev, DEV_PM_QOS_MIN_PERF);
> + s32 max_value = dev_pm_qos_read_value(dev, DEV_PM_QOS_MAX_PERF);
> + s32 new_value;
> +
> + if (kstrtoint(buf, 0, &new_value))
> + return -EINVAL;
> +
> + if (new_value < PM_QOS_MIN_PERF_DEFAULT_VALUE ||
> + new_value > PM_QOS_MAX_PERF_DEFAULT_VALUE)
> + return -EINVAL;
> +
> + if (max && (new_value < min_value))
> + return -EINVAL;
> +
> + if (!max && (new_value > max_value))
> + return -EINVAL;
> +
> + ret = dev_pm_qos_update_request(max ? dev->power.qos->perf_max_req :
> + dev->power.qos->perf_min_req, new_value);
> +
> + return ret < 0 ? ret : n;
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return pm_qos_perf_limit_min_max_show(dev, attr, buf, false);
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + return pm_qos_perf_limit_min_max_store(dev, attr, buf, n, false);
> +}
> +
> +static ssize_t pm_qos_perf_limit_max_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return pm_qos_perf_limit_min_max_show(dev, attr, buf, true);
> +}
> +
> +static ssize_t pm_qos_perf_limit_max_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + return pm_qos_perf_limit_min_max_store(dev, attr, buf, n, true);
> +}
> +
> +static DEVICE_ATTR_RW(pm_qos_perf_limit_min);
> +static DEVICE_ATTR_RW(pm_qos_perf_limit_max);
> +
> #ifdef CONFIG_PM_SLEEP
> static const char _enabled[] = "enabled";
> static const char _disabled[] = "disabled";
> @@ -686,6 +756,17 @@ static struct attribute *pm_qos_flags_attrs[] = {
> &dev_attr_pm_qos_no_power_off.attr,
> NULL,
> };
> +
> +static struct attribute *pm_qos_perf_limit_attrs[] = {
> + &dev_attr_pm_qos_perf_limit_min.attr,
> + &dev_attr_pm_qos_perf_limit_max.attr,
> + NULL,
> +};
> +static const struct attribute_group pm_qos_perf_limit_attr_group = {
> + .name = power_group_name,
> + .attrs = pm_qos_perf_limit_attrs,
> +};
> +
> static const struct attribute_group pm_qos_flags_attr_group = {
> .name = power_group_name,
> .attrs = pm_qos_flags_attrs,
> @@ -821,6 +902,17 @@ void pm_qos_sysfs_remove_latency_tolerance(struct device *dev)
> sysfs_unmerge_group(&dev->kobj, &pm_qos_latency_tolerance_attr_group);
> }
>
> +int pm_qos_sysfs_add_perf_limit(struct device *dev)
> +{
> + return sysfs_merge_group(&dev->kobj,
> + &pm_qos_perf_limit_attr_group);
> +}
> +
> +void pm_qos_sysfs_remove_perf_limit(struct device *dev)
> +{
> + sysfs_unmerge_group(&dev->kobj, &pm_qos_perf_limit_attr_group);
> +}
> +
> void rpm_sysfs_remove(struct device *dev)
> {
> sysfs_unmerge_group(&dev->kobj, &pm_runtime_attr_group);
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 08a8ba4bfd2d..b33fc4db4277 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -83,6 +83,8 @@ struct cpufreq_policy {
> struct range_constraints constraints;
> struct range_qos_request *min_freq_req;
> struct range_qos_request *max_freq_req;
> + struct range_qos_request *min_perf_req;
> + struct range_qos_request *max_perf_req;
>
> struct cpufreq_frequency_table *freq_table;
> enum cpufreq_table_sorting freq_table_sorted;
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 5f5d967ede32..ab4412877b59 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -34,6 +34,8 @@ enum pm_qos_flags_status {
> #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0
> #define PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE 0
> #define PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE FREQ_QOS_MAX_DEFAULT_VALUE
> +#define PM_QOS_MIN_PERF_DEFAULT_VALUE 0
> +#define PM_QOS_MAX_PERF_DEFAULT_VALUE 1024
> #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1)
>
> #define PM_QOS_FLAG_NO_POWER_OFF (1 << 0)
> @@ -102,6 +104,8 @@ enum dev_pm_qos_req_type {
> DEV_PM_QOS_LATENCY_TOLERANCE,
> DEV_PM_QOS_MIN_FREQUENCY,
> DEV_PM_QOS_MAX_FREQUENCY,
> + DEV_PM_QOS_MIN_PERF,
> + DEV_PM_QOS_MAX_PERF,
> DEV_PM_QOS_FLAGS,
> };
>
> @@ -111,6 +115,7 @@ struct dev_pm_qos_request {
> struct plist_node pnode;
> struct pm_qos_flags_request flr;
> struct range_qos_request freq;
> + struct range_qos_request perf;
> } data;
> struct device *dev;
> };
> @@ -119,10 +124,13 @@ struct dev_pm_qos {
> struct pm_qos_constraints resume_latency;
> struct pm_qos_constraints latency_tolerance;
> struct range_constraints freq;
> + struct range_constraints perf;
> struct pm_qos_flags flags;
> struct dev_pm_qos_request *resume_latency_req;
> struct dev_pm_qos_request *latency_tolerance_req;
> struct dev_pm_qos_request *flags_req;
> + struct dev_pm_qos_request *perf_min_req;
> + struct dev_pm_qos_request *perf_max_req;
> };
>
> /* Action requested to pm_qos_update_target */
> @@ -192,6 +200,8 @@ s32 dev_pm_qos_get_user_latency_tolerance(struct device *dev);
> int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val);
> int dev_pm_qos_expose_latency_tolerance(struct device *dev);
> void dev_pm_qos_hide_latency_tolerance(struct device *dev);
> +int dev_pm_qos_expose_perf_limit(struct device *dev);
> +void dev_pm_qos_hide_perf_limit(struct device *dev);
>
> static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
> {
> @@ -228,6 +238,10 @@ static inline s32 dev_pm_qos_read_value(struct device *dev,
> return PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
> case DEV_PM_QOS_MAX_FREQUENCY:
> return PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
> + case DEV_PM_QOS_MIN_PERF:
> + return PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + case DEV_PM_QOS_MAX_PERF:
> + return PM_QOS_MAX_PERF_DEFAULT_VALUE;
> default:
> WARN_ON(1);
> return 0;
> @@ -281,6 +295,10 @@ static inline int dev_pm_qos_expose_latency_tolerance(struct device *dev)
> { return 0; }
> static inline void dev_pm_qos_hide_latency_tolerance(struct device *dev) {}
>
> +static inline int dev_pm_qos_expose_perf_limit(struct device *dev)
> + { return 0; }
> +void dev_pm_qos_hide_perf_limit(struct device *dev) {}
> +
> static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
> {
> return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> @@ -317,4 +335,28 @@ int freq_qos_remove_notifier(struct range_constraints *qos,
> enum range_qos_req_type type,
> struct notifier_block *notifier);
>
> +static inline int perf_qos_request_active(struct range_qos_request *req)
> +{
> + return !IS_ERR_OR_NULL(req->qos);
> +}
> +
> +s32 perf_qos_read_value(struct range_constraints *qos,
> + enum range_qos_req_type type);
> +
> +int perf_qos_apply(struct range_qos_request *req,
> + enum pm_qos_req_action action, s32 value);
> +
> +int perf_qos_add_request(struct range_constraints *qos,
> + struct range_qos_request *req,
> + enum range_qos_req_type type, s32 value);
> +int perf_qos_update_request(struct range_qos_request *req, s32 new_value);
> +int perf_qos_remove_request(struct range_qos_request *req);
> +
> +int perf_qos_add_notifier(struct range_constraints *qos,
> + enum range_qos_req_type type,
> + struct notifier_block *notifier);
> +int perf_qos_remove_notifier(struct range_constraints *qos,
> + enum range_qos_req_type type,
> + struct notifier_block *notifier);
> +
> #endif
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 39919a2eed73..2787473e6048 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -680,3 +680,228 @@ int freq_qos_remove_notifier(struct range_constraints *qos,
> return ret;
> }
> EXPORT_SYMBOL_GPL(freq_qos_remove_notifier);
> +
> +static inline bool perf_qos_value_invalid(s32 value)
> +{
> + return value < 0 && value != PM_QOS_DEFAULT_VALUE;
> +}
> +
> +/**
> + * perf_qos_apply - Add/modify/remove performance QoS request.
> + * @req: Constraint request to apply.
> + * @action: Action to perform (add/update/remove).
> + * @value: Value to assign to the QoS request.
> + *
> + * This is only meant to be called from inside pm_qos, not drivers.
> + */
> +int perf_qos_apply(struct range_qos_request *req,
> + enum pm_qos_req_action action, s32 value)
> +{
> + int ret;
> +
> + switch(req->type) {
> + case RANGE_QOS_MIN:
> + ret = pm_qos_update_target(&req->qos->lower_bound, &req->pnode,
> + action, value);
> + break;
> + case RANGE_QOS_MAX:
> + ret = pm_qos_update_target(&req->qos->upper_bound, &req->pnode,
> + action, value);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * perf_qos_read_value - Get performance QoS constraint for a given list.
> + * @qos: Constraints to evaluate.
> + * @type: QoS request type.
> + */
> +s32 perf_qos_read_value(struct range_constraints *qos,
> + enum range_qos_req_type type)
> +{
> + s32 ret;
> +
> + switch (type) {
> + case RANGE_QOS_MIN:
> + ret = IS_ERR_OR_NULL(qos) ?
> + PM_QOS_MIN_PERF_DEFAULT_VALUE :
> + pm_qos_read_value(&qos->lower_bound);
> + break;
> + case RANGE_QOS_MAX:
> + ret = IS_ERR_OR_NULL(qos) ?
> + PM_QOS_MAX_PERF_DEFAULT_VALUE :
> + pm_qos_read_value(&qos->upper_bound);
> + break;
> + default:
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_read_value);
> +
> +/**
> + * perf_qos_add_request - Insert new performance QoS request into a given list.
> + * @qos: Constraints to update.
> + * @req: Preallocated request object.
> + * @type: Request type.
> + * @value: Request value.
> + *
> + * Insert a new entry into the @qos list of requests, recompute the effective
> + * QoS constraint value for that list and initialize the @req object. The
> + * caller needs to save that object for later use in updates and removal.
> + *
> + * Return 1 if the effective constraint value has changed, 0 if the effective
> + * constraint value has not changed, or a negative error code on failures.
> + */
> +int perf_qos_add_request(struct range_constraints *qos,
> + struct range_qos_request *req,
> + enum range_qos_req_type type, s32 value)
> +{
> + int ret;
> +
> + if (IS_ERR_OR_NULL(qos) || !req || perf_qos_value_invalid(value))
> + return -EINVAL;
> +
> + if (WARN(perf_qos_request_active(req),
> + "%s() called for active request\n", __func__))
> + return -EINVAL;
> +
> + req->qos = qos;
> + req->type = type;
> + ret = perf_qos_apply(req, PM_QOS_ADD_REQ, value);
> + if (ret < 0) {
> + req->qos = NULL;
> + req->type = 0;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_add_request);
> +
> +/**
> + * perf_qos_update_request - Modify existing performance QoS request.
> + * @req: Request to modify.
> + * @new_value: New request value.
> + *
> + * Update an existing performance QoS request along with the effective
> + * constraint value for the list of requests it belongs to.
> + *
> + * Return 1 if the effective constraint value has changed, 0 if the effective
> + * constraint value has not changed, or a negative error code on failures.
> + */
> +int perf_qos_update_request(struct range_qos_request *req, s32 new_value)
> +{
> + if (!req || perf_qos_value_invalid(new_value))
> + return -EINVAL;
> +
> + if (WARN(!perf_qos_request_active(req),
> + "%s() called for unknown object\n", __func__))
> + return -EINVAL;
> +
> + if (req->pnode.prio == new_value)
> + return 0;
> +
> + return perf_qos_apply(req, PM_QOS_UPDATE_REQ, new_value);
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_update_request);
> +
> +/**
> + * perf_qos_remove_request - Remove performance QoS request from its list.
> + * @req: Request to remove.
> + *
> + * Remove the given performance QoS request from the list of
> + * constraints it belongs to and recompute the effective constraint
> + * value for that list.
> + *
> + * Return 1 if the effective constraint value has changed, 0 if the effective
> + * constraint value has not changed, or a negative error code on failures.
> + */
> +int perf_qos_remove_request(struct range_qos_request *req)
> +{
> + int ret;
> +
> + if (!req)
> + return -EINVAL;
> +
> + if (WARN(!perf_qos_request_active(req),
> + "%s() called for unknown object\n", __func__))
> + return -EINVAL;
> +
> + ret = perf_qos_apply(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
> + req->qos = NULL;
> + req->type = 0;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_remove_request);
> +
> +/**
> + * perf_qos_add_notifier - Add performance QoS change notifier.
> + * @qos: List of requests to add the notifier to.
> + * @type: Request type.
> + * @notifier: Notifier block to add.
> + */

Before there are any users of this, it is not clear whether or not
notifiers will be useful.

I'd add them later, when necessary.

> +int perf_qos_add_notifier(struct range_constraints *qos,
> + enum range_qos_req_type type,
> + struct notifier_block *notifier)
> +{
> + int ret;
> +
> + if (IS_ERR_OR_NULL(qos) || !notifier)
> + return -EINVAL;
> +
> + switch (type) {
> + case RANGE_QOS_MIN:
> + ret = blocking_notifier_chain_register(qos->lower_boundnotifiers,
> + notifier);
> + break;
> + case RANGE_QOS_MAX:
> + ret = blocking_notifier_chain_register(qos->upper_boundnotifiers,
> + notifier);
> + break;
> + default:
> + WARN_ON(1);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_add_notifier);
> +
> +/**
> + * perf_qos_remove_notifier - Remove performance QoS change notifier.
> + * @qos: List of requests to remove the notifier from.
> + * @type: Request type.
> + * @notifier: Notifier block to remove.
> + */
> +int perf_qos_remove_notifier(struct range_constraints *qos,
> + enum range_qos_req_type type,
> + struct notifier_block *notifier)
> +{
> + int ret;
> +
> + if (IS_ERR_OR_NULL(qos) || !notifier)
> + return -EINVAL;
> +
> + switch (type) {
> + case RANGE_QOS_MIN:
> + ret = blocking_notifier_chain_unregister(qos->lower_bound.notifiers,
> + notifier);
> + break;
> + case RANGE_QOS_MAX:
> + ret = blocking_notifier_chain_unregister(qos->upper_bound.notifiers,
> + notifier);
> + break;
> + default:
> + WARN_ON(1);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_remove_notifier);
> --

2024-02-14 10:44:07

by Daniel Lezcano

[permalink] [raw]

Subject: Re: [PATCH v1 2/2] PM: QoS: Add a performance QoS

Hi Rafael,

On 12/02/2024 18:30, Rafael J. Wysocki wrote:
> On Wed, Dec 13, 2023 at 6:58 PM Daniel Lezcano
> <[email protected]> wrote:
>>

[ ... ]

>
> I sent a comment on the first patch in the series and please note that
> 0-day has reported build issues with this one. I guess it needs to be
> refreshed.
>
> At the general level, I think that it would be good to add at least
> one user of this framework along with it, as a matter of illustration
> how it is going to (or at least how it is intended to) be used. For
> example, what is going to happen when user space updates one of the
> limits via sysfs for a given device?

I think Caleb is doing a port of the QMI TMD series he posted to the
performance QoS.

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2024-02-15 10:03:21

by Dhruva Gole

[permalink] [raw]

Subject: Re: [PATCH v1 2/2] PM: QoS: Add a performance QoS

Hi,

On Dec 13, 2023 at 18:58:18 +0100, Daniel Lezcano wrote:
> Currently cpufreq and devfreq are using the freq QoS to aggregate the
> requests for frequency ranges.
>
> However, there are new devices wanting to act not on a frequency range
> but on a performance index range. Those need also to export to
> userspace the knob to act on their performance limits.
>
> This change provides a performance limiter QoS based on a minimum /
> maximum performance values. At init time, the limits of the interval
> are 0 / 1024. It is up to the backend to convert the 1024 to the
> maximum performance state. So if the performance must be limited to
> 50%, it should set to maximum limit to 512 where the backend will end
> up by converting (max performance index / 2). The same applies for the
> minimum. Obviously, the min can not be greater than the max.
>
> 1. With the example above, if there is a odd number like 5 for the
> number of performance indexes and we ask for 512 (so 50%), what would
> be the performance index computed? (5/2=2 or 5/2=3)? (I would say the
> minimum otherwise we end up with a performance limit greater than
> what we actually asked for).
>
> 2. The conversion from 1024 to a performance index will inevatibly
> end up to a state above or below the percentage given. Shall it be
> reflected in the value set? eg. We want to apply a performance limit
> to be 33% maximum. So it is, 1024 x 0.333333 = 314. If there are 20
> performance indexes, that will be (20 x 314) / 1024 = 6.13, so index
> 6. Shall we convert this index back to the requested performance
> limit to (6.13 x 1024) / 20 = 307 ? (So requested is 314 but it is
> actually 307).
>
> The end goal is to make the freq QoS and perf QoS to co-exist together
> in the next changes in the different backends. A change of one of the
> QoS impacts the other. For instance if there are 5 performance states
> and we set a performance limit to 80%, then the maximum state will 4.
>
> For the long term, when those can co-exist, then we can implement a
> cooling device based on the performance Qos which will be generic for
> all devices using this QoS. That will imply the CPUs, the GPUs and any
> devfreq devices. So devfreq and cpufreq cooling devices can be merged
> into a single performance cooling device which will be generic for all
> devices with a performance limit QoS.
>
> In a similar way, in the future, a power QoS could be added also and a
> power based cooling device. So any device with the energy model and a
> power capping feature can become a cooling device and the power
> computation part in the cooling devices will move to the back ends. We
> will end up with a generic power cooling device compatible with all
> power capable devices.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
> drivers/base/power/power.h | 2 +
> drivers/base/power/qos.c | 158 +++++++++++++++++++++++++-
> drivers/base/power/sysfs.c | 92 +++++++++++++++
> include/linux/cpufreq.h | 2 +
> include/linux/pm_qos.h | 42 +++++++
> kernel/power/qos.c | 225 +++++++++++++++++++++++++++++++++++++
> 6 files changed, 517 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index 922ed457db19..eb1a77a7a0f4 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
> @@ -78,6 +78,8 @@ extern int pm_qos_sysfs_add_flags(struct device *dev);
> extern void pm_qos_sysfs_remove_flags(struct device *dev);
> extern int pm_qos_sysfs_add_latency_tolerance(struct device *dev);
> extern void pm_qos_sysfs_remove_latency_tolerance(struct device *dev);
> +extern int pm_qos_sysfs_add_perf_limit(struct device *dev);
> +extern void pm_qos_sysfs_remove_perf_limit(struct device *dev);
> extern int dpm_sysfs_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid);
>
> #else /* CONFIG_PM */
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index ae0b9d2573ec..a71cff1f8048 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -128,6 +128,14 @@ s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
> ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE
> : freq_qos_read_value(&qos->freq, FREQ_QOS_MAX);
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MIN_PERF_DEFAULT_VALUE
> + : perf_qos_read_value(&qos->perf, RANGE_QOS_MIN);
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_PERF_DEFAULT_VALUE
> + : perf_qos_read_value(&qos->perf, RANGE_QOS_MAX);
> + break;
> default:
> WARN_ON(1);
> ret = 0;
> @@ -177,6 +185,10 @@ static int apply_constraint(struct dev_pm_qos_request *req,
> ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
> action, value);
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + case DEV_PM_QOS_MAX_PERF:
> + ret = perf_qos_apply(&req->data.perf, action, value);
> + break;
> default:
> ret = -EINVAL;
> }
> @@ -223,6 +235,20 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
> c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
> c->type = PM_QOS_MIN;
>
> + c = &qos->perf.lower_bound;
> + plist_head_init(&c->list);
> + c->target_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->default_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->no_constraint_value = PM_QOS_MIN_PERF_DEFAULT_VALUE;
> + c->type = PM_QOS_MAX;
> +
> + c = &qos->perf.upper_bound;
> + plist_head_init(&c->list);
> + c->target_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->default_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->no_constraint_value = PM_QOS_MAX_PERF_DEFAULT_VALUE;
> + c->type = PM_QOS_MIN;
> +
> freq_constraints_init(&qos->freq);
>
> INIT_LIST_HEAD(&qos->flags.list);
> @@ -299,6 +325,20 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
> memset(req, 0, sizeof(*req));
> }
>
> + c = &qos->perf.lower_bound;
> + plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
> + apply_constraint(req, PM_QOS_REMOVE_REQ,
> + PM_QOS_MIN_PERF_DEFAULT_VALUE);
> + memset(req, 0, sizeof(*req));
> + }
> +
> + c = &qos->perf.upper_bound;
> + plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
> + apply_constraint(req, PM_QOS_REMOVE_REQ,
> + PM_QOS_MAX_PERF_DEFAULT_VALUE);
> + memset(req, 0, sizeof(*req));
> + }
> +
> f = &qos->flags;
> list_for_each_entry_safe(req, tmp, &f->list, data.flr.node) {
> apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
> @@ -349,17 +389,32 @@ static int __dev_pm_qos_add_request(struct device *dev,
>
> req->dev = dev;
> req->type = type;
> - if (req->type == DEV_PM_QOS_MIN_FREQUENCY)
> +
> + switch (type) {
> + case DEV_PM_QOS_MIN_FREQUENCY:
> ret = freq_qos_add_request(&dev->power.qos->freq,
> &req->data.freq,
> FREQ_QOS_MIN, value);
> - else if (req->type == DEV_PM_QOS_MAX_FREQUENCY)
> + break;
> + case DEV_PM_QOS_MAX_FREQUENCY:
> ret = freq_qos_add_request(&dev->power.qos->freq,
> &req->data.freq,
> FREQ_QOS_MAX, value);
> - else
> + break;
> + case DEV_PM_QOS_MIN_PERF:
> + ret = perf_qos_add_request(&dev->power.qos->perf,
> + &req->data.perf,
> + RANGE_QOS_MIN, value);
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + ret = perf_qos_add_request(&dev->power.qos->perf,
> + &req->data.perf,
> + RANGE_QOS_MAX, value);
> + break;
> + default:
> ret = apply_constraint(req, PM_QOS_ADD_REQ, value);
> -
> + break;
> + }
> return ret;
> }
>
> @@ -427,6 +482,10 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
> case DEV_PM_QOS_MAX_FREQUENCY:
> curr_value = req->data.freq.pnode.prio;
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + case DEV_PM_QOS_MAX_PERF:
> + curr_value = req->data.perf.pnode.prio;
> + break;
> case DEV_PM_QOS_FLAGS:
> curr_value = req->data.flr.flags;
> break;
> @@ -674,6 +733,14 @@ static void __dev_pm_qos_drop_user_request(struct device *dev,
> req = dev->power.qos->flags_req;
> dev->power.qos->flags_req = NULL;
> break;
> + case DEV_PM_QOS_MIN_PERF:
> + req = dev->power.qos->perf_min_req;
> + dev->power.qos->perf_min_req = NULL;
> + break;
> + case DEV_PM_QOS_MAX_PERF:
> + req = dev->power.qos->perf_max_req;
> + dev->power.qos->perf_max_req = NULL;
> + break;
> default:
> WARN_ON(1);
> return;
> @@ -980,3 +1047,86 @@ void dev_pm_qos_hide_latency_tolerance(struct device *dev)
> pm_runtime_put(dev);
> }
> EXPORT_SYMBOL_GPL(dev_pm_qos_hide_latency_tolerance);
> +
> +int dev_pm_qos_expose_perf_limit(struct device *dev)
> +{
> + struct dev_pm_qos_request *req_min;
> + struct dev_pm_qos_request *req_max;
> + int ret;
> +
> + if (!device_is_registered(dev))
> + return -EINVAL;
> +
> + req_min = kzalloc(sizeof(*req_min), GFP_KERNEL);
> + if (!req_min)
> + return -ENOMEM;
> +
> + req_max = kzalloc(sizeof(*req_max), GFP_KERNEL);
> + if (!req_max) {
> + kfree(req_min);
> + return -ENOMEM;
> + }
> +

Oops, looks like we forgot to run checkpatch ;)

There are many more errors with these patches and I'd urge you to run
checkpatch and fix and re-spin.
Do keep me in CC from next rev.

> + ret = dev_pm_qos_add_request(dev, req_min, DEV_PM_QOS_MIN_PERF,
> + PM_QOS_MIN_PERF_DEFAULT_VALUE);
> + if (ret < 0) {
> + kfree(req_min);
> + kfree(req_max);
> + return ret;
> + }
> +
> + ret = dev_pm_qos_add_request(dev, req_max, DEV_PM_QOS_MAX_PERF,
> + PM_QOS_MAX_PERF_DEFAULT_VALUE);
> + if (ret < 0) {
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + return ret;
> + }
> +
> + mutex_lock(&dev_pm_qos_sysfs_mtx);
> +
> + mutex_lock(&dev_pm_qos_mtx);
> +
> + if (IS_ERR_OR_NULL(dev->power.qos))
> + ret = -ENODEV;
> + else if (dev->power.qos->perf_min_req || dev->power.qos->perf_max_req)
> + ret = -EEXIST;
> +
> + if (ret < 0) {
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> + mutex_unlock(&dev_pm_qos_mtx);
> + goto out;
> + }
> +
> + dev->power.qos->perf_min_req = req_min;
> + dev->power.qos->perf_max_req = req_max;
> +
> + mutex_unlock(&dev_pm_qos_mtx);
> +
> + ret = pm_qos_sysfs_add_perf_limit(dev);
> + if (ret) {
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> + }
> +out:
> + mutex_unlock(&dev_pm_qos_sysfs_mtx);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_qos_expose_perf_limit);
> +
> +void dev_pm_qos_hide_perf_limit(struct device *dev)
> +{
> + mutex_lock(&dev_pm_qos_sysfs_mtx);
> +
> + pm_qos_sysfs_remove_perf_limit(dev);
> +
> + mutex_lock(&dev_pm_qos_mtx);
> +
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MIN_PERF);
> + __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_MAX_PERF);
> +

whitespace.. ^^

> + mutex_unlock(&dev_pm_qos_mtx);
> +
> + mutex_unlock(&dev_pm_qos_sysfs_mtx);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_qos_hide_perf_limit);
> diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
> index a1474fb67db9..5a45191006c1 100644
> --- a/drivers/base/power/sysfs.c
> +++ b/drivers/base/power/sysfs.c
> @@ -317,6 +317,76 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,
>
> static DEVICE_ATTR_RW(pm_qos_no_power_off);
>
> +
> +static ssize_t pm_qos_perf_limit_min_max_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf, bool max)
> +{
> + s32 value = dev_pm_qos_read_value(dev, max ? DEV_PM_QOS_MAX_PERF :
> + DEV_PM_QOS_MIN_PERF);
> +
> + return sysfs_emit(buf, "%d\n", value);
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_max_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n, bool max)
> +{
> + int ret;

Your function return type is ssize_t, do you want to change the type of
ret too?

> + s32 min_value = dev_pm_qos_read_value(dev, DEV_PM_QOS_MIN_PERF);
> + s32 max_value = dev_pm_qos_read_value(dev, DEV_PM_QOS_MAX_PERF);
> + s32 new_value;
> +
> + if (kstrtoint(buf, 0, &new_value))
> + return -EINVAL;
> +
> + if (new_value < PM_QOS_MIN_PERF_DEFAULT_VALUE ||
> + new_value > PM_QOS_MAX_PERF_DEFAULT_VALUE)
> + return -EINVAL;
> +
> + if (max && (new_value < min_value))
> + return -EINVAL;
> +
> + if (!max && (new_value > max_value))
> + return -EINVAL;

No strong opinions, but might help debug better if you print why each
EINVAL was returned?

> +
> + ret = dev_pm_qos_update_request(max ? dev->power.qos->perf_max_req :
> + dev->power.qos->perf_min_req, new_value);
> +
> + return ret < 0 ? ret : n;
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return pm_qos_perf_limit_min_max_show(dev, attr, buf, false);
> +}
> +
> +static ssize_t pm_qos_perf_limit_min_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + return pm_qos_perf_limit_min_max_store(dev, attr, buf, n, false);
> +}
> +
> +static ssize_t pm_qos_perf_limit_max_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return pm_qos_perf_limit_min_max_show(dev, attr, buf, true);
> +}
> +
> +static ssize_t pm_qos_perf_limit_max_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + return pm_qos_perf_limit_min_max_store(dev, attr, buf, n, true);
> +}
> +
> +static DEVICE_ATTR_RW(pm_qos_perf_limit_min);
> +static DEVICE_ATTR_RW(pm_qos_perf_limit_max);
> +
[...]

--
Best regards,
Dhruva Gole <[email protected]>