[PATCH 0/9] PM: domains/opp/arm_scmi: Extend performance scaling support

[PATCH 0/9] PM: domains/opp/arm_scmi: Extend performance scaling support

[Ulf Hansson]

Consumer drivers for devices that are attached to the SCMI performance domain,
are currently not able to use the OPP library to scale performance. This series
is enabling the support for this.

The OPPs for SCMI performance domain are encoded in the FW, rather than being
described through DT. To better support this, some changes have also been done
to the OPP library and for PM domains in general. More details are available in
the commit messages.

I have based the series on the scmi tree and the for-next/scmi/updates branch,
so the OPP and PM domain changes would require acks from Viresh and Rafael to
be allow to be picked up and funneled through the scmi tree. Or, let's just
discuss what works best for us in this regards.

Note that, I am running this on the Qemu virt platform with Optee running an
SCMI server. If you want some more details about my test setup, I can certainly
share this with you, just let me know.

Looking forward to your feedback!

Kind regards
Ulf Hansson


Ulf Hansson (9):
  PM: domains: Introduce dev_pm_domain_set_performance_state()
  PM: domains: Implement the ->set_performance_state() callback for
    genpd
  OPP: Add dev_pm_opp_add_dynamic() to allow more flexibility
  OPP: Extend dev_pm_opp_data with a level
  OPP: Switch to use dev_pm_domain_set_performance_state()
  OPP: Extend support for the opp-level beyond required-opps
  firmware: arm_scmi: Simplify error path in scmi_dvfs_device_opps_add()
  firmware: arm_scmi: Specify the performance level when adding an OPP
  firmware: arm_scmi: Add generic OPP support to the SCMI performance
    domain

 drivers/base/power/common.c             | 21 ++++++++++
 drivers/base/power/domain.c             | 33 +++++++++------
 drivers/firmware/arm_scmi/perf.c        | 22 +++++-----
 drivers/opp/core.c                      | 54 +++++++++++++++++++------
 drivers/opp/of.c                        | 10 +++--
 drivers/opp/opp.h                       |  2 +-
 drivers/pmdomain/arm/scmi_perf_domain.c | 34 ++++++++++++++++
 include/linux/pm.h                      |  2 +
 include/linux/pm_domain.h               |  6 +++
 include/linux/pm_opp.h                  | 31 ++++++++++++--
 10 files changed, 168 insertions(+), 47 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/9] PM: domains: Introduce dev_pm_domain_set_performance_state()

[Ulf Hansson]

The generic PM domain is currently the only PM domain variant that supports
performance scaling. To allow performance scaling to be supported through a
common interface, let's add an optional callback ->set_performance_state(),
in the struct dev_pm_domain.

Moreover, let's add a function, dev_pm_domain_set_performance_state(), that
may be called by consumers to request a new performance state for a device
through its PM domain.

Note that, in most cases it's preferred that a consumer use the OPP library
to request a new performance state for its device. Although, this requires
some additional changes to be supported, which are being implemented from
subsequent changes.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/common.c | 21 +++++++++++++++++++++
 include/linux/pm.h          |  2 ++
 include/linux/pm_domain.h   |  6 ++++++
 3 files changed, 29 insertions(+)

diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index 72115917e0bd..44ec20918a4d 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -228,3 +228,24 @@ void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd)
 	device_pm_check_callbacks(dev);
 }
 EXPORT_SYMBOL_GPL(dev_pm_domain_set);
+
+/**
+ * dev_pm_domain_set_performance_state - Request a new performance state.
+ * @dev: The device to make the request for.
+ * @state: Target performance state for the device.
+ *
+ * This function should be called when a new performance state needs to be
+ * requested for a device that is attached to a PM domain. Note that, the
+ * support for performance scaling for PM domains is optional.
+ *
+ * Returns 0 on success and when performance scaling isn't supported, negative
+ * error code on failure.
+ */
+int dev_pm_domain_set_performance_state(struct device *dev, unsigned int state)
+{
+	if (dev->pm_domain && dev->pm_domain->set_performance_state)
+		return dev->pm_domain->set_performance_state(dev, state);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dev_pm_domain_set_performance_state);
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 1400c37b29c7..4c9f571609c8 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -719,6 +719,7 @@ extern void dev_pm_put_subsys_data(struct device *dev);
  * @activate: Called before executing probe routines for bus types and drivers.
  * @sync: Called after successful driver probe.
  * @dismiss: Called after unsuccessful driver probe and after driver removal.
+ * @set_performance_state: Called to request a new performance state.
  *
  * Power domains provide callbacks that are executed during system suspend,
  * hibernation, system resume and during runtime PM transitions instead of
@@ -731,6 +732,7 @@ struct dev_pm_domain {
 	int (*activate)(struct device *dev);
 	void (*sync)(struct device *dev);
 	void (*dismiss)(struct device *dev);
+	int (*set_performance_state)(struct device *dev, unsigned int state);
 };
 
 /*
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 05ad8cefdff1..34663d0d5c55 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -435,6 +435,7 @@ struct device *dev_pm_domain_attach_by_name(struct device *dev,
 void dev_pm_domain_detach(struct device *dev, bool power_off);
 int dev_pm_domain_start(struct device *dev);
 void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
+int dev_pm_domain_set_performance_state(struct device *dev, unsigned int state);
 #else
 static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
 {
@@ -457,6 +458,11 @@ static inline int dev_pm_domain_start(struct device *dev)
 }
 static inline void dev_pm_domain_set(struct device *dev,
 				     struct dev_pm_domain *pd) {}
+static inline int dev_pm_domain_set_performance_state(struct device *dev,
+						      unsigned int state)
+{
+	return 0;
+}
 #endif
 
 #endif /* _LINUX_PM_DOMAIN_H */
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/9] PM: domains: Implement the ->set_performance_state() callback for genpd

[Ulf Hansson]

To enable generic support for performance scaling for PM domains, let's
implement the ->set_performance_state() callback for genpd.

Beyond this change, users of the corresponding genpd specific API,
dev_pm_genpd_set_performance_state() are encouraged to switch to the common
dev_pm_domain_set_performance_state() API.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index c74edf80417f..da1777e39eaa 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -420,6 +420,25 @@ static void genpd_restore_performance_state(struct device *dev,
 		genpd_set_performance_state(dev, state);
 }
 
+static int genpd_dev_pm_set_performance_state(struct device *dev,
+					      unsigned int state)
+{
+	struct generic_pm_domain *genpd = dev_to_genpd(dev);
+	int ret = 0;
+
+	genpd_lock(genpd);
+	if (pm_runtime_suspended(dev)) {
+		dev_gpd_data(dev)->rpm_pstate = state;
+	} else {
+		ret = genpd_set_performance_state(dev, state);
+		if (!ret)
+			dev_gpd_data(dev)->rpm_pstate = 0;
+	}
+	genpd_unlock(genpd);
+
+	return ret;
+}
+
 /**
  * dev_pm_genpd_set_performance_state- Set performance state of device's power
  * domain.
@@ -438,7 +457,6 @@ static void genpd_restore_performance_state(struct device *dev,
 int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state)
 {
 	struct generic_pm_domain *genpd;
-	int ret = 0;
 
 	genpd = dev_to_genpd_safe(dev);
 	if (!genpd)
@@ -448,17 +466,7 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state)
 		     !dev->power.subsys_data->domain_data))
 		return -EINVAL;
 
-	genpd_lock(genpd);
-	if (pm_runtime_suspended(dev)) {
-		dev_gpd_data(dev)->rpm_pstate = state;
-	} else {
-		ret = genpd_set_performance_state(dev, state);
-		if (!ret)
-			dev_gpd_data(dev)->rpm_pstate = 0;
-	}
-	genpd_unlock(genpd);
-
-	return ret;
+	return genpd_dev_pm_set_performance_state(dev, state);
 }
 EXPORT_SYMBOL_GPL(dev_pm_genpd_set_performance_state);
 
@@ -2080,6 +2088,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
 	genpd->domain.ops.restore_noirq = genpd_restore_noirq;
 	genpd->domain.ops.complete = genpd_complete;
 	genpd->domain.start = genpd_dev_pm_start;
+	genpd->domain.set_performance_state = genpd_dev_pm_set_performance_state;
 
 	if (genpd->flags & GENPD_FLAG_PM_CLK) {
 		genpd->dev_ops.stop = pm_clk_suspend;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/9] OPP: Add dev_pm_opp_add_dynamic() to allow more flexibility

[Ulf Hansson]

The dev_pm_opp_add() API is limited to add dynamic OPPs with a frequency
and a voltage level. To enable more flexibility, let's add a new API,
dev_pm_opp_add_dynamic() that's takes a struct dev_pm_opp_data* instead of
a list of in-parameters.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/opp/core.c     | 22 ++++++++++------------
 drivers/opp/of.c       | 10 ++++++----
 drivers/opp/opp.h      |  2 +-
 include/linux/pm_opp.h | 29 +++++++++++++++++++++++++----
 4 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 919cc53bc02e..54b6138e1189 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2002,8 +2002,7 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
  * _opp_add_v1() - Allocate a OPP based on v1 bindings.
  * @opp_table:	OPP table
  * @dev:	device for which we do this operation
- * @freq:	Frequency in Hz for this OPP
- * @u_volt:	Voltage in uVolts for this OPP
+ * @data:	The OPP data for the OPP to add
  * @dynamic:	Dynamically added OPPs.
  *
  * This function adds an opp definition to the opp table and returns status.
@@ -2021,10 +2020,10 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
  * -ENOMEM	Memory allocation failure
  */
 int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
