Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <33dd969d9bdb1eb93f8f2a2167efeb535455cf74.camel@linux.intel.com>
Subject: Re: [PATCH 0/2] intel_pstate: fix turbo not being used after a
 processor is rebooted
From:   srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
To:     Pratyush Yadav <ptyadav@amazon.de>
Cc:     linux-pm@vger.kernel.org, "Rafael J. Wysocki" <rafael@kernel.org>,
        Len Brown <lenb@kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Robert Moore <robert.moore@intel.com>,
        linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
        devel@acpica.org
Date:   Tue, 27 Dec 2022 08:40:21 -0800
In-Reply-To: <mafs07cycdfh4.fsf_-_@amazon.de>
References: <20221221155203.11347-1-ptyadav@amazon.de>
         <72bcd14eef038ec9181d30b3d196b0a872f47ccb.camel@linux.intel.com>
         <mafs0k02jd8oh.fsf_-_@dev-dsk-ptyadav-1c-37607b33.eu-west-1.amazon.com>
         <2ed9702b67832e3e33ef352808124980206c1e95.camel@linux.intel.com>
         <8e2cc66f7dadcfb04099aac7c4eef0b02075c91b.camel@linux.intel.com>
         <mafs07cycdfh4.fsf_-_@amazon.de>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 (3.42.4-2.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 2022-12-27 at 16:38 +0100, Pratyush Yadav wrote:
> Hi Srinivas,
> 
> On Sat, Dec 24 2022, srinivas pandruvada wrote:
> 
> > On Fri, 2022-12-23 at 10:10 -0800, srinivas pandruvada wrote:
> > > Hi Pratyush,
> > > 
> > > On Thu, 2022-12-22 at 11:39 +0100, Pratyush Yadav wrote:
> > > > 
> > > > Hi Srinivas,
> > > > 
> > > > On Wed, Dec 21 2022, srinivas pandruvada wrote:
> > > > > On Wed, 2022-12-21 at 16:52 +0100, Pratyush Yadav wrote:
> > > > > > When a processor is brought offline and online again, it is
> > > > > > unable to
> > > > > > use Turbo mode because the _PSS table does not contain the
> > > > > > whole
> > > > > > turbo
> > > > > > frequency range, but only +1 MHz above the max non-turbo
> > > > > > frequency.
> > > > > > This
> > > > > > causes problems when ACPI processor driver tries to set
> > > > > > frequency
> > > > > > constraints. See patch 2 for more details.
> > > > > > 
> > > I can reproduce on a Broadwell server platform. But not on a
> > > client
> > > system with acpi_ppc usage.
> > > 
> > > Need to check what change broke this.
> > 
> > When PPC limits enforcement changed to PM QOS, this broke.
> > Previously
> > acpi_processor_get_platform_limit() was not enforcing any limits.
> > It
> > was just setting variable. So any update done after
> > acpi_register_performance_state() call to pr->performance-
> > > states[ppc].core_frequency, was effective.
> > 
> > We don't really need to call
> >         ret = freq_qos_update_request(&pr->perflib_req,
> >                         pr->performance->states[ppc].core_frequency
> > *
> > 1000);
> > 
> > if the PPC is not changed. When PPC is changed, this gets called
> > again,
> > so then we can call the above function to update cpufreq limit.
> > 
> > The below change fixed for me.
> 
> Right. 
I think, this is the only change you require to fix this. In addition
set pr->performance_platform_limit = 0 in
acpi_processor_unregister_performance().

Thanks,
Srinivas

> Should I re-roll my patches with your diff below then? Or do you
> think my patches should be good to merge as-is?
> 
> > 
> > diff --git a/drivers/acpi/processor_perflib.c
> > b/drivers/acpi/processor_perflib.c
> > index 757a98f6d7a2..c6ced89c00dd 100644
> > --- a/drivers/acpi/processor_perflib.c
> > +++ b/drivers/acpi/processor_perflib.c
> > @@ -75,6 +75,11 @@ static int
> > acpi_processor_get_platform_limit(struct
> > acpi_processor *pr)
> >         pr_debug("CPU %d: _PPC is %d - frequency %s limited\n", pr-
> > >id,
> >                        (int)ppc, ppc ? "" : "not");
> > 
> > +       if (ppc == pr->performance_platform_limit) {
> > +               pr_debug("CPU %d: _PPC is %d - frequency not
> > changed\n", pr->id, ppc);
> > +               return 0;
> > +       }
> > +
> >         pr->performance_platform_limit = (int)ppc;
> > 
> >         if (ppc >= pr->performance->state_count ||
> > 
>