[RFC 0/4] opp: Parse required-opp as dev_pm_qos_request

[RFC 0/4] opp: Parse required-opp as dev_pm_qos_request

[Leonard Crestez]

The "required-opps" property can be placed on any device and point to
any OPP table according to bindings doc but this is not fully
implemented. In practice it can only point from the opp table of a
device to the opp table of a power domain.

As part of my investingating QOS mechanisms I implemented support for
parsing "required-opps" into a DEV_PM_QOS_MIN_FREQUENCY
dev_pm_qos_request. Since OPPs can be shared between devices this only
works when OPP tables are unshared.

This would need to be called from a device probe function and any
suspend/resume handling (which likely means disabling the QOS requests)
would also be handled manually by each driver.

This is RFC mostly because I plan to use the "interconnect" framework
for device requests instead. In theory this could be used if you don't
care about implementing smart aggregation and just want to "set bus freq
to high".

Devfreq support for dev_pm_qos is here: https://patchwork.kernel.org/patch/11078475/

Leonard Crestez (4):
  opp: Drop const from opp_device struct device
  opp: Add dev_pm_opp_table_get_device
  opp: Add dev_pm_parse_required_opp_as_qos
  PM / QoS: Add dev_pm_qos_get_curr_value

 drivers/base/power/qos.c | 59 +++++++++++++++++++++++++-----------
 drivers/opp/core.c       | 34 +++++++++++++++++++--
 drivers/opp/of.c         | 65 ++++++++++++++++++++++++++++++++++++++++
 drivers/opp/opp.h        |  4 +--
 include/linux/pm_opp.h   | 15 ++++++++++
 include/linux/pm_qos.h   |  1 +
 6 files changed, 157 insertions(+), 21 deletions(-)

-- 
2.17.1

[RFC 1/4] opp: Drop const from opp_device struct device

[Leonard Crestez]

This is required for fetching struct device from struct opp_table with
casts

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

---
Does "const" here have any particular significance?

 drivers/opp/core.c | 4 ++--
 drivers/opp/opp.h  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 3b7ffd0234e9..77814d3bc4e6 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -913,11 +913,11 @@ static void _remove_opp_dev(struct opp_device *opp_dev,
 	opp_debug_unregister(opp_dev, opp_table);
 	list_del(&opp_dev->node);
 	kfree(opp_dev);
 }
 