-		unsigned long freq, long u_volt, bool dynamic)
+		struct dev_pm_opp_data *data, bool dynamic)
 {
 	struct dev_pm_opp *new_opp;
-	unsigned long tol;
+	unsigned long tol, u_volt = data->u_volt;
 	int ret;
 
 	if (!assert_single_clk(opp_table))
@@ -2035,7 +2034,7 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
 		return -ENOMEM;
 
 	/* populate the opp table */
-	new_opp->rates[0] = freq;
+	new_opp->rates[0] = data->freq;
 	tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
 	new_opp->supplies[0].u_volt = u_volt;
 	new_opp->supplies[0].u_volt_min = u_volt - tol;
@@ -2825,10 +2824,9 @@ int dev_pm_opp_xlate_performance_state(struct opp_table *src_table,
 }
 
 /**
- * dev_pm_opp_add()  - Add an OPP table from a table definitions
- * @dev:	device for which we do this operation
- * @freq:	Frequency in Hz for this OPP
- * @u_volt:	Voltage in uVolts for this OPP
+ * dev_pm_opp_add_dynamic()  - Add an OPP table from a table definitions
+ * @dev:	The device for which we do this operation
+ * @data:	The OPP data for the OPP to add
  *
  * This function adds an opp definition to the opp table and returns status.
  * The opp is made available by default and it can be controlled using
@@ -2841,7 +2839,7 @@ int dev_pm_opp_xlate_performance_state(struct opp_table *src_table,
  *		Duplicate OPPs (both freq and volt are same) and !opp->available
  * -ENOMEM	Memory allocation failure
  */
-int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
+int dev_pm_opp_add_dynamic(struct device *dev, struct dev_pm_opp_data *data)
 {
 	struct opp_table *opp_table;
 	int ret;
@@ -2853,13 +2851,13 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 	/* Fix regulator count for dynamic OPPs */
 	opp_table->regulator_count = 1;
 
-	ret = _opp_add_v1(opp_table, dev, freq, u_volt, true);
+	ret = _opp_add_v1(opp_table, dev, data, true);
 	if (ret)
 		dev_pm_opp_put_opp_table(opp_table);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(dev_pm_opp_add);
+EXPORT_SYMBOL_GPL(dev_pm_opp_add_dynamic);
 
 /**
  * _opp_set_availability() - helper to set the availability of an opp
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index ada4963c7cfa..ade6d42cae46 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -1077,13 +1077,15 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table)
 
 	val = prop->value;
 	while (nr) {
-		unsigned long freq = be32_to_cpup(val++) * 1000;
-		unsigned long volt = be32_to_cpup(val++);
+		struct dev_pm_opp_data data = {
+			.freq = be32_to_cpup(val++) * 1000,
+			.u_volt = be32_to_cpup(val++),
+		};
 
-		ret = _opp_add_v1(opp_table, dev, freq, volt, false);
+		ret = _opp_add_v1(opp_table, dev, &data, false);
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP %ld (%d)\n",
-				__func__, freq, ret);
+				__func__, data.freq, ret);
 			goto remove_static_opp;
 		}
 		nr -= 2;
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 8a5ea38f3a3d..fefdf9845692 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -251,7 +251,7 @@ struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table);
 void _opp_free(struct dev_pm_opp *opp);
 int _opp_compare_key(struct opp_table *opp_table, struct dev_pm_opp *opp1, struct dev_pm_opp *opp2);
 int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table);
-int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic);
+int _opp_add_v1(struct opp_table *opp_table, struct device *dev, struct dev_pm_opp_data *data, bool dynamic);
 void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, int last_cpu);
 struct opp_table *_add_opp_table_indexed(struct device *dev, int index, bool getclk);
 void _put_opp_list_kref(struct opp_table *opp_table);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 91f87d7e807c..a8ee93ba41d8 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -92,6 +92,16 @@ struct dev_pm_opp_config {
 	struct device ***virt_devs;
 };
 
+/**
+ * struct dev_pm_opp_data - The data to use to initialize an OPP.
+ * @freq: The clock rate in Hz for the OPP.
+ * @u_volt: The voltage in uV for the OPP.
+ */
+struct dev_pm_opp_data {
+	unsigned long freq;
+	unsigned long u_volt;
+};
+
 #if defined(CONFIG_PM_OPP)
 
 struct opp_table *dev_pm_opp_get_opp_table(struct device *dev);
@@ -152,8 +162,8 @@ struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev,
 
 void dev_pm_opp_put(struct dev_pm_opp *opp);
 
-int dev_pm_opp_add(struct device *dev, unsigned long freq,
-		   unsigned long u_volt);
+int dev_pm_opp_add_dynamic(struct device *dev, struct dev_pm_opp_data *opp);
+
 void dev_pm_opp_remove(struct device *dev, unsigned long freq);
 void dev_pm_opp_remove_all_dynamic(struct device *dev);
 
@@ -322,8 +332,8 @@ static inline struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev,
 
 static inline void dev_pm_opp_put(struct dev_pm_opp *opp) {}
 
-static inline int dev_pm_opp_add(struct device *dev, unsigned long freq,
-					unsigned long u_volt)
+static inline int
+dev_pm_opp_add_dynamic(struct device *dev, struct dev_pm_opp_data *opp)
 {
 	return -EOPNOTSUPP;
 }
@@ -519,6 +529,17 @@ static inline int dev_pm_opp_of_find_icc_paths(struct device *dev, struct opp_ta
 
 /* OPP Configuration helpers */
 
+static inline int dev_pm_opp_add(struct device *dev, unsigned long freq,
+				 unsigned long u_volt)
+{
+	struct dev_pm_opp_data data = {
+		.freq = freq,
+		.u_volt = u_volt,
+	};
+
+	return dev_pm_opp_add_dynamic(dev, &data);
+}
+
 /* Regulators helpers */
 static inline int dev_pm_opp_set_regulators(struct device *dev,
 					    const char * const names[])
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/9] OPP: Extend dev_pm_opp_data with a level

[Ulf Hansson]

Let's extend the dev_pm_opp_data with a level variable, to allow users to
specify a corresponding level (performance state) for a dynamically added
OPP.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/opp/core.c     | 1 +
 include/linux/pm_opp.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 54b6138e1189..ca8d1304b508 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2035,6 +2035,7 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
 
 	/* populate the opp table */
 	new_opp->rates[0] = data->freq;
+	new_opp->level = data->level;
 	tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
 	new_opp->supplies[0].u_volt = u_volt;
 	new_opp->supplies[0].u_volt_min = u_volt - tol;
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index a8ee93ba41d8..9ad168f4cbf1 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -94,10 +94,12 @@ struct dev_pm_opp_config {
 
 /**
  * struct dev_pm_opp_data - The data to use to initialize an OPP.
+ * @level: The performance level for the OPP.
  * @freq: The clock rate in Hz for the OPP.
  * @u_volt: The voltage in uV for the OPP.
  */
 struct dev_pm_opp_data {
+	unsigned int level;
 	unsigned long freq;
 	unsigned long u_volt;
 };
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/9] OPP: Switch to use dev_pm_domain_set_performance_state()

[Ulf Hansson]

To support performance scaling for any kinds of PM domains, let's move away
from using the genpd specific API, dev_pm_genpd_set_performance_state(), to
the common dev_pm_domain_set_performance_state().

No intended functional impact at this point.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/opp/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index ca8d1304b508..60dca60ac4af 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1030,7 +1030,7 @@ static int _set_performance_state(struct device *dev, struct device *pd_dev,
 	if (!pd_dev)
 		return 0;
 
-	ret = dev_pm_genpd_set_performance_state(pd_dev, pstate);
+	ret = dev_pm_domain_set_performance_state(pd_dev, pstate);
 	if (ret) {
 		dev_err(dev, "Failed to set performance state of %s: %d (%d)\n",
 			dev_name(pd_dev), pstate, ret);
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps

[Ulf Hansson]

At this point the level (performance state) for an OPP is currently limited
to be requested for a device that is attached to a PM domain.  Moreover,
the device needs to have the so called required-opps assigned to it, which
are based upon OPP tables being described in DT.

To extend the support beyond required-opps and DT, let's enable the level
to be set for all OPPs. More precisely, if the requested OPP has a valid
level let's try to request it through the device's optional PM domain, via
calling dev_pm_domain_set_performance_state().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/opp/core.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 60dca60ac4af..afb73978cdcb 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table)
 		opp_table->set_required_opps = _opp_set_required_opps_generic;
 }
 
+static int _set_opp_level(struct device *dev, struct opp_table *opp_table,
+			  struct dev_pm_opp *opp)
+{
+	int ret = 0;
+
+	/* Request a new performance state through the device's PM domain. */
+	if (opp && opp->level) {
+		ret = dev_pm_domain_set_performance_state(dev, opp->level);
+		if (ret)
+			dev_err(dev, "Failed to set performance state %u (%d)\n",
+				opp->level, ret);
+	}
+
+	return ret;
+}
+
 static void _find_current_opp(struct device *dev, struct opp_table *opp_table)
 {
 	struct dev_pm_opp *opp = ERR_PTR(-ENODEV);
@@ -1154,8 +1170,13 @@ static int _disable_opp_table(struct device *dev, struct opp_table *opp_table)
 	if (opp_table->regulators)
 		regulator_disable(opp_table->regulators[0]);
 
+	ret = _set_opp_level(dev, opp_table, NULL);
+	if (ret)
+		goto out;
+
 	ret = _set_required_opps(dev, opp_table, NULL, false);
 
+out:
 	opp_table->enabled = false;
 	return ret;
 }
@@ -1198,6 +1219,10 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,
 			return ret;
 		}
 
+		ret = _set_opp_level(dev, opp_table, opp);
+		if (ret)
+			return ret;
+
 		ret = _set_opp_bw(opp_table, opp, dev);
 		if (ret) {
 			dev_err(dev, "Failed to set bw: %d\n", ret);
@@ -1241,6 +1266,10 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,
 			return ret;
 		}
 
+		ret = _set_opp_level(dev, opp_table, opp);
+		if (ret)
+			return ret;
+
 		ret = _set_required_opps(dev, opp_table, opp, false);
 		if (ret) {
 			dev_err(dev, "Failed to set required opps: %d\n", ret);
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps

[Ulf Hansson]

On Mon, 25 Sept 2023 at 15:18, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> At this point the level (performance state) for an OPP is currently limited
> to be requested for a device that is attached to a PM domain.  Moreover,
> the device needs to have the so called required-opps assigned to it, which
> are based upon OPP tables being described in DT.
>
> To extend the support beyond required-opps and DT, let's enable the level
> to be set for all OPPs. More precisely, if the requested OPP has a valid
> level let's try to request it through the device's optional PM domain, via
> calling dev_pm_domain_set_performance_state().
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/opp/core.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 60dca60ac4af..afb73978cdcb 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table)
>                 opp_table->set_required_opps = _opp_set_required_opps_generic;
>  }
>
> +static int _set_opp_level(struct device *dev, struct opp_table *opp_table,
> +                         struct dev_pm_opp *opp)
> +{
> +       int ret = 0;
> +
> +       /* Request a new performance state through the device's PM domain. */
> +       if (opp && opp->level) {
> +               ret = dev_pm_domain_set_performance_state(dev, opp->level);
> +               if (ret)
> +                       dev_err(dev, "Failed to set performance state %u (%d)\n",
> +                               opp->level, ret);
> +       }

Okay, so reviewing my own code found a problem here. We need an "else"
here, that should request the performance state to be set to 0.

I am not sending a new version at this point, but awaiting more feedback first.

> +
> +       return ret;
> +}
> +
>  static void _find_current_opp(struct device *dev, struct opp_table *opp_table)
>  {
>         struct dev_pm_opp *opp = ERR_PTR(-ENODEV);
> @@ -1154,8 +1170,13 @@ static int _disable_opp_table(struct device *dev, struct opp_table *opp_table)
>         if (opp_table->regulators)
>                 regulator_disable(opp_table->regulators[0]);
>
> +       ret = _set_opp_level(dev, opp_table, NULL);
> +       if (ret)
> +               goto out;
> +
>         ret = _set_required_opps(dev, opp_table, NULL, false);
>
> +out:
>         opp_table->enabled = false;
>         return ret;
>  }
> @@ -1198,6 +1219,10 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,
>                         return ret;
>                 }
>
> +               ret = _set_opp_level(dev, opp_table, opp);
> +               if (ret)
> +                       return ret;
> +
>                 ret = _set_opp_bw(opp_table, opp, dev);
>                 if (ret) {
>                         dev_err(dev, "Failed to set bw: %d\n", ret);
> @@ -1241,6 +1266,10 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,
>                         return ret;
>                 }
>
> +               ret = _set_opp_level(dev, opp_table, opp);
> +               if (ret)
> +                       return ret;
> +
>                 ret = _set_required_opps(dev, opp_table, opp, false);
>                 if (ret) {
>                         dev_err(dev, "Failed to set required opps: %d\n", ret);
> --
> 2.34.1
>

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps

[Viresh Kumar]

On 25-09-23, 15:33, Ulf Hansson wrote:
> On Mon, 25 Sept 2023 at 15:18, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > At this point the level (performance state) for an OPP is currently limited
> > to be requested for a device that is attached to a PM domain.  Moreover,
> > the device needs to have the so called required-opps assigned to it, which
> > are based upon OPP tables being described in DT.
> >
> > To extend the support beyond required-opps and DT, let's enable the level
> > to be set for all OPPs. More precisely, if the requested OPP has a valid
> > level let's try to request it through the device's optional PM domain, via
> > calling dev_pm_domain_set_performance_state().
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >  drivers/opp/core.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > index 60dca60ac4af..afb73978cdcb 100644
> > --- a/drivers/opp/core.c
> > +++ b/drivers/opp/core.c
> > @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table)
> >                 opp_table->set_required_opps = _opp_set_required_opps_generic;
> >  }
> >
> > +static int _set_opp_level(struct device *dev, struct opp_table *opp_table,
> > +                         struct dev_pm_opp *opp)
> > +{
> > +       int ret = 0;
> > +
> > +       /* Request a new performance state through the device's PM domain. */
> > +       if (opp && opp->level) {
> > +               ret = dev_pm_domain_set_performance_state(dev, opp->level);
> > +               if (ret)
> > +                       dev_err(dev, "Failed to set performance state %u (%d)\n",
> > +                               opp->level, ret);
> > +       }
> 
> Okay, so reviewing my own code found a problem here. We need an "else"
> here, that should request the performance state to be set to 0.
> 
> I am not sending a new version at this point, but awaiting more feedback first.

I am looking to add below to this patch, is that okay with you ?

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 7b505316bb1c..a113e9caaa5a 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1135,16 +1135,22 @@ void _update_set_required_opps(struct opp_table *opp_table)
 static int _set_opp_level(struct device *dev, struct opp_table *opp_table,
                          struct dev_pm_opp *opp)
 {
+       unsigned int level = 0;
        int ret = 0;

-       /* Request a new performance state through the device's PM domain. */
-       if (opp && opp->level) {
-               ret = dev_pm_domain_set_performance_state(dev, opp->level);
-               if (ret)
-                       dev_err(dev, "Failed to set performance state %u (%d)\n",
-                               opp->level, ret);
+       if (opp) {
+               if (!opp->level)
+                       return 0;
+
+               level = opp->level;
        }

+       /* Request a new performance state through the device's PM domain. */
+       ret = dev_pm_genpd_set_performance_state(dev, level);
+       if (ret)
+               dev_err(dev, "Failed to set performance state %u (%d)\n", level,
+                       ret);
+
        return ret;
 }

I am switching back to dev_pm_genpd_set_performance_state() as I won't be
applying the power domain changes. I will then push out a branch and you can
rebase your patches on top of it ? And then probably Sudeep or someone else can
apply everything ?

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps

[Ulf Hansson]

On Tue, 3 Oct 2023 at 10:23, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 25-09-23, 15:33, Ulf Hansson wrote:
> > On Mon, 25 Sept 2023 at 15:18, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > At this point the level (performance state) for an OPP is currently limited
> > > to be requested for a device that is attached to a PM domain.  Moreover,
> > > the device needs to have the so called required-opps assigned to it, which
> > > are based upon OPP tables being described in DT.
> > >
> > > To extend the support beyond required-opps and DT, let's enable the level
> > > to be set for all OPPs. More precisely, if the requested OPP has a valid
> > > level let's try to request it through the device's optional PM domain, via
> > > calling dev_pm_domain_set_performance_state().
> > >
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > >  drivers/opp/core.c | 29 +++++++++++++++++++++++++++++
> > >  1 file changed, 29 insertions(+)
> > >
> > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > > index 60dca60ac4af..afb73978cdcb 100644
> > > --- a/drivers/opp/core.c
> > > +++ b/drivers/opp/core.c
> > > @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table)
> > >                 opp_table->set_required_opps = _opp_set_required_opps_generic;
> > >  }
> > >
> > > +static int _set_opp_level(struct device *dev, struct opp_table *opp_table,
> > > +                         struct dev_pm_opp *opp)
> > > +{
> > > +       int ret = 0;
> > > +
> > > +       /* Request a new performance state through the device's PM domain. */
> > > +       if (opp && opp->level) {
> > > +               ret = dev_pm_domain_set_performance_state(dev, opp->level);
> > > +               if (ret)
> > > +                       dev_err(dev, "Failed to set performance state %u (%d)\n",
> > > +                               opp->level, ret);
> > > +       }
> >
> > Okay, so reviewing my own code found a problem here. We need an "else"
> > here, that should request the performance state to be set to 0.
> >
> > I am not sending a new version at this point, but awaiting more feedback first.
>
> I am looking to add below to this patch, is that okay with you ?
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 7b505316bb1c..a113e9caaa5a 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1135,16 +1135,22 @@ void _update_set_required_opps(struct opp_table *opp_table)
>  static int _set_opp_level(struct device *dev, struct opp_table *opp_table,
>                           struct dev_pm_opp *opp)
>  {
> +       unsigned int level = 0;
>         int ret = 0;
>
> -       /* Request a new performance state through the device's PM domain. */
> -       if (opp && opp->level) {
> -               ret = dev_pm_domain_set_performance_state(dev, opp->level);
> -               if (ret)
> -                       dev_err(dev, "Failed to set performance state %u (%d)\n",
> -                               opp->level, ret);
> +       if (opp) {
> +               if (!opp->level)
> +                       return 0;
> +
> +               level = opp->level;
>         }
>
> +       /* Request a new performance state through the device's PM domain. */
> +       ret = dev_pm_genpd_set_performance_state(dev, level);
> +       if (ret)
> +               dev_err(dev, "Failed to set performance state %u (%d)\n", level,
> +                       ret);
> +
>         return ret;
>  }
>
> I am switching back to dev_pm_genpd_set_performance_state() as I won't be
> applying the power domain changes.

Can you please explain further on this. Rafael has acked those
patches, so it should be perfectly fine for you to pick them via your
tree too. There is no need to defer them.

> I will then push out a branch and you can
> rebase your patches on top of it ? And then probably Sudeep or someone else can
> apply everything ?

Or are you suggesting to just take one of the patches from my series,
and then I will re-base everything on top?

Just trying to understand the way forward. :-)

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps

[Viresh Kumar]

On 03-10-23, 14:36, Ulf Hansson wrote:
> Can you please explain further on this. Rafael has acked those
> patches, so it should be perfectly fine for you to pick them via your
> tree too. There is no need to defer them.

Ahh, then it is fine.

> > I will then push out a branch and you can
> > rebase your patches on top of it ? And then probably Sudeep or someone else can
> > apply everything ?
> 
> Or are you suggesting to just take one of the patches from my series,
> and then I will re-base everything on top?
> 
> Just trying to understand the way forward. :-)

