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
Re: [PATCH v5 0/3] Add required-opps support to devfreq passive gov
On Thu, Feb 4, 2021 at 1: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 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
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
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 = <_top CLK_ACLK_G2D_400>;
> clock-names = "bus";
> operating-points-v2 = <_g2d_400_opp_table>;
> status = "disabled";
> };
>
> bus_noc2: bus9 {
> compatible = "samsung,exynos-bus";
> clocks = <_mif CLK_ACLK_BUS2_400>;
> clock-names = "bus";
> operating-points-v2 = <_noc2_opp_table>;
> status = "disabled";
> };
>
> bus_g2d_400_opp_table: opp_table2 {
> compatible = "operating-points-v2";
> opp-shared;
>
> opp-4 {
> opp-hz = /bits/ 64 <4>;
> opp-microvolt = <1075000>;
> required-opps = <_400>;
> };
> opp-26700 {
> opp-hz = /bits/ 64 <26700>;
> opp-microvolt = <100>;
> required-opps = <_200>;
> };
> opp-2 {
> opp-hz = /bits/ 64 <2>;
> opp-microvolt = <975000>;
> required-opps = <_200>;
> };
> opp-16000 {
> opp-hz = /bits/ 64 <16000>;
> opp-microvolt = <962500>;
> required-opps = <_134>;
> };
> opp-13400 {
> opp-hz = /bits/ 64 <13400>;
> opp-microvolt = <95>;
> required-opps = <_134>;
> };
> opp-1 {
> opp-hz = /bits/ 64 <1>;
> opp-microvolt = <937500>;
> required-opps = <_100>;
> };
> };
>
> bus_noc2_opp_table: opp_table6 {
> compatible = "operating-points-v2";
>
> noc2_400: opp-4 {
> opp-hz = /bits/ 64 <4>;
> };
> noc2_200: opp-2 {
> opp-hz = /bits/ 64 <2>;
> };
> noc2_134: opp-13400 {
> opp-hz = /bits/ 64 <13400>;
> };
> noc2_100: opp-1 {
> opp-hz = /bits/ 64 <1>;
> };
> };
>
> -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.
> -
[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 = <_top CLK_ACLK_G2D_400>;
clock-names = "bus";
operating-points-v2 = <_g2d_400_opp_table>;
status = "disabled";
};
bus_noc2: bus9 {
compatible = "samsung,exynos-bus";
clocks = <_mif CLK_ACLK_BUS2_400>;
clock-names = "bus";
operating-points-v2 = <_noc2_opp_table>;
status = "disabled";
};
bus_g2d_400_opp_table: opp_table2 {
compatible = "operating-points-v2";
opp-shared;
opp-4 {
opp-hz = /bits/ 64 <4>;
opp-microvolt = <1075000>;
required-opps = <_400>;
};
opp-26700 {
opp-hz = /bits/ 64 <26700>;
opp-microvolt = <100>;
required-opps = <_200>;
};
opp-2 {
opp-hz = /bits/ 64 <2>;
opp-microvolt = <975000>;
required-opps = <_200>;
};
opp-16000 {
opp-hz = /bits/ 64 <16000>;
opp-microvolt = <962500>;
required-opps = <_134>;
};
opp-13400 {
opp-hz = /bits/ 64 <13400>;
opp-microvolt = <95>;
required-opps = <_134>;
};
opp-1 {
opp-hz = /bits/ 64 <1>;
opp-microvolt = <937500>;
required-opps = <_100>;
};
};
bus_noc2_opp_table: opp_table6 {
compatible = "operating-points-v2";
noc2_400: opp-4 {
opp-hz = /bits/ 64 <4>;
};
noc2_200: opp-2 {
opp-hz = /bits/ 64 <2>;
};
noc2_134: opp-13400 {
opp-hz = /bits/ 64 <13400>;
};
noc2_100: opp-1 {
opp-hz = /bits/ 64 <1>;
};
};
-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