-static struct opp_device *_add_opp_dev_unlocked(const struct device *dev,
+static struct opp_device *_add_opp_dev_unlocked(struct device *dev,
 						struct opp_table *opp_table)
 {
 	struct opp_device *opp_dev;
 
 	opp_dev = kzalloc(sizeof(*opp_dev), GFP_KERNEL);
@@ -933,11 +933,11 @@ static struct opp_device *_add_opp_dev_unlocked(const struct device *dev,
 	opp_debug_register(opp_dev, opp_table);
 
 	return opp_dev;
 }
 
-struct opp_device *_add_opp_dev(const struct device *dev,
+struct opp_device *_add_opp_dev(struct device *dev,
 				struct opp_table *opp_table)
 {
 	struct opp_device *opp_dev;
 
 	mutex_lock(&opp_table->lock);
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 01a500e2c40a..5a7ddd55bd84 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -103,11 +103,11 @@ struct dev_pm_opp {
  * This is an internal data structure maintaining the devices that are managed
  * by 'struct opp_table'.
  */
 struct opp_device {
 	struct list_head node;
-	const struct device *dev;
+	struct device *dev;
 
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 #endif
 };
@@ -207,11 +207,11 @@ struct opp_table {
 void dev_pm_opp_get(struct dev_pm_opp *opp);
 void _opp_remove_all_static(struct opp_table *opp_table);
 void _get_opp_table_kref(struct opp_table *opp_table);
 int _get_opp_count(struct opp_table *opp_table);
 struct opp_table *_find_opp_table(struct device *dev);
-struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table);
+struct opp_device *_add_opp_dev(struct device *dev, struct opp_table *opp_table);
 void _dev_pm_opp_find_and_remove_table(struct device *dev);
 struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table);
 void _opp_free(struct dev_pm_opp *opp);
 int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table, bool rate_not_available);
 int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic);
-- 
2.17.1

[RFC 2/4] opp: Add dev_pm_opp_table_get_device

[Leonard Crestez]

This API is the opposite of dev_pm_opp_get_opp_table. OPP tables can be
shared between devices but this ambiguity can be handled by returning an
error if opp table is not exclusive.

This can used for fetching the device pointed to by "required-opps".

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/opp/core.c     | 30 ++++++++++++++++++++++++++++++
 include/linux/pm_opp.h |  6 ++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 77814d3bc4e6..b8bbabbbe44a 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -87,10 +87,40 @@ struct opp_table *_find_opp_table(struct device *dev)
 	mutex_unlock(&opp_table_lock);
 
 	return opp_table;
 }
 
+/**
+ * _find_opp_table() - find device struct using opp_table pointer
+ *
+ * OPP table must be exclusive: not be shared between devices.
+ */
+struct device *dev_pm_opp_table_get_device(struct opp_table *opp_table)
+{
+	struct opp_device *opp_dev;
+	struct device *dev = ERR_PTR(-EINVAL);
+	int opp_dev_cnt = 0;
+
+	mutex_lock(&opp_table->lock);
+
+	/* OPP table must not be shared: only one device */
+	if (opp_table->shared_opp != OPP_TABLE_ACCESS_EXCLUSIVE)
+		goto out;
+	list_for_each_entry(opp_dev, &opp_table->dev_list, node)
+		opp_dev_cnt++;
+	if (opp_dev_cnt != 1)
+		goto out;
+
+	opp_dev = list_first_entry(&opp_table->dev_list, struct opp_device, node);
+	dev = opp_dev->dev;
+
+out:
+	mutex_unlock(&opp_table->lock);
+	return dev;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_table_get_device);
+
 /**
  * dev_pm_opp_get_voltage() - Gets the voltage corresponding to an opp
  * @opp:	opp for which voltage has to be returned for
  *
  * Return: voltage in micro volt corresponding to the opp, else
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index b8197ab014f2..a4db3f42d787 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -76,10 +76,11 @@ struct dev_pm_set_opp_data {
 #if defined(CONFIG_PM_OPP)
 
 struct opp_table *dev_pm_opp_get_opp_table(struct device *dev);
 struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *dev, int index);
 void dev_pm_opp_put_opp_table(struct opp_table *opp_table);
+struct device *dev_pm_opp_table_get_device(struct opp_table *opp_table);
 
 unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
 
 unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp);
 
@@ -149,10 +150,15 @@ static inline struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *
 	return ERR_PTR(-ENOTSUPP);
 }
 
 static inline void dev_pm_opp_put_opp_table(struct opp_table *opp_table) {}
 
+struct struct device *dev_pm_opp_table_get_device(struct opp_table *opp_table)
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+
 static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
 {
 	return 0;
 }
 
-- 
2.17.1

[RFC 3/4] opp: Add dev_pm_parse_required_opp_as_qos

[Leonard Crestez]

The "required-opps" property can be placed on any device and point to
any OPP table according to bindings doc but this is not fully
implemented. In practice it can only point from the opp table of a
device to the opp table of a power domain.

Add support for parsing "required-opps" as a DEV_PM_QOS_MIN_FREQUENCY
request.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/opp/of.c       | 65 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_opp.h |  9 ++++++
 2 files changed, 74 insertions(+)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 1813f5ad5fa2..a086bb120fec 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -16,10 +16,11 @@
 #include <linux/of_device.h>
 #include <linux/pm_domain.h>
 #include <linux/slab.h>
 #include <linux/export.h>
 #include <linux/energy_model.h>
+#include <linux/pm_qos.h>
 
 #include "opp.h"
 
 /*
  * Returns opp descriptor node for a device node, caller must
@@ -1119,5 +1120,69 @@ void dev_pm_opp_of_register_em(struct cpumask *cpus)
 		return;
 
 	em_register_perf_domain(cpus, nr_opp, &em_cb);
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_register_em);
+
+int dev_pm_parse_required_opp_as_qos(struct device_node *node,
+				     struct dev_pm_qos_request *req)
+{
+	struct device_node *opp_node;
+	struct opp_table *opp_table;
+	struct dev_pm_opp *opp;
+	struct device *req_dev;
+	unsigned long req_val;
+	int ret;
+
+	mutex_lock(&opp_table_lock);
+
+	opp_node = of_parse_required_opp(node, 0);
+	if (!opp_node) {
+		ret = -ENOENT;
+		goto unlock_opp_tables;
+	}
+
+	opp_table = _find_table_of_opp_np(opp_node);
+	if (IS_ERR(opp_table)) {
+		ret = -EPROBE_DEFER;
+		goto put_opp_node;
+	}
+
+	opp = _find_opp_of_np(opp_table, opp_node);
+	if (!opp) {
+		ret = -ENOENT;
+		goto put_opp_table;
+	}
+
+	req_dev = dev_pm_opp_table_get_device(opp_table);
+	if (IS_ERR(req_dev)) {
+		pr_err("%pOF: failed to fetch device for table %pOF\n",
+				node, opp_table->np);
+		ret = PTR_ERR(req_dev);
+		goto put_opp;
+	}
+
+	req_val = dev_pm_opp_get_freq(opp);
+	if (req_val > S32_MAX) {
+		ret = -ERANGE;
+		goto put_opp;
+	}
+
+	mutex_unlock(&opp_table_lock);
+
+	ret = dev_pm_qos_add_request(req_dev, req,
+			DEV_PM_QOS_MIN_FREQUENCY, req_val);
+	if (ret < 0)
+		pr_err("%pOF: failed to add dev_pm_qos request: %d\n", node, ret);
+	ret = 0;
+
+put_opp:
+	dev_pm_opp_put(opp);
+put_opp_table:
+	dev_pm_opp_put_opp_table(opp_table);
+put_opp_node:
+	of_node_put(opp_node);
+unlock_opp_tables:
+	mutex_unlock(&opp_table_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_parse_required_opp_as_qos);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index a4db3f42d787..949c35e8f5ae 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -17,10 +17,11 @@
 struct clk;
 struct regulator;
 struct dev_pm_opp;
 struct device;
 struct opp_table;
+struct dev_pm_qos_request;
 
 enum dev_pm_opp_event {
 	OPP_EVENT_ADD, OPP_EVENT_REMOVE, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
 };
 
@@ -352,10 +353,12 @@ void dev_pm_opp_of_cpumask_remove_table(const struct cpumask *cpumask);
 int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
 struct device_node *dev_pm_opp_of_get_opp_desc_node(struct device *dev);
 struct device_node *dev_pm_opp_get_of_node(struct dev_pm_opp *opp);
 int of_get_required_opp_performance_state(struct device_node *np, int index);
 void dev_pm_opp_of_register_em(struct cpumask *cpus);
+int dev_pm_parse_required_opp_as_qos(struct device_node *node,
+				     struct dev_pm_qos_request *req);
 #else
 static inline int dev_pm_opp_of_add_table(struct device *dev)
 {
 	return -ENOTSUPP;
 }
@@ -399,8 +402,14 @@ static inline void dev_pm_opp_of_register_em(struct cpumask *cpus)
 
 static inline int of_get_required_opp_performance_state(struct device_node *np, int index)
 {
 	return -ENOTSUPP;
 }
+
+int dev_pm_parse_required_opp_as_qos(struct device_node *node,
+				     struct dev_pm_qos_request *req)
+{
+	return -ENOTSUPP;
+}
 #endif
 
 #endif		/* __LINUX_OPP_H__ */
-- 
2.17.1

[RFC 4/4] PM / QoS: Add dev_pm_qos_get_curr_value

[Leonard Crestez]

Add a new API for fetching the value set with dev_pm_qos_add_request by
refactoring __dev_pm_qos_update_request.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/base/power/qos.c | 59 ++++++++++++++++++++++++++++------------
 include/linux/pm_qos.h   |  1 +
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 6c90fd7e2ff8..f171a7137c5d 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -402,10 +402,49 @@ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
 	mutex_unlock(&dev_pm_qos_mtx);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dev_pm_qos_add_request);
 
