From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Stephane Gasparini <stephane.gasparini@linux.intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        Josh Boyer <jwboyer@fedoraproject.org>,
        Philippe Longepe <philippe.longepe@linux.intel.com>,
        Len Brown <lenb@kernel.org>, Viresh Kumar <viresh.kumar@linaro.org>,
        Linux PM list <linux-pm@vger.kernel.org>,
        "Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>
Subject: Re: intel_pstate oopses and lockdep report with Linux v4.5-1822-g63e30271b04c
Date: Mon, 21 Mar 2016 15:11:48 +0100
Message-ID: <2299553.MTuVWJXJ99@vostro.rjw.lan>
User-Agent: KMail/4.11.5 (Linux/4.5.0-rc1+; KDE/4.11.5; x86_64; ; )
In-Reply-To: <94B138B5-2AA0-4756-BF7E-1EA8764860F2@linux.intel.com>
References: <CA+5PVA5=CJ=yf0EAZqNJX0XMc+ofDDJ-CHBrwuaANQwdhh2XbA@mail.gmail.com> <1458323535.3861.76.camel@linux.intel.com> <94B138B5-2AA0-4756-BF7E-1EA8764860F2@linux.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8BIT
Content-Type: text/plain; charset="utf-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2583
Lines: 71

On Monday, March 21, 2016 10:28:09 AM Stephane Gasparini wrote:
> 
> —
> Steph
> 
> 
> 
> 
> > On Mar 18, 2016, at 6:52 PM, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> > 
> > On Fri, 2016-03-18 at 17:13 +0100, Stephane Gasparini wrote:
> >> Rafael,
> >> 
> >> Why in step 3) both atom_set_pstate() and atom_set_pstate() were not
> >> both
> >> changed to use wrmsrl ?
> > Initial Atom support was experimental as there were no users, till
> > Chrome started using. So it was just a miss.
> > 
> > We should never have to use wrmsrl_on_cpu. But looks like
> > cpufreq_driver.init() can't guarantee that.
> > 
> >> BTW, what is the interest of setting the pstate to LFM during
> >> initialization ?
> >> The BIOS is setting the pstate to either LFM, HFM or BFM, and why
> >> bothering
> >> changing it.
> > This is a different issue. BIOS has different configuration option to
> > enable fast boot modes which are not necessarily optimized for Linux.
> > Some aggressive setting will force system to reboot on boot. So I will
> > leave the way it is.
> 
> Here is my understanding.
> 
> 1) until the driver starts, the CPUS will anyway starts at the P-State set by the BIOS.
> 2) even if you force it to Lowest P-State in init Intel P-State init, if there is load associated
>    to the execution, 10ms after (or may be quicker with the new scheduler based option) the
>    P-State will again set to P0.
> 
> so because 1) and 2) you’ll have basically the following behavior assuming we have high load
> during boot, as this what can cause a reboot due to high frequencies I assume
> 
> a) BIOS set LFM
> 10 ms after init of intel P-State, CPUs will go to Turbo according to load.
> 
> b) BIOS set HFM
> CPU will boot to HFM until we reach intel_state init.
> During 10ms, CPU will be at LFM.
> Due to load they will go up to BFM.
> 
> c) BIOS set to BFM.
> CPU will boot to BFM until we reach intel_state init.
> During 10ms, CPU will be at LFM.
> Due to load they will go up to BFM.
> 
> So I may have miss something but I do not see what is the real benefit of doing this init to LFM 
> that will last for 10ms.
> 
> I still think this initialization is useless and complexity the code.
> 
> Can you point me to case where having this initialization did solve an issue so that I understand 
> the interest of doing this initialization ?

What you're saying above makes sense, but that change wouldn't belong to the
patch in question anyway.

Please consider submitting another patch to make that change if you think it's
worth the effort.

Thanks,
Rafael