LinuxLists.cc - [PATCH] Framebuffer: client notification mecanism & PM

[permalink] [raw]

Subject: Re: [PATCH] Framebuffer: client notification mecanism & PM

On Tue, 2003-07-29 at 09:39, Petr Vandrovec wrote:

> Oops... You probably did not want IBM LCD diffs in fbmem.c posted.
>
> matroxfb tried to use a 'dead' for handling hot removal, but your code
> looks definitely saner

Hrm... right, that ibmlcd bit can go ;) Hopefully someone else will
get the real ibmlcd driver in sooner or later...

My code wasn't really intended to deal with hot-removal, more with
"what happens if printk occurs during the Power Management suspend
sequence", but I tried to keep the notification mecanism simple
enough so it could be used for that as well. Also, indeed, the fbcon
changes should deal with hot-removal in some way (though you surely
also want to "deatch" from the fbdev's in this case).

Among other things that could be used for is live monitor insertion/
removal detection (some HW are able to do that), "pivot" monitor
kind of things, etc... typically via the mode_changed hook.

Ben.

2003-07-29 16:55:28

[permalink] [raw]

Subject: Re: [PATCH] Framebuffer: client notification mecanism & PM

> > matroxfb tried to use a 'dead' for handling hot removal, but your code
> > looks definitely saner
>
> My code wasn't really intended to deal with hot-removal, more with
> "what happens if printk occurs during the Power Management suspend
> sequence", but I tried to keep the notification mecanism simple
> enough so it could be used for that as well. Also, indeed, the fbcon
> changes should deal with hot-removal in some way (though you surely
> also want to "deatch" from the fbdev's in this case).
>
> Among other things that could be used for is live monitor insertion/
> removal detection (some HW are able to do that), "pivot" monitor
> kind of things, etc... typically via the mode_changed hook.

I knew it was a matter of time before "client" management would happen.
Is this a 2.6.X thing tho or shoudl we wait for the next developement
cycle. I don't mind working on experimental stuff.

2003-07-29 20:23:10

[permalink] [raw]

Subject: Re: [PATCH] Framebuffer: client notification mecanism & PM

On Tue, 2003-07-29 at 12:55, James Simmons wrote:
> > > matroxfb tried to use a 'dead' for handling hot removal, but your code
> > > looks definitely saner
> >
> > My code wasn't really intended to deal with hot-removal, more with
> > "what happens if printk occurs during the Power Management suspend
> > sequence", but I tried to keep the notification mecanism simple
> > enough so it could be used for that as well. Also, indeed, the fbcon
> > changes should deal with hot-removal in some way (though you surely
> > also want to "deatch" from the fbdev's in this case).
> >
> > Among other things that could be used for is live monitor insertion/
> > removal detection (some HW are able to do that), "pivot" monitor
> > kind of things, etc... typically via the mode_changed hook.
>
> I knew it was a matter of time before "client" management would happen.
> Is this a 2.6.X thing tho or shoudl we wait for the next developement
> cycle. I don't mind working on experimental stuff.

We need that now for proper power management.

Ben.

2003-07-29 21:34:13

[permalink] [raw]

Subject: Re: [PATCH] Framebuffer: client notification mecanism & PM

2003-07-30 06:28:11

[permalink] [raw]

Subject: Re: [PATCH] Framebuffer: client notification mecanism & PM

Hi!

> There is room for improvement, but this is a first step that makes
> suspend/resume work more reliably on pmacs. I'll send aty128fb and
> radeonfb bits using this framework once it's merged.

> @@ -199,7 +199,8 @@
> if (!info || (info->cursor.rop == ROP_COPY))
> return;
> info->cursor.enable ^= 1;
> - info->fbops->fb_cursor(info, &info->cursor);
> + if (!info->suspended)
> + info->fbops->fb_cursor(info, &info->cursor);
> }
>
> y_break = p->vrows - p->yscroll;

Would not it be simpler to replace info->fbops with some dummy ones?
--
Pavel
Written on sharp zaurus, because my Velo1 broke. If you have Velo you don't need...

2003-07-30 12:14:59

[permalink] [raw]

Subject: Re: [PATCH] Framebuffer: client notification mecanism & PM

>
> Would not it be simpler to replace info->fbops with some dummy ones?

I did that for 2.4, that's indeed an option. I suppose we are completely
sure fbcon will never touch the framebuffer without going through those
fb_ops (fb_fillrect, fb_imageblit, etc...) ? That mean we would still
run a lot of useless code, but that's not too bad.

We still want the notification mecanism though, at least for proper
refresh on wakeup, and other "clients" (the DRI comes to mind) may
make good use of it as well.

Ben.

2003-07-30 12:23:08

[permalink] [raw]

Subject: Re: [PATCH] Framebuffer: client notification mecanism & PM

> > I knew it was a matter of time before "client" management would happen.
> > Is this a 2.6.X thing tho or shoudl we wait for the next developement
> > cycle. I don't mind working on experimental stuff.

Note, BTW, there's also a locking issue. Typically, we must make sure
the fbcon (which may be running another CPU) have finished doing whatever
it was doing before returning, and we don't get any mode change or
whatever ioctl initiated thing at that moment (or be in the middle of
doing it).

Right now, my code assumes any operation on the fbdev is protected by
the console sem. However, looking at the console & fbmem code, that is
not always true. Typically, userland ioctls in fbmem aren't, and vt.c
may call into consoles in some cases without holding it neither.

That should probably be fixed, though I haven't had time to work on
it yet.

Ben.

2003-07-30 13:14:52

[permalink] [raw]

Subject: [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

(resent with proper recipients list, sorry)

So, here's a new version that removes checking for suspend field
all over fbcon. I tried it here with a modified radeonfb that sets
the fbops to some dummy functions (provided by fbmem.c in this
patch though you may want to move them around).
In theory, I could implement power management properly without the client
notification mecanism by just setting the dummy fbops and directly calling
update_screen from the low level fbdev on resume, but that doesn't smell
good. The console subsystem is a bit of a nightmare to me, James, can we
currently have several consoles on several heads ? (that currcons in
vt.c is disturbing me). It may be worth doing better than just update_screen
(fg_console) when resuming in fbcon.c... (this callback will be called for
each fbdev who is waking up).

Let me know what you think, in all cases, asap so I can push the remaining
fbdev driver bits.

Ben.

===== drivers/video/fbmem.c 1.77 vs edited =====
--- 1.77/drivers/video/fbmem.c Mon May 26 20:51:43 2003
+++ edited/drivers/video/fbmem.c Wed Jul 30 08:59:06 2003
@@ -392,6 +392,9 @@
struct fb_info *registered_fb[FB_MAX];
int num_registered_fb;

+static LIST_HEAD(fb_clients);
+static DECLARE_MUTEX(fb_clients_lock);
+
#ifdef CONFIG_FB_OF
static int ofonly __initdata = 0;
#endif
@@ -935,6 +938,8 @@
fb_pan_display(info, &info->var);

fb_set_cmap(&info->cmap, 1, info);
+
+ fb_clients_call_mode_changed(info);
}
}
return 0;
@@ -1371,6 +1376,107 @@

__setup("video=", video_setup);

+/*
+ * Dummy operations used typically when power managing
+ */
+
+int fb_dummy_cursor(struct fb_info *info, struct fb_cursor *cursor)
+{
+ return 0;
+}
+
+void fb_dummy_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
+{
+}
+
+void fb_dummy_copyarea(struct fb_info *info, const struct fb_copyarea *area)
+{
+}
+
+void fb_dummy_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+}
+
+/*
+ * Wrappers to client calls
+ */
+
+int
+fb_set_suspend(struct fb_info *info, int suspended)
+{
+ if (suspended == info->suspended)
+ return 0;
+
+ info->suspended = suspended;
+ if (suspended)
+ fb_clients_call_suspended(info);
+ else
+ fb_clients_call_resumed(info);
+
+ return 0;
+}
+
+int register_fb_client(struct fb_client_ops *ops, void *data)
+{
+ struct fb_client *client;
+
+ client = kmalloc(sizeof(*client), GFP_KERNEL);
+ if (client == NULL)
+ return -ENOMEM;
+
+ memset(client, 0, sizeof(*client));
+ client->ops = ops;
+ client->data = data;
+
+ down(&fb_clients_lock);
+ list_add(&client->link, &fb_clients);
+ up(&fb_clients_lock);
+
+ return 0;
+}
+
+int unregister_fb_client(struct fb_client_ops *ops)
+{
+ struct fb_client *client = NULL;
+ struct list_head *pos;
+
+ down(&fb_clients_lock);
+ list_for_each(pos, &fb_clients) {
+ client = list_entry(pos, struct fb_client, link);
+ if (client->ops == ops) {
+ list_del(&client->link);
+ kfree(client);
+ break;
+ }
+ }
+ up(&fb_clients_lock);
+
+ return 0;
+}
+
+#define make_fb_client_call(name) \
+int fb_clients_call_##name(struct fb_info *info) \
+{ \
+ struct fb_client *client = NULL; \
+ struct list_head *pos; \
+\
+ down(&fb_clients_lock); \
+ list_for_each(pos, &fb_clients) { \
+ client = list_entry(pos, struct fb_client, link); \
+ if (try_module_get(client->ops->owner)) { \
+ if (client->ops->name) \
+ client->ops->name(client->data, info); \
+ module_put(client->ops->owner); \
+ } \
+ } \
+ up(&fb_clients_lock); \
+ return 0; \
+}
+
+make_fb_client_call(mode_changed)
+make_fb_client_call(suspended)
+make_fb_client_call(resumed)
+
/*
* Visible symbols for modules
*/
@@ -1387,5 +1493,15 @@
EXPORT_SYMBOL(fb_get_buffer_offset);
EXPORT_SYMBOL(move_buf_unaligned);
EXPORT_SYMBOL(move_buf_aligned);
+EXPORT_SYMBOL(fb_set_suspend);
+EXPORT_SYMBOL(register_fb_client);
+EXPORT_SYMBOL(unregister_fb_client);
+EXPORT_SYMBOL(fb_clients_call_mode_changed);
+EXPORT_SYMBOL(fb_clients_call_suspended);
+EXPORT_SYMBOL(fb_clients_call_resumed);
+EXPORT_SYMBOL(fb_dummy_fillrect);
+EXPORT_SYMBOL(fb_dummy_copyarea);
+EXPORT_SYMBOL(fb_dummy_imageblit);
+EXPORT_SYMBOL(fb_dummy_cursor);

