MIME-Version: 1.0
References: <cover.1677063656.git.viresh.kumar@linaro.org>
In-Reply-To: <cover.1677063656.git.viresh.kumar@linaro.org>
From:   Jun Nie <jun.nie@linaro.org>
Date:   Thu, 23 Feb 2023 17:56:28 +0800
Message-ID: <CABymUCMhoKoFHy8K6-ohrcAbyTpDe0Hig3oUM_wH4Db0-9yx+g@mail.gmail.com>
Subject: Re: [PATCH 0/3] OPP: Simplify set_required_opp handling
To:     Viresh Kumar <viresh.kumar@linaro.org>, cw00.choi@samsung.com
Cc:     Nishanth Menon <nm@ti.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Viresh Kumar <vireshk@kernel.org>, linux-pm@vger.kernel.org,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Viresh Kumar <viresh.kumar@linaro.org> =E4=BA=8E2023=E5=B9=B42=E6=9C=8822=
=E6=97=A5=E5=91=A8=E4=B8=89 19:06=E5=86=99=E9=81=93=EF=BC=9A
>
> Hello,
>
> The required-opps configuration is closely tied to genpd and performance
> states at the moment and it is not very obvious that required-opps can
> live without genpds. Though we don't support configuring required-opps
> for non-genpd cases currently.
>
> This patchset aims at cleaning up this a bit, just like what's done for c=
lk and
> regulators. This also makes it possible for platforms to provide their ow=
n
> version of set_required_opps() helper, which can be used to configure the
> devfreq device propertly.
>
> Jun,
>
> I haven't found time to test this through yet, though there isn't much an=
yway I
> guess. Can you see if these can solve your problem properly ?

Hi Viresh,

It looks promising. The function get_target_freq_with_cpufreq() can be wrap=
ped
to act as set_required_opps() callback. But my case is a bit
complicated. CPU opp
depends on both genpd opp and devfreq opp. So the genpd_virt_devs array nee=
d
to be modified or add another array for devfreq case. While genpd_virt_devs=
 is
bounded with genpd directly and coupled with "power-domains" list in
device tree.
Current required-opp nodes are designed to be aligned with the list. I
am considering
what's the best way for back compatibility.

Hi Chanwoo,

Do you have any comments on this proposal? This proposal arise because opp
lib reports error when cpufreq driver try to set required opp for
non-genpd case.
Another possible fix is to ignore non-genpd opp in opp lib. But a
unified and recursive
opp management looks nicer, just like clock tree management.

>
> Viresh Kumar (3):
>   OPP: Handle all genpd cases together in _set_required_opps()
>   OPP: Move required opps configuration to specialized callback
>   OPP: Allow platforms to add a set_required_opps() callback
>
>  drivers/opp/core.c     | 113 ++++++++++++++++++++++++++++-------------
>  drivers/opp/of.c       |   3 ++
>  drivers/opp/opp.h      |   4 ++
>  include/linux/pm_opp.h |   5 ++
>  4 files changed, 91 insertions(+), 34 deletions(-)
>
> --
> 2.31.1.272.g89b43f80a514
>