+/**
+ * __dev_pm_qos_get_curr_value - Modify an existing device PM QoS request.
+ * @req : PM QoS request to query
+ */
+static int __dev_pm_qos_get_curr_value(struct dev_pm_qos_request *req,
+				       s32 *curr_value)
+{
+	if (WARN(!dev_pm_qos_request_active(req),
+		 "%s() called for unknown object\n", __func__))
+		return -EINVAL;
+
+	switch(req->type) {
+	case DEV_PM_QOS_RESUME_LATENCY:
+	case DEV_PM_QOS_LATENCY_TOLERANCE:
+	case DEV_PM_QOS_MIN_FREQUENCY:
+	case DEV_PM_QOS_MAX_FREQUENCY:
+		*curr_value = req->data.pnode.prio;
+		return 0;
+	case DEV_PM_QOS_FLAGS:
+		*curr_value = req->data.flr.flags;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+int dev_pm_qos_get_curr_value(struct dev_pm_qos_request *req,
+				     s32 *curr_value)
+{
+	int ret;
+
+	mutex_lock(&dev_pm_qos_mtx);
+	ret = __dev_pm_qos_get_curr_value(req, curr_value);
+	mutex_unlock(&dev_pm_qos_mtx);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_qos_get_curr_value);
+
 /**
  * __dev_pm_qos_update_request - Modify an existing device PM QoS request.
  * @req : PM QoS request to modify.
  * @new_value: New value to request.
  */
@@ -416,31 +455,17 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
 	int ret = 0;
 
 	if (!req) /*guard against callers passing in null */
 		return -EINVAL;
 
-	if (WARN(!dev_pm_qos_request_active(req),
-		 "%s() called for unknown object\n", __func__))
-		return -EINVAL;
+	ret = __dev_pm_qos_get_curr_value(req, &curr_value);
+	if (ret)
+		return ret;
 
 	if (IS_ERR_OR_NULL(req->dev->power.qos))
 		return -ENODEV;
 
-	switch(req->type) {
-	case DEV_PM_QOS_RESUME_LATENCY:
-	case DEV_PM_QOS_LATENCY_TOLERANCE:
-	case DEV_PM_QOS_MIN_FREQUENCY:
-	case DEV_PM_QOS_MAX_FREQUENCY:
-		curr_value = req->data.pnode.prio;
-		break;
-	case DEV_PM_QOS_FLAGS:
-		curr_value = req->data.flr.flags;
-		break;
-	default:
-		return -EINVAL;
-	}
-
 	trace_dev_pm_qos_update_request(dev_name(req->dev), req->type,
 					new_value);
 	if (curr_value != new_value)
 		ret = apply_constraint(req, PM_QOS_UPDATE_REQ, new_value);
 
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 2aebbc5b9950..ae42e58d02bb 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -149,10 +149,11 @@ enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev, s32 mask);
 enum pm_qos_flags_status dev_pm_qos_flags(struct device *dev, s32 mask);
 s32 __dev_pm_qos_resume_latency(struct device *dev);
 s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type);
 int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
 			   enum dev_pm_qos_req_type type, s32 value);