MODULE_LICENSE("GPL");
===== drivers/video/console/fbcon.c 1.102 vs edited =====
--- 1.102/drivers/video/console/fbcon.c Tue May 6 09:50:50 2003
+++ edited/drivers/video/console/fbcon.c Wed Jul 30 08:59:44 2003
@@ -2259,6 +2259,19 @@
return 0;
}

+static void fbcon_suspended(void *data, struct fb_info *info)
+{
+ /* Here, we should do something to properly erase the
+ * cursor and synchronize with the cursor interrupt on
+ * SMP... (may not be that critical though...)
+ */
+}
+
+static void fbcon_resumed(void *data, struct fb_info *info)
+{
+ update_screen(fg_console);
+}
+
/*
* The console `switch' structure for the frame buffer based console
*/
@@ -2285,16 +2298,25 @@
.con_resize = fbcon_resize,
};

+static struct fb_client_ops fbcon_client = {
+ .owner = THIS_MODULE,
+ .mode_changed = NULL, /* TODO */
+ .suspended = fbcon_suspended,
+ .resumed = fbcon_resumed,
+};
+
int __init fb_console_init(void)
{
if (!num_registered_fb)
return -ENODEV;
take_over_console(&fb_con, first_fb_vc, last_fb_vc, fbcon_is_default);
+ register_fb_client(&fbcon_client, NULL);
return 0;
}

void __exit fb_console_exit(void)
{
+ unregister_fb_client(&fbcon_client);
give_up_console(&fb_con);
}

===== include/linux/fb.h 1.53 vs edited =====
--- 1.53/include/linux/fb.h Thu Apr 24 06:30:41 2003
+++ edited/include/linux/fb.h Wed Jul 30 09:01:06 2003
@@ -352,6 +352,44 @@
struct fb_info;
struct vm_area_struct;
struct file;
+struct fb_client;
+
+ /*
+ * Framebuffer clients. Currently, this is only used
+ * by fbcon to get notified of events on the framebuffer,
+ * though that should be extended to the userland interface
+ * some way.
+ *
+ * We should also add more callbacks to better deal with
+ * hotplug displays (add/removal notification). This is
+ * not to replaced by a device class, though it could be
+ * wrapped in a device interface according to the driver
+ * model, I have to think more about it.
+ *
+ * Locking rules: The callback should not take the console
+ * semaphore explicitely (call acquire_console_sem()) as it
+ * will typically already be owned.
+ *
+ */
+struct fb_client_ops {
+ struct module *owner;
+
+ /* Userland initiated mode change */
+ void (*mode_changed)(void *data, struct fb_info *info);
+ /* The device is beeing suspended, do not access from
+ * that point
+ */
+ void (*suspended)(void *data, struct fb_info *info);
+ /* The device is back to life, refresh screen
+ */
+ void (*resumed)(void *data, struct fb_info *info);
+};
+
+struct fb_client {
+ struct list_head link;
+ struct fb_client_ops *ops;
+ void *data;
+};

/*
* Frame buffer operations
@@ -399,6 +437,7 @@
int node;
int flags;
int open; /* Has this been open already ? */
+ int suspended; /* Is this currently suspended ? */
#define FBINFO_FLAG_MODULE 1 /* Low-level driver is a module */
struct fb_var_screeninfo var; /* Current var */
struct fb_fix_screeninfo fix; /* Current fix */
@@ -412,6 +451,7 @@
struct vc_data *display_fg; /* Console visible on this display */
int currcon; /* Current VC. */
void *pseudo_palette; /* Fake palette of 16 colors */
+
/* From here on everything is device dependent */
void *par;
};
@@ -475,6 +515,10 @@
extern void cfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
extern void cfb_copyarea(struct fb_info *info, const struct fb_copyarea *area);
extern void cfb_imageblit(struct fb_info *info, const struct fb_image *image);
+extern int fb_dummy_cursor(struct fb_info *info, struct fb_cursor *cursor);
+extern void fb_dummy_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
+extern void fb_dummy_copyarea(struct fb_info *info, const struct fb_copyarea *area);
+extern void fb_dummy_imageblit(struct fb_info *info, const struct fb_image *image);

/* drivers/video/fbmem.c */
extern int register_framebuffer(struct fb_info *fb_info);
@@ -574,6 +618,22 @@
const struct fb_videomode *default_mode,
unsigned int default_bpp);
#endif
+
+/* Power Management: called by low driver to notify other layers,
+ * driver should have acquired the console semaphore prior to
+ * calling this
+ */
+extern int fb_set_suspend(struct fb_info *info, int suspended);
+
+/*
+ * fb_client operations
+ */
+
+extern int register_fb_client(struct fb_client_ops *ops, void *data);
+extern int unregister_fb_client(struct fb_client_ops *ops);
+extern int fb_clients_call_mode_changed(struct fb_info *info);
+extern int fb_clients_call_suspended(struct fb_info *info);
+extern int fb_clients_call_resumed(struct fb_info *info);

#endif /* __KERNEL__ */

2003-07-30 13:11:44

[permalink] [raw]

Subject: [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

So, here's a new version that removes checking for suspend field
all over fbcon. I tried it here with a modified radeonfb that sets
the fbops to some dummy functions (provided by fbmem.c in this
patch though you may want to move them around).
In theory, I could implement power management properly without the client
notification mecanism by just setting the dummy fbops and directly calling
update_screen from the low level fbdev on resume, but that doesn't smell
good. The console subsystem is a bit of a nightmare to me, James, can we
currently have several consoles on several heads ? (that currcons in
vt.c is disturbing me). It may be worth doing better than just update_screen
(fg_console) when resuming in fbcon.c... (this callback will be called for
each fbdev who is waking up).

Let me know what you think, in all cases, asap so I can push the remaining
fbdev driver bits.

Ben.

===== drivers/video/fbmem.c 1.77 vs edited =====
--- 1.77/drivers/video/fbmem.c Mon May 26 20:51:43 2003
+++ edited/drivers/video/fbmem.c Wed Jul 30 08:59:06 2003
@@ -392,6 +392,9 @@
struct fb_info *registered_fb[FB_MAX];
int num_registered_fb;

+static LIST_HEAD(fb_clients);
+static DECLARE_MUTEX(fb_clients_lock);
+
#ifdef CONFIG_FB_OF
static int ofonly __initdata = 0;
#endif
@@ -935,6 +938,8 @@
fb_pan_display(info, &info->var);

fb_set_cmap(&info->cmap, 1, info);
+
+ fb_clients_call_mode_changed(info);
}
}
return 0;
@@ -1371,6 +1376,107 @@

