Re: [linux-pm] OpPoint summary

From: Matthew Locke
Date: Sun Sep 17 2006 - 18:44:00 EST


Dave,

I am confused as to why you continue to duplicate work already done in PowerOP. We are still waiting to hear why you need to have a different operating point interface. As Eugeny and I have pointed out before (refer to the thread in my PowerOP vs OPpoint email), your interface doesn't make sense on a mulit cpu, SMP or otherwise, system. I recommend you redo your patches to use PowerOP which already supports this properly.


On Sep 16, 2006, at 10:07 PM, David Singleton wrote:

Pavel and Greg,

I've incorporated Pavels suggestions and only put suspend states
in the /sys/power/state file. The control file for frequency and
voltage operating
point transitions is now in /sys/power/operating_points/current_point.

The /sys/power/operating_points dirctory still contains the operating
points themselves, with a frequency, voltage and latency file
for each operating point.

The oppointd power manager has been changed to use the
new control file for operating points. It has been tested on
a centrino laptop, the 4 way Xeon server and the arm-pxa27x.

I finally got SMP tested on a 4 way Xeon server. The patch
that supports SMP Xeon's is the oppoint-x86-p4.patch in the series.

The only files in the core framework patch now are:

kernel/power/main.c
include/linux/pm.h
kernel/power/power.h

The full patch set is at

http://source.mvista.com/~dsingleton/2.6.18-rc7

The power manager source and patch is at:

http://source.mvista.com/~dsingleton/oppointd-1.2.3

Attached is the oppoint-core.patch.

David

Signed-Off-by: David Singleton <dsingleton@xxxxxxxxxx>

include/linux/pm.h | 30 +++-
kernel/power/main.c | 361 +++++++++++++++++++++++++++++++++++++++++++++------
kernel/power/power.h | 2
3 files changed, 350 insertions(+), 43 deletions(-)

Index: linux-2.6.17/kernel/power/main.c
===================================================================
--- linux-2.6.17.orig/kernel/power/main.c
+++ linux-2.6.17/kernel/power/main.c
@@ -16,6 +16,7 @@
#include <linux/init.h>
#include <linux/pm.h>
#include <linux/console.h>
+#include <linux/module.h>

#include "power.h"

@@ -49,7 +50,7 @@ void pm_set_ops(struct pm_ops * ops)
* the platform can enter the requested state.
*/

-static int suspend_prepare(suspend_state_t state)
+static int suspend_prepare(struct oppoint * state)
{
int error = 0;
unsigned int free_pages;
@@ -82,7 +83,7 @@ static int suspend_prepare(suspend_state
}

if (pm_ops->prepare) {
- if ((error = pm_ops->prepare(state)))
+ if ((error = pm_ops->prepare(state->type)))
goto Thaw;
}

@@ -94,7 +95,7 @@ static int suspend_prepare(suspend_state
return 0;
Finish:
if (pm_ops->finish)
- pm_ops->finish(state);
+ pm_ops->finish(state->type);
Thaw:
thaw_processes();
Enable_cpu:
@@ -104,7 +105,7 @@ static int suspend_prepare(suspend_state
}


-int suspend_enter(suspend_state_t state)
+int suspend_enter(struct oppoint * state)
{
int error = 0;
unsigned long flags;
@@ -115,7 +116,7 @@ int suspend_enter(suspend_state_t state)
printk(KERN_ERR "Some devices failed to power down\n");
goto Done;
}
- error = pm_ops->enter(state);
+ error = pm_ops->enter(state->type);
device_power_up();
Done:
local_irq_restore(flags);
@@ -131,36 +132,82 @@ int suspend_enter(suspend_state_t state)
* console that we've allocated. This is not called for suspend-to-disk.
*/

-static void suspend_finish(suspend_state_t state)
+static void suspend_finish(struct oppoint * state)
{
device_resume();
resume_console();
thaw_processes();
enable_nonboot_cpus();
if (pm_ops && pm_ops->finish)
- pm_ops->finish(state);
+ pm_ops->finish(state->type);
pm_restore_console();
}

+struct list_head pm_list;
+static struct oppoint standby = {
+ .name = "standby",
+ .type = PM_SUSPEND_STANDBY,
+};

+static struct oppoint mem = {
+ .name = "mem",
+ .type = PM_SUSPEND_MEM,
+ .frequency = 0,
+ .voltage = 0,
+ .latency = 150,
+};

-
-static const char * const pm_states[PM_SUSPEND_MAX] = {
- [PM_SUSPEND_STANDBY] = "standby",
- [PM_SUSPEND_MEM] = "mem",
#ifdef CONFIG_SOFTWARE_SUSPEND
- [PM_SUSPEND_DISK] = "disk",
+struct oppoint disk = {
+ .name = "disk",
+ .type = PM_SUSPEND_DISK,
+};
#endif
+
+struct oppoint pm_states = {
+ .name = "default",
+ .type = PM_FREQ_CHANGE,
};
+struct oppoint *current_state;
+
+/*
+ *
+ */
+static int pm_change_state(struct oppoint *state)
+{
+ int error = 0;
+
+ printk("OpPoint: changing from %s to %s\n", current_state->name,
+ state->name);
+ /*
+ * compare to current operating point.
+ * if different change to new operating point.
+ */
+ if (current_state == state)
+ goto out;
+
+ if ((error = state->prepare_transition(current_state, state)))
+ goto out;
+
+ if ((error = state->transition(current_state, state)))
+ state = current_state;
+
+ if ((error = state->finish_transition(current_state, state)) == 0)
+ current_state = state;
+
+out:
+ printk("OpPoint: State change returned %d\n", error);
+ return error;
+}

-static inline int valid_state(suspend_state_t state)
+static inline int valid_state(struct oppoint * state)
{
/* Suspend-to-disk does not really need low-level support.
* It can work with reboot if needed. */
- if (state == PM_SUSPEND_DISK)
+ if (state->type == PM_SUSPEND_DISK)
return 1;

- if (pm_ops && pm_ops->valid && !pm_ops->valid(state))
+ if (pm_ops && pm_ops->valid && !pm_ops->valid(state->type))
return 0;
return 1;
}
@@ -168,7 +215,7 @@ static inline int valid_state(suspend_st

/**
* enter_state - Do common work of entering low-power state.
- * @state: pm_state structure for state we're entering.
+ * @state: oppoint structure for state we're entering.
*
* Make sure we're the only ones trying to enter a sleep state. Fail
* if someone has beat us to it, since we don't want anything weird to
@@ -177,7 +224,7 @@ static inline int valid_state(suspend_st
* we've woken up).
*/

-static int enter_state(suspend_state_t state)
+static int enter_state(struct oppoint *state)
{
int error;

@@ -186,16 +233,21 @@ static int enter_state(suspend_state_t s
if (down_trylock(&pm_sem))
return -EBUSY;

- if (state == PM_SUSPEND_DISK) {
+ if (state->type == PM_SUSPEND_DISK) {
error = pm_suspend_disk();
goto Unlock;
}

- pr_debug("PM: Preparing system for %s sleep\n", pm_states[state]);
+ if (state->type == PM_FREQ_CHANGE || state->type == PM_VOLT_CHANGE) {
+ error = pm_change_state(state);
+ goto Unlock;
+ }
+
+ pr_debug("PM: Preparing system for %s sleep\n", state->name);
if ((error = suspend_prepare(state)))
goto Unlock;

- pr_debug("PM: Entering %s sleep\n", pm_states[state]);
+ pr_debug("PM: Entering %s sleep\n", state->name);
error = suspend_enter(state);

pr_debug("PM: Finishing wakeup.\n");
@@ -211,7 +263,15 @@ static int enter_state(suspend_state_t s
*/
int software_suspend(void)
{
- return enter_state(PM_SUSPEND_DISK);
+ struct oppoint *this, *next;
+ struct list_head *head = &mem.list;
+ int error = 0;
+
+ list_for_each_entry_safe(this, next, head, list) {
+ if (this->type == PM_SUSPEND_DISK)
+ error= enter_state(this);
+ }
+ return error;
}


@@ -223,9 +283,9 @@ int software_suspend(void)
* structure, and enter (above).
*/

-int pm_suspend(suspend_state_t state)
+int pm_suspend(struct oppoint * state)
{
- if (state > PM_SUSPEND_ON && state <= PM_SUSPEND_MAX)
+ if (state->type > PM_SUSPEND_ON && state->type <= PM_SUSPEND_MAX)
return enter_state(state);
return -EINVAL;
}
@@ -248,36 +308,35 @@ decl_subsys(power,NULL,NULL);

static ssize_t state_show(struct subsystem * subsys, char * buf)
{
- int i;
- char * s = buf;
+ struct oppoint *this, *next;
+ struct list_head *head = &pm_list;
+ char *s = buf;
+
+ list_for_each_entry_safe(this, next, head, list)
+ s += sprintf(s,"%s ", this->name);

- for (i = 0; i < PM_SUSPEND_MAX; i++) {
- if (pm_states[i] && valid_state(i))
- s += sprintf(s,"%s ", pm_states[i]);
- }
s += sprintf(s,"\n");
+
return (s - buf);
}

static ssize_t state_store(struct subsystem * subsys, const char *
buf, size_t n)
{
- suspend_state_t state = PM_SUSPEND_STANDBY;
- const char * const *s;
+ struct oppoint *this, *next;
+ struct list_head *head = &mem.list;
char *p;
- int error;
+ int error = -EINVAL;
int len;

p = memchr(buf, '\n', n);
len = p ? p - buf : n;
-
- for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) {
- if (*s && !strncmp(buf, *s, len))
+ list_for_each_entry_safe(this, next, head, list) {
+ if ((strlen(this->name) == len) &&
+ (!strncmp(this->name, buf, len))) {
+ error = enter_state(this);
break;
+ }
}
- if (state < PM_SUSPEND_MAX && *s)
- error = enter_state(state);
- else
- error = -EINVAL;
return error ? error : n;
}

@@ -292,12 +351,234 @@ static struct attribute_group attr_group
.attrs = g,
};

+static struct kobject oppoint_kobj = {
+ .kset = &power_subsys.kset,
+};
+
+struct oppoint_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct kobject * kobj, char * buf);
+ ssize_t (*store)(struct kobject * kobj, const char * buf,
size_t count);
+};
+
+#define to_oppoint(obj) container_of(obj,struct oppoint,kobj)
+#define to_oppoint_attr(_attr) container_of(_attr,struct
oppoint_attribute,attr)
+/*
+ * the frequency, voltage and latency files are readonly
+ */
+
+static ssize_t oppoint_voltage_show(struct kobject * kobj, char * buf)
+{
+ ssize_t len;
+ struct oppoint *opt = to_oppoint(kobj);
+
+ len = sprintf(buf, "%8d\n", opt->voltage);
+
+ return len;
+}
+
+static ssize_t oppoint_voltage_store(struct kobject * kobj, const char * buf,
+ size_t n)
+{
+ return -EINVAL;
+
+}
+
+static ssize_t oppoint_frequency_show(struct kobject * kobj, char * buf)
+{
+ ssize_t len;
+ struct oppoint *opt = to_oppoint(kobj);
+
+ len = sprintf(buf, "%8d\n", opt->frequency);
+
+ return len;
+}
+
+static ssize_t oppoint_frequency_store(struct kobject * kobj,
+ const char * buf, size_t n)
+{
+ return -EINVAL;
+
+}
+
+static ssize_t oppoint_point_show(struct kobject * kobj, char * buf)
+{
+ ssize_t len;
+
+ len = sprintf(buf, "%s\n", current_state->name);
+
+ return len;
+}
+
+static ssize_t oppoint_point_store(struct kobject * kobj, const char * buf,
+ size_t n)
+{
+ struct oppoint *this, *next;
+ struct list_head *head = &pm_states.list;
+ char *p;
+ int error = -EINVAL;
+ int len;
+
+ p = memchr(buf, '\n', n);
+ len = p ? p - buf : n;
+ list_for_each_entry_safe(this, next, head, list) {
+ if ((strlen(this->name) == len) &&
+ (!strncmp(this->name, buf, len))) {
+ error = enter_state(this);
+ break;
+ }
+ }
+ return error ? error : n;
+}
+
+static ssize_t oppoint_latency_show(struct kobject * kobj, char * buf)
+{
+ ssize_t len;
+ struct oppoint *opt = to_oppoint(kobj);
+
+ len = sprintf(buf, "%8d\n", opt->latency);
+
+ return len;
+}
+
+static ssize_t oppoint_latency_store(struct kobject * kobj,
+ const char * buf, size_t n)
+{
+ return -EINVAL;
+
+}
+
+static struct oppoint_attribute point_attr = {
+ .attr = {
+ .name = "current_point",
+ .mode = 0600,
+ },
+ .show = oppoint_point_show,
+ .store = oppoint_point_store,
+};
+
+static struct oppoint_attribute frequency_attr = {
+ .attr = {
+ .name = "frequency",
+ .mode = 0400,
+ },
+ .show = oppoint_frequency_show,
+ .store = oppoint_frequency_store,
+};
+
+static struct oppoint_attribute voltage_attr = {
+ .attr = {
+ .name = "voltage",
+ .mode = 0400,
+ },
+ .show = oppoint_voltage_show,
+ .store = oppoint_voltage_store,
+};
+
+static struct oppoint_attribute latency_attr = {
+ .attr = {
+ .name = "latency",
+ .mode = 0400,
+ },
+ .show = oppoint_latency_show,
+ .store = oppoint_latency_store,
+};
+
+static ssize_t
+oppoint_attr_show(struct kobject * kobj, struct attribute * attr, char * buf)
+{
+ struct oppoint_attribute * opt_attr = to_oppoint_attr(attr);
+ ssize_t ret = 0;
+
+ if (opt_attr->show)
+ ret = opt_attr->show(kobj,buf);
+ return ret;
+}
+
+static ssize_t
+oppoint_attr_store(struct kobject * kobj, struct attribute * attr,
+ const char * buf, size_t count)
+{
+ return -EINVAL;
+}
+
+static void oppoint_kobj_release(struct kobject *kobj)
+{
+ return;
+}
+
+static struct sysfs_ops oppoint_sysfs_ops = {
+ .show = oppoint_attr_show,
+ .store = oppoint_attr_store,
+};
+
+static struct attribute * oppoint_default_attrs[] = {
+ &frequency_attr.attr,
+ &voltage_attr.attr,
+ &latency_attr.attr,
+ NULL,
+};
+
+static struct kobj_type ktype_operating_point = {
+ .release = oppoint_kobj_release,
+ .sysfs_ops = &oppoint_sysfs_ops,
+ .default_attrs = oppoint_default_attrs,
+};
+
+int unregister_operating_point(struct oppoint *opt)
+{
+ down(&pm_sem);
+ list_del_init(&opt->list);
+ sysfs_remove_file(&opt->kobj, &frequency_attr.attr);
+ sysfs_remove_file(&opt->kobj, &voltage_attr.attr);
+ sysfs_remove_file(&opt->kobj, &latency_attr.attr);
+ up(&pm_sem);
+
+ return 0;
+}
+EXPORT_SYMBOL(unregister_operating_point);
+
+int register_operating_point(struct oppoint *opt)
+{
+ down(&pm_sem);
+ kobject_set_name(&opt->kobj, opt->name);
+ opt->kobj.kset = &power_subsys.kset;
+ opt->kobj.parent = &oppoint_kobj;
+ opt->kobj.ktype = &ktype_operating_point;
+ kobject_register(&opt->kobj);
+
+ sysfs_create_file(&opt->kobj, &frequency_attr.attr);
+ sysfs_create_file(&opt->kobj, &voltage_attr.attr);
+ sysfs_create_file(&opt->kobj, &latency_attr.attr);
+
+ list_add_tail(&opt->list, &pm_states.list);
+ up(&pm_sem);
+ return 0;
+}
+EXPORT_SYMBOL(register_operating_point);

static int __init pm_init(void)
{
+
int error = subsystem_register(&power_subsys);
- if (!error)
+ if (!error) {
error = sysfs_create_group(&power_subsys.kset.kobj,&attr_group);
+ kobject_set_name(&oppoint_kobj, "operating_points");
+ kobject_register(&oppoint_kobj);
+ sysfs_create_file(&oppoint_kobj, &point_attr.attr);
+ }
+
+
+ INIT_LIST_HEAD(&pm_states.list);
+ INIT_LIST_HEAD(&pm_list);
+
+#ifdef CONFIG_SOFTWARE_SUSPEND
+ list_add(&disk.list, &pm_list);
+#endif
+ list_add(&standby.list, &pm_list);
+ list_add(&mem.list, &pm_list);
+ current_state = &pm_states;
+
return error;
}

Index: linux-2.6.17/include/linux/pm.h
===================================================================
--- linux-2.6.17.orig/include/linux/pm.h
+++ linux-2.6.17/include/linux/pm.h
@@ -24,6 +24,7 @@
#ifdef __KERNEL__

#include <linux/list.h>
+#include <linux/kobject.h>
#include <asm/atomic.h>

/*
@@ -108,7 +109,32 @@ typedef int __bitwise suspend_state_t;
#define PM_SUSPEND_STANDBY ((__force suspend_state_t) 1)
#define PM_SUSPEND_MEM ((__force suspend_state_t) 3)
#define PM_SUSPEND_DISK ((__force suspend_state_t) 4)
-#define PM_SUSPEND_MAX ((__force suspend_state_t) 5)
+#define PM_FREQ_CHANGE ((__force suspend_state_t) 5)
+#define PM_VOLT_CHANGE ((__force suspend_state_t) 6)
+#define PM_SUSPEND_MAX ((__force suspend_state_t) 7)
+
+struct oppoint {
+ struct list_head list;
+ suspend_state_t type;
+ char *name;
+ unsigned int flags;
+ unsigned int frequency; /* in KHz */
+ unsigned int voltage; /* mV */
+ unsigned int latency; /* transition latency in us */
+ int (*prepare_transition)(struct oppoint *cur, struct oppoint *new);
+ int (*transition)(struct oppoint *cur, struct oppoint *new);
+ int (*finish_transition)(struct oppoint *cur, struct oppoint *new);
+
+ void *md_data; /* arch dependent data */
+ struct kobject kobj;
+};
+
+
+extern struct oppoint pm_states;
+extern struct oppoint *current_state;
+extern int register_operating_point(struct oppoint *opt);
+extern int unregister_operating_point(struct oppoint *opt);
+struct notifier_block;

typedef int __bitwise suspend_disk_method_t;

@@ -128,7 +154,7 @@ struct pm_ops {

extern void pm_set_ops(struct pm_ops *);
extern struct pm_ops *pm_ops;
-extern int pm_suspend(suspend_state_t state);
+extern int pm_suspend(struct oppoint *state);


/*
Index: linux-2.6.17/kernel/power/power.h
===================================================================
--- linux-2.6.17.orig/kernel/power/power.h
+++ linux-2.6.17/kernel/power/power.h
@@ -113,4 +113,4 @@ extern int swsusp_resume(void);
extern int swsusp_read(void);
extern int swsusp_write(void);
extern void swsusp_close(void);
-extern int suspend_enter(suspend_state_t state);
+extern int suspend_enter(struct oppoint * state);
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxx
https://lists.osdl.org/mailman/listinfo/linux-pm


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/