2021-02-03 09:29:34

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v5 0/3] Add required-opps support to devfreq passive gov

The devfreq passive governor scales the frequency of a "child" device based
on the current frequency of a "parent" device (not parent/child in the
sense of device hierarchy). As of today, the passive governor requires one
of the following to work correctly:
1. The parent and child device have the same number of frequencies
2. The child device driver passes a mapping function to translate from
parent frequency to child frequency.

When (1) is not true, (2) is the only option right now. But often times,
all that is required is a simple mapping from parent's frequency to child's
frequency.

Since OPPs already support pointing to other "required-opps", add support
for using that to map from parent device frequency to child device
frequency. That way, every child device driver doesn't have to implement a
separate mapping function anytime (1) isn't true.

Some common (but not comprehensive) reason for needing a devfreq passive
governor to adjust the frequency of one device based on another are:

1. These were the combination of frequencies that were validated/screened
during the manufacturing process.
2. These are the sensible performance combinations between two devices
interacting with each other. So that when one runs fast the other
doesn't become the bottleneck.
3. Hardware bugs requiring some kind of frequency ratio between devices.

For example, the following mapping can't be captured in DT as it stands
today because the parent and child device have different number of OPPs.
But with this patch series, this mapping can be captured cleanly.

In arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi you have something
like this with the following changes:

bus_g2d_400: bus0 {
compatible = "samsung,exynos-bus";
clocks = <&cmu_top CLK_ACLK_G2D_400>;
clock-names = "bus";
operating-points-v2 = <&bus_g2d_400_opp_table>;
status = "disabled";
};

bus_noc2: bus9 {
compatible = "samsung,exynos-bus";
clocks = <&cmu_mif CLK_ACLK_BUS2_400>;
clock-names = "bus";
operating-points-v2 = <&bus_noc2_opp_table>;
status = "disabled";
};

bus_g2d_400_opp_table: opp_table2 {
compatible = "operating-points-v2";
opp-shared;

opp-400000000 {
opp-hz = /bits/ 64 <400000000>;
opp-microvolt = <1075000>;
required-opps = <&noc2_400>;
};
opp-267000000 {
opp-hz = /bits/ 64 <267000000>;
opp-microvolt = <1000000>;
required-opps = <&noc2_200>;
};
opp-200000000 {
opp-hz = /bits/ 64 <200000000>;
opp-microvolt = <975000>;
required-opps = <&noc2_200>;
};
opp-160000000 {
opp-hz = /bits/ 64 <160000000>;
opp-microvolt = <962500>;
required-opps = <&noc2_134>;
};
opp-134000000 {
opp-hz = /bits/ 64 <134000000>;
opp-microvolt = <950000>;
required-opps = <&noc2_134>;
};
opp-100000000 {
opp-hz = /bits/ 64 <100000000>;
opp-microvolt = <937500>;
required-opps = <&noc2_100>;
};
};

bus_noc2_opp_table: opp_table6 {
compatible = "operating-points-v2";

noc2_400: opp-400000000 {
opp-hz = /bits/ 64 <400000000>;
};
noc2_200: opp-200000000 {
opp-hz = /bits/ 64 <200000000>;
};
noc2_134: opp-134000000 {
opp-hz = /bits/ 64 <134000000>;
};
noc2_100: opp-100000000 {
opp-hz = /bits/ 64 <100000000>;
};
};

-Saravana

v4 -> v5:
- drop patch "OPP: Improve required-opps linking" and rebase to
https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/log/?h=opp/linux-next
- Compare pointers in dev_pm_opp_xlate_required_opp().

v3 -> v4:
- Fixed documentation comments
- Fixed order of functions in .h file
- Renamed the new xlate API
- Caused _set_required_opps() to fail if all required opps tables aren't
linked.
v2 -> v3:
- Rebased onto linux-next.
- Added documentation comment for new fields.
- Added support for lazy required-opps linking.
- Updated Ack/Reviewed-bys.
v1 -> v2:
- Cached OPP table reference in devfreq to avoid looking up every time.
- Renamed variable in passive governor to be more intuitive.
- Updated cover letter with examples.

Saravana Kannan (3):
OPP: Add function to look up required OPP's for a given OPP
PM / devfreq: Cache OPP table reference in devfreq
PM / devfreq: Add required OPPs support to passive governor

drivers/devfreq/devfreq.c | 6 ++++
drivers/devfreq/governor_passive.c | 20 ++++++++---
drivers/opp/core.c | 58 ++++++++++++++++++++++++++++++
include/linux/devfreq.h | 2 ++
include/linux/pm_opp.h | 11 ++++++
5 files changed, 92 insertions(+), 5 deletions(-)

--
2.30.0.365.g02bc693789-goog


2021-02-03 09:29:59

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v5 1/3] OPP: Add function to look up required OPP's for a given OPP

From: Saravana Kannan <[email protected]>

Add a function that allows looking up required OPPs given a source OPP
table, destination OPP table and the source OPP.

Signed-off-by: Saravana Kannan <[email protected]>
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
drivers/opp/core.c | 58 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_opp.h | 11 ++++++++
2 files changed, 69 insertions(+)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index dc95d29e94c1b..878f066b972cc 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2398,6 +2398,64 @@ devm_pm_opp_attach_genpd(struct device *dev, const char **names,
}
EXPORT_SYMBOL_GPL(devm_pm_opp_attach_genpd);