__setup("video=", video_setup);

+/*
+ * Dummy operations used typically when power managing
+ */
+
+int fb_dummy_cursor(struct fb_info *info, struct fb_cursor *cursor)
+{
+ return 0;
+}
+
+void fb_dummy_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
+{
+}
+
+void fb_dummy_copyarea(struct fb_info *info, const struct fb_copyarea *area)
+{
+}
+
+void fb_dummy_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+}
+
+/*
+ * Wrappers to client calls
+ */
+
+int
+fb_set_suspend(struct fb_info *info, int suspended)
+{
+ if (suspended == info->suspended)
+ return 0;
+
+ info->suspended = suspended;
+ if (suspended)
+ fb_clients_call_suspended(info);
+ else
+ fb_clients_call_resumed(info);
+
+ return 0;
+}
+
+int register_fb_client(struct fb_client_ops *ops, void *data)
+{
+ struct fb_client *client;
+
+ client = kmalloc(sizeof(*client), GFP_KERNEL);
+ if (client == NULL)
+ return -ENOMEM;
+
+ memset(client, 0, sizeof(*client));
+ client->ops = ops;
+ client->data = data;
+
+ down(&fb_clients_lock);
+ list_add(&client->link, &fb_clients);
+ up(&fb_clients_lock);
+
+ return 0;
+}
+
+int unregister_fb_client(struct fb_client_ops *ops)
+{
+ struct fb_client *client = NULL;
+ struct list_head *pos;
+
+ down(&fb_clients_lock);
+ list_for_each(pos, &fb_clients) {
+ client = list_entry(pos, struct fb_client, link);
+ if (client->ops == ops) {
+ list_del(&client->link);
+ kfree(client);
+ break;
+ }
+ }
+ up(&fb_clients_lock);
+
+ return 0;
+}
+
+#define make_fb_client_call(name) \
+int fb_clients_call_##name(struct fb_info *info) \
+{ \
+ struct fb_client *client = NULL; \
+ struct list_head *pos; \
+\
+ down(&fb_clients_lock); \
+ list_for_each(pos, &fb_clients) { \
+ client = list_entry(pos, struct fb_client, link); \
+ if (try_module_get(client->ops->owner)) { \
+ if (client->ops->name) \
+ client->ops->name(client->data, info); \
+ module_put(client->ops->owner); \
+ } \
+ } \
+ up(&fb_clients_lock); \
+ return 0; \
+}
+
+make_fb_client_call(mode_changed)
+make_fb_client_call(suspended)
+make_fb_client_call(resumed)
+
/*
* Visible symbols for modules
*/
@@ -1387,5 +1493,15 @@
EXPORT_SYMBOL(fb_get_buffer_offset);
EXPORT_SYMBOL(move_buf_unaligned);
EXPORT_SYMBOL(move_buf_aligned);
+EXPORT_SYMBOL(fb_set_suspend);
+EXPORT_SYMBOL(register_fb_client);
+EXPORT_SYMBOL(unregister_fb_client);
+EXPORT_SYMBOL(fb_clients_call_mode_changed);
+EXPORT_SYMBOL(fb_clients_call_suspended);
+EXPORT_SYMBOL(fb_clients_call_resumed);
+EXPORT_SYMBOL(fb_dummy_fillrect);
+EXPORT_SYMBOL(fb_dummy_copyarea);
+EXPORT_SYMBOL(fb_dummy_imageblit);
+EXPORT_SYMBOL(fb_dummy_cursor);

MODULE_LICENSE("GPL");
===== drivers/video/console/fbcon.c 1.102 vs edited =====
--- 1.102/drivers/video/console/fbcon.c Tue May 6 09:50:50 2003
+++ edited/drivers/video/console/fbcon.c Wed Jul 30 08:59:44 2003
@@ -2259,6 +2259,19 @@
return 0;
}

