Hello, this is my first time posting to this list so please forgive me
if I'm violating protocol in some way. :) I've written a patch to the
framebuffer code to modify its behavior a bit. I am running on a
dual-headed system and I noticed when I was working in one console on
one monitor, the console on the other monitor was "frozen", not
updating itself. After some digging through the code I realized this
is because the two framebuffer drivers share the same framebuffer code
which stores a single pointer to the "current" virtual console. If a
VC is not current it is considered invisible and is not updated. So I
patched the code to store a pointer for each framebuffer to the
"foreground" VC on each one. It seems to work well but I'd like to get
others' input as this is my first time writing any kernel code, and to
be honest there is so much code it's difficult to get a clear picture
in my head of how the whole system works.
I've attached the patches to this message, a small one to
drivers/video/console/fbcon.h, and a larger one to fbcon.c. I would
appreciate it if some of you guys could look over this code and look
for any obvious errors, or better yet, hopefully someone else has a
multihead system they can try it on. The patches are against the
latest GIT source tree (torvalds, main branch, 2.6.15rc2) as of last
night.
Any replies to this message should be CC'ed directly to my e-mail
account (nathan dot cline .. gmail dot com) as I am not currently
subscribed to this list. I look forward to getting this patch of high
enough quality to submit to Linus. Thanks!
Nathan Cline wrote:
> Hello, this is my first time posting to this list so please forgive me
> if I'm violating protocol in some way. :) I've written a patch to the
> framebuffer code to modify its behavior a bit. I am running on a
> dual-headed system and I noticed when I was working in one console on
> one monitor, the console on the other monitor was "frozen", not
> updating itself. After some digging through the code I realized this
> is because the two framebuffer drivers share the same framebuffer code
> which stores a single pointer to the "current" virtual console. If a
> VC is not current it is considered invisible and is not updated.
Yes, there is only 1 active console at one time. And many multi-head
users has been asking (and confused) about this for some time now.
> So I
> patched the code to store a pointer for each framebuffer to the
> "foreground" VC on each one.
Well, you really don't need to store currcon_ptr in fbcon_ops. Using
vc_cons[ops->currcon].d should be enough to get the current vc attached
to the framebuffer. It will also make your patch smaller, and hopefully
easier to spot regressions.
> It seems to work well but I'd like to get
> others' input as this is my first time writing any kernel code, and to
> be honest there is so much code it's difficult to get a clear picture
> in my head of how the whole system works.
Yes, the console code is very confusing. I have been looking at it for
a long time and I still don't know every code pathway it's going to take.
Anyway, with single-head systems, I can't really see your patch causing
regressions, so that's good. With multi-head, I don't know.
Anyway, maybe Andrew can give this a whirl in the -mm tree? Just resubmit
the patch without the currcon_ptr from fbcon_ops. And add a
Signed-off-by: line. See:
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt
Tony
Nathan Cline wrote:
> --- fbcon.c 2005-11-23 23:49:10.000000000 -0600
> +++ fbcon.c.mine 2005-11-23 23:12:16.000000000 -0600
> @@ -390,12 +390,15 @@
> int mode;
>
> if (ops->currcon != -1)
> - vc = vc_cons[ops->currcon].d;
> + vc = ops->currcon_ptr;
As mentioned in the other thread, you don't need ops->currcon_ptr.
vc_cons[ops->currcon].d should give you the same results.
>
> - if (!vc || !CON_IS_VISIBLE(vc) ||
> + if (!vc)
> + return;
> +
> + if (!CON_IS_VISIBLE(vc) ||
> fbcon_is_inactive(vc, info) ||
> registered_fb[con2fb_map[vc->vc_num]] != info ||
> - vc_cons[ops->currcon].d->vc_deccm != 1)
> + vc->vc_deccm != 1)
> return;
> acquire_console_sem();
> p = &fb_display[vc->vc_num];
> @@ -753,6 +756,7 @@
> struct fbcon_ops *ops = info->fbcon_par;
>
> ops->currcon = fg_console;
> + ops->currcon_ptr = vc_cons[fg_console].d;
no need
>
> if (info->fbops->fb_set_par && !(ops->flags & FBCON_FLAGS_INIT))
> info->fbops->fb_set_par(info);
> @@ -766,7 +770,7 @@
> fbcon_preset_disp(info, &info->var, unit);
>
> if (show_logo) {
> - struct vc_data *fg_vc = vc_cons[fg_console].d;
> + struct vc_data *fg_vc = ops->currcon_ptr;
or *fg_vc = vc_cons[ops->currcon].d
> struct fb_info *fg_info =
> registered_fb[con2fb_map[fg_console]];
>
> @@ -775,7 +779,7 @@
> fg_vc->vc_rows);
> }
>
> - update_screen(vc_cons[fg_console].d);
> + update_screen(ops->currcon_ptr);
similarly, update_screen(vc_cons[ops->currcon].d);
> }
>
> /**
> @@ -929,6 +933,7 @@
>
> memset(ops, 0, sizeof(struct fbcon_ops));
> ops->currcon = -1;
> + ops->currcon_ptr = NULL;
no need.
> ops->graphics = 1;
> ops->cur_rotate = -1;
> info->fbcon_par = ops;
> @@ -1055,6 +1060,15 @@
> return;
>
> cap = info->flags;
> + ops = info->fbcon_par;
> +
> + if (ops->currcon == -1)
> + {
> + ops->currcon = vc->vc_num;
> + ops->currcon_ptr = vc;
no need for the last line.
> + }
> +
> + vc->vc_display_fg = &(ops->currcon_ptr);
>
> if (vc != svc || logo_shown == FBCON_LOGO_DONTSHOW ||
> (info->fix.type == FB_TYPE_TEXT))
> @@ -1091,7 +1105,6 @@
> if (!*vc->vc_uni_pagedir_loc)
> con_copy_unimap(vc, svc);
>
> - ops = info->fbcon_par;
> p->con_rotate = rotate;
> set_blitting_type(vc, info, NULL);
>
> @@ -1296,6 +1309,8 @@
> struct fbcon_ops *ops = info->fbcon_par;
> int rows, cols, charcnt = 256;
>
> + vc->vc_display_fg = &(ops->currcon_ptr);
> +
or vc->vc_display_fg = &vc_cons[ops->currcon].d;
> if (var_to_display(p, var, info))
> return;
> t = &fb_display[svc->vc_num];
> @@ -2048,7 +2063,7 @@
> struct fbcon_ops *ops;
> struct display *p = &fb_display[vc->vc_num];
> struct fb_var_screeninfo var;
> - int i, prev_console;
> + int prev_console;
>
> info = registered_fb[con2fb_map[vc->vc_num]];
> ops = info->fbcon_par;
> @@ -2073,21 +2088,10 @@
> prev_console = ops->currcon;
> if (prev_console != -1)
> old_info = registered_fb[con2fb_map[prev_console]];
> - /*
> - * FIXME: If we have multiple fbdev's loaded, we need to
> - * update all info->currcon. Perhaps, we can place this
> - * in a centralized structure, but this might break some
> - * drivers.
> - *
> - * info->currcon = vc->vc_num;
> - */
> - for (i = 0; i < FB_MAX; i++) {
> - if (registered_fb[i] != NULL && registered_fb[i]->fbcon_par) {
> - struct fbcon_ops *o = registered_fb[i]->fbcon_par;
>
> - o->currcon = vc->vc_num;
> - }
> - }
This hunk is the actual culprit why fbcon is behaving like it is :-)
> + ops->currcon = vc->vc_num;
> + ops->currcon_ptr = vc;
> +
> memset(&var, 0, sizeof(struct fb_var_screeninfo));
> display_to_var(&var, p);
> var.activate = FB_ACTIVATE_NOW;
> @@ -2103,13 +2107,6 @@
> fb_set_var(info, &var);
> ops->var = info->var;
>
> - if (old_info != NULL && old_info != info) {
> - if (info->fbops->fb_set_par)
> - info->fbops->fb_set_par(info);
Don't remove the above hunk. It's possible to have multiple fb_info's
(ie vga16fb, vesafb, xxxfb) driving the same hardware. This gives
the driver a chance to initialize the hardware when it's their turn
to take over.
> - fbcon_del_cursor_timer(old_info);
> - fbcon_add_cursor_timer(info);
> - }
> -
The above is safe to remove.
> set_blitting_type(vc, info, p);
> ops->cursor_reset = 1;
>
> @@ -2691,7 +2688,7 @@
>
> if (!ops || ops->currcon < 0)
> return;
> - vc = vc_cons[ops->currcon].d;
> + vc = ops->currcon_ptr;
Again, equivalent.
>
> /* Clear cursor, restore saved data */
> fbcon_cursor(vc, CM_ERASE);
> @@ -2704,7 +2701,7 @@
>
> if (!ops || ops->currcon < 0)
> return;
> - vc = vc_cons[ops->currcon].d;
> + vc = ops->currcon_ptr;
equivalent
>
> update_screen(vc);
> }
> @@ -2718,7 +2715,7 @@
>
> if (!ops || ops->currcon < 0)
> return;
> - vc = vc_cons[ops->currcon].d;
> + vc = ops->currcon_ptr;
equivalent
> if (vc->vc_mode != KD_TEXT ||
> registered_fb[con2fb_map[ops->currcon]] != info)
> return;
> @@ -2841,7 +2838,7 @@
> if (!ops || ops->currcon < 0)
> return;
>
> - vc = vc_cons[ops->currcon].d;
> + vc = ops->currcon_ptr;
equivalent
> if (vc->vc_mode != KD_TEXT ||
> registered_fb[con2fb_map[ops->currcon]] != info)
> return;
>
>
> ------------------------------------------------------------------------
>
> --- fbcon.h 2005-11-23 23:49:10.000000000 -0600
> +++ fbcon.h.mine 2005-11-23 23:46:07.000000000 -0600
> @@ -73,6 +73,7 @@
> struct fb_cursor cursor_state;
> struct display *p;
> int currcon; /* Current VC. */
> + struct vc_data *currcon_ptr;
No need for currcon_ptr.
Tony
Nathan Cline wrote:
>Hello, this is my first time posting to this list so please forgive me
>if I'm violating protocol in some way. :) I've written a patch to the
>framebuffer code to modify its behavior a bit. I am running on a
>dual-headed system and I noticed when I was working in one console on
>one monitor, the console on the other monitor was "frozen", not
>updating itself. After some digging through the code I realized this
>is because the two framebuffer drivers share the same framebuffer code
>which stores a single pointer to the "current" virtual console. If a
>VC is not current it is considered invisible and is not updated. So I
>patched the code to store a pointer for each framebuffer to the
>"foreground" VC on each one. It seems to work well but I'd like to get
>others' input as this is my first time writing any kernel code, and to
>be honest there is so much code it's difficult to get a clear picture
>in my head of how the whole system works.
>I've attached the patches to this message, a small one to
>drivers/video/console/fbcon.h, and a larger one to fbcon.c. I would
>appreciate it if some of you guys could look over this code and look
>for any obvious errors, or better yet, hopefully someone else has a
>multihead system they can try it on. The patches are against the
>latest GIT source tree (torvalds, main branch, 2.6.15rc2) as of last
>night.
>Any replies to this message should be CC'ed directly to my e-mail
>account (nathan dot cline .. gmail dot com) as I am not currently
>subscribed to this list. I look forward to getting this patch of high
>enough quality to submit to Linus. Thanks!
>
>
[PATCH SNIPPED]
Nathan you may also want to send a copy of your cleaned up patch to the
folks at [email protected] as that is list that
deals with multihead development / issues. We also have patches for
getting multiheaded X working as well.
Matt Keenan
On 11/24/05, Nathan Cline <[email protected]> wrote:
> Hello, this is my first time posting to this list so please forgive me
> if I'm violating protocol in some way. :)
Please read Documentation/SubmittingPatches before you resend.
Pekka