Messages in this thread | | | Date | Wed, 25 Oct 2023 18:16:23 +0200 | From | Stephan Gerhold <> | Subject | Re: [PATCH 1/2] OPP: Use _set_opp_level() for single genpd case |
| |
On Wed, Oct 25, 2023 at 08:54:31PM +0530, Viresh Kumar wrote: > On 25-10-23, 15:47, Stephan Gerhold wrote: > > FWIW I'm hitting this WARNing when trying to set up the parent domain > > setup for CPR->RPMPD(MX) on MSM8916 that I discussed with Uffe recently > > [1]. I know, me and all my weird OPP setups. :'D > > > > Basically, I have cpufreq voting for performance states of the CPR genpd > > (via required-opps). CPR is supposed to have <&rpmpd MSM8916_VDDMX_AO> > > as parent genpd and translates to the parent performance state using the > > "required-opps" in the *CPR* OPP table: > > > > cpr: power-controller@b018000 { > > compatible = "qcom,msm8916-cpr", "qcom,cpr"; > > reg = <0x0b018000 0x1000>; > > /* ... */ > > #power-domain-cells = <0>; > > operating-points-v2 = <&cpr_opp_table>; > > /* Supposed to be parent domain, not consumer */ > > power-domains = <&rpmpd MSM8916_VDDMX_AO>; > > > > cpr_opp_table: opp-table { > > compatible = "operating-points-v2-qcom-level"; > > > > cpr_opp1: opp1 { > > opp-level = <1>; > > qcom,opp-fuse-level = <1>; > > required-opps = <&rpmpd_opp_svs_soc>; > > }; > > cpr_opp2: opp2 { > > opp-level = <2>; > > qcom,opp-fuse-level = <2>; > > required-opps = <&rpmpd_opp_nom>; > > }; > > cpr_opp3: opp3 { > > opp-level = <3>; > > qcom,opp-fuse-level = <3>; > > required-opps = <&rpmpd_opp_super_turbo>; > > }; > > }; > > }; > > I have forgotten a bit about this usecase. How exactly does the > configurations work currently for this ? I mean genpd core must be > setting the vote finally for only one of them or something else ? >
I'm not sure if I understand your question correctly. Basically, setting <&rpmpd MSM8916_VDDMX_AO> as "parent genpd" of <&cpr> is supposed to describe that there is a direct relationship between the performance states of CPR and VDDMX. When changing the CPR performance state, VDDMX should also be adjusted accordingly.
This is implemented in the genpd core in _genpd_set_performance_state(). It loops over the parent genpds, and re-evaluates the performance states of each of them. Translation happens using genpd_xlate_performance_state() which is just a direct call to dev_pm_opp_xlate_performance_state(). This will look up the required-opps from the OPP table above. However, the genpd core calls ->set_performance_state() on the parent genpd directly, so dev_pm_opp_set_opp() isn't involved in this case.
Overall the call sequence for a CPUfreq switch will look something like:
- cpu0: dev_pm_opp_set_rate(998.4 MHz) - cpu0: _set_required_opps(opp-998400000) - genpd:1:cpu0: dev_pm_opp_set_opp(&cpr_opp3) - genpd:1:cpu0: _set_opp_level(&cpr_opp3) - cpr: _genpd_set_performance_state(3)
# genpd: translate & adjust parent performance states - cpr: genpd_xlate_performance_state(parent=VDDMX_AO) => &rpmpd_opp_super_turbo = 6 - VDDMX_AO: _genpd_set_performance_state(6) - rpmpd: ->set_performance_state(VDDMX_AO, 6)
# genpd: change actual performance state - cpr: ->set_performance_state(cpr, 3)
Before the discussion with Uffe I did not describe this relationship between CPR<->VDDMX as parent-child, I just had them as two separate power domains in the CPU OPP table. That worked fine too but Uffe suggested the parent-child representation might be better. Does that help or were you looking for something else? :D
Thanks, Stephan
| |