+static void fbcon_suspended(void *data, struct fb_info *info)
+{
+ /* Here, we should do something to properly erase the
+ * cursor and synchronize with the cursor interrupt on
+ * SMP... (may not be that critical though...)
+ */
+}
+
+static void fbcon_resumed(void *data, struct fb_info *info)
+{
+ update_screen(fg_console);
+}
+
/*
* The console `switch' structure for the frame buffer based console
*/
@@ -2285,16 +2298,25 @@
.con_resize = fbcon_resize,
};

+static struct fb_client_ops fbcon_client = {
+ .owner = THIS_MODULE,
+ .mode_changed = NULL, /* TODO */
+ .suspended = fbcon_suspended,
+ .resumed = fbcon_resumed,
+};
+
int __init fb_console_init(void)
{
if (!num_registered_fb)
return -ENODEV;
take_over_console(&fb_con, first_fb_vc, last_fb_vc, fbcon_is_default);
+ register_fb_client(&fbcon_client, NULL);
return 0;
}

void __exit fb_console_exit(void)
{
+ unregister_fb_client(&fbcon_client);
give_up_console(&fb_con);
}

===== include/linux/fb.h 1.53 vs edited =====
--- 1.53/include/linux/fb.h Thu Apr 24 06:30:41 2003
+++ edited/include/linux/fb.h Wed Jul 30 09:01:06 2003
@@ -352,6 +352,44 @@
struct fb_info;
struct vm_area_struct;
struct file;
+struct fb_client;
+
+ /*
+ * Framebuffer clients. Currently, this is only used
+ * by fbcon to get notified of events on the framebuffer,
+ * though that should be extended to the userland interface
+ * some way.
+ *
+ * We should also add more callbacks to better deal with
+ * hotplug displays (add/removal notification). This is
+ * not to replaced by a device class, though it could be
+ * wrapped in a device interface according to the driver
+ * model, I have to think more about it.
+ *
+ * Locking rules: The callback should not take the console
+ * semaphore explicitely (call acquire_console_sem()) as it
+ * will typically already be owned.
+ *
+ */
+struct fb_client_ops {
+ struct module *owner;
+
+ /* Userland initiated mode change */
+ void (*mode_changed)(void *data, struct fb_info *info);
+ /* The device is beeing suspended, do not access from
+ * that point
+ */
+ void (*suspended)(void *data, struct fb_info *info);
+ /* The device is back to life, refresh screen
+ */
+ void (*resumed)(void *data, struct fb_info *info);
+};
+
+struct fb_client {
+ struct list_head link;
+ struct fb_client_ops *ops;
+ void *data;
+};

/*
* Frame buffer operations
@@ -399,6 +437,7 @@
int node;
int flags;
int open; /* Has this been open already ? */
+ int suspended; /* Is this currently suspended ? */
#define FBINFO_FLAG_MODULE 1 /* Low-level driver is a module */
struct fb_var_screeninfo var; /* Current var */
struct fb_fix_screeninfo fix; /* Current fix */
@@ -412,6 +451,7 @@
struct vc_data *display_fg; /* Console visible on this display */
int currcon; /* Current VC. */
void *pseudo_palette; /* Fake palette of 16 colors */
+
/* From here on everything is device dependent */
void *par;
};
@@ -475,6 +515,10 @@
extern void cfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
extern void cfb_copyarea(struct fb_info *info, const struct fb_copyarea *area);
extern void cfb_imageblit(struct fb_info *info, const struct fb_image *image);
+extern int fb_dummy_cursor(struct fb_info *info, struct fb_cursor *cursor);
+extern void fb_dummy_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
+extern void fb_dummy_copyarea(struct fb_info *info, const struct fb_copyarea *area);
+extern void fb_dummy_imageblit(struct fb_info *info, const struct fb_image *image);