Applied patches 1-6/9 and the fixed routine looks like this now:

+static int _set_opp_level(struct device *dev, struct opp_table *opp_table,
+                         struct dev_pm_opp *opp)
+{
+       unsigned int level = 0;
+       int ret = 0;
+
+       if (opp) {
+               if (!opp->level)
+                       return 0;
+
+               level = opp->level;
+       }
+
+       /* Request a new performance state through the device's PM domain. */
+       ret = dev_pm_domain_set_performance_state(dev, level);
+       if (ret)
+               dev_err(dev, "Failed to set performance state %u (%d)\n", level,
+                       ret);
+
+       return ret;
+}

Does it look okay now ?

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps

[Ulf Hansson]

On Wed, 4 Oct 2023 at 07:04, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 03-10-23, 14:36, Ulf Hansson wrote:
> > Can you please explain further on this. Rafael has acked those
> > patches, so it should be perfectly fine for you to pick them via your
> > tree too. There is no need to defer them.
>
> Ahh, then it is fine.
>
> > > I will then push out a branch and you can
> > > rebase your patches on top of it ? And then probably Sudeep or someone else can
> > > apply everything ?
> >
> > Or are you suggesting to just take one of the patches from my series,
> > and then I will re-base everything on top?
> >
> > Just trying to understand the way forward. :-)
>
> Applied patches 1-6/9 and the fixed routine looks like this now:
>
> +static int _set_opp_level(struct device *dev, struct opp_table *opp_table,
> +                         struct dev_pm_opp *opp)
> +{
> +       unsigned int level = 0;
> +       int ret = 0;
> +
> +       if (opp) {
> +               if (!opp->level)
> +                       return 0;
> +
> +               level = opp->level;
> +       }
> +
> +       /* Request a new performance state through the device's PM domain. */
> +       ret = dev_pm_domain_set_performance_state(dev, level);
> +       if (ret)
> +               dev_err(dev, "Failed to set performance state %u (%d)\n", level,
> +                       ret);
> +
> +       return ret;
> +}
>
> Does it look okay now ?