+int dev_pm_qos_get_curr_value(struct dev_pm_qos_request *req, s32 *value);
 int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
 int dev_pm_qos_remove_request(struct dev_pm_qos_request *req);
 int dev_pm_qos_add_notifier(struct device *dev,
 			    struct notifier_block *notifier,
 			    enum dev_pm_qos_req_type type);
-- 
2.17.1

Re: [RFC 0/4] opp: Parse required-opp as dev_pm_qos_request

[Viresh Kumar]

On 06-08-19, 14:12, Leonard Crestez wrote:
> The "required-opps" property can be placed on any device and point to
> any OPP table according to bindings doc but this is not fully
> implemented. In practice it can only point from the opp table of a
> device to the opp table of a power domain.
> 
> As part of my investingating QOS mechanisms I implemented support for
> parsing "required-opps" into a DEV_PM_QOS_MIN_FREQUENCY
> dev_pm_qos_request. Since OPPs can be shared between devices this only
> works when OPP tables are unshared.
> 
> This would need to be called from a device probe function and any
> suspend/resume handling (which likely means disabling the QOS requests)
> would also be handled manually by each driver.
> 
> This is RFC mostly because I plan to use the "interconnect" framework
> for device requests instead. In theory this could be used if you don't
> care about implementing smart aggregation and just want to "set bus freq
> to high".
> 
> Devfreq support for dev_pm_qos is here: https://patchwork.kernel.org/patch/11078475/

