Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: MyungJoo Ham <myungjoo.ham@samsung.com>,
	Viresh Kumar <viresh.kumar@linaro.org>
Cc: "Chanwoo Choi" <cw00.choi@samsung.com>,
	"Leonard Crestez" <leonard.crestez@nxp.com>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	"Artur Świgoń" <a.swigon@partner.samsung.com>,
	"Saravana Kannan" <saravanak@google.com>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Alexandre Bailon" <abailon@baylibre.com>,
	"Georgi Djakov" <georgi.djakov@linaro.org>,
	"Abel Vesa" <abel.vesa@nxp.com>, "Jacky Bai" <ping.bai@nxp.com>,
	"Lukasz Luba" <l.luba@partner.samsung.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v9 6/8] PM / devfreq: Introduce get_freq_range helper
Date: Fri, 11 Oct 2019 11:29:07 -0700
Message-ID: <20191011182907.GN87296@google.com> (raw)
In-Reply-To: <20191003181938.GJ87296@google.com>

On Thu, Oct 03, 2019 at 11:19:38AM -0700, Matthias Kaehlcke wrote:
> On Wed, Oct 02, 2019 at 10:25:09PM +0300, Leonard Crestez wrote:
> > Moving handling of min/max freq to a single function and call it from
> > update_devfreq and for printing min/max freq values in sysfs.
> > 
> > This changes the behavior of out-of-range min_freq/max_freq: clamping
> > is now done at evaluation time. This means that if an out-of-range
> > constraint is imposed by sysfs and it later becomes valid then it will
> > be enforced.
> > 
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  drivers/devfreq/devfreq.c | 110 +++++++++++++++++++++-----------------
> >  1 file changed, 62 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index 87eff789ce24..2d63692903ff 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> >
> > ...
> >
> >  static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
> >  			     char *buf)
> >  {
> >  	struct devfreq *df = to_devfreq(dev);
> > +	unsigned long min_freq, max_freq;
> >  
> > -	return sprintf(buf, "%lu\n", max(df->scaling_min_freq, df->min_freq));
> > +	mutex_lock(&df->lock);
> > +	get_freq_range(df, &min_freq, &max_freq);
> 
> With this min/max_freq shown aren't necessarily those set through sysfs,
> but the aggregated PM QoS values (plus OPP constraints).
> 
> I did some testing with a WIP patch that converts devfreq_cooling.c to
> PM QoS. When reading sysfs min/max values to double check the limits
> set earlier I found it utterly confusing to see the sysfs min/max values
> fluctuating due to thermal throttling, and not being able to see the
> configured values.
> 
> Looks like cpufreq does the same, but I'm not convinced this is a good
> idea. I think we want to display the values set by userspace, as done
> before managing the limits through PM QoS. Viresh, was this change of
> userspace visible behavior done intentionally for cpufreq?

ping

Viresh / devfreq maintainers:

Do we really want to expose through sysfs the potentially constantly
changing aggregate min/max values, instead of those set by userspace?
At least for cpufreq that's a divergence from previous behavior, and
from a userspace perspective it seems odd that you can't reliably read
back the limit set by userspace.

  parent reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 19:25 [PATCH v9 0/8] PM / devfreq: Add dev_pm_qos support Leonard Crestez
2019-10-02 19:25 ` [PATCH v9 1/8] PM / devfreq: Don't fail devfreq_dev_release if not in list Leonard Crestez
2019-10-02 19:25 ` [PATCH v9 2/8] PM / devfreq: Fix devfreq_notifier_call returning errno Leonard Crestez
2019-10-02 21:24   ` Matthias Kaehlcke
2019-10-02 23:47     ` Leonard Crestez
2019-10-02 19:25 ` [PATCH v9 3/8] PM / devfreq: Set scaling_max_freq to max on OPP notifier error Leonard Crestez
2019-10-02 21:33   ` Matthias Kaehlcke
2019-10-02 19:25 ` [PATCH v9 4/8] PM / devfreq: Move more initialization before registration Leonard Crestez
2019-10-02 19:25 ` [PATCH v9 5/8] PM / devfreq: Don't take lock in devfreq_add_device Leonard Crestez
2019-10-02 19:25 ` [PATCH v9 6/8] PM / devfreq: Introduce get_freq_range helper Leonard Crestez
2019-10-03 18:19   ` Matthias Kaehlcke
2019-10-03 19:16     ` Leonard Crestez
2019-10-03 19:36       ` Matthias Kaehlcke
2019-10-11 18:29     ` Matthias Kaehlcke [this message]
2019-10-02 19:25 ` [PATCH v9 7/8] PM / devfreq: Add PM QoS support Leonard Crestez
2019-10-02 22:22   ` Matthias Kaehlcke
2019-10-04 17:04   ` Matthias Kaehlcke
2019-10-02 19:25 ` [PATCH v9 8/8] PM / devfreq: Use PM QoS for sysfs min/max_freq Leonard Crestez
2019-10-04 17:05   ` Matthias Kaehlcke

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191011182907.GN87296@google.com \
    --to=mka@chromium.org \
    --cc=a.swigon@partner.samsung.com \
    --cc=abailon@baylibre.com \
    --cc=abel.vesa@nxp.com \
    --cc=cw00.choi@samsung.com \
    --cc=georgi.djakov@linaro.org \
    --cc=krzk@kernel.org \
    --cc=kyungmin.park@samsung.com \
    --cc=l.luba@partner.samsung.com \
    --cc=leonard.crestez@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-pm@vger.kernel.org \
    --cc=myungjoo.ham@samsung.com \
    --cc=ping.bai@nxp.com \
    --cc=saravanak@google.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org linux-pm@archiver.kernel.org
	public-inbox-index linux-pm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox