Messages in this thread |  | | Date | Fri, 09 Dec 2005 12:53:27 +0100 | From | Knut Petersen <> | Subject | Re: [PATCH 1/1 2.6.15-rc4-git1] Fix switching to KD_TEXT |
| |
Hi Tony!
>Sorry, NAK for now. Unless other people agree that it is okay for them >to have an unconditional call to set_par() for every console switch. Note >that the set_par() in some drivers is terribly slow (several seconds at least). > >Let's wait a few days, if nobody disagrees with you, so be it. > > > Which hardware needs several seconds? I do run relatively slow hardware, but a complete set_par() needs less than 0.003 seconds with cyblafb.
If there really is hardware that needs _several_ seconds for _one_ set_par(), then we should really be more carefull when to call that function.
Let´s have a look at a switch from the framebuffer console to X and back.
Kernel is a 2.6.15-rc4-git1 with my current cyblafb + your patches needed for cyblafb + the patch we are talking about.
Framebuffer console is 1 with mode 1280x1024, X console is 7 and has been set to 1024x768 before X was activated.
Now I do run "chvt 7;sleep 3;chvt 1" on the vt 1.
[ 972.360841] [<c0282331>] cyblafb_set_par+0x31/0xd80 [ 972.360898] [<c027bb38>] fb_set_var+0x1e8/0x210 [ 972.360925] [<c02742fc>] fbcon_switch+0x16c/0x690 [ 972.360951] [<c02cc645>] redraw_screen+0xe5/0x1c0 [ 972.360989] [<c02c811a>] complete_change_console+0x2a/0xd0 [ 972.361018] [<c02c8203>] change_console+0x43/0x90 [ 972.361043] [<c02ceebd>] console_callback+0xed/0x110 [ 972.361101] [<c0127e6a>] worker_thread+0x1ba/0x260 [ 972.361143] [<c012c115>] kthread+0x95/0xd0 [ 972.361171] [<c0100c15>] kernel_thread_helper+0x5/0x10 [ 972.361212] cyblafb: Switching to new mode: fbset -g 1024 768 1024 768 8 -t 15385 160 24 29 3 136 6 [ 972.363308] cyblafb: pixclock = 64.99 MHz, k/m/n 1 b 6e
[ 972.363916] [<c0282331>] cyblafb_set_par+0x31/0xd80 [ 972.363945] [<c0274759>] fbcon_switch+0x5c9/0x690 [ 972.363969] [<c02cc645>] redraw_screen+0xe5/0x1c0 [ 972.363996] [<c02c811a>] complete_change_console+0x2a/0xd0 [ 972.364023] [<c02c8203>] change_console+0x43/0x90 [ 972.364047] [<c02ceebd>] console_callback+0xed/0x110 [ 972.364093] [<c0127e6a>] worker_thread+0x1ba/0x260 [ 972.364121] [<c012c115>] kthread+0x95/0xd0 [ 972.364143] [<c0100c15>] kernel_thread_helper+0x5/0x10 [ 972.364181] cyblafb: Switching to new mode: fbset -g 1024 768 1024 768 8 -t 15385 160 24 29 3 136 6 [ 972.366271] cyblafb: pixclock = 64.99 MHz, k/m/n 1 b 6e [ 972.366325] cyblafb: Switching to KD_GRAPHICS
Not so nice. Now let´s have a look at the log entries caused switching back to the framebuffer console:
This call to set_par() is the result of my patch:
[ 975.751407] [<c0282331>] cyblafb_set_par+0x31/0xd80 [ 975.751437] [<c027bb38>] fb_set_var+0x1e8/0x210 [ 975.751462] [<c02742fc>] fbcon_switch+0x16c/0x690 [ 975.751486] [<c02cc645>] redraw_screen+0xe5/0x1c0 [ 975.751513] [<c02c811a>] complete_change_console+0x2a/0xd0 [ 975.751540] [<c02c765d>] vt_ioctl+0x103d/0x19b0 [ 975.751564] [<c02c17eb>] tty_ioctl+0x18b/0x3d0 [ 975.751587] [<c016e92a>] do_ioctl+0x5a/0x90 [ 975.751613] [<c016eaeb>] vfs_ioctl+0x5b/0x1d0 [ 975.751640] [<c016eca5>] sys_ioctl+0x45/0x70 [ 975.751666] [<c01029a9>] syscall_call+0x7/0xb [ 975.751702] cyblafb: Switching to new mode: fbset -g 1280 1024 2048 4096 8 -t 7407 232 16 39 0 160 3 [ 975.753795] cyblafb: pixclock = 135.00 MHz, k/m/n 0 5 3a
Without that prior set_par() that´s the point where the enhanced cyblafb would have serious problems:
[ 975.753839] cyblafb_pan_display: entry [ 975.753853] [<c0281f7c>] cyblafb_pan_display+0x9c/0xb0 [ 975.753883] [<c027b911>] fb_pan_display+0x61/0xa0 [ 975.753925] [<c027ba4f>] fb_set_var+0xff/0x210 [ 975.753949] [<c02742fc>] fbcon_switch+0x16c/0x690 [ 975.753973] [<c02cc645>] redraw_screen+0xe5/0x1c0 [ 975.754000] [<c02c811a>] complete_change_console+0x2a/0xd0 [ 975.754027] [<c02c765d>] vt_ioctl+0x103d/0x19b0 [ 975.754051] [<c02c17eb>] tty_ioctl+0x18b/0x3d0 [ 975.754074] [<c016e92a>] do_ioctl+0x5a/0x90 [ 975.754100] [<c016eaeb>] vfs_ioctl+0x5b/0x1d0 [ 975.754127] [<c016eca5>] sys_ioctl+0x45/0x70 [ 975.754153] [<c01029a9>] syscall_call+0x7/0xb
Again a call to set_par:
[ 975.754744] [<c0282331>] cyblafb_set_par+0x31/0xd80 [ 975.754773] [<c0274759>] fbcon_switch+0x5c9/0x690 [ 975.754796] [<c02cc645>] redraw_screen+0xe5/0x1c0 [ 975.754823] [<c02c811a>] complete_change_console+0x2a/0xd0 [ 975.754850] [<c02c765d>] vt_ioctl+0x103d/0x19b0 [ 975.754875] [<c02c17eb>] tty_ioctl+0x18b/0x3d0 [ 975.754917] [<c016e92a>] do_ioctl+0x5a/0x90 [ 975.754943] [<c016eaeb>] vfs_ioctl+0x5b/0x1d0 [ 975.754970] [<c016eca5>] sys_ioctl+0x45/0x70 [ 975.754996] [<c01029a9>] syscall_call+0x7/0xb [ 975.755032] cyblafb: Switching to new mode: fbset -g 1280 1024 2048 4096 8 -t 7407 232 16 39 0 160 3 [ 975.757123] cyblafb: pixclock = 135.00 MHz, k/m/n 0 5 3a
Now the save_state() function is called:
[ 975.757745] cyblafb: Switching to KD_TEXT
and again a call to set_par():
[ 975.757759] [<c0282331>] cyblafb_set_par+0x31/0xd80 [ 975.757789] [<c027bb38>] fb_set_var+0x1e8/0x210 [ 975.757813] [<c027498a>] fbcon_blank+0x11a/0x1b0 [ 975.757837] [<c02cff17>] do_unblank_screen+0x67/0x120 [ 975.757869] [<c02c8130>] complete_change_console+0x40/0xd0 [ 975.757955] [<c02c765d>] vt_ioctl+0x103d/0x19b0 [ 975.757979] [<c02c17eb>] tty_ioctl+0x18b/0x3d0 [ 975.758002] [<c016e92a>] do_ioctl+0x5a/0x90 [ 975.758029] [<c016eaeb>] vfs_ioctl+0x5b/0x1d0 [ 975.758055] [<c016eca5>] sys_ioctl+0x45/0x70 [ 975.758081] [<c01029a9>] syscall_call+0x7/0xb [ 975.758117] cyblafb: Switching to new mode: fbset -g 1280 1024 2048 4096 8 -t 7407 232 16 39 0 160 3 [ 975.760205] cyblafb: pixclock = 135.00 MHz, k/m/n 0 5 3a
And now, the be really shure, again, a call to set_par():
[ 975.760827] [<c0282331>] cyblafb_set_par+0x31/0xd80 [ 975.760856] [<c0274759>] fbcon_switch+0x5c9/0x690 [ 975.760880] [<c02cc645>] redraw_screen+0xe5/0x1c0 [ 975.760923] [<c02749e6>] fbcon_blank+0x176/0x1b0 [ 975.760947] [<c02cff17>] do_unblank_screen+0x67/0x120 [ 975.760976] [<c02c8130>] complete_change_console+0x40/0xd0 [ 975.761002] [<c02c765d>] vt_ioctl+0x103d/0x19b0 [ 975.761027] [<c02c17eb>] tty_ioctl+0x18b/0x3d0 [ 975.761050] [<c016e92a>] do_ioctl+0x5a/0x90 [ 975.761076] [<c016eaeb>] vfs_ioctl+0x5b/0x1d0 [ 975.761102] [<c016eca5>] sys_ioctl+0x45/0x70 [ 975.761129] [<c01029a9>] syscall_call+0x7/0xb [ 975.761164] cyblafb: Switching to new mode: fbset -g 1280 1024 2048 4096 8 -t 7407 232 16 39 0 160 3 [ 975.763280] cyblafb: pixclock = 135.00 MHz, k/m/n 0 5 3a
First of all: set_par is executed too often. With and without my patch.
If we would know that vt x is nothing but the vt used for X, we could introduce a policy to completely ignore all framebuffer operations for that console. That also would result in a much nicer look of the modeswitch. Do we need a new kernel parameter or should this be implemented by detected which vt is switched to KD_GRAPHICS?
Switching from X to the console could be optimized a lot, I´m shure that you do agree. We do need _one_ call to set_par() etc, but this must be at the right place. And the right place is somewhere before any other driver operation is called, with the exception of check_var() and init functions obviously.
Think about your NAK carefully. I might submit a patch to skeletonfb.c that inserts a warning at all places that might be unexpectedly called with a hardware state different to that set by set_par(). ;-))))
And please don´t argue that X or certain releases are broken. That is true, but ordinary users will use those broken versions for years.
cu, Knut - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |