* [PATCH v2 25/27] kernel: power: use kzalloc rather than kmalloc followed with memset
From: Fuqian Huang @ 2019-06-28 2:50 UTC (permalink / raw)
Cc: Fuqian Huang, Rafael J. Wysocki, Len Brown, Pavel Machek,
linux-pm, linux-kernel
Use zeroing allocator instead of using allocator
followed with memset with 0
Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
kernel/power/swap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index e1912ad13bdc..ca0fcb5ced71 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -974,12 +974,11 @@ static int get_swap_reader(struct swap_map_handle *handle,
last = handle->maps = NULL;
offset = swsusp_header->image;
while (offset) {
- tmp = kmalloc(sizeof(*handle->maps), GFP_KERNEL);
+ tmp = kzalloc(sizeof(*handle->maps), GFP_KERNEL);
if (!tmp) {
release_swap_reader(handle);
return -ENOMEM;
}
- memset(tmp, 0, sizeof(*tmp));
if (!handle->maps)
handle->maps = tmp;
if (last)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] PM / wakeup: show wakeup sources stats in sysfs
From: Tri Vo @ 2019-06-27 22:57 UTC (permalink / raw)
To: Greg KH
Cc: Rafael J. Wysocki, Viresh Kumar, Rafael J. Wysocki,
Hridya Valsaraju, Sandeep Patil, LKML, Linux PM,
Cc: Android Kernel
In-Reply-To: <20190627000412.GA527@kroah.com>
On Wed, Jun 26, 2019 at 5:04 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 26, 2019 at 03:48:58PM -0700, Tri Vo wrote:
> > On Tue, Jun 25, 2019 at 6:33 PM Tri Vo <trong@android.com> wrote:
> > >
> > > On Tue, Jun 25, 2019 at 6:12 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Tue, Jun 25, 2019 at 05:54:49PM -0700, Tri Vo wrote:
> > > > > Embedding a struct kobject into struct wakeup_source changes lifetime
> > > > > requirements on the latter. To that end, change deallocation of struct
> > > > > wakeup_source using kfree to kobject_put().
> > > >
> > > > Ick, are you sure you need a new kobject here? Why wouldn't a named
> > > > attribute group work instead? That should keep this patch much smaller
> > > > and simpler.
> > >
> > > Yeah, named attribute groups might be a much cleaner way to do this.
> > > Let me investigate.
> >
> > Say, we read /sys/power/wakeup_sources/foo/active_count.
>
> What is "foo" here? You didn't include Documentation of the sysfs
> files so it was pretty impossible to say what exactly your heirachy was
> going to be in order to determine this :)
Sorry about that. I sent out a new version of the patch with updated
documentation.
^ permalink raw reply
* [PATCH v2] PM / wakeup: show wakeup sources stats in sysfs
From: Tri Vo @ 2019-06-27 22:53 UTC (permalink / raw)
To: rjw, gregkh, viresh.kumar
Cc: rafael, hridya, sspatil, kaleshsingh, linux-kernel, linux-pm,
kernel-team, Tri Vo
In-Reply-To: <20190627000412.GA527@kroah.com>
Userspace can use wakeup_sources debugfs node to plot history of suspend
blocking wakeup sources over device's boot cycle. This information can
then be used (1) for power-specific bug reporting and (2) towards
attributing battery consumption to specific processes over a period of
time.
However, debugfs doesn't have stable ABI. For this reason, expose wakeup
sources statistics in sysfs under /sys/power/wakeup_sources/<name>/
Embedding a struct kobject into struct wakeup_source changes lifetime
requirements on the latter. To that end, change deallocation of struct
wakeup_source using kfree to kobject_put().
Change struct wakelock's wakeup_source member to a pointer to decouple
lifetimes of struct wakelock and struct wakeup_source for above reason.
Introduce CONFIG_PM_SLEEP_STATS that enables/disables showing wakeup
source statistics in sysfs.
Signed-off-by: Tri Vo <trong@android.com>
---
Documentation/ABI/testing/sysfs-power | 80 ++++++++++-
drivers/base/power/Makefile | 2 +-
drivers/base/power/power.h | 7 +
drivers/base/power/wakeup.c | 16 ++-
drivers/base/power/wakeup_stats.c | 194 ++++++++++++++++++++++++++
include/linux/pm_wakeup.h | 15 ++
kernel/power/Kconfig | 10 ++
kernel/power/wakelock.c | 42 ++++--
8 files changed, 347 insertions(+), 19 deletions(-)
create mode 100644 drivers/base/power/wakeup_stats.c
v2:
- Updated Documentation/ABI/, as per Greg.
- Removed locks in attribute functions, as per Greg.
- Lifetimes of struct wakelock and struck wakeup_source are now different due to
the latter embedding a refcounted kobject. Changed it so that struct wakelock
only has a pointer to struct wakeup_source, instead of embedding it.
- Added CONFIG_PM_SLEEP_STATS that enables/disables wakeup source statistics in
sysfs.
diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power
index 18b7dc929234..ba35f1eef73b 100644
--- a/Documentation/ABI/testing/sysfs-power
+++ b/Documentation/ABI/testing/sysfs-power
@@ -300,4 +300,82 @@ Description:
attempt.
Using this sysfs file will override any values that were
- set using the kernel command line for disk offset.
\ No newline at end of file
+ set using the kernel command line for disk offset.
+
+What: /sys/power/wakeup_sources/.../
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../ directory contains attributes
+ that expose statistics about a given wakeup source to user
+ space. Attributes in this directory are read-only.
+
+What: /sys/power/wakeup_sources/.../active_count
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../active_count file contains the
+ number of times the wakeup source was activated.
+
+What: /sys/power/wakeup_sources/.../event_count
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../event_count file contains the
+ number of signaled wakeup events associated with the wakeup
+ source.
+
+What: /sys/power/wakeup_sources/.../wakeup_count
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../wakeup_count file contains the
+ number of times the wakeup source might abort suspend.
+
+What: /sys/power/wakeup_sources/.../expire_count
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../expire_count file contains the
+ number of times the wakeup source's timeout has expired.
+
+What: /sys/power/wakeup_sources/.../active_time_ms
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../active_time_ms file contains
+ the amount of time the wakeup source has been continuously
+ active, in milliseconds. If the wakeup source is not active,
+ this file contains '0'.
+
+What: /sys/power/wakeup_sources/.../total_time_ms
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../total_time_ms file contains
+ the total amount of time this wakeup source has been active, in
+ milliseconds.
+
+What: /sys/power/wakeup_sources/.../max_time_ms
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../max_time_ms file contains the
+ maximum amount of time this wakeup source has been continuously
+ active, in milliseconds.
+
+What: /sys/power/wakeup_sources/.../last_change_ms
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../last_change_ms file contains
+ the monotonic clock time when the wakeup source was touched last
+ time, in milliseconds.
+
+What: /sys/power/wakeup_sources/.../prevent_suspend_time_ms
+Date: June 2019
+Contact: Tri Vo <trong@android.com>
+Description:
+ The /sys/power/wakeup_sources/.../prevent_suspend_time_ms file
+ contains the total amount of time this wakeup source has been
+ preventing autosleep, in milliseconds.
diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
index e1bb691cf8f1..ec5bb190b9d0 100644
--- a/drivers/base/power/Makefile
+++ b/drivers/base/power/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_PM) += sysfs.o generic_ops.o common.o qos.o runtime.o wakeirq.o
-obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o
+obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o wakeup_stats.o
obj-$(CONFIG_PM_TRACE_RTC) += trace.o
obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o domain_governor.o
obj-$(CONFIG_HAVE_CLK) += clock_ops.o
diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index ec33fbdb919b..c288298f57c0 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -149,3 +149,10 @@ static inline void device_pm_init(struct device *dev)
device_pm_sleep_init(dev);
pm_runtime_init(dev);
}
+
+#ifdef CONFIG_PM_SLEEP
+
+/* drivers/base/power/wakeup_stats.c */
+extern struct kobj_type wakeup_source_ktype;
+
+#endif /* CONFIG_PM_SLEEP */
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 5b2b6a05a4f3..620c87923a3e 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -102,6 +102,9 @@ struct wakeup_source *wakeup_source_create(const char *name)
return NULL;
wakeup_source_prepare(ws, name ? kstrdup_const(name, GFP_KERNEL) : NULL);
+
+ kobject_init(&ws->kobj, &wakeup_source_ktype);
+
return ws;
}
EXPORT_SYMBOL_GPL(wakeup_source_create);
@@ -147,8 +150,7 @@ void wakeup_source_destroy(struct wakeup_source *ws)
__pm_relax(ws);
wakeup_source_record(ws);
- kfree_const(ws->name);
- kfree(ws);
+ kobject_put(&ws->kobj);
}
EXPORT_SYMBOL_GPL(wakeup_source_destroy);
@@ -205,11 +207,17 @@ EXPORT_SYMBOL_GPL(wakeup_source_remove);
struct wakeup_source *wakeup_source_register(const char *name)
{
struct wakeup_source *ws;
+ int ret;
ws = wakeup_source_create(name);
- if (ws)
+ if (ws) {
+ ret = wakeup_source_sysfs_add(ws);
+ if (ret) {
+ kobject_put(&ws->kobj);
+ return NULL;
+ }
wakeup_source_add(ws);
-
+ }
return ws;
}
EXPORT_SYMBOL_GPL(wakeup_source_register);
diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
new file mode 100644
index 000000000000..05ab8283ed66
--- /dev/null
+++ b/drivers/base/power/wakeup_stats.c
@@ -0,0 +1,194 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/slab.h>
+
+#include "power.h"
+
+struct wakeup_source_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr, char *buf);
+ ssize_t (*store)(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr, const char *buf,
+ size_t count);
+};
+
+#define to_wakeup_source_obj(x) container_of(x, struct wakeup_source, kobj)
+#define to_wakeup_source_attr(x) \
+ container_of(x, struct wakeup_source_attribute, attr)
+
+static ssize_t wakeup_source_attr_show(struct kobject *kobj,
+ struct attribute *attr,
+ char *buf)
+{
+ struct wakeup_source_attribute *attribute;
+ struct wakeup_source *ws;
+
+ ws = to_wakeup_source_obj(kobj);
+ attribute = to_wakeup_source_attr(attr);
+
+ if (!attribute->show)
+ return -EIO;
+
+ return attribute->show(ws, attribute, buf);
+}
+
+static const struct sysfs_ops wakeup_source_sysfs_ops = {
+ .show = wakeup_source_attr_show,
+};
+
+static void wakeup_source_release(struct kobject *kobj)
+{
+ struct wakeup_source *ws;
+
+ ws = to_wakeup_source_obj(kobj);
+ kfree_const(ws->name);
+ kfree(ws);
+}
+
+static ssize_t wakeup_source_count_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ unsigned long var;
+
+ if (strcmp(attr->attr.name, "active_count") == 0)
+ var = ws->active_count;
+ else if (strcmp(attr->attr.name, "event_count") == 0)
+ var = ws->event_count;
+ else if (strcmp(attr->attr.name, "wakeup_count") == 0)
+ var = ws->wakeup_count;
+ else
+ var = ws->expire_count;
+
+ return sprintf(buf, "%lu\n", var);
+}
+
+#define wakeup_source_count_attr(_name) \
+static struct wakeup_source_attribute _name##_attr = { \
+ .attr = { \
+ .name = __stringify(_name), \
+ .mode = 0444, \
+ }, \
+ .show = wakeup_source_count_show, \
+}
+
+wakeup_source_count_attr(active_count);
+wakeup_source_count_attr(event_count);
+wakeup_source_count_attr(wakeup_count);
+wakeup_source_count_attr(expire_count);
+
+#define wakeup_source_attr(_name) \
+static struct wakeup_source_attribute _name##_attr = __ATTR_RO(_name)
+
+static ssize_t active_time_ms_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ ktime_t active_time =
+ ws->active ? ktime_sub(ktime_get(), ws->last_time) : 0;
+ return sprintf(buf, "%lld\n", ktime_to_ms(active_time));
+}
+wakeup_source_attr(active_time_ms);
+
+static ssize_t total_time_ms_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ ktime_t active_time;
+ ktime_t total_time = ws->total_time;
+
+ if (ws->active) {
+ active_time = ktime_sub(ktime_get(), ws->last_time);
+ total_time = ktime_add(total_time, active_time);
+ }
+ return sprintf(buf, "%lld\n", ktime_to_ms(total_time));
+}
+wakeup_source_attr(total_time_ms);
+
+static ssize_t max_time_ms_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ ktime_t active_time;
+ ktime_t max_time = ws->max_time;
+
+ if (ws->active) {
+ active_time = ktime_sub(ktime_get(), ws->last_time);
+ if (active_time > max_time)
+ max_time = active_time;
+ }
+ return sprintf(buf, "%lld\n", ktime_to_ms(max_time));
+}
+wakeup_source_attr(max_time_ms);
+
+static ssize_t last_change_ms_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%lld\n", ktime_to_ms(ws->last_time));
+}
+wakeup_source_attr(last_change_ms);
+
+static ssize_t prevent_suspend_time_ms_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ ktime_t prevent_sleep_time = ws->prevent_sleep_time;
+
+ if (ws->active && ws->autosleep_enabled) {
+ prevent_sleep_time = ktime_add(prevent_sleep_time,
+ ktime_sub(ktime_get(), ws->start_prevent_time));
+ }
+ return sprintf(buf, "%lld\n", ktime_to_ms(prevent_sleep_time));
+}
+wakeup_source_attr(prevent_suspend_time_ms);
+
+static struct attribute *wakeup_source_default_attrs[] = {
+ &active_count_attr.attr,
+ &event_count_attr.attr,
+ &wakeup_count_attr.attr,
+ &expire_count_attr.attr,
+ &active_time_ms_attr.attr,
+ &total_time_ms_attr.attr,
+ &max_time_ms_attr.attr,
+ &last_change_ms_attr.attr,
+ &prevent_suspend_time_ms_attr.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(wakeup_source_default);
+
+struct kobj_type wakeup_source_ktype = {
+ .sysfs_ops = &wakeup_source_sysfs_ops,
+ .release = wakeup_source_release,
+ .default_groups = wakeup_source_default_groups,
+};
+
+#ifdef CONFIG_PM_SLEEP_STATS
+
+static struct kset *wakeup_source_kset;
+
+/**
+ * wakeup_source_sysfs_add - Add wakeup_source attributes to sysfs.
+ * @ws: Wakeup source to be exposed in sysfs.
+ */
+int wakeup_source_sysfs_add(struct wakeup_source *ws)
+{
+ ws->kobj.kset = wakeup_source_kset;
+ return kobject_add(&ws->kobj, NULL, "%s", ws->name);
+}
+EXPORT_SYMBOL_GPL(wakeup_source_sysfs_add);
+
+static int __init wakeup_sources_sysfs_init(void)
+{
+ wakeup_source_kset = kset_create_and_add("wakeup_sources", NULL,
+ power_kobj);
+ if (!wakeup_source_kset)
+ return -ENOMEM;
+
+ return 0;
+}
+
+postcore_initcall(wakeup_sources_sysfs_init);
+
+#endif /* !CONFIG_PM_SLEEP_STATS */
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index ce57771fab9b..67026c52975c 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -57,6 +57,7 @@ struct wakeup_source {
unsigned long wakeup_count;
bool active:1;
bool autosleep_enabled:1;
+ struct kobject kobj;
};
#ifdef CONFIG_PM_SLEEP
@@ -181,6 +182,20 @@ static inline void pm_wakeup_dev_event(struct device *dev, unsigned int msec,
#endif /* !CONFIG_PM_SLEEP */
+#ifdef CONFIG_PM_SLEEP_STATS
+
+/* drivers/base/power/wakeup_stats.c */
+extern int wakeup_source_sysfs_add(struct wakeup_source *ws);
+
+#else /* !CONFIG_PM_SLEEP_STATS */
+
+static inline int wakeup_source_sysfs_add(struct wakeup_source *ws)
+{
+ return 0;
+}
+
+#endif /* !CONFIG_PM_SLEEP_STATS */
+
static inline void wakeup_source_init(struct wakeup_source *ws,
const char *name)
{
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index ff8592ddedee..4d258f34020b 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -195,6 +195,16 @@ config PM_SLEEP_DEBUG
def_bool y
depends on PM_DEBUG && PM_SLEEP
+config PM_SLEEP_STATS
+ bool "Wakeup sources statistics"
+ depends on PM_SLEEP
+ help
+ Expose wakeup sources statistics to user space via sysfs. Collected
+ statistics are located under /sys/power/wakeup_sources. For more
+ information, read <file:Documentation/ABI/testing/sysfs-power>.
+
+ If in doubt, say N.
+
config DPM_WATCHDOG
bool "Device suspend/resume watchdog"
depends on PM_DEBUG && PSTORE && EXPERT
diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
index 4210152e56f0..eebbc537321f 100644
--- a/kernel/power/wakelock.c
+++ b/kernel/power/wakelock.c
@@ -27,7 +27,7 @@ static DEFINE_MUTEX(wakelocks_lock);
struct wakelock {
char *name;
struct rb_node node;
- struct wakeup_source ws;
+ struct wakeup_source *ws;
#ifdef CONFIG_PM_WAKELOCKS_GC
struct list_head lru;
#endif
@@ -46,7 +46,7 @@ ssize_t pm_show_wakelocks(char *buf, bool show_active)
for (node = rb_first(&wakelocks_tree); node; node = rb_next(node)) {
wl = rb_entry(node, struct wakelock, node);
- if (wl->ws.active == show_active)
+ if (wl->ws->active == show_active)
str += scnprintf(str, end - str, "%s ", wl->name);
}
if (str > buf)
@@ -112,18 +112,19 @@ static void __wakelocks_gc(struct work_struct *work)
u64 idle_time_ns;
bool active;
- spin_lock_irq(&wl->ws.lock);
- idle_time_ns = ktime_to_ns(ktime_sub(now, wl->ws.last_time));
- active = wl->ws.active;
- spin_unlock_irq(&wl->ws.lock);
+ spin_lock_irq(&wl->ws->lock);
+ idle_time_ns = ktime_to_ns(ktime_sub(now, wl->ws->last_time));
+ active = wl->ws->active;
+ spin_unlock_irq(&wl->ws->lock);
if (idle_time_ns < ((u64)WL_GC_TIME_SEC * NSEC_PER_SEC))
break;
if (!active) {
- wakeup_source_remove(&wl->ws);
+ wakeup_source_remove(wl->ws);
rb_erase(&wl->node, &wakelocks_tree);
list_del(&wl->lru);
+ kobject_put(&wl->ws->kobj);
kfree(wl->name);
kfree(wl);
decrement_wakelocks_number();
@@ -153,6 +154,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
struct rb_node **node = &wakelocks_tree.rb_node;
struct rb_node *parent = *node;
struct wakelock *wl;
+ int ret;
while (*node) {
int diff;
@@ -187,9 +189,23 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
kfree(wl);
return ERR_PTR(-ENOMEM);
}
- wl->ws.name = wl->name;
- wl->ws.last_time = ktime_get();
- wakeup_source_add(&wl->ws);
+
+ wl->ws = wakeup_source_create(wl->name);
+ if (!wl->ws) {
+ kfree(wl);
+ return ERR_PTR(-ENOMEM);
+ }
+ wl->ws->last_time = ktime_get();
+
+ ret = wakeup_source_sysfs_add(wl->ws);
+ if (ret) {
+ kobject_put(&wl->ws->kobj);
+ kfree(wl->name);
+ kfree(wl);
+ return ERR_PTR(ret);
+ }
+
+ wakeup_source_add(wl->ws);
rb_link_node(&wl->node, parent, node);
rb_insert_color(&wl->node, &wakelocks_tree);
wakelocks_lru_add(wl);
@@ -233,9 +249,9 @@ int pm_wake_lock(const char *buf)
u64 timeout_ms = timeout_ns + NSEC_PER_MSEC - 1;
do_div(timeout_ms, NSEC_PER_MSEC);
- __pm_wakeup_event(&wl->ws, timeout_ms);
+ __pm_wakeup_event(wl->ws, timeout_ms);
} else {
- __pm_stay_awake(&wl->ws);
+ __pm_stay_awake(wl->ws);
}
wakelocks_lru_most_recent(wl);
@@ -271,7 +287,7 @@ int pm_wake_unlock(const char *buf)
ret = PTR_ERR(wl);
goto out;
}
- __pm_relax(&wl->ws);
+ __pm_relax(wl->ws);
wakelocks_lru_most_recent(wl);
wakelocks_gc();
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: [PATCH 0/3] powercap/intel_rapl: add support for more Icelake platforms
From: Rafael J. Wysocki @ 2019-06-27 22:03 UTC (permalink / raw)
To: Zhang Rui; +Cc: linux-pm
In-Reply-To: <1560778395-15088-1-git-send-email-rui.zhang@intel.com>
On Monday, June 17, 2019 3:33:12 PM CEST Zhang Rui wrote:
> This patch series add intel_rapl support for more IceLake platforms including
> ICX, ICX-D and ICL desktop.
>
> Note that all the patches depend on commit e35faeb64146
> ("x86/CPU: Add more Icelake model numbers") which is just merged in 5.2-rc5.
>
> thanks,
> rui
>
>
All three applied, thanks!
^ permalink raw reply
* Re: [PATCH 2/2] powercap/rapl: Add Ice Lake NNPI support to RAPL driver
From: Rafael J. Wysocki @ 2019-06-27 22:02 UTC (permalink / raw)
To: Rajneesh Bhardwaj; +Cc: linux-kernel, len.brown, rafael.j.wysocki, linux-pm
In-Reply-To: <20190614080523.13464-1-rajneesh.bhardwaj@linux.intel.com>
On Friday, June 14, 2019 10:05:23 AM CEST Rajneesh Bhardwaj wrote:
> Enables support for ICL-NNPI, which is a neural network processor for deep
> learning inference. From RAPL point of view it is same as Ice Lake Mobile
> processor.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: linux-pm@vger.kernel.org
> Link: https://lkml.org/lkml/2019/6/5/1034
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@linux.intel.com>
> ---
> drivers/powercap/intel_rapl.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
> index 4347f15165f8..431c8c8bdf07 100644
> --- a/drivers/powercap/intel_rapl.c
> +++ b/drivers/powercap/intel_rapl.c
> @@ -1157,6 +1157,7 @@ static const struct x86_cpu_id rapl_ids[] __initconst = {
> INTEL_CPU_FAM6(KABYLAKE_DESKTOP, rapl_defaults_core),
> INTEL_CPU_FAM6(CANNONLAKE_MOBILE, rapl_defaults_core),
> INTEL_CPU_FAM6(ICELAKE_MOBILE, rapl_defaults_core),
> + INTEL_CPU_FAM6(ICELAKE_NNPI, rapl_defaults_core),
>
> INTEL_CPU_FAM6(ATOM_SILVERMONT, rapl_defaults_byt),
> INTEL_CPU_FAM6(ATOM_AIRMONT, rapl_defaults_cht),
>
It is in my queue, but I get build errors when I try to apply it.
I guess the definition of ICELAKE_NNPI is not there in the Linus' tree yet.
^ permalink raw reply
* Re: [PATCH] power: avs: smartreflex: no need to check return value of debugfs_create functions
From: Rafael J. Wysocki @ 2019-06-27 22:01 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Kevin Hilman, Nishanth Menon, linux-pm
In-Reply-To: <20190611180547.GA13520@kroah.com>
On Tuesday, June 11, 2019 8:05:47 PM CEST Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> And even when not checking the return value, no need to cast away the
> call to (void), as these functions were never a "must check" type of a
> function, so remove that odd cast.
>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/power/avs/smartreflex.c | 41 +++++++++------------------------
> 1 file changed, 11 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index c96c01e09740..4684e7df833a 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -899,38 +899,19 @@ static int omap_sr_probe(struct platform_device *pdev)
> }
>
> dev_info(&pdev->dev, "%s: SmartReflex driver initialized\n", __func__);
> - if (!sr_dbg_dir) {
> + if (!sr_dbg_dir)
> sr_dbg_dir = debugfs_create_dir("smartreflex", NULL);
> - if (IS_ERR_OR_NULL(sr_dbg_dir)) {
> - ret = PTR_ERR(sr_dbg_dir);
> - pr_err("%s:sr debugfs dir creation failed(%d)\n",
> - __func__, ret);
> - goto err_list_del;
> - }
> - }
>
> sr_info->dbg_dir = debugfs_create_dir(sr_info->name, sr_dbg_dir);
> - if (IS_ERR_OR_NULL(sr_info->dbg_dir)) {
> - dev_err(&pdev->dev, "%s: Unable to create debugfs directory\n",
> - __func__);
> - ret = PTR_ERR(sr_info->dbg_dir);
> - goto err_debugfs;
> - }
>
> - (void) debugfs_create_file("autocomp", S_IRUGO | S_IWUSR,
> - sr_info->dbg_dir, (void *)sr_info, &pm_sr_fops);
> - (void) debugfs_create_x32("errweight", S_IRUGO, sr_info->dbg_dir,
> - &sr_info->err_weight);
> - (void) debugfs_create_x32("errmaxlimit", S_IRUGO, sr_info->dbg_dir,
> - &sr_info->err_maxlimit);
> + debugfs_create_file("autocomp", S_IRUGO | S_IWUSR, sr_info->dbg_dir,
> + (void *)sr_info, &pm_sr_fops);
> + debugfs_create_x32("errweight", S_IRUGO, sr_info->dbg_dir,
> + &sr_info->err_weight);
> + debugfs_create_x32("errmaxlimit", S_IRUGO, sr_info->dbg_dir,
> + &sr_info->err_maxlimit);
>
> nvalue_dir = debugfs_create_dir("nvalue", sr_info->dbg_dir);
> - if (IS_ERR_OR_NULL(nvalue_dir)) {
> - dev_err(&pdev->dev, "%s: Unable to create debugfs directory for n-values\n",
> - __func__);
> - ret = PTR_ERR(nvalue_dir);
> - goto err_debugfs;
> - }
>
> if (sr_info->nvalue_count == 0 || !sr_info->nvalue_table) {
> dev_warn(&pdev->dev, "%s: %s: No Voltage table for the corresponding vdd. Cannot create debugfs entries for n-values\n",
> @@ -945,12 +926,12 @@ static int omap_sr_probe(struct platform_device *pdev)
>
> snprintf(name, sizeof(name), "volt_%lu",
> sr_info->nvalue_table[i].volt_nominal);
> - (void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
> - &(sr_info->nvalue_table[i].nvalue));
> + debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
> + &(sr_info->nvalue_table[i].nvalue));
> snprintf(name, sizeof(name), "errminlimit_%lu",
> sr_info->nvalue_table[i].volt_nominal);
> - (void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
> - &(sr_info->nvalue_table[i].errminlimit));
> + debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
> + &(sr_info->nvalue_table[i].errminlimit));
>
> }
>
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] cpufreq: pcc-cpufreq: Fail initialization if driver cannot be registered
From: Rafael J. Wysocki @ 2019-06-27 22:00 UTC (permalink / raw)
To: Viresh Kumar, David Arcari; +Cc: linux-pm, linux-kernel
In-Reply-To: <20190607030546.7kjg466wbkzxfym4@vireshk-i7>
On Friday, June 7, 2019 5:05:46 AM CEST Viresh Kumar wrote:
> On 06-06-19, 14:50, David Arcari wrote:
> > Make pcc_cpufreq_init() return error codes when the driver cannot be
> > registered. Otherwise the driver can shows up loaded via lsmod even
> > though it failed initialization. This is confusing to the user.
> >
> > Signed-off-by: David Arcari <darcari@redhat.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > drivers/cpufreq/pcc-cpufreq.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
> > index 1e5e64643c3a..fdc767fdbe6a 100644
> > --- a/drivers/cpufreq/pcc-cpufreq.c
> > +++ b/drivers/cpufreq/pcc-cpufreq.c
> > @@ -582,10 +582,10 @@ static int __init pcc_cpufreq_init(void)
> >
> > /* Skip initialization if another cpufreq driver is there. */
> > if (cpufreq_get_current_driver())
> > - return 0;
> > + return -EEXIST;
> >
> > if (acpi_disabled)
> > - return 0;
> > + return -ENODEV;
> >
> > ret = pcc_cpufreq_probe();
> > if (ret) {
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2] powerpc/power: Expose pfn_is_nosave prototype
From: Rafael J. Wysocki @ 2019-06-27 22:00 UTC (permalink / raw)
To: Mathieu Malaterre
Cc: Michael Ellerman, Christophe Leroy, Benjamin Herrenschmidt,
Paul Mackerras, Martin Schwidefsky, Heiko Carstens, Pavel Machek,
Len Brown, linuxppc-dev, linux-kernel, linux-s390, linux-pm
In-Reply-To: <20190524104418.17194-1-malat@debian.org>
On Friday, May 24, 2019 12:44:18 PM CEST Mathieu Malaterre wrote:
> The declaration for pfn_is_nosave is only available in
> kernel/power/power.h. Since this function can be override in arch,
> expose it globally. Having a prototype will make sure to avoid warning
> (sometime treated as error with W=1) such as:
>
> arch/powerpc/kernel/suspend.c:18:5: error: no previous prototype for 'pfn_is_nosave' [-Werror=missing-prototypes]
>
> This moves the declaration into a globally visible header file and add
> missing include to avoid a warning on powerpc. Also remove the
> duplicated prototypes since not required anymore.
>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
> v2: As suggestion by christophe remove duplicates prototypes
>
> arch/powerpc/kernel/suspend.c | 1 +
> arch/s390/kernel/entry.h | 1 -
> include/linux/suspend.h | 1 +
> kernel/power/power.h | 2 --
> 4 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/suspend.c b/arch/powerpc/kernel/suspend.c
> index a531154cc0f3..9e1b6b894245 100644
> --- a/arch/powerpc/kernel/suspend.c
> +++ b/arch/powerpc/kernel/suspend.c
> @@ -8,6 +8,7 @@
> */
>
> #include <linux/mm.h>
> +#include <linux/suspend.h>
> #include <asm/page.h>
> #include <asm/sections.h>
>
> diff --git a/arch/s390/kernel/entry.h b/arch/s390/kernel/entry.h
> index 20420c2b8a14..b2956d49b6ad 100644
> --- a/arch/s390/kernel/entry.h
> +++ b/arch/s390/kernel/entry.h
> @@ -63,7 +63,6 @@ void __init startup_init(void);
> void die(struct pt_regs *regs, const char *str);
> int setup_profiling_timer(unsigned int multiplier);
> void __init time_init(void);
> -int pfn_is_nosave(unsigned long);
> void s390_early_resume(void);
> unsigned long prepare_ftrace_return(unsigned long parent, unsigned long sp, unsigned long ip);
>
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 6b3ea9ea6a9e..e8b8a7bede90 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -395,6 +395,7 @@ extern bool system_entering_hibernation(void);
> extern bool hibernation_available(void);
> asmlinkage int swsusp_save(void);
> extern struct pbe *restore_pblist;
> +int pfn_is_nosave(unsigned long pfn);
> #else /* CONFIG_HIBERNATION */
> static inline void register_nosave_region(unsigned long b, unsigned long e) {}
> static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 9e58bdc8a562..44bee462ff57 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -75,8 +75,6 @@ static inline void hibernate_reserved_size_init(void) {}
> static inline void hibernate_image_size_init(void) {}
> #endif /* !CONFIG_HIBERNATION */
>
> -extern int pfn_is_nosave(unsigned long);
> -
> #define power_attr(_name) \
> static struct kobj_attribute _name##_attr = { \
> .attr = { \
>
Applied, thanks!
^ permalink raw reply
* Re: [GIT PULL] cpupower update for Linux 5.2-rc6
From: Rafael J. Wysocki @ 2019-06-27 21:58 UTC (permalink / raw)
To: Shuah Khan
Cc: Rafael J. Wysocki, Linux PM, Thomas Renninger,
Linux Kernel Mailing List
In-Reply-To: <376ea5d7-110a-71fe-7b02-efe1b0aed88e@linuxfoundation.org>
On Wednesday, June 12, 2019 10:27:03 PM CEST Shuah Khan wrote:
> This is a multi-part message in MIME format.
> --------------DA1BC82BAC95C219E9AB2661
> Content-Type: text/plain; charset=utf-8; format=flowed
> Content-Transfer-Encoding: 7bit
>
> Hi Rafael,
>
> Please pull the following update for Linux 5.2-rc6 or 5.3 depending on
> your pull request schedule for Linus.
>
> This cpupower update for Linux 5.2-rc6 consists of a fix and a minor
> spelling correction.
>
> diff is attached.
>
> thanks,
> -- Shuah
>
> ----------------------------------------------------------------
> The following changes since commit f2c7c76c5d0a443053e94adb9f0918fa2fb85c3a:
>
> Linux 5.2-rc3 (2019-06-02 13:55:33 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux
> tags/linux-cpupower-5.2-rc6
>
> for you to fetch changes up to 04507c0a9385cc8280f794a36bfff567c8cc1042:
>
> cpupower : frequency-set -r option misses the last cpu in related cpu
> list (2019-06-04 09:06:50 -0600)
Pulled (long ago) and queued for 5.3.
Thanks, and sorry for the delayed response.
^ permalink raw reply
* Re: [PATCH V2 3/5] cpufreq: Use has_target() instead of !setpolicy
From: Rafael J. Wysocki @ 2019-06-27 21:52 UTC (permalink / raw)
To: Viresh Kumar; +Cc: linux-pm, Vincent Guittot, linux-kernel
In-Reply-To: <56d8e01d8febb81917aded319249145fdc73daec.1560999838.git.viresh.kumar@linaro.org>
On Thursday, June 20, 2019 5:05:48 AM CEST Viresh Kumar wrote:
> For code consistency, use has_target() instead of !setpolicy everywhere,
> as it is already done at several places. Maybe we should also use
> "!has_target()" instead of "cpufreq_driver->setpolicy" where we need to
> check if the driver supports setpolicy, so to use only one expression
> for this kind of differentiation.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 41ac701e324f..5f5c7a516c74 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -632,7 +632,7 @@ static int cpufreq_parse_policy(char *str_governor,
> }
>
> /**
> - * cpufreq_parse_governor - parse a governor string only for !setpolicy
> + * cpufreq_parse_governor - parse a governor string only for has_target()
> */
> static int cpufreq_parse_governor(char *str_governor,
> struct cpufreq_policy *policy)
> @@ -1301,7 +1301,7 @@ static int cpufreq_online(unsigned int cpu)
> policy->max = policy->user_policy.max;
> }
>
> - if (cpufreq_driver->get && !cpufreq_driver->setpolicy) {
> + if (cpufreq_driver->get && has_target()) {
> policy->cur = cpufreq_driver->get(policy->cpu);
> if (!policy->cur) {
> pr_err("%s: ->get() failed\n", __func__);
> @@ -2401,7 +2401,7 @@ void cpufreq_update_policy(unsigned int cpu)
> * BIOS might change freq behind our back
> * -> ask driver for current freq and notify governors about a change
> */
> - if (cpufreq_driver->get && !cpufreq_driver->setpolicy &&
> + if (cpufreq_driver->get && has_target() &&
> (cpufreq_suspended || WARN_ON(!cpufreq_update_current_freq(policy))))
> goto unlock;
>
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH V2 1/5] cpufreq: Remove the redundant !setpolicy check
From: Rafael J. Wysocki @ 2019-06-27 21:52 UTC (permalink / raw)
To: Viresh Kumar; +Cc: linux-pm, Vincent Guittot, linux-kernel
In-Reply-To: <b9bac95bcc36f5f70e910e4801be5d4f8fd32d0c.1560999838.git.viresh.kumar@linaro.org>
On Thursday, June 20, 2019 5:05:46 AM CEST Viresh Kumar wrote:
> cpufreq_start_governor() is only called for !setpolicy case, checking it
> again is not required.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 85ff958e01f1..54befd775bd6 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2153,7 +2153,7 @@ static int cpufreq_start_governor(struct cpufreq_policy *policy)
>
> pr_debug("%s: for CPU %u\n", __func__, policy->cpu);
>
> - if (cpufreq_driver->get && !cpufreq_driver->setpolicy)
> + if (cpufreq_driver->get)
> cpufreq_update_current_freq(policy);
>
> if (policy->governor->start) {
>
Applied, thanks!
^ permalink raw reply
* Re: [GIT PULL] OPP changes for 5.3
From: Rafael J. Wysocki @ 2019-06-27 21:34 UTC (permalink / raw)
To: Viresh Kumar; +Cc: Rafael J. Wysocki, Linux PM
In-Reply-To: <20190626072027.kjqfwwbvacn2s47o@vireshk-i7>
On Wed, Jun 26, 2019 at 9:20 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Hi Rafael,
>
> This pull request contains:
>
> - OPP core changes to support a wider range of devices, like IO
> devices (Rajendra Nayak and Stehpen Boyd).
> - Fixes around genpd_virt_devs (Viresh Kumar).
> - Fix for platform with set_opp() callback (Dmitry Osipenko).
>
> --
> viresh
>
> -------------------------8<-------------------------
>
> The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9:
>
> Linux 5.2-rc1 (2019-05-19 15:47:09 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/linux-next
>
> for you to fetch changes up to 560d1bcad715c215e7ffe5d7cffe045974b623d0:
>
> opp: Don't use IS_ERR on invalid supplies (2019-06-24 12:48:29 +0530)
>
Pulled, thanks!
^ permalink raw reply
* Re: [GIT PULL] cpufreq/arm changes for 5.3
From: Rafael J. Wysocki @ 2019-06-27 21:34 UTC (permalink / raw)
To: Viresh Kumar; +Cc: Rafael J. Wysocki, Linux PM
In-Reply-To: <20190626071643.enxekm6qxx4qgpht@vireshk-i7>
On Wed, Jun 26, 2019 at 9:16 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Hi Rafael,
>
> This pull request contains:
>
> - Minor fixes for brcmstb driver (Florian).
> - New imx-cpufreq driver, its bindings and code around it (Leonard).
> - New Raspberry Pi driver (Nicolas).
> - Minor fix for s5pv210 driver (Pawel).
> - Minor cleanup for armada driver (YueHaibing).
>
> --
> viresh
>
> -------------------------8<-------------------------
> The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9:
>
> Linux 5.2-rc1 (2019-05-19 15:47:09 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git cpufreq/arm/linux-next
>
> for you to fetch changes up to f9020441dbc39133591ff72b420f21f51896afc5:
>
> cpufreq: s5pv210: Don't flood kernel log after cpufreq change (2019-06-24 10:27:02 +0530)
>
Pulled, thanks!
^ permalink raw reply
* Re: [PATCH V4 1/3] cpufreq: Move the IS_ENABLED(CPU_THERMAL) macro in a stub
From: Rafael J. Wysocki @ 2019-06-27 21:31 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Viresh Kumar, Rafael J. Wysocki, Eduardo Valentin,
Linux Kernel Mailing List,
open list:CPU FREQUENCY SCALING FRAMEWORK
In-Reply-To: <20190627210209.32600-1-daniel.lezcano@linaro.org>
On Thu, Jun 27, 2019 at 11:02 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> The cpufreq_online and the cpufreq_offline [un]register the driver as
> a cooling device. This is done if the driver is flagged as a cooling
> device in addition with a IS_ENABLED macro to compile out the branching
> code.
>
> Group this test in a stub function added in the cpufreq header instead
> of having the IS_ENABLED in the code path.
>
> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
This one has been queued up for 5.3 already, no need to resend.
Thanks!
> ---
> drivers/cpufreq/cpufreq.c | 6 ++----
> include/linux/cpufreq.h | 6 ++++++
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 85ff958e01f1..aee024e42618 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1378,8 +1378,7 @@ static int cpufreq_online(unsigned int cpu)
> if (cpufreq_driver->ready)
> cpufreq_driver->ready(policy);
>
> - if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
> - cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV)
> + if (cpufreq_thermal_control_enabled(cpufreq_driver))
> policy->cdev = of_cpufreq_cooling_register(policy);
>
> pr_debug("initialization complete\n");
> @@ -1469,8 +1468,7 @@ static int cpufreq_offline(unsigned int cpu)
> goto unlock;
> }
>
> - if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
> - cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV) {
> + if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
> cpufreq_cooling_unregister(policy->cdev);
> policy->cdev = NULL;
> }
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index d01a74fbc4db..a1467aa7f58b 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -409,6 +409,12 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
> const char *cpufreq_get_current_driver(void);
> void *cpufreq_get_driver_data(void);
>
> +static inline int cpufreq_thermal_control_enabled(struct cpufreq_driver *drv)
> +{
> + return IS_ENABLED(CONFIG_CPU_THERMAL) &&
> + (drv->flags & CPUFREQ_IS_COOLING_DEV);
> +}
> +
> static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
> unsigned int min, unsigned int max)
> {
> --
> 2.17.1
>
^ permalink raw reply
* Re: [RFC] clk: imx8mm: Add dram freq switch support
From: Stephen Boyd @ 2019-06-27 21:14 UTC (permalink / raw)
To: Alexandre Bailon, Jacky Bai, Leonard Crestez, Michael Turquette
Cc: Viresh Kumar, Anson Huang, Abel Vesa, Peng Fan, Shawn Guo,
Aisheng Dong, Fabio Estevam, Georgi Djakov, Cedric Neveux,
Silvano Di Ninno, Nitin Garg, Rafael J. Wysocki,
Krzysztof Kozlowski, linux-pm@vger.kernel.org,
linux-clk@vger.kernel.org, kernel@pengutronix.de, dl-linux-imx,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <475e0250b1e77a660c095749e78427fde318d5f6.1559200405.git.leonard.crestez@nxp.com>
Quoting Leonard Crestez (2019-05-30 00:13:51)
> Add a wrapper clock encapsulating dram frequency switch support for
> imx8m chips. This allows higher-level DVFS code to manipulate dram
> frequency using standard clock framework APIs.
>
> Linux-side implementation is similar in principle to imx_clk_cpu or a
> composite clock. Only some preparation is done inside the kernel, the
> actual freq switch is performed from TF-A code which runs from an SRAM
> area. Cores other than the one performing the switch are also made to
> spin inside TF-A by sending each an IRQ.
>
> This is an early proof-of-concept which only support low/high mode on
> imx8mm but NXP has secure-world dram freq switching implementations for
> multiple other chips and this approach can be extended.
>
> This was tested using a large pile of NXP out-of-tree patches. Code for
> the "busfreq core" from last release can be seen here:
> https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/soc/imx/busfreq-imx8mq.c?h=imx_4.14.98_2.0.0_ga
>
> It can be likely made to work with interconnect RFC:
> https://patchwork.kernel.org/cover/10851705/
>
> This RFC effectively refactors a common part between them.
>
> Among the possible cleanups:
> * Handle errors in low/high busfreq code and back off
> * Move irq to secure world
> * Try to use fewer clk parameters
> * More chips and frequencies
>
> Many platforms handle this kind of stuff externally but cpufreq is quite
> insistent that actual rates are set by clk code and that new platforms
> use cpufreq-dt.
>
> Let me know if there are objections to handling dram freq via clk.
Can it be an interconnect driver instead? I don't see how this is a clk
driver. It looks more like a driver that itself manages a collection of
clks, and you've put the coordination of those clks behind the clk_ops
interface. We don't want to have clk_ops calling clk consumer APIs in
general, so the whole approach doesn't seem correct. Hopefully this can
work out as some other sort of driver that is used directly from devfreq
or interconnect core instead and then have a different consumer driver
of devfreq or interconnect core that knows how to drive the clk tree.
^ permalink raw reply
* [PATCH v3 01/22] PM / devfreq: tegra30: Change irq type to unsigned int
From: Dmitry Osipenko @ 2019-06-27 21:10 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
IRQ numbers are always positive, hence the corresponding variable should
be unsigned to keep types consistent. This is a minor change that cleans
up code a tad more.
Suggested-by: Thierry Reding <thierry.reding@gmail.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index a6ba75f4106d..a27300f40b0b 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -160,7 +160,7 @@ struct tegra_devfreq {
struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
- int irq;
+ unsigned int irq;
};
struct tegra_actmon_emc_ratio {
@@ -618,12 +618,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
return PTR_ERR(tegra->emc_clock);
}
- tegra->irq = platform_get_irq(pdev, 0);
- if (tegra->irq < 0) {
- err = tegra->irq;
+ err = platform_get_irq(pdev, 0);
+ if (err < 0) {
dev_err(&pdev->dev, "Failed to get IRQ: %d\n", err);
return err;
}
+ tegra->irq = err;
reset_control_assert(tegra->reset);
--
2.22.0
^ permalink raw reply related
* [PATCH v3 02/22] PM / devfreq: tegra30: Keep interrupt disabled while governor is stopped
From: Dmitry Osipenko @ 2019-06-27 21:10 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
There is no real need to keep interrupt always-enabled, will be nicer
to keep it disabled while governor is inactive.
Suggested-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 43 ++++++++++++++++---------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index a27300f40b0b..5e2b133babdd 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -11,6 +11,7 @@
#include <linux/devfreq.h>
#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/irq.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/platform_device.h>
@@ -416,8 +417,6 @@ static void tegra_actmon_start(struct tegra_devfreq *tegra)
{
unsigned int i;
- disable_irq(tegra->irq);
-
actmon_writel(tegra, ACTMON_SAMPLING_PERIOD - 1,
ACTMON_GLB_PERIOD_CTRL);
@@ -442,8 +441,6 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
}
actmon_write_barrier(tegra);
-
- enable_irq(tegra->irq);
}
static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
@@ -552,6 +549,12 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
{
struct tegra_devfreq *tegra = dev_get_drvdata(devfreq->dev.parent);
+ /*
+ * Couple device with the governor early as it is needed at
+ * the moment of governor's start (used by ISR).
+ */
+ tegra->devfreq = devfreq;
+
switch (event) {
case DEVFREQ_GOV_START:
devfreq_monitor_start(devfreq);
@@ -586,10 +589,11 @@ static struct devfreq_governor tegra_devfreq_governor = {
static int tegra_devfreq_probe(struct platform_device *pdev)
{
- struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
- unsigned int i;
+ struct tegra_devfreq *tegra;
+ struct devfreq *devfreq;
unsigned long rate;
+ unsigned int i;
int err;
tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
@@ -625,6 +629,16 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
}
tegra->irq = err;
+ irq_set_status_flags(tegra->irq, IRQ_NOAUTOEN);
+
+ err = devm_request_threaded_irq(&pdev->dev, tegra->irq, NULL,
+ actmon_thread_isr, IRQF_ONESHOT,
+ "tegra-devfreq", tegra);
+ if (err) {
+ dev_err(&pdev->dev, "Interrupt request failed: %d\n", err);
+ return err;
+ }
+
reset_control_assert(tegra->reset);
err = clk_prepare_enable(tegra->clock);
@@ -672,28 +686,15 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
}
tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock);
- tegra->devfreq = devfreq_add_device(&pdev->dev,
- &tegra_devfreq_profile,
- "tegra_actmon",
- NULL);
+ devfreq = devfreq_add_device(&pdev->dev, &tegra_devfreq_profile,
+ "tegra_actmon", NULL);
if (IS_ERR(tegra->devfreq)) {
err = PTR_ERR(tegra->devfreq);
goto remove_governor;
}
- err = devm_request_threaded_irq(&pdev->dev, tegra->irq, NULL,
- actmon_thread_isr, IRQF_ONESHOT,
- "tegra-devfreq", tegra);
- if (err) {
- dev_err(&pdev->dev, "Interrupt request failed: %d\n", err);
- goto remove_devfreq;
- }
-
return 0;
-remove_devfreq:
- devfreq_remove_device(tegra->devfreq);
-
remove_governor:
devfreq_remove_governor(&tegra_devfreq_governor);
--
2.22.0
^ permalink raw reply related
* [PATCH v3 03/22] PM / devfreq: tegra30: Handle possible round-rate error
From: Dmitry Osipenko @ 2019-06-27 21:10 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
The EMC clock rate rounding technically could fail, hence let's handle
the error cases properly.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 5e2b133babdd..5e606ae3f238 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -592,8 +592,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
struct tegra_devfreq_device *dev;
struct tegra_devfreq *tegra;
struct devfreq *devfreq;
- unsigned long rate;
unsigned int i;
+ long rate;
int err;
tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
@@ -650,8 +650,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
reset_control_deassert(tegra->reset);
- tegra->max_freq = clk_round_rate(tegra->emc_clock, ULONG_MAX) / KHZ;
+ rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
+ if (rate < 0) {
+ dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate);
+ return rate;
+ }
+
tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ;
+ tegra->max_freq = rate / KHZ;
for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
dev = tegra->devices + i;
@@ -662,6 +668,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
rate = clk_round_rate(tegra->emc_clock, rate);
+ if (rate < 0) {
+ dev_err(&pdev->dev,
+ "Failed to round clock rate: %ld\n", rate);
+ err = rate;
+ goto remove_opps;
+ }
+
err = dev_pm_opp_add(&pdev->dev, rate, 0);
if (err) {
dev_err(&pdev->dev, "Failed to add OPP: %d\n", err);
--
2.22.0
^ permalink raw reply related
* [PATCH v3 04/22] PM / devfreq: tegra30: Drop write-barrier
From: Dmitry Osipenko @ 2019-06-27 21:10 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
There is no need in a write-barrier now, given that interrupt masking is
handled by CPU's GIC now. Hence we know exactly that interrupt won't fire
after stopping the devfreq's governor. In other cases we don't care about
potential buffering of the writes to hardware and thus there is no need to
stall CPU.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 5e606ae3f238..4be7858c33bc 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -230,12 +230,6 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
ACTMON_DEV_LOWER_WMARK);
}
-static void actmon_write_barrier(struct tegra_devfreq *tegra)
-{
- /* ensure the update has reached the ACTMON */
- readl(tegra->regs + ACTMON_GLB_STATUS);
-}
-
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
@@ -287,8 +281,6 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
-
- actmon_write_barrier(tegra);
}
static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra,
@@ -376,8 +368,6 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
tegra_devfreq_update_wmark(tegra, dev);
}
- actmon_write_barrier(tegra);
-
return NOTIFY_OK;
}
@@ -423,8 +413,6 @@ static void tegra_actmon_start(struct tegra_devfreq *tegra)
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_configure_device(tegra, &tegra->devices[i]);
- actmon_write_barrier(tegra);
-
enable_irq(tegra->irq);
}
@@ -439,8 +427,6 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
device_writel(&tegra->devices[i], ACTMON_INTR_STATUS_CLEAR,
ACTMON_DEV_INTR_STATUS);
}
-
- actmon_write_barrier(tegra);
}
static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
--
2.22.0
^ permalink raw reply related
* [PATCH v3 06/22] PM / devfreq: tegra30: Tune up boosting thresholds
From: Dmitry Osipenko @ 2019-06-27 21:10 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
Now that average-sustain coefficient / multiplier is gone, it won't hurt
to re-tune the boosting thresholds to get a bit harder boosting for MCALL
clients, resulting in a more reactive governing in a case of multimedia
applications usage like 3d / video.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 0c84ea44c1c0..7662e54f0e70 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -111,8 +111,8 @@ static struct tegra_devfreq_device_config actmon_device_configs[] = {
.irq_mask = 1 << 26,
.boost_up_coeff = 200,
.boost_down_coeff = 50,
- .boost_up_threshold = 60,
- .boost_down_threshold = 40,
+ .boost_up_threshold = 50,
+ .boost_down_threshold = 25,
},
{
/* MCCPU: memory accesses from the CPUs */
--
2.22.0
^ permalink raw reply related
* [PATCH v3 05/22] PM / devfreq: tegra30: Set up watermarks properly
From: Dmitry Osipenko @ 2019-06-27 21:10 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
The current implementation is inaccurate and results in very intensive
interrupt activity, which neglects the whole idea of polling offload to
hardware. The reason of the shortcoming is that watermarks are not set
up correctly and this results in ACTMON constantly asking to change freq
and then these requests are ignored. The end result of this patch is that
there are few hundreds of ACTMON's interrupts instead of tens thousands
after few minutes of a working devfreq, meanwhile the transitions activity
stays about the same and governor becomes more reactive.
Since watermarks are set precisely correct now, the boosting logic is
changed a tad to accommodate the change. The "average sustain coefficient"
multiplier is gone now since there is no need to compensate the improper
watermarks and EMC frequency-bump happens once boosting hits the upper
watermark enough times, depending on the per-device boosting threshold.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 294 +++++++++++++++++++++---------
1 file changed, 209 insertions(+), 85 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 4be7858c33bc..0c84ea44c1c0 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -47,6 +47,8 @@
#define ACTMON_DEV_INTR_CONSECUTIVE_UPPER BIT(31)
#define ACTMON_DEV_INTR_CONSECUTIVE_LOWER BIT(30)
+#define ACTMON_DEV_INTR_AVG_BELOW_WMARK BIT(25)
+#define ACTMON_DEV_INTR_AVG_ABOVE_WMARK BIT(24)
#define ACTMON_ABOVE_WMARK_WINDOW 1
#define ACTMON_BELOW_WMARK_WINDOW 3
@@ -63,9 +65,8 @@
* ACTMON_AVERAGE_WINDOW_LOG2: default value for @DEV_CTRL_K_VAL, which
* translates to 2 ^ (K_VAL + 1). ex: 2 ^ (6 + 1) = 128
*/
-#define ACTMON_AVERAGE_WINDOW_LOG2 6
-#define ACTMON_SAMPLING_PERIOD 12 /* ms */
-#define ACTMON_DEFAULT_AVG_BAND 6 /* 1/10 of % */
+#define ACTMON_AVERAGE_WINDOW_LOG2 6
+#define ACTMON_SAMPLING_PERIOD 12 /* ms */
#define KHZ 1000
@@ -142,9 +143,6 @@ struct tegra_devfreq_device {
* watermark breaches.
*/
unsigned long boost_freq;
-
- /* Optimal frequency calculated from the stats for this device */
- unsigned long target_freq;
};
struct tegra_devfreq {
@@ -156,7 +154,6 @@ struct tegra_devfreq {
struct clk *emc_clock;
unsigned long max_freq;
- unsigned long cur_freq;
struct notifier_block rate_change_nb;
struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
@@ -205,42 +202,182 @@ static unsigned long do_percent(unsigned long val, unsigned int pct)
return val * pct / 100;
}
+static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra)
+{
+ struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
+ unsigned int cpu_freq = cpufreq_get(0);
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(actmon_emc_ratios); i++, ratio++) {
+ if (cpu_freq >= ratio->cpu_freq) {
+ if (ratio->emc_freq >= tegra->max_freq)
+ return tegra->max_freq;
+ else
+ return ratio->emc_freq;
+ }
+ }
+
+ return 0;
+}
+
+static unsigned long
+tegra_actmon_account_cpu_freq(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev,
+ unsigned long target_freq)
+{
+ unsigned long static_cpu_emc_freq;
+
+ if (dev->config->avg_dependency_threshold &&
+ dev->config->avg_dependency_threshold < dev->avg_count) {
+ static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra);
+ target_freq = max(target_freq, static_cpu_emc_freq);
+ }
+
+ return target_freq;
+}
+
+static unsigned long tegra_actmon_lower_freq(struct tegra_devfreq *tegra,
+ unsigned long target_freq)
+{
+ unsigned long lower = target_freq;
+ struct dev_pm_opp *opp;
+
+ opp = dev_pm_opp_find_freq_floor(tegra->devfreq->dev.parent, &lower);
+ if (IS_ERR(opp))
+ lower = 0;
+ else
+ dev_pm_opp_put(opp);
+
+ return lower;
+}
+
+static unsigned long tegra_actmon_upper_freq(struct tegra_devfreq *tegra,
+ unsigned long target_freq)
+{
+ unsigned long upper = target_freq + 1;
+ struct dev_pm_opp *opp;
+
+ opp = dev_pm_opp_find_freq_ceil(tegra->devfreq->dev.parent, &upper);
+ if (IS_ERR(opp))
+ upper = ULONG_MAX;
+ else
+ dev_pm_opp_put(opp);
+
+ return upper;
+}
+
+static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev,
+ unsigned long target_freq,
+ unsigned long *lower,
+ unsigned long *upper)
+{
+ /*
+ * Memory frequencies are guaranteed to have 1MHz granularity
+ * and thus we need this rounding down to get a proper watermarks
+ * range in a case where target_freq falls into a range of
+ * next_possible_opp_freq - 1MHz.
+ */
+ target_freq = round_down(target_freq, 1000000);
+
+ /* watermarks are set at the borders of the corresponding OPPs */
+ *lower = tegra_actmon_lower_freq(tegra, target_freq);
+ *upper = tegra_actmon_upper_freq(tegra, target_freq);
+
+ *lower /= KHZ;
+ *upper /= KHZ;
+
+ /*
+ * The upper watermark should take into account CPU's frequency
+ * because cpu_to_emc_rate() may override the target_freq with
+ * a higher value and thus upper watermark need to be set up
+ * accordingly to avoid parasitic upper-events.
+ */
+ *upper = tegra_actmon_account_cpu_freq(tegra, dev, *upper);
+
+ *lower *= ACTMON_SAMPLING_PERIOD;
+ *upper *= ACTMON_SAMPLING_PERIOD;
+}
+
static void tegra_devfreq_update_avg_wmark(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 avg = dev->avg_count;
- u32 avg_band_freq = tegra->max_freq * ACTMON_DEFAULT_AVG_BAND / KHZ;
- u32 band = avg_band_freq * ACTMON_SAMPLING_PERIOD;
+ unsigned long lower, upper, freq;
+
+ freq = dev->avg_count / ACTMON_SAMPLING_PERIOD * KHZ;
+ tegra_actmon_get_lower_upper(tegra, dev, freq, &lower, &upper);
- device_writel(dev, avg + band, ACTMON_DEV_AVG_UPPER_WMARK);
+ /*
+ * We want to get interrupts when MCCPU client crosses the
+ * dependency threshold in order to take into / out of account
+ * the CPU's freq.
+ */
+ if (lower < dev->config->avg_dependency_threshold &&
+ upper > dev->config->avg_dependency_threshold) {
+ if (dev->avg_count < dev->config->avg_dependency_threshold)
+ upper = dev->config->avg_dependency_threshold;
+ else
+ lower = dev->config->avg_dependency_threshold;
+ }
- avg = max(dev->avg_count, band);
- device_writel(dev, avg - band, ACTMON_DEV_AVG_LOWER_WMARK);
+ device_writel(dev, lower, ACTMON_DEV_AVG_LOWER_WMARK);
+ device_writel(dev, upper, ACTMON_DEV_AVG_UPPER_WMARK);
}
static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
- struct tegra_devfreq_device *dev)
+ struct tegra_devfreq_device *dev,
+ unsigned long freq)
{
- u32 val = tegra->cur_freq * ACTMON_SAMPLING_PERIOD;
+ unsigned long lower, upper, delta;
+
+ /*
+ * Boosting logic kicks-in once lower / upper watermark is hit.
+ * The watermarks are based on the updated EMC rate and the
+ * average activity.
+ *
+ * The higher watermark is set in accordance to the EMC rate
+ * because we want to set it to the highest mark here and EMC rate
+ * represents that mark. The consecutive-upper interrupts are
+ * always enabled and we don't want to receive them if they won't
+ * do anything useful, hence the upper watermark is capped to maximum.
+ * Note that the EMC rate is changed once boosting pushed the rate
+ * too high, in that case boosting-up will be stopped because
+ * upper watermark is much higher now and it is *important* to
+ * stop the unwanted interrupts.
+ */
+ tegra_actmon_get_lower_upper(tegra, dev, freq - 1, &lower, &upper);
+
+ delta = do_percent(upper - lower, dev->config->boost_up_threshold);
+ device_writel(dev, lower + delta, ACTMON_DEV_UPPER_WMARK);
- device_writel(dev, do_percent(val, dev->config->boost_up_threshold),
- ACTMON_DEV_UPPER_WMARK);
+ /*
+ * Meanwhile the lower mark is based on the average value
+ * because it is the lowest possible consecutive-mark for this
+ * device. Once that mark is hit and boosting is stopped, the
+ * interrupt is disabled by ISR.
+ */
+ freq = dev->avg_count / ACTMON_SAMPLING_PERIOD * KHZ;
+ tegra_actmon_get_lower_upper(tegra, dev, freq, &lower, &upper);
- device_writel(dev, do_percent(val, dev->config->boost_down_threshold),
- ACTMON_DEV_LOWER_WMARK);
+ delta = do_percent(upper - lower, dev->config->boost_down_threshold);
+ device_writel(dev, lower + delta, ACTMON_DEV_LOWER_WMARK);
}
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 intr_status, dev_ctrl;
+ u32 intr_status, dev_ctrl, avg_intr_mask;
dev->avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
- tegra_devfreq_update_avg_wmark(tegra, dev);
-
intr_status = device_readl(dev, ACTMON_DEV_INTR_STATUS);
dev_ctrl = device_readl(dev, ACTMON_DEV_CTRL);
+ avg_intr_mask = ACTMON_DEV_INTR_AVG_BELOW_WMARK |
+ ACTMON_DEV_INTR_AVG_ABOVE_WMARK;
+
+ if (intr_status & avg_intr_mask)
+ tegra_devfreq_update_avg_wmark(tegra, dev);
+
if (intr_status & ACTMON_DEV_INTR_CONSECUTIVE_UPPER) {
/*
* new_boost = min(old_boost * up_coef + step, max_freq)
@@ -253,8 +390,6 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
if (dev->boost_freq >= tegra->max_freq)
dev->boost_freq = tegra->max_freq;
- else
- dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_EN;
} else if (intr_status & ACTMON_DEV_INTR_CONSECUTIVE_LOWER) {
/*
* new_boost = old_boost * down_coef
@@ -263,63 +398,36 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
dev->boost_freq = do_percent(dev->boost_freq,
dev->config->boost_down_coeff);
- dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_EN;
-
if (dev->boost_freq < (ACTMON_BOOST_FREQ_STEP >> 1))
dev->boost_freq = 0;
- else
- dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
}
- if (dev->config->avg_dependency_threshold) {
- if (dev->avg_count >= dev->config->avg_dependency_threshold)
- dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
- else if (dev->boost_freq == 0)
- dev_ctrl &= ~ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
+ if (intr_status & avg_intr_mask) {
+ /*
+ * Once average watermark is hit, it means that the memory
+ * activity changed significantly and thus boosting-up shall
+ * be reset because EMC clock rate will be changed and
+ * boosting will restart in this case.
+ */
+ dev->boost_freq = 0;
}
- device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
+ /* no boosting => no need for consecutive-down interrupt */
+ if (dev->boost_freq == 0)
+ dev_ctrl &= ~ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
+ device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
}
-static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra,
- unsigned long cpu_freq)
-{
- unsigned int i;
- struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
-
- for (i = 0; i < ARRAY_SIZE(actmon_emc_ratios); i++, ratio++) {
- if (cpu_freq >= ratio->cpu_freq) {
- if (ratio->emc_freq >= tegra->max_freq)
- return tegra->max_freq;
- else
- return ratio->emc_freq;
- }
- }
-
- return 0;
-}
-
-static void actmon_update_target(struct tegra_devfreq *tegra,
- struct tegra_devfreq_device *dev)
-{
- unsigned long cpu_freq = 0;
- unsigned long static_cpu_emc_freq = 0;
- unsigned int avg_sustain_coef;
-
- if (dev->config->avg_dependency_threshold) {
- cpu_freq = cpufreq_get(0);
- static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra, cpu_freq);
- }
+static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev)
+{ unsigned long target_freq;
- dev->target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD;
- avg_sustain_coef = 100 * 100 / dev->config->boost_up_threshold;
- dev->target_freq = do_percent(dev->target_freq, avg_sustain_coef);
- dev->target_freq += dev->boost_freq;
+ target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD + dev->boost_freq;
+ target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq);
- if (dev->avg_count >= dev->config->avg_dependency_threshold)
- dev->target_freq = max(dev->target_freq, static_cpu_emc_freq);
+ return target_freq;
}
static irqreturn_t actmon_thread_isr(int irq, void *data)
@@ -351,8 +459,8 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
unsigned long action, void *ptr)
{
struct clk_notifier_data *data = ptr;
- struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
+ struct tegra_devfreq *tegra;
unsigned int i;
if (action != POST_RATE_CHANGE)
@@ -360,12 +468,28 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
tegra = container_of(nb, struct tegra_devfreq, rate_change_nb);
- tegra->cur_freq = data->new_rate / KHZ;
-
+ /*
+ * EMC rate could change due to three reasons:
+ *
+ * 1. Average watermark hit
+ * 2. Boosting overflow
+ * 3. CPU freq change
+ *
+ * Once rate is changed, the consecutive watermarks need to be
+ * updated in order for boosting to work properly and to avoid
+ * unnecessary interrupts. Note that the consecutive range is set for
+ * all of devices using the same rate, hence if CPU is doing much
+ * less than the other memory clients, then its upper watermark will
+ * be very high in comparison to the actual activity (lower watermark)
+ * and thus unnecessary upper-interrupts will be suppressed.
+ *
+ * The average watermarks also should be updated because of 3.
+ */
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
dev = &tegra->devices[i];
- tegra_devfreq_update_wmark(tegra, dev);
+ tegra_devfreq_update_avg_wmark(tegra, dev);
+ tegra_devfreq_update_wmark(tegra, dev, data->new_rate);
}
return NOTIFY_OK;
@@ -374,15 +498,14 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 val = 0;
-
- dev->target_freq = tegra->cur_freq;
+ u32 val = 0, target_freq;
- dev->avg_count = tegra->cur_freq * ACTMON_SAMPLING_PERIOD;
+ target_freq = clk_get_rate(tegra->emc_clock) / KHZ;
+ dev->avg_count = target_freq * ACTMON_SAMPLING_PERIOD;
device_writel(dev, dev->avg_count, ACTMON_DEV_INIT_AVG);
tegra_devfreq_update_avg_wmark(tegra, dev);
- tegra_devfreq_update_wmark(tegra, dev);
+ tegra_devfreq_update_wmark(tegra, dev, target_freq);
device_writel(dev, ACTMON_COUNT_WEIGHT, ACTMON_DEV_COUNT_WEIGHT);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
@@ -469,13 +592,13 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
struct tegra_devfreq_device *actmon_dev;
unsigned long cur_freq;
- cur_freq = READ_ONCE(tegra->cur_freq);
+ cur_freq = clk_get_rate(tegra->emc_clock);
/* To be used by the tegra governor */
stat->private_data = tegra;
/* The below are to be used by the other governors */
- stat->current_frequency = cur_freq * KHZ;
+ stat->current_frequency = cur_freq;
actmon_dev = &tegra->devices[MCALL];
@@ -486,7 +609,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
stat->busy_time *= 100 / BUS_SATURATION_RATIO;
/* Number of cycles in a sampling period */
- stat->total_time = ACTMON_SAMPLING_PERIOD * cur_freq;
+ stat->total_time = cur_freq / KHZ * ACTMON_SAMPLING_PERIOD;
stat->busy_time = min(stat->busy_time, stat->total_time);
@@ -505,6 +628,7 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
struct devfreq_dev_status *stat;
struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
+ unsigned long dev_target_freq;
unsigned long target_freq = 0;
unsigned int i;
int err;
@@ -520,9 +644,9 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
dev = &tegra->devices[i];
- actmon_update_target(tegra, dev);
+ dev_target_freq = actmon_update_target(tegra, dev);
- target_freq = max(target_freq, dev->target_freq);
+ target_freq = max(target_freq, dev_target_freq);
}
*freq = target_freq * KHZ;
@@ -642,7 +766,6 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
return rate;
}
- tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ;
tegra->max_freq = rate / KHZ;
for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
@@ -671,7 +794,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, tegra);
tegra->rate_change_nb.notifier_call = tegra_actmon_rate_notify_cb;
- err = clk_notifier_register(tegra->emc_clock, &tegra->rate_change_nb);
+ err = clk_notifier_register(tegra->emc_clock,
+ &tegra->rate_change_nb);
if (err) {
dev_err(&pdev->dev,
"Failed to register rate change notifier\n");
--
2.22.0
^ permalink raw reply related
* [PATCH v3 07/22] PM / devfreq: tegra30: Use CPUFreq notifier
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
The CPU's client need to take into account that CPUFreq may change
while memory activity not, staying high. Thus an appropriate frequency
notifier should be used in addition to the clk-notifier.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 105 +++++++++++++++++++++++++-----
1 file changed, 88 insertions(+), 17 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 7662e54f0e70..502511ac4602 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -17,6 +17,7 @@
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/reset.h>
+#include <linux/workqueue.h>
#include "governor.h"
@@ -154,7 +155,10 @@ struct tegra_devfreq {
struct clk *emc_clock;
unsigned long max_freq;
- struct notifier_block rate_change_nb;
+ struct notifier_block clk_rate_change_nb;
+
+ struct work_struct update_work;
+ struct notifier_block cpu_rate_change_nb;
struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
@@ -455,8 +459,8 @@ static irqreturn_t actmon_thread_isr(int irq, void *data)
return handled ? IRQ_HANDLED : IRQ_NONE;
}
-static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
- unsigned long action, void *ptr)
+static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
+ unsigned long action, void *ptr)
{
struct clk_notifier_data *data = ptr;
struct tegra_devfreq_device *dev;
@@ -466,7 +470,7 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
if (action != POST_RATE_CHANGE)
return NOTIFY_OK;
- tegra = container_of(nb, struct tegra_devfreq, rate_change_nb);
+ tegra = container_of(nb, struct tegra_devfreq, clk_rate_change_nb);
/*
* EMC rate could change due to three reasons:
@@ -495,6 +499,37 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
return NOTIFY_OK;
}
+static void tegra_actmon_delayed_update(struct work_struct *work)
+{
+ struct tegra_devfreq *tegra = container_of(work, struct tegra_devfreq,
+ update_work);
+
+ mutex_lock(&tegra->devfreq->lock);
+ update_devfreq(tegra->devfreq);
+ mutex_unlock(&tegra->devfreq->lock);
+}
+
+static int tegra_actmon_cpu_notify_cb(struct notifier_block *nb,
+ unsigned long action, void *ptr)
+{
+ struct tegra_devfreq *tegra;
+
+ if (action != CPUFREQ_POSTCHANGE)
+ return NOTIFY_OK;
+
+ tegra = container_of(nb, struct tegra_devfreq, cpu_rate_change_nb);
+
+ /*
+ * CPUFreq driver should support CPUFREQ_ASYNC_NOTIFICATION in order
+ * to allow asynchronous notifications. This means we can't block
+ * here for too long, otherwise CPUFreq's core will complain with a
+ * warning splat.
+ */
+ schedule_work(&tegra->update_work);
+
+ return NOTIFY_OK;
+}
+
static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
@@ -526,9 +561,16 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
device_writel(dev, val, ACTMON_DEV_CTRL);
}
-static void tegra_actmon_start(struct tegra_devfreq *tegra)
+static void tegra_actmon_stop_device(struct tegra_devfreq_device *dev)
+{
+ device_writel(dev, 0x00000000, ACTMON_DEV_CTRL);
+ device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
+}
+
+static int tegra_actmon_start(struct tegra_devfreq *tegra)
{
unsigned int i;
+ int err;
actmon_writel(tegra, ACTMON_SAMPLING_PERIOD - 1,
ACTMON_GLB_PERIOD_CTRL);
@@ -536,7 +578,30 @@ static void tegra_actmon_start(struct tegra_devfreq *tegra)
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_configure_device(tegra, &tegra->devices[i]);
+ /*
+ * We are estimating CPU's memory bandwidth requirement based on
+ * amount of memory accesses and system's load, judging by CPU's
+ * frequency. We also don't want to receive events about CPU's
+ * frequency transaction when governor is stopped, hence notifier
+ * is registered dynamically.
+ */
+ err = cpufreq_register_notifier(&tegra->cpu_rate_change_nb,
+ CPUFREQ_TRANSITION_NOTIFIER);
+ if (err) {
+ dev_err(tegra->devfreq->dev.parent,
+ "Failed to register rate change notifier: %d\n", err);
+ goto err_stop;
+ }
+
enable_irq(tegra->irq);
+
+ return 0;
+
+err_stop:
+ for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
+ tegra_actmon_stop_device(&tegra->devices[i]);
+
+ return err;
}
static void tegra_actmon_stop(struct tegra_devfreq *tegra)
@@ -545,11 +610,13 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
disable_irq(tegra->irq);
- for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
- device_writel(&tegra->devices[i], 0x00000000, ACTMON_DEV_CTRL);
- device_writel(&tegra->devices[i], ACTMON_INTR_STATUS_CLEAR,
- ACTMON_DEV_INTR_STATUS);
- }
+ cpufreq_unregister_notifier(&tegra->cpu_rate_change_nb,
+ CPUFREQ_TRANSITION_NOTIFIER);
+
+ cancel_work_sync(&tegra->update_work);
+
+ for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
+ tegra_actmon_stop_device(&tegra->devices[i]);
}
static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
@@ -658,6 +725,7 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct tegra_devfreq *tegra = dev_get_drvdata(devfreq->dev.parent);
+ int ret = 0;
/*
* Couple device with the governor early as it is needed at
@@ -668,7 +736,7 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
switch (event) {
case DEVFREQ_GOV_START:
devfreq_monitor_start(devfreq);
- tegra_actmon_start(tegra);
+ ret = tegra_actmon_start(tegra);
break;
case DEVFREQ_GOV_STOP:
@@ -683,11 +751,11 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
case DEVFREQ_GOV_RESUME:
devfreq_monitor_resume(devfreq);
- tegra_actmon_start(tegra);
+ ret = tegra_actmon_start(tegra);
break;
}
- return 0;
+ return ret;
}
static struct devfreq_governor tegra_devfreq_governor = {
@@ -793,9 +861,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, tegra);
- tegra->rate_change_nb.notifier_call = tegra_actmon_rate_notify_cb;
+ tegra->cpu_rate_change_nb.notifier_call = tegra_actmon_cpu_notify_cb;
+ INIT_WORK(&tegra->update_work, tegra_actmon_delayed_update);
+
+ tegra->clk_rate_change_nb.notifier_call = tegra_actmon_clk_notify_cb;
err = clk_notifier_register(tegra->emc_clock,
- &tegra->rate_change_nb);
+ &tegra->clk_rate_change_nb);
if (err) {
dev_err(&pdev->dev,
"Failed to register rate change notifier\n");
@@ -822,7 +893,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
devfreq_remove_governor(&tegra_devfreq_governor);
unreg_notifier:
- clk_notifier_unregister(tegra->emc_clock, &tegra->rate_change_nb);
+ clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
remove_opps:
dev_pm_opp_remove_all_dynamic(&pdev->dev);
@@ -840,7 +911,7 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
devfreq_remove_device(tegra->devfreq);
devfreq_remove_governor(&tegra_devfreq_governor);
- clk_notifier_unregister(tegra->emc_clock, &tegra->rate_change_nb);
+ clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
dev_pm_opp_remove_all_dynamic(&pdev->dev);
reset_control_reset(tegra->reset);
--
2.22.0
^ permalink raw reply related
* [PATCH v3 08/22] PM / devfreq: tegra30: Move clk-notifier's registration to governor's start
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
There is no point in receiving of the notifications while governor is
stopped, let's keep them disabled like we do for the CPU freq-change
notifications. This also fixes a potential use-after-free bug if
notification happens after device's removal.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 33 ++++++++++++++++++-------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 502511ac4602..1751d17f9a1c 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -575,6 +575,19 @@ static int tegra_actmon_start(struct tegra_devfreq *tegra)
actmon_writel(tegra, ACTMON_SAMPLING_PERIOD - 1,
ACTMON_GLB_PERIOD_CTRL);
+ /*
+ * CLK notifications are needed in order to reconfigure the upper
+ * consecutive watermark in accordance to the actual clock rate
+ * to avoid unnecessary upper interrupts.
+ */
+ err = clk_notifier_register(tegra->emc_clock,
+ &tegra->clk_rate_change_nb);
+ if (err) {
+ dev_err(tegra->devfreq->dev.parent,
+ "Failed to register rate change notifier\n");
+ return err;
+ }
+
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_configure_device(tegra, &tegra->devices[i]);
@@ -601,6 +614,8 @@ static int tegra_actmon_start(struct tegra_devfreq *tegra)
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_stop_device(&tegra->devices[i]);
+ clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
+
return err;
}
@@ -617,6 +632,8 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_stop_device(&tegra->devices[i]);
+
+ clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
}
static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
@@ -861,22 +878,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, tegra);
+ tegra->clk_rate_change_nb.notifier_call = tegra_actmon_clk_notify_cb;
tegra->cpu_rate_change_nb.notifier_call = tegra_actmon_cpu_notify_cb;
INIT_WORK(&tegra->update_work, tegra_actmon_delayed_update);
- tegra->clk_rate_change_nb.notifier_call = tegra_actmon_clk_notify_cb;
- err = clk_notifier_register(tegra->emc_clock,
- &tegra->clk_rate_change_nb);
- if (err) {
- dev_err(&pdev->dev,
- "Failed to register rate change notifier\n");
- goto remove_opps;
- }
-
err = devfreq_add_governor(&tegra_devfreq_governor);
if (err) {
dev_err(&pdev->dev, "Failed to add governor: %d\n", err);
- goto unreg_notifier;
+ goto remove_opps;
}
tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock);
@@ -892,9 +901,6 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
remove_governor:
devfreq_remove_governor(&tegra_devfreq_governor);
-unreg_notifier:
- clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
-
remove_opps:
dev_pm_opp_remove_all_dynamic(&pdev->dev);
@@ -911,7 +917,6 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
devfreq_remove_device(tegra->devfreq);
devfreq_remove_governor(&tegra_devfreq_governor);
- clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
dev_pm_opp_remove_all_dynamic(&pdev->dev);
reset_control_reset(tegra->reset);
--
2.22.0
^ permalink raw reply related
* [PATCH v3 09/22] PM / devfreq: tegra30: Reset boosting on startup
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
Governor could be stopped while boosting is active. We have assumption
that everything is reset on governor's restart, including the boosting
value, which was missed.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 1751d17f9a1c..cca2fc15bafe 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -535,6 +535,9 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
{
u32 val = 0, target_freq;
+ /* we don't want boosting on restart */
+ dev->boost_freq = 0;
+
target_freq = clk_get_rate(tegra->emc_clock) / KHZ;
dev->avg_count = target_freq * ACTMON_SAMPLING_PERIOD;
device_writel(dev, dev->avg_count, ACTMON_DEV_INIT_AVG);
--
2.22.0
^ permalink raw reply related
* [PATCH v3 10/22] PM / devfreq: tegra30: Don't enable consecutive-down interrupt on startup
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
The consecutive-down event tells that we should perform frequency
de-boosting, but boosting is in a reset state on start and hence the
event won't do anything useful for us and it will be just a dummy
interrupt request.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index cca2fc15bafe..65f0363dbf1b 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -557,7 +557,6 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
<< ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_NUM_SHIFT;
val |= ACTMON_DEV_CTRL_AVG_ABOVE_WMARK_EN;
val |= ACTMON_DEV_CTRL_AVG_BELOW_WMARK_EN;
- val |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
val |= ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_EN;
val |= ACTMON_DEV_CTRL_ENB;
--
2.22.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox