Re: [RFC PATCH] cpufreq: add support for HiSilicon SoC HIP09

[Hanjun Guo]

Hi Thanu, Souvik, Sudeep,

Thanks a lot for the prompt reply, and it clarify a lot for
us, comment inline below.

On 2020/5/6 22:57, Souvik Chakravarty wrote:
> Hi,
>
> > From: Thanu Rangarajan <Thanu.Rangarajan@xxxxxxx>
> > Sent: Wednesday, May 6, 2020 1:58 PM
> >
> > Hi,
> > ACPI CPPC already supports the notion of boost. Not sure we need any
> > enhancements there.
> >
> > Regards,
> > Thanu
> >
> > ïOn 06/05/2020, 18:19, "Sudeep Holla" <sudeep.holla@xxxxxxx> wrote:
> >
> >      + Thanu, Souvik who work with ASWG
> >
> >      On Wed, May 06, 2020 at 05:36:51PM +0800, Hanjun Guo wrote:
> >      > Hi Sudeep,
> >      >
> >      > On 2020/4/30 17:55, Sudeep Holla wrote:
> >      > > On Thu, Apr 30, 2020 at 02:19:59PM +0800, Xiongfeng Wang wrote:
> >      > > > HiSilicon SoC has a separate System Control Processor(SCP)
> > dedicated for
> >      > > > clock frequency adjustment and has been using the cpufreq driver
> >      > > > 'cppc-cpufreq'. New HiSilicon SoC HIP09 add support for CPU Boost,
> > but
> >      > > > ACPI CPPC doesn't support this. In HiSilicon SoC HIP09, each core has
> >      > > > its own clock domain. It is better for the core itself to adjust its
> >      > > > frequency when we require fast response. In this patch, we add a
> >      > > > separate cpufreq driver for HiSilicon SoC HIP09.
> >      > > >
> >      > >
> >      > > I disagree with this approach unless you have tried to extend the CPPC
> >      > > in ACPI to accommodate this boost feature you need. Until you show
> > those
> >      > > efforts and disagreement to do that from ASWG, I am NACKing this
> > approach.
> >      >
> >      > Unfortunately we are not in ASWG at now, could you please give some
> >      > help about extending CPPC in ACPI to support boost feature?
> >      >
> >
> >      You may have to provide more details than the commit log for sure as I
> >      haven't understood the boost feature and what is missing in ACPI CPPC.
>
> I would agree with Thanu here regarding the ACPI spec part - everything is already there.

I take another detail read of the spec and as you said,
everything is already there,thanks!. I was misleading by the
CPPC code which it's using the 'Highest Performance' as
the max performance not the 'Nominal Performance', so seems
that 'Highest Performance' is the normal max performance but
not the boost performance.

> It seems to me that the .set_boost is today not handled in cppc_cpufreq.c. In fact if a platform provides a value for Highest Performance which is different than Nominal Performance, then the entire range of performance is always requested, which works well for platforms which do boost enable/disable selection at hardware/firmware level.

Yes, so for now, the CPPC code will enable the boost feature in
default if the firmware provides a value for Highest Performance
which is different than Nominal Performance.

> .boost hook could potentially be useful in cppc_cpufreq.c for platforms which would manage the boost selection in software. But it would be good to keep a common implementation which can choose between "software-triggered or auto-boost" selection for different platforms.

Thanks for the clarify, it helps a lot, Xiongfeng prepared
some patches to enable .boost hook, but needs to set the
'Nominal Performance' as the max performance, and
'Highest Performance' is the max boost performance, will
send out for comments.

Best Regards,
Hanjun