Yes, perfect! Thanks for helping out!

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 7/9] firmware: arm_scmi: Simplify error path in scmi_dvfs_device_opps_add()

[Ulf Hansson]

Let's simplify the code in scmi_dvfs_device_opps_add() by using
dev_pm_opp_remove_all_dynamic() in the error path.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/firmware/arm_scmi/perf.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 9eb58df9124d..733c5ebeb555 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -787,30 +787,22 @@ static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
 {
 	int idx, ret;
 	unsigned long freq;
-	struct scmi_opp *opp;
 	struct perf_dom_info *dom;
 
 	dom = scmi_perf_domain_lookup(ph, domain);
 	if (IS_ERR(dom))
 		return PTR_ERR(dom);
 
-	for (opp = dom->opp, idx = 0; idx < dom->opp_count; idx++, opp++) {
+	for (idx = 0; idx < dom->opp_count; idx++) {
 		if (!dom->level_indexing_mode)
-			freq = opp->perf * dom->mult_factor;
+			freq = dom->opp[idx].perf * dom->mult_factor;
 		else
-			freq = opp->indicative_freq * 1000;
+			freq = dom->opp[idx].indicative_freq * 1000;
 
 		ret = dev_pm_opp_add(dev, freq, 0);
 		if (ret) {
 			dev_warn(dev, "failed to add opp %luHz\n", freq);
-
-			while (idx-- > 0) {
-				if (!dom->level_indexing_mode)
-					freq = (--opp)->perf * dom->mult_factor;
-				else
-					freq = (--opp)->indicative_freq * 1000;
-				dev_pm_opp_remove(dev, freq);
-			}
+			dev_pm_opp_remove_all_dynamic(dev);
 			return ret;
 		}
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 8/9] firmware: arm_scmi: Specify the performance level when adding an OPP

[Ulf Hansson]

To enable the performance level to be used for OPPs, let's convert into
using the dev_pm_opp_add_dynamic() API when creating them. This will be
particularly useful for the SCMI performance domain, as shown through
subsequent changes.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/firmware/arm_scmi/perf.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 733c5ebeb555..c6a1332164c1 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -787,6 +787,7 @@ static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
 {
 	int idx, ret;
 	unsigned long freq;
+	struct dev_pm_opp_data data = {};
 	struct perf_dom_info *dom;
 
 	dom = scmi_perf_domain_lookup(ph, domain);
@@ -799,7 +800,10 @@ static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
 		else
 			freq = dom->opp[idx].indicative_freq * 1000;
 
-		ret = dev_pm_opp_add(dev, freq, 0);
+		data.level = dom->opp[idx].perf;
+		data.freq = freq;
+
+		ret = dev_pm_opp_add_dynamic(dev, &data);
 		if (ret) {
 			dev_warn(dev, "failed to add opp %luHz\n", freq);
 			dev_pm_opp_remove_all_dynamic(dev);
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain

[Ulf Hansson]

To allow a consumer driver to use the OPP library to scale the performance
for its device, let's dynamically add the OPP table when the device gets
attached to its SCMI performance domain.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/arm/scmi_perf_domain.c | 34 +++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/pmdomain/arm/scmi_perf_domain.c b/drivers/pmdomain/arm/scmi_perf_domain.c
index aa100270500f..bc3f78abb6da 100644
--- a/drivers/pmdomain/arm/scmi_perf_domain.c
+++ b/drivers/pmdomain/arm/scmi_perf_domain.c
@@ -9,6 +9,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_opp.h>
 #include <linux/scmi_protocol.h>
 #include <linux/slab.h>
 
@@ -42,6 +43,37 @@ scmi_pd_set_perf_state(struct generic_pm_domain *genpd, unsigned int state)
 	return ret;
 }
 
+static int
+scmi_pd_attach_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	struct scmi_perf_domain *pd = to_scmi_pd(genpd);
+	int ret;
+
+	/*
+	 * Allow the device to be attached, but don't add the OPP table unless
+	 * the performance level can be changed.
+	 */
+	if (!pd->info->set_perf)
+		return 0;
+
+	ret = pd->perf_ops->device_opps_add(pd->ph, dev, pd->domain_id);
+	if (ret)
+		dev_warn(dev, "failed to add OPPs for the device\n");
+
+	return ret;
+}
+
+static void
+scmi_pd_detach_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	struct scmi_perf_domain *pd = to_scmi_pd(genpd);
+
+	if (!pd->info->set_perf)
+		return;
+
+	dev_pm_opp_remove_all_dynamic(dev);
+}
+
 static int scmi_perf_domain_probe(struct scmi_device *sdev)
 {
 	struct device *dev = &sdev->dev;
@@ -95,6 +127,8 @@ static int scmi_perf_domain_probe(struct scmi_device *sdev)
 		scmi_pd->genpd.flags = GENPD_FLAG_ALWAYS_ON |
 				       GENPD_FLAG_OPP_TABLE_FW;
 		scmi_pd->genpd.set_performance_state = scmi_pd_set_perf_state;
+		scmi_pd->genpd.attach_dev = scmi_pd_attach_dev;
+		scmi_pd->genpd.detach_dev = scmi_pd_detach_dev;
 
 		ret = pm_genpd_init(&scmi_pd->genpd, NULL, false);
 		if (ret)
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain

[Sudeep Holla]

On Mon, Sep 25, 2023 at 03:17:15PM +0200, Ulf Hansson wrote:
> To allow a consumer driver to use the OPP library to scale the performance
> for its device, let's dynamically add the OPP table when the device gets
> attached to its SCMI performance domain.
>

The SCMI changes(patches 7-9) look fine to me. Rafael was fine with genpd
changes, Viresh if you are OK with OPP changes I can take it via SCMI as
there are some dependent patches as Ulf has pointed out in the cover letter.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain

[Viresh Kumar]

On 29-09-23, 17:25, Sudeep Holla wrote:
> On Mon, Sep 25, 2023 at 03:17:15PM +0200, Ulf Hansson wrote:
> > To allow a consumer driver to use the OPP library to scale the performance
> > for its device, let's dynamically add the OPP table when the device gets
> > attached to its SCMI performance domain.
> >
> 
> The SCMI changes(patches 7-9) look fine to me. Rafael was fine with genpd
> changes, Viresh if you are OK with OPP changes I can take it via SCMI as
> there are some dependent patches as Ulf has pointed out in the cover letter.

I would like to take OPP patches via my tree as there are some changes in my
tree and I plan to add some more changes on top of this. I can give an immutable
branch though.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain

[Sudeep Holla]

On Tue, Oct 03, 2023 at 01:51:33PM +0530, Viresh Kumar wrote:
> On 29-09-23, 17:25, Sudeep Holla wrote:
> > On Mon, Sep 25, 2023 at 03:17:15PM +0200, Ulf Hansson wrote:
> > > To allow a consumer driver to use the OPP library to scale the performance
> > > for its device, let's dynamically add the OPP table when the device gets
> > > attached to its SCMI performance domain.
> > >
> > 
> > The SCMI changes(patches 7-9) look fine to me. Rafael was fine with genpd
> > changes, Viresh if you are OK with OPP changes I can take it via SCMI as
> > there are some dependent patches as Ulf has pointed out in the cover letter.
> 
> I would like to take OPP patches via my tree as there are some changes in my
> tree and I plan to add some more changes on top of this. I can give an immutable
> branch though.
>

Works for me. Please do share it once you have it ready.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain

[Viresh Kumar]

On 03-10-23, 12:26, Sudeep Holla wrote:
> On Tue, Oct 03, 2023 at 01:51:33PM +0530, Viresh Kumar wrote:
> > On 29-09-23, 17:25, Sudeep Holla wrote:
> > > On Mon, Sep 25, 2023 at 03:17:15PM +0200, Ulf Hansson wrote:
> > > > To allow a consumer driver to use the OPP library to scale the performance
> > > > for its device, let's dynamically add the OPP table when the device gets
> > > > attached to its SCMI performance domain.
> > > >
> > > 
> > > The SCMI changes(patches 7-9) look fine to me. Rafael was fine with genpd
> > > changes, Viresh if you are OK with OPP changes I can take it via SCMI as
> > > there are some dependent patches as Ulf has pointed out in the cover letter.
> > 
> > I would like to take OPP patches via my tree as there are some changes in my
> > tree and I plan to add some more changes on top of this. I can give an immutable
> > branch though.
> >
> 
> Works for me. Please do share it once you have it ready.

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/pm-domain-scmi

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain

[Sudeep Holla]

On Wed, Oct 04, 2023 at 10:38:06AM +0530, Viresh Kumar wrote:
> On 03-10-23, 12:26, Sudeep Holla wrote:
> > On Tue, Oct 03, 2023 at 01:51:33PM +0530, Viresh Kumar wrote:
> > > On 29-09-23, 17:25, Sudeep Holla wrote:
> > > > On Mon, Sep 25, 2023 at 03:17:15PM +0200, Ulf Hansson wrote:
> > > > > To allow a consumer driver to use the OPP library to scale the performance
> > > > > for its device, let's dynamically add the OPP table when the device gets
> > > > > attached to its SCMI performance domain.
> > > > >
> > > > 
> > > > The SCMI changes(patches 7-9) look fine to me. Rafael was fine with genpd
> > > > changes, Viresh if you are OK with OPP changes I can take it via SCMI as
> > > > there are some dependent patches as Ulf has pointed out in the cover letter.
> > > 
> > > I would like to take OPP patches via my tree as there are some changes in my
> > > tree and I plan to add some more changes on top of this. I can give an immutable
> > > branch though.
> > >
> > 
> > Works for me. Please do share it once you have it ready.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/pm-domain-scmi
> 

Thanks, I wasn't expecting these patches in the branch, but if you are
aware off and are fine with it, I have no objections.

	dt-bindings: opp: opp-v2-kryo-cpu: Allow opp-peak-kBps
	OPP: debugfs: Fix warning with W=1 builds
	OPP: Remove doc style comments for internal routines
	OPP: Add dev_pm_opp_find_level_floor()

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain

[Viresh Kumar]

On 05-10-23, 16:06, Sudeep Holla wrote:
> Thanks, I wasn't expecting these patches in the branch, but if you are
> aware off and are fine with it, I have no objections.
> 
> 	dt-bindings: opp: opp-v2-kryo-cpu: Allow opp-peak-kBps
> 	OPP: debugfs: Fix warning with W=1 builds
> 	OPP: Remove doc style comments for internal routines
> 	OPP: Add dev_pm_opp_find_level_floor()

Ahh, I didn't create a fresh branch for this but added them on top of
OPP changes. Fixed it now. Fetch the repo again.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain

[Sudeep Holla]

On Fri, Oct 06, 2023 at 12:39:23PM +0530, Viresh Kumar wrote:
> On 05-10-23, 16:06, Sudeep Holla wrote:
> > Thanks, I wasn't expecting these patches in the branch, but if you are
> > aware off and are fine with it, I have no objections.
> > 
> > 	dt-bindings: opp: opp-v2-kryo-cpu: Allow opp-peak-kBps
> > 	OPP: debugfs: Fix warning with W=1 builds
> > 	OPP: Remove doc style comments for internal routines
> > 	OPP: Add dev_pm_opp_find_level_floor()
> 
> Ahh, I didn't create a fresh branch for this but added them on top of
> OPP changes. Fixed it now. Fetch the repo again.

Thanks, glad I asked, initially assumed you were aware of it :).

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/9] PM: domains/opp/arm_scmi: Extend performance scaling support

[Rafael J. Wysocki]

On Mon, Sep 25, 2023 at 3:17 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Consumer drivers for devices that are attached to the SCMI performance domain,
> are currently not able to use the OPP library to scale performance. This series
> is enabling the support for this.
>
> The OPPs for SCMI performance domain are encoded in the FW, rather than being
> described through DT. To better support this, some changes have also been done
> to the OPP library and for PM domains in general. More details are available in
> the commit messages.
>
> I have based the series on the scmi tree and the for-next/scmi/updates branch,
> so the OPP and PM domain changes would require acks from Viresh and Rafael to
> be allow to be picked up and funneled through the scmi tree. Or, let's just
> discuss what works best for us in this regards.
>
> Note that, I am running this on the Qemu virt platform with Optee running an
> SCMI server. If you want some more details about my test setup, I can certainly
> share this with you, just let me know.
>
> Looking forward to your feedback!
>
> Kind regards
> Ulf Hansson
>
>
> Ulf Hansson (9):
>   PM: domains: Introduce dev_pm_domain_set_performance_state()
>   PM: domains: Implement the ->set_performance_state() callback for
>     genpd

I have no strong opinion regarding the two patches above, so if you
need ACKs from me on them, please feel free to add them.

>   OPP: Add dev_pm_opp_add_dynamic() to allow more flexibility
>   OPP: Extend dev_pm_opp_data with a level
>   OPP: Switch to use dev_pm_domain_set_performance_state()
>   OPP: Extend support for the opp-level beyond required-opps
>   firmware: arm_scmi: Simplify error path in scmi_dvfs_device_opps_add()
>   firmware: arm_scmi: Specify the performance level when adding an OPP
>   firmware: arm_scmi: Add generic OPP support to the SCMI performance
>     domain
>
>  drivers/base/power/common.c             | 21 ++++++++++
>  drivers/base/power/domain.c             | 33 +++++++++------
>  drivers/firmware/arm_scmi/perf.c        | 22 +++++-----
>  drivers/opp/core.c                      | 54 +++++++++++++++++++------
>  drivers/opp/of.c                        | 10 +++--
>  drivers/opp/opp.h                       |  2 +-
>  drivers/pmdomain/arm/scmi_perf_domain.c | 34 ++++++++++++++++
>  include/linux/pm.h                      |  2 +
>  include/linux/pm_domain.h               |  6 +++
>  include/linux/pm_opp.h                  | 31 ++++++++++++--
>  10 files changed, 168 insertions(+), 47 deletions(-)
>
> --

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: (subset) [PATCH 0/9] PM: domains/opp/arm_scmi: Extend performance scaling support

[Sudeep Holla]

On Mon, 25 Sep 2023 15:17:06 +0200, Ulf Hansson wrote:
> Consumer drivers for devices that are attached to the SCMI performance domain,
> are currently not able to use the OPP library to scale performance. This series
> is enabling the support for this.
> 
> The OPPs for SCMI performance domain are encoded in the FW, rather than being
> described through DT. To better support this, some changes have also been done
> to the OPP library and for PM domains in general. More details are available in
> the commit messages.
> 
> [...]

Applied to sudeep.holla/linux (for-next/scmi/updates), thanks!

[1/9 - 6/9] Merged via branch 'opp/pm-domain-scmi' of git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm

[7/9] firmware: arm_scmi: Simplify error path in scmi_dvfs_device_opps_add()
      https://git.kernel.org/sudeep.holla/c/033ca4de1296
[8/9] firmware: arm_scmi: Specify the performance level when adding an OPP
      https://git.kernel.org/sudeep.holla/c/5a6a10419352
[9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain
      https://git.kernel.org/sudeep.holla/c/0a81b0eb81ca
--
Regards,
Sudeep


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel