MIME-Version: 1.0
References: <20230215071649.9078-1-quic_mkshah@quicinc.com>
 <20230227153848.auqs4e5hf2qmwmg2@ripper> <CAD=FV=UK9zyQ=Mg3BaVCwOGBG3G7rW2pdpMFtGptR88p8ce9kg@mail.gmail.com>
 <CAPDyKFouhMbFFMHeT5vEo_LUQWVe5wKpagx8HUtDpcV7LCa4zQ@mail.gmail.com> <CAD=FV=V+fdfjUbAtvmeG43Nk=J3=Td4RSCZajrsxu8n8ykmH0w@mail.gmail.com>
In-Reply-To: <CAD=FV=V+fdfjUbAtvmeG43Nk=J3=Td4RSCZajrsxu8n8ykmH0w@mail.gmail.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Wed, 1 Mar 2023 15:47:06 +0100
Message-ID: <CAPDyKFruXa2a83TRrpzBF7hsDi8QxOQmrwRwxSjWm_ziFF6fSA@mail.gmail.com>
Subject: Re: [PATCH 0/1] Use PSCI OS initiated mode for sc7280
To:     Doug Anderson <dianders@chromium.org>
Cc:     Bjorn Andersson <andersson@kernel.org>,
        Maulik Shah <quic_mkshah@quicinc.com>, swboyd@chromium.org,
        wingers@google.com, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org, quic_lsrao@quicinc.com,
        quic_rjendra@quicinc.com, Julius Werner <jwerner@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, 28 Feb 2023 at 16:25, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Feb 28, 2023 at 4:17=E2=80=AFAM Ulf Hansson <ulf.hansson@linaro.o=
rg> wrote:
> >
> > On Mon, 27 Feb 2023 at 17:10, Doug Anderson <dianders@chromium.org> wro=
te:
> > >
> > > Hi,
> > >
> > > On Mon, Feb 27, 2023 at 7:35=E2=80=AFAM Bjorn Andersson <andersson@ke=
rnel.org> wrote:
> > > >
> > > > On Wed, Feb 15, 2023 at 12:46:48PM +0530, Maulik Shah wrote:
> > > > > This change adds power-domains for cpuidle states to use PSCI OS
> > > > > initiated mode for sc7280.
> > > > >
> > > > > This change depends on external project changes [1] & [2] which a=
re under
> > > > > review/discussion to add PSCI os-initiated support in Arm Trusted=
 Firmware.
> > > > >
> > > > > I can update here once the dependency are in and change is ready =
to merge.
> > > > >
> > > >
> > > > Please do, I will drop this from the queue for now.
> > >
> > > I'm a bit confused about why we're doing this. There's always been a
> > > question about exactly why we need OSI mode. As far as I can tell it
> > > can't be for "correctness" reasons because we managed to ship sc7180
> > > without OSI mode. ...so I guess somehow the argument is that OSI mode
> > > is more performant in some cases? Are there actual numbers backing
> > > this up, or is it all theoretical? Before making such a big change, i=
t
> > > would be good to actually understand what the motivation is and see
> > > real data. This should be easy to collect since we currently have
> > > things working without OSI and (presumably) you have OSI working. It
> > > would also be good to document this motivation in the commit message
> > > and/or cover letter.
> >
> > I certainly don't object to what you say here. Although, let me also
> > share some more background to these suggested changes.
> >
> > As you know, for mobile platforms, Qcom have been using OS-initiated
> > mode for years, but on Chromium platforms that has been limited to the
> > default platform-coordinated mode. Whether that is a deliberate
> > decision for the Chromium platforms or rather because the PSCI
> > implementation in TF-A has been lacking OSI support, I don't know.
> > Maybe you have some more insight to share around this?
>
> You hit the reason exactly. Nobody on the ChromeOS team objected to
> OSI, per say, but it was never supported in ARM Trusted Firmware. I
> still don't have anything against OSI mode, but I just want to make
> sure that the data is there and that we're not just arbitrarily
> churning things. ;-)

Thanks for sharing this information! It certainly helps to better
understand the background for all of us.

>
> I think sc7180's ship has sailed at this point. While we could update
> the firmware for testing, I don't think we'd switch production sc7180
> devices over to OSI. That means that we'll always need to support PC
> mode for sc7180. Switching sc7280 over to OSI needs to be justified
> given that we'll have to continue to support sc7180 with PC mode
> anyway.

Right. Supporting both OSI and PC mode, dynamically based upon what
the PSCI firmware (TF-A) supports should work fine from Linux point of
view.

I may be overlooking something for this particular case - and in that
case, I am happy to help to fix it!

>
>
> > Note that, Wing has been working on adding support for PSCI OSI mode
> > to TF-A [1], which hopefully should land soon. In this regard, it
> > seems like we are getting closer to finally being able to run some
> > more in-depth tests, that should allow us to better compare the
> > behaviour of the PSCI CPU-suspend modes - at least on some platforms.
> > In fact, Maulik/Wing also presented their work around this topic,
> > including some results around performance/energy tests at the last
> > TF-A call [2]. I think some of that data could be shared in the commit
> > message too.
>
> Yup, I was mostly just asking for data like you provided to be in the
> commit message.

Great, thanks for confirming!

>
>
> > Kind regards
> > Uffe
> >
> > [1]
> > https://review.trustedfirmware.org/q/topic:psci-osi
> >
> > [2]
> > https://www.trustedfirmware.org/meetings/tf-a-technical-forum

Kind regards
Uffe