Re: [PATCH] cros_ec_spi: Even though we're RT priority, don't bump cpu freq

[Doug Anderson]

Hi,

On Fri, Jun 12, 2020 at 5:34 AM Qais Yousef <qais.yousef@xxxxxxx> wrote:
>
> On 06/12/20 10:24, Quentin Perret wrote:
> > +CC Qais [FYI]
>
> Thanks for the CC.
>
> >
> > On Thursday 11 Jun 2020 at 10:48:40 (-0700), Doug Anderson wrote:
> > > Hrm.  I guess my first instinct is to say that we still want this
> > > patch even if we have something that is applied more globally.
> >
> > Fair enough.
> >
> > > Specifically it sounds as if the patch you point at is suggesting that
> > > we'd tweak the boost value to something other than max but we'd still
> > > have a boost value.  In the case of cros_ec_spi I don't believe we
> > > need any boost value at all, so my patch would still be useful.  The
> > > computational needs of cros_ec_spi are very modest and it can do its
> > > work at lower CPU frequencies just fine.  It just can't be interrupted
> > > for large swaths of time.
> > >
> > >
> > > > IOW, how do you feel about 20200511154053.7822-1-qais.yousef@xxxxxxx ?
> > >
> > > I'm not totally a fan, but I'm definitely not an expert in this area
> > > (I've also only read the patch description and not the patch or the
> > > whole thread).  I really don't want yet another value that I need to
> > > tune from board to board.  Even worse, this tuning value isn't
> > > board-specific but a combination of board and software specific.  By
> > > this, I'd imagine a scenario where you're using a real-time task to
> > > get audio decoding done within a certain latency.  I guess you'd tune
> > > this value to make sure that you can get all your audio decoding done
> > > in time but also not burn extra power.  Now, imagine that the OS
> > > upgrades and the audio task suddenly has to decode more complex
> > > streams.  You've got to go across all of your boards and re-tune every
> > > one?  ...or, nobody thinks about it and older boards start getting
> > > stuttery audio?  Perhaps the opposite happens and someone comes up
> > > with a newer lower-cpu-intensive codec and you could save power.
> > > Sounds like a bit of a nightmare.
>
> Generally I would expect this global tunable to be part of a vendor's SoC BSP.

I think I'm coming at this from a very different world than the one
you're thinking of.  The concept of a BSP makes me think of a SoC
vendor providing a drop of Linux with all their own weird hacks in it
that only ever works on that one SoC and will never, ever, ever be
updated.  5 years down the road if a software update is needed
(security fix?) some poor soul will be in charge of tracking down the
exact ancient code base that was used to build the original kernel,
making the tweak, and building a new kernel.  ;-)

In the world of Chrome OS we try very very hard to build everything
from a clean code base and are trying hard to stay even closer to
upstream and away from per-device weirdness...


> People tend to think of the flagship SoCs which are powerful, but if you
> consider the low and medium end devices there's a massive spectrum over there
> that this range is trying to cover.

Yeah, perhaps you're right.  When thinking about a laptop it's almost
always a fairly powerful device and things could certainly be
different for very low end chips trying to run Linux.


> I would expect older boards init script to be separate for newer boards init
> script. The OS by default boosts all RT tasks unless a platform specific script
> overrides that. So I can't see how an OS upgrade would affect older boards.

In the Chrome OS world I'm part of, the people supporting the boards
are not always the people adding new whizbang features.  We get new
versions of the OS every 6 weeks and those new versions may change the
way things work pretty significantly.  We ship those new versions off
to all boards.  Certainly they undergo testing, but there are a lot of
boards and if something is not tuned as well as it was when the device
first shipped it's likely nobody will really notice.  This is my bias
against needing per-board tuning.


> This knob still allows you to disable the max boosting and use the per-task
> uclamp interface to boost only those tasks you care about. AFAIK this is
> already done in a hacky way in android devices via special vendors provisions.

Yeah.  I don't think I have enough skin in the game to really say one
way or the other what the API should be so I'll probably stay off the
other, bigger thread and let others decide what the best API should
be.  For Chrome OS I'd probably advocate just disabling the default
boost but even there I'd be willing to defer to the folks doing the
actual work.


-Doug