/* drivers/video/fbmem.c */
extern int register_framebuffer(struct fb_info *fb_info);
@@ -574,6 +618,22 @@
const struct fb_videomode *default_mode,
unsigned int default_bpp);
#endif
+
+/* Power Management: called by low driver to notify other layers,
+ * driver should have acquired the console semaphore prior to
+ * calling this
+ */
+extern int fb_set_suspend(struct fb_info *info, int suspended);
+
+/*
+ * fb_client operations
+ */
+
+extern int register_fb_client(struct fb_client_ops *ops, void *data);
+extern int unregister_fb_client(struct fb_client_ops *ops);
+extern int fb_clients_call_mode_changed(struct fb_info *info);
+extern int fb_clients_call_suspended(struct fb_info *info);
+extern int fb_clients_call_resumed(struct fb_info *info);

#endif /* __KERNEL__ */

2003-08-01 10:05:38

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

James, if you are ok, can you get that upstream to Linus asap so
I can start pushing the driver bits for radeon & aty128 ?

Ben.

2003-08-06 23:54:12

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

> James, if you are ok, can you get that upstream to Linus asap so
> I can start pushing the driver bits for radeon & aty128 ?

Working on it. I'm thinking about also how it effects userland and how
userland affects the console if present. Basically the logic will go

pci suspend -> framebuffer driver supend function -> call each client

Just give me a few days to piece it together.

2003-08-07 09:38:51

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

On Thu, 2003-08-07 at 01:52, James Simmons wrote:
> > James, if you are ok, can you get that upstream to Linus asap so
> > I can start pushing the driver bits for radeon & aty128 ?
>
> Working on it. I'm thinking about also how it effects userland and how
> userland affects the console if present. Basically the logic will go
>
> pci suspend -> framebuffer driver supend function -> call each client
>
> Just give me a few days to piece it together.

Right now, we don't have a proper userland notification. So far, the
main affected thing is XFree, but this is ok as it will have received
a suspend request via /dev/apm_bios (which we emulate on PowerMacs),
and so won't touch the framebuffer until resumed.

There isn't much we can do against a userland client tapping the
framebuffer that it mmap'ed previously. I don't know how feasible it
would be to sort of "hack" this process mapping on the fly (would
involve some nasty SMP synchronisation issues) so that the userland
process is just put to sleep on fb access while the fb is suspended
(or get a SEGV). We probably want to extend the notification mecanism
to userland in some way, but this isn't something i cover in this
patch.

Ben.

2003-08-07 10:03:27

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

Hi!

> > > James, if you are ok, can you get that upstream to Linus asap so
> > > I can start pushing the driver bits for radeon & aty128 ?
> >
> > Working on it. I'm thinking about also how it effects userland and how
> > userland affects the console if present. Basically the logic will go
> >
> > pci suspend -> framebuffer driver supend function -> call each client
> >
> > Just give me a few days to piece it together.
>
> Right now, we don't have a proper userland notification. So far, the
> main affected thing is XFree, but this is ok as it will have received
> a suspend request via /dev/apm_bios (which we emulate on PowerMacs),
> and so won't touch the framebuffer until resumed.
>
> There isn't much we can do against a userland client tapping the
> framebuffer that it mmap'ed previously. I don't know how feasible it
> would be to sort of "hack" this process mapping on the fly (would
> involve some nasty SMP synchronisation issues) so that the userland
> process is just put to sleep on fb access while the fb is suspended
> (or get a SEGV). We probably want to extend the notification mecanism
> to userland in some way, but this isn't something i cover in this
> patch.

I believe solution to this is simple: always switch to kernel-owned
console during suspend. (swsusp does it, there's patch for S3 to do
the same). That way, Xfree (or qtopia or whoever) should clean up
after themselves and leave the console to the kernel. (See
kernel/power/console.c)
Pavel

--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

2003-08-07 13:34:35

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

On Thu, 2003-08-07 at 12:03, Pavel Machek wrote:

> I believe solution to this is simple: always switch to kernel-owned
> console during suspend. (swsusp does it, there's patch for S3 to do
> the same). That way, Xfree (or qtopia or whoever) should clean up
> after themselves and leave the console to the kernel. (See
> kernel/power/console.c)

I admit I quite like this solution. It would also help displaying
something sane (blank, pattern, whatever) on screen during driver
teardown instead of the junk left by X...

I'll look into including that switch into my pmac code as well
and see if it works properly in all cases (I think so). Also,
recent DRI CVS finally has working suspend/resume (works on
console switch too).