+/**
+ * dev_pm_opp_xlate_required_opp() - Find required OPP for @src_table OPP.
+ * @src_table: OPP table which has @dst_table as one of its required OPP table.
+ * @dst_table: Required OPP table of the @src_table.
+ *
+ * This function returns the OPP (present in @dst_table) pointed out by the
+ * "required-opps" property of the OPP (present in @src_table).
+ *
+ * The callers are required to call dev_pm_opp_put() for the returned OPP after
+ * use.
+ *
+ * Return: destination table OPP on success, otherwise NULL on errors.
+ */
+struct dev_pm_opp *dev_pm_opp_xlate_required_opp(struct opp_table *src_table,
+ struct opp_table *dst_table,
+ struct dev_pm_opp *src_opp)
+{
+ struct dev_pm_opp *opp, *dest_opp = NULL;
+ int i;
+
+ if (!src_table || !dst_table || !src_opp ||
+ !src_table->required_opp_tables)
+ return NULL;
+
+ /* required-opps not fully initialized yet */
+ if (lazy_linking_pending(src_table))
+ return NULL;
+
+ for (i = 0; i < src_table->required_opp_count; i++) {
+ if (src_table->required_opp_tables[i] == dst_table)
+ break;
+ }
+
+ if (unlikely(i == src_table->required_opp_count)) {
+ pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
+ __func__, src_table, dst_table);
+ return NULL;
+ }
+
+ mutex_lock(&src_table->lock);
+
+ list_for_each_entry(opp, &src_table->opp_list, node) {
+ if (opp == src_opp) {
+ dest_opp = opp->required_opps[i];
+ dev_pm_opp_get(dest_opp);
+ goto unlock;
+ }
+ }
+
+ pr_err("%s: Couldn't find matching OPP (%p: %p)\n", __func__, src_table,
+ dst_table);
+
+unlock:
+ mutex_unlock(&src_table->lock);
+
+ return dest_opp;
+}
+
/**
* dev_pm_opp_xlate_performance_state() - Find required OPP's pstate for src_table.
* @src_table: OPP table which has dst_table as one of its required OPP table.
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index ab1d15ce559db..6f5f72a7f601c 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -156,6 +156,9 @@ struct opp_table *devm_pm_opp_register_set_opp_helper(struct device *dev, int (*
struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names, struct device ***virt_devs);
void dev_pm_opp_detach_genpd(struct opp_table *opp_table);
struct opp_table *devm_pm_opp_attach_genpd(struct device *dev, const char **names, struct device ***virt_devs);
+struct dev_pm_opp *dev_pm_opp_xlate_required_opp(struct opp_table *src_table,
+ struct opp_table *dst_table,
+ struct dev_pm_opp *src_opp);
int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
int dev_pm_opp_set_opp(struct device *dev, struct dev_pm_opp *opp);
@@ -367,6 +370,14 @@ static inline struct opp_table *devm_pm_opp_attach_genpd(struct device *dev,
return ERR_PTR(-EOPNOTSUPP);
}

+static inline struct dev_pm_opp *dev_pm_opp_xlate_required_opp(
+ struct opp_table *src_table,
+ struct opp_table *dst_table,
+ struct dev_pm_opp *src_opp)
+{
+ return NULL;
+}
+
static inline int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate)
{
return -EOPNOTSUPP;
--
2.30.0.365.g02bc693789-goog

2021-02-03 09:30:23

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v5 2/3] PM / devfreq: Cache OPP table reference in devfreq

From: Saravana Kannan <[email protected]>

The OPP table can be used often in devfreq. Trying to get it each time can
be expensive, so cache it in the devfreq struct.

Signed-off-by: Saravana Kannan <[email protected]>
Reviewed-by: Chanwoo Choi <[email protected]>
Acked-by: MyungJoo Ham <[email protected]>
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
drivers/devfreq/devfreq.c | 6 ++++++
include/linux/devfreq.h | 2 ++
2 files changed, 8 insertions(+)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 6aa10de792b33..a5899c9ae16fc 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -757,6 +757,8 @@ static void devfreq_dev_release(struct device *dev)
if (devfreq->profile->exit)
devfreq->profile->exit(devfreq->dev.parent);

+ if (devfreq->opp_table)
+ dev_pm_opp_put_opp_table(devfreq->opp_table);
mutex_destroy(&devfreq->lock);
kfree(devfreq);
}
@@ -844,6 +846,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
}

devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
+ devfreq->opp_table = dev_pm_opp_get_opp_table(dev);
+ if (IS_ERR(devfreq->opp_table))
+ devfreq->opp_table = NULL;
+
atomic_set(&devfreq->suspend_count, 0);

dev_set_name(&devfreq->dev, "%s", dev_name(dev));
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index b6d3bae1c74d8..26ea0850be9bb 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -137,6 +137,7 @@ struct devfreq_stats {
* using devfreq.
* @profile: device-specific devfreq profile
* @governor: method how to choose frequency based on the usage.
+ * @opp_table: Reference to OPP table of dev.parent, if one exists.
* @nb: notifier block used to notify devfreq object that it should
* reevaluate operable frequencies. Devfreq users may use
* devfreq.nb to the corresponding register notifier call chain.
@@ -173,6 +174,7 @@ struct devfreq {
struct device dev;
struct devfreq_dev_profile *profile;
const struct devfreq_governor *governor;
+ struct opp_table *opp_table;
struct notifier_block nb;
struct delayed_work work;

--
2.30.0.365.g02bc693789-goog

2021-02-03 09:58:52

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v5 0/3] Add required-opps support to devfreq passive gov

Hi Hsin-Yi,

Thanks for the patch. I already reviewed this patch.
But, I'll check these again and test it.

On 2/3/21 6:23 PM, Hsin-Yi Wang wrote:
> The devfreq passive governor scales the frequency of a "child" device based
> on the current frequency of a "parent" device (not parent/child in the
> sense of device hierarchy). As of today, the passive governor requires one
> of the following to work correctly:
> 1. The parent and child device have the same number of frequencies
> 2. The child device driver passes a mapping function to translate from
> parent frequency to child frequency.
>
> When (1) is not true, (2) is the only option right now. But often times,
> all that is required is a simple mapping from parent's frequency to child's
> frequency.
>
> Since OPPs already support pointing to other "required-opps", add support
> for using that to map from parent device frequency to child device
> frequency. That way, every child device driver doesn't have to implement a
> separate mapping function anytime (1) isn't true.
>
> Some common (but not comprehensive) reason for needing a devfreq passive
> governor to adjust the frequency of one device based on another are:
>
> 1. These were the combination of frequencies that were validated/screened
> during the manufacturing process.
> 2. These are the sensible performance combinations between two devices
> interacting with each other. So that when one runs fast the other
> doesn't become the bottleneck.
> 3. Hardware bugs requiring some kind of frequency ratio between devices.
>
> For example, the following mapping can't be captured in DT as it stands
> today because the parent and child device have different number of OPPs.
> But with this patch series, this mapping can be captured cleanly.
>
> In arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi you have something
> like this with the following changes:
>
> bus_g2d_400: bus0 {
> compatible = "samsung,exynos-bus";
> clocks = <&cmu_top CLK_ACLK_G2D_400>;
> clock-names = "bus";
> operating-points-v2 = <&bus_g2d_400_opp_table>;
> status = "disabled";
> };
>
> bus_noc2: bus9 {
> compatible = "samsung,exynos-bus";
> clocks = <&cmu_mif CLK_ACLK_BUS2_400>;
> clock-names = "bus";
> operating-points-v2 = <&bus_noc2_opp_table>;
> status = "disabled";
> };
>
> bus_g2d_400_opp_table: opp_table2 {
> compatible = "operating-points-v2";
> opp-shared;
>
> opp-400000000 {
> opp-hz = /bits/ 64 <400000000>;
> opp-microvolt = <1075000>;
> required-opps = <&noc2_400>;
> };
> opp-267000000 {
> opp-hz = /bits/ 64 <267000000>;
> opp-microvolt = <1000000>;
> required-opps = <&noc2_200>;
> };
> opp-200000000 {
> opp-hz = /bits/ 64 <200000000>;
> opp-microvolt = <975000>;
> required-opps = <&noc2_200>;
> };
> opp-160000000 {
> opp-hz = /bits/ 64 <160000000>;
> opp-microvolt = <962500>;
> required-opps = <&noc2_134>;
> };
> opp-134000000 {
> opp-hz = /bits/ 64 <134000000>;
> opp-microvolt = <950000>;
> required-opps = <&noc2_134>;
> };
> opp-100000000 {
> opp-hz = /bits/ 64 <100000000>;
> opp-microvolt = <937500>;
> required-opps = <&noc2_100>;
> };
> };
>
> bus_noc2_opp_table: opp_table6 {
> compatible = "operating-points-v2";
>
> noc2_400: opp-400000000 {
> opp-hz = /bits/ 64 <400000000>;
> };
> noc2_200: opp-200000000 {
> opp-hz = /bits/ 64 <200000000>;
> };
> noc2_134: opp-134000000 {
> opp-hz = /bits/ 64 <134000000>;
> };
> noc2_100: opp-100000000 {
> opp-hz = /bits/ 64 <100000000>;
> };
> };
>
> -Saravana
>
> v4 -> v5:
> - drop patch "OPP: Improve required-opps linking" and rebase to
> https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/log/?h=opp/linux-next
> - Compare pointers in dev_pm_opp_xlate_required_opp().
>
> v3 -> v4:
> - Fixed documentation comments
> - Fixed order of functions in .h file
> - Renamed the new xlate API
> - Caused _set_required_opps() to fail if all required opps tables aren't
> linked.
> v2 -> v3:
> - Rebased onto linux-next.
> - Added documentation comment for new fields.
> - Added support for lazy required-opps linking.
> - Updated Ack/Reviewed-bys.
> v1 -> v2:
> - Cached OPP table reference in devfreq to avoid looking up every time.
> - Renamed variable in passive governor to be more intuitive.
> - Updated cover letter with examples.
>
> Saravana Kannan (3):
> OPP: Add function to look up required OPP's for a given OPP
> PM / devfreq: Cache OPP table reference in devfreq
> PM / devfreq: Add required OPPs support to passive governor
>
> drivers/devfreq/devfreq.c | 6 ++++
> drivers/devfreq/governor_passive.c | 20 ++++++++---
> drivers/opp/core.c | 58 ++++++++++++++++++++++++++++++
> include/linux/devfreq.h | 2 ++
> include/linux/pm_opp.h | 11 ++++++
> 5 files changed, 92 insertions(+), 5 deletions(-)
>


--
Best Regards,
Chanwoo Choi
Samsung Electronics

2021-02-04 05:44:28

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v5 0/3] Add required-opps support to devfreq passive gov

On 03-02-21, 17:23, Hsin-Yi Wang wrote:
> The devfreq passive governor scales the frequency of a "child" device based
> on the current frequency of a "parent" device (not parent/child in the
> sense of device hierarchy). As of today, the passive governor requires one
> of the following to work correctly:
> 1. The parent and child device have the same number of frequencies
> 2. The child device driver passes a mapping function to translate from
> parent frequency to child frequency.
>
> When (1) is not true, (2) is the only option right now. But often times,
> all that is required is a simple mapping from parent's frequency to child's
> frequency.
>
> Since OPPs already support pointing to other "required-opps", add support
> for using that to map from parent device frequency to child device
> frequency. That way, every child device driver doesn't have to implement a
> separate mapping function anytime (1) isn't true.

So you guys aren't interested in dev_pm_opp_set_opp() but just the
translation of the required-OPPs ?

I am fine with most of the stuff and I would like to take it via OPP
tree, hope that would be fine ?

--
viresh

2021-02-04 05:52:14

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v5 1/3] OPP: Add function to look up required OPP's for a given OPP

On 03-02-21, 17:23, Hsin-Yi Wang wrote:
> From: Saravana Kannan <[email protected]>
>
> Add a function that allows looking up required OPPs given a source OPP
> table, destination OPP table and the source OPP.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> Signed-off-by: Hsin-Yi Wang <[email protected]>
> ---
> drivers/opp/core.c | 58 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/pm_opp.h | 11 ++++++++
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index dc95d29e94c1b..878f066b972cc 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -2398,6 +2398,64 @@ devm_pm_opp_attach_genpd(struct device *dev, const char **names,
> }
> EXPORT_SYMBOL_GPL(devm_pm_opp_attach_genpd);
>
> +/**
> + * dev_pm_opp_xlate_required_opp() - Find required OPP for @src_table OPP.
> + * @src_table: OPP table which has @dst_table as one of its required OPP table.
> + * @dst_table: Required OPP table of the @src_table.
> + *
> + * This function returns the OPP (present in @dst_table) pointed out by the
> + * "required-opps" property of the OPP (present in @src_table).
> + *
> + * The callers are required to call dev_pm_opp_put() for the returned OPP after
> + * use.
> + *
> + * Return: destination table OPP on success, otherwise NULL on errors.
> + */
> +struct dev_pm_opp *dev_pm_opp_xlate_required_opp(struct opp_table *src_table,
> + struct opp_table *dst_table,
> + struct dev_pm_opp *src_opp)
> +{
> + struct dev_pm_opp *opp, *dest_opp = NULL;
> + int i;
> +
> + if (!src_table || !dst_table || !src_opp ||
> + !src_table->required_opp_tables)
> + return NULL;
> +
> + /* required-opps not fully initialized yet */
> + if (lazy_linking_pending(src_table))
> + return NULL;
> +
> + for (i = 0; i < src_table->required_opp_count; i++) {
> + if (src_table->required_opp_tables[i] == dst_table)
> + break;
> + }
> +
> + if (unlikely(i == src_table->required_opp_count)) {
> + pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
> + __func__, src_table, dst_table);
> + return NULL;
> + }
> +
> + mutex_lock(&src_table->lock);
> +
> + list_for_each_entry(opp, &src_table->opp_list, node) {
> + if (opp == src_opp) {
> + dest_opp = opp->required_opps[i];
> + dev_pm_opp_get(dest_opp);
> + goto unlock;
> + }
> + }
> +
> + pr_err("%s: Couldn't find matching OPP (%p: %p)\n", __func__, src_table,
> + dst_table);
> +
> +unlock:
> + mutex_unlock(&src_table->lock);
> +
> + return dest_opp;
> +}
> +
> /**
> * dev_pm_opp_xlate_performance_state() - Find required OPP's pstate for src_table.
> * @src_table: OPP table which has dst_table as one of its required OPP table.
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index ab1d15ce559db..6f5f72a7f601c 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -156,6 +156,9 @@ struct opp_table *devm_pm_opp_register_set_opp_helper(struct device *dev, int (*
> struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names, struct device ***virt_devs);
> void dev_pm_opp_detach_genpd(struct opp_table *opp_table);
> struct opp_table *devm_pm_opp_attach_genpd(struct device *dev, const char **names, struct device ***virt_devs);
> +struct dev_pm_opp *dev_pm_opp_xlate_required_opp(struct opp_table *src_table,
> + struct opp_table *dst_table,
> + struct dev_pm_opp *src_opp);
> int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
> int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
> int dev_pm_opp_set_opp(struct device *dev, struct dev_pm_opp *opp);
> @@ -367,6 +370,14 @@ static inline struct opp_table *devm_pm_opp_attach_genpd(struct device *dev,
> return ERR_PTR(-EOPNOTSUPP);
> }
>
> +static inline struct dev_pm_opp *dev_pm_opp_xlate_required_opp(
> + struct opp_table *src_table,
> + struct opp_table *dst_table,
> + struct dev_pm_opp *src_opp)
> +{
> + return NULL;
> +}

Like other routines that return opp *, don't return NULL on errors but
a valid error number instead. And follow the declaration format of
other routines from this file, don't break lines etc..

> +
> static inline int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate)
> {
> return -EOPNOTSUPP;
> --
> 2.30.0.365.g02bc693789-goog

--
viresh

2021-02-04 08:14:39

by Hsin-Yi Wang

[permalink] [raw]
Subject: Re: [PATCH v5 0/3] Add required-opps support to devfreq passive gov

On Thu, Feb 4, 2021 at 1:41 PM Viresh Kumar <[email protected]> wrote:
>
> On 03-02-21, 17:23, Hsin-Yi Wang wrote:
> > The devfreq passive governor scales the frequency of a "child" device based
> > on the current frequency of a "parent" device (not parent/child in the
> > sense of device hierarchy). As of today, the passive governor requires one
> > of the following to work correctly:
> > 1. The parent and child device have the same number of frequencies
> > 2. The child device driver passes a mapping function to translate from
> > parent frequency to child frequency.
> >
> > When (1) is not true, (2) is the only option right now. But often times,
> > all that is required is a simple mapping from parent's frequency to child's
> > frequency.
> >
> > Since OPPs already support pointing to other "required-opps", add support
> > for using that to map from parent device frequency to child device
> > frequency. That way, every child device driver doesn't have to implement a
> > separate mapping function anytime (1) isn't true.
>
> So you guys aren't interested in dev_pm_opp_set_opp() but just the
> translation of the required-OPPs ?
>
I think this series focuses on required-opps.

> I am fine with most of the stuff and I would like to take it via OPP
> tree, hope that would be fine ?
>
Sounds good to me, thanks.

> --
> viresh

2021-02-04 23:32:55

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v5 0/3] Add required-opps support to devfreq passive gov

Hi Viresh,

On 2/4/21 2:41 PM, Viresh Kumar wrote:
> On 03-02-21, 17:23, Hsin-Yi Wang wrote:
>> The devfreq passive governor scales the frequency of a "child" device based
>> on the current frequency of a "parent" device (not parent/child in the
>> sense of device hierarchy). As of today, the passive governor requires one
>> of the following to work correctly:
>> 1. The parent and child device have the same number of frequencies
>> 2. The child device driver passes a mapping function to translate from
>> parent frequency to child frequency.
>>
>> When (1) is not true, (2) is the only option right now. But often times,
>> all that is required is a simple mapping from parent's frequency to child's
>> frequency.
>>
>> Since OPPs already support pointing to other "required-opps", add support
>> for using that to map from parent device frequency to child device
>> frequency. That way, every child device driver doesn't have to implement a
>> separate mapping function anytime (1) isn't true.
>
> So you guys aren't interested in dev_pm_opp_set_opp() but just the
> translation of the required-OPPs ?
>
> I am fine with most of the stuff and I would like to take it via OPP
> tree, hope that would be fine ?

I agree. Take these patches to OPP tree.

--
Best Regards,
Chanwoo Choi
Samsung Electronics