Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20360841.iInq7taT2Z@kreacher> <3342398.tGQZsKHvNY@kreacher> <20201208124344.GX2414@hirez.programming.kicks-ass.net>
In-Reply-To: <20201208124344.GX2414@hirez.programming.kicks-ass.net>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Tue, 8 Dec 2020 18:10:56 +0100
Message-ID: <CAJZ5v0h6KM1V2_JoN+WFxAq8pKvOfsOcCVXYchUzb4xyfdzsGw@mail.gmail.com>
Subject: Re: [PATCH v1 4/4] cpufreq: intel_pstate: Implement the
 ->adjust_perf() callback
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux PM <linux-pm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        Doug Smythies <dsmythies@telus.net>,
        Giovanni Gherdovich <ggherdovich@suse.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Dec 8, 2020 at 1:44 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Dec 07, 2020 at 05:38:58PM +0100, Rafael J. Wysocki wrote:
>
> > +static void intel_cpufreq_adjust_perf(unsigned int cpunum,
> > +                                   unsigned long min_perf,
> > +                                   unsigned long target_perf,
> > +                                   unsigned long capacity)
> > +{
> > +     struct cpudata *cpu = all_cpu_data[cpunum];
> > +     int old_pstate = cpu->pstate.current_pstate;
> > +     int cap_pstate, min_pstate, max_pstate, target_pstate;
> > +
> > +     update_turbo_state();
> > +     cap_pstate = global.turbo_disabled ? cpu->pstate.max_pstate :
> > +                                          cpu->pstate.turbo_pstate;
> > +
> > +     /* Optimization: Avoid unnecessary divisions. */
> > +
> > +     target_pstate = cap_pstate;
> > +     if (target_perf < capacity)
> > +             target_pstate = DIV_ROUND_UP(cap_pstate * target_perf, capacity);
> > +
> > +     min_pstate = cap_pstate;
> > +     if (min_perf < capacity)
> > +             min_pstate = DIV_ROUND_UP(cap_pstate * min_perf, capacity);
> > +
> > +     if (min_pstate < cpu->pstate.min_pstate)
> > +             min_pstate = cpu->pstate.min_pstate;
> > +
> > +     if (min_pstate < cpu->min_perf_ratio)
> > +             min_pstate = cpu->min_perf_ratio;
> > +
> > +     max_pstate = min(cap_pstate, cpu->max_perf_ratio);
> > +     if (max_pstate < min_pstate)
> > +             max_pstate = min_pstate;
> > +
> > +     target_pstate = clamp_t(int, target_pstate, min_pstate, max_pstate);
> > +
> > +     intel_cpufreq_adjust_hwp(cpu, min_pstate, max_pstate, target_pstate, true);
>
> I'm confused... HWP doesn't do pstate, yet everything here is now called
> pstate, help?

HWP.REQ.MIN, HWP.REQ.MAX and HWP.REQ.DESIRED all are in the same space
of values as the original PERF_CTL MSR, which is P-states, at least
effectively.

> > +
> > +     cpu->pstate.current_pstate = target_pstate;
> > +     intel_cpufreq_trace(cpu, INTEL_PSTATE_TRACE_FAST_SWITCH, old_pstate);
> > +}