Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
MIME-Version: 1.0
References: <cover.1697710527.git.viresh.kumar@linaro.org> <6de4fcb5bb943a131d0cdf0a858bd35af02a2f88.1697710527.git.viresh.kumar@linaro.org>
 <ZTeoGiMQZ-OoYJBG@kernkonzept.com> <20231025073634.4et5epfog25o2pxf@vireshk-i7>
 <ZTkHVVq61lrpRUOv@gerhold.net> <20231025152000.vt42gvti2e4rewwc@vireshk-i7>
In-Reply-To: <20231025152000.vt42gvti2e4rewwc@vireshk-i7>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Wed, 25 Oct 2023 18:03:30 +0200
Message-ID: <CAPDyKFrEdHGVeKo0oh0f2uJjtASbg4eye2UbEaE8s3sy+2tWDA@mail.gmail.com>
Subject: Re: [PATCH 2/2] OPP: Call dev_pm_opp_set_opp() for required OPPs
To:     Viresh Kumar <viresh.kumar@linaro.org>
Cc:     Stephan Gerhold <stephan@gerhold.net>,
        Stephan Gerhold <stephan.gerhold@kernkonzept.com>,
        Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
        Stephen Boyd <sboyd@kernel.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>, linux-pm@vger.kernel.org,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 25 Oct 2023 at 17:20, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 25-10-23, 14:17, Stephan Gerhold wrote:
> > Thanks, this seems to work fine.
>
> Thanks a lot.
>
> > I found another small problem: In my OPP setup for MSM8916, some of the
> > required-opps point to an OPP with "opp-level = <0>" (see [1], the
> > <&rpmpd_opp_none> in the cpu-opp-table). This is because the vote for
> > the "CX" domain is for the CPU PLL clock source, which is only used for
> > the higher CPU frequencies (>= 998.4 MHz). With the previous code you
> > made it possible for me to vote for opp-level = <0> in commit
> > a5663c9b1e31 ("opp: Allow opp-level to be set to 0", discussion in [2]).
> > I think now it's broken because the _set_opp_level() added by Uffe
> > checks for if (!opp->level) as a sign that there is no opp-level defined
> > at all.

I don't think my patch broke it, I just followed the similar semantics
of how we treated the OPPs for the required opps.

As far as I understood it, the only way we could request the
performance-level to be zero, was if the consumer driver would call
dev_pm_opp_set_opp(dev, NULL);

So, unless I am overlooking something, things can have been screwed up
earlier too?

>
> Yes, we broke that. I think a simple fix is to initialize the level
> with an impossible value, like -1 and then 0 becomes valid.

Yep, make sense, let's fix it!

Are you sending a patch?

[...]

Kind regards
Uffe