Some work is going on in related field. Please have a look at this as well.

https://lore.kernel.org/lkml/20190724014222.110767-1-saravanak@google.com

-- 
viresh

Re: [RFC 0/4] opp: Parse required-opp as dev_pm_qos_request

[Leonard Crestez]

On 20.08.2019 09:52, Viresh Kumar wrote:
> On 06-08-19, 14:12, Leonard Crestez wrote:
>> The "required-opps" property can be placed on any device and point to
>> any OPP table according to bindings doc but this is not fully
>> implemented. In practice it can only point from the opp table of a
>> device to the opp table of a power domain.
>>
>> As part of my investingating QOS mechanisms I implemented support for
>> parsing "required-opps" into a DEV_PM_QOS_MIN_FREQUENCY
>> dev_pm_qos_request. Since OPPs can be shared between devices this only
>> works when OPP tables are unshared.
>>
>> This would need to be called from a device probe function and any
>> suspend/resume handling (which likely means disabling the QOS requests)
>> would also be handled manually by each driver.
>>
>> This is RFC mostly because I plan to use the "interconnect" framework
>> for device requests instead. In theory this could be used if you don't
>> care about implementing smart aggregation and just want to "set bus freq
>> to high".
>>
>> Devfreq support for dev_pm_qos is here: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F11078475%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C09dabcdb17434862317508d7253aeac8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637018807295295723&sdata=vXvTIowhuqDkTxVZMHq%2BQxrKuqYv7n%2FU01ZDA7fdB0c%3D&reserved=0
> 
> Some work is going on in related field. Please have a look at this as well.

I noticed that series but other than touching "required-opp" there is 
little in common. It seems to be mostly an expansion of the passive 
governor.

My series doesn't even depend on devfreq; in theory you could even use 
required-opp = <&opp_1200mhz> on a cpu device.

--
Regards,
Leonard

Re: [RFC 0/4] opp: Parse required-opp as dev_pm_qos_request

[Viresh Kumar]

On 20-08-19, 09:02, Leonard Crestez wrote:
> On 20.08.2019 09:52, Viresh Kumar wrote:
> > On 06-08-19, 14:12, Leonard Crestez wrote:
> >> The "required-opps" property can be placed on any device and point to
> >> any OPP table according to bindings doc but this is not fully
> >> implemented. In practice it can only point from the opp table of a
> >> device to the opp table of a power domain.
> >>
> >> As part of my investingating QOS mechanisms I implemented support for
> >> parsing "required-opps" into a DEV_PM_QOS_MIN_FREQUENCY
> >> dev_pm_qos_request. Since OPPs can be shared between devices this only
> >> works when OPP tables are unshared.
> >>
> >> This would need to be called from a device probe function and any
> >> suspend/resume handling (which likely means disabling the QOS requests)
> >> would also be handled manually by each driver.
> >>
> >> This is RFC mostly because I plan to use the "interconnect" framework
> >> for device requests instead. In theory this could be used if you don't
> >> care about implementing smart aggregation and just want to "set bus freq
> >> to high".
> >>
> >> Devfreq support for dev_pm_qos is here: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F11078475%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C09dabcdb17434862317508d7253aeac8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637018807295295723&sdata=vXvTIowhuqDkTxVZMHq%2BQxrKuqYv7n%2FU01ZDA7fdB0c%3D&reserved=0
> > 
> > Some work is going on in related field. Please have a look at this as well.
> 
> I noticed that series but other than touching "required-opp" there is 
> little in common. It seems to be mostly an expansion of the passive 
> governor.
> 
> My series doesn't even depend on devfreq; in theory you could even use 
> required-opp = <&opp_1200mhz> on a cpu device.