Ben.

2003-08-07 14:42:46

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

> I believe solution to this is simple: always switch to kernel-owned
> console during suspend. (swsusp does it, there's patch for S3 to do
> the same). That way, Xfree (or qtopia or whoever) should clean up
> after themselves and leave the console to the kernel. (See
> kernel/power/console.c)

I tried using it on pmac, but it causes hell with XFree. I'm not sure
what's up yet, I suspect it may be XFree still doing things after
calling the RELDISP ioctl but I'm not completely sure yet.

The setup XFree + DRI is working without switching to suspend console
(with only the apm_bios emulation for XFree to suspend/restore itself)
but not when switching to suspend console right before doing the apm
emulation callbacks (which should be ignored by X since it's no longer
the frontmost process at this point).

For some reason, it seems that after we have switched to the suspend
console, we race with the X server on accel engine, and on resume, the X
server just crashes.

Ben.

2003-08-07 17:28:05

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

> For some reason, it seems that after we have switched to the suspend
> console, we race with the X server on accel engine, and on resume, the X
> server just crashes.

Are you shutting down the accel engine in the fbdev driver on suspend?

2003-08-07 17:23:17

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

2003-08-07 17:26:22

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

> > I believe solution to this is simple: always switch to kernel-owned
> > console during suspend. (swsusp does it, there's patch for S3 to do
> > the same). That way, Xfree (or qtopia or whoever) should clean up
> > after themselves and leave the console to the kernel. (See
> > kernel/power/console.c)
>
> I admit I quite like this solution. It would also help displaying
> something sane (blank, pattern, whatever) on screen during driver
> teardown instead of the junk left by X...

There is the case of framebuffer without VT console. Of course X can't
work except with specific patches. X shouldn't be touching the console.

2003-08-07 20:29:29

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

Hi!

> > I believe solution to this is simple: always switch to kernel-owned
> > console during suspend. (swsusp does it, there's patch for S3 to do
> > the same). That way, Xfree (or qtopia or whoever) should clean up
> > after themselves and leave the console to the kernel. (See
> > kernel/power/console.c)
>
> I tried using it on pmac, but it causes hell with XFree. I'm not sure
> what's up yet, I suspect it may be XFree still doing things after
> calling the RELDISP ioctl but I'm not completely sure yet.

Sounds like XFree bug to me ;-).
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

2003-08-07 20:30:13

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

Hi!

> > I believe solution to this is simple: always switch to kernel-owned
> > console during suspend. (swsusp does it, there's patch for S3 to do
> > the same). That way, Xfree (or qtopia or whoever) should clean up
> > after themselves and leave the console to the kernel. (See
> > kernel/power/console.c)
>
> Not very helpful on embedded systems that use the framebuffer without the
> VT console.

Okay, that might be a problem. But such system will need some special
notification... Is VT so much overhead?
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

2003-08-07 21:39:34

by Nigel Cunningham

[permalink] [raw]

Subject: Re: [Linux-fbdev-devel] [PATCH] Framebuffer: 2nd try: client notification mecanism & PM

Hi.

Would this play well with the suspend process itself displaying output?
(Eg progress, errors...).

Regards,

Nigel

On Fri, 2003-08-08 at 01:32, Benjamin Herrenschmidt wrote:
> On Thu, 2003-08-07 at 12:03, Pavel Machek wrote:
>
> > I believe solution to this is simple: always switch to kernel-owned
> > console during suspend. (swsusp does it, there's patch for S3 to do
> > the same). That way, Xfree (or qtopia or whoever) should clean up
> > after themselves and leave the console to the kernel. (See
> > kernel/power/console.c)
>
> I admit I quite like this solution. It would also help displaying
> something sane (blank, pattern, whatever) on screen during driver
> teardown instead of the junk left by X...
>
> I'll look into including that switch into my pmac code as well
> and see if it works properly in all cases (I think so). Also,
> recent DRI CVS finally has working suspend/resume (works on
> console switch too).
>
> Ben.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Nigel Cunningham
495 St Georges Road South, Hastings 4201, New Zealand

You see, at just the right time, when we were still powerless,
Christ died for the ungodly.
-- Romans 5:6, NIV.

2003-08-07 21:52:03