Re: [PATCH 0/1] Use PSCI OS initiated mode for sc7280

[Doug Anderson]

Hi,

On Tue, Feb 28, 2023 at 4:17 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>
> On Mon, 27 Feb 2023 at 17:10, Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > On Mon, Feb 27, 2023 at 7:35 AM Bjorn Andersson <andersson@xxxxxxxxxx> 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 are 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, it
> > 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. ;-)

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.


> 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.


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