What is the exact use case you are targeting here or the problem you are trying
to solve ?

-- 
viresh

Re: [RFC 0/4] opp: Parse required-opp as dev_pm_qos_request

[Leonard Crestez]

On 20.08.2019 12:22, Viresh Kumar wrote:
> On 20-08-19, 09:02, Leonard Crestez wrote:
>> On 20.08.2019 09:52, Viresh Kumar wrote:
>>> On 06-08-19, 14:12, Leonard Crestez wrote:
>>>> The "required-opps" property can be placed on any device and point to
>>>> any OPP table according to bindings doc but this is not fully
>>>> implemented. In practice it can only point from the opp table of a
>>>> device to the opp table of a power domain.
>>>>
>>>> As part of my investingating QOS mechanisms I implemented support for
>>>> parsing "required-opps" into a DEV_PM_QOS_MIN_FREQUENCY
>>>> dev_pm_qos_request. Since OPPs can be shared between devices this only
>>>> works when OPP tables are unshared.
>>>>
>>>> This would need to be called from a device probe function and any
>>>> suspend/resume handling (which likely means disabling the QOS requests)
>>>> would also be handled manually by each driver.
>>>>
>>>> This is RFC mostly because I plan to use the "interconnect" framework
>>>> for device requests instead. In theory this could be used if you don't
>>>> care about implementing smart aggregation and just want to "set bus freq
>>>> to high".
>>>>
>>>> Devfreq support for dev_pm_qos is here: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F11078475%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C9ff357888cba49c522ce08d7254fe2c4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637018897344276009&sdata=NV2Xnop9%2BplnKdIqrMCHF05xpt9y651ed%2BhwFK8gEKI%3D&reserved=0
>>>
>>> Some work is going on in related field. Please have a look at this as well.
>>
>> I noticed that series but other than touching "required-opp" there is
>> little in common. It seems to be mostly an expansion of the passive
>> governor.
>>
>> My series doesn't even depend on devfreq; in theory you could even use
>> required-opp = <&opp_1200mhz> on a cpu device.
> 
> What is the exact use case you are targeting here or the problem you are trying
> to solve ?

My exact use case is that devices (such as display, gpu, audio etc) need 
DRAM to run at a minimum frequency. This is currently done in imx vendor 
tree with a custom API but I'm trying to upstream via devfreq:

https://patchwork.kernel.org/cover/11104113/

The "required-opp" documentation looked like it would be a fit but 
apparently it requires all requesting devices to have OPPs and the 
target to be a power domain? This seemed very restrictive so this series 
implements a different way to handle required-opp, via dev_pm_qos.

The interconnect subsystem has additional capabilities (scaling along a 
path) so I plan to use that instead. You can treat this series as a 
"curiosity".

--
Regards,
Leonard

Re: [RFC 0/4] opp: Parse required-opp as dev_pm_qos_request

[Viresh Kumar]

On 20-08-19, 15:48, Leonard Crestez wrote:
> The interconnect subsystem has additional capabilities (scaling along a 
> path) so I plan to use that instead. You can treat this series as a 
> "curiosity".

So I can just ignore it ;)

-- 
viresh