2002-12-04 14:28:20

by Antonino A. Daplas

[permalink] [raw]
Subject: [PATCH 1/3: FBDEV: VGA State Save/Restore module

Hi,

Attached is a patch against linux-2.5.50 + James Simmons fbdev.diff to
save and restore the VGA state. This includes character maps (plane
0-3), the colormap, and the video mode. This can be used in fb_open()
and fb_release() to go back to VGA text/graphics mode.

Usage:

struct fb_vgastate state;

/* To save VGA state */
state.flags = VGA_SAVE_MODE | VGA_SAVE_CMAP | VGA_SAVE_FONTS;
fb_save_vga(&state);

/* To restore VGA state */
fb_restore_vga(&state);

Limitations:
1. Restoring the VGA state from high-resolution graphics mode may
result in a corrupt display which can be corrected by switching
consoles. May need a screen redraw at this point. Restoring from VGA
graphics mode to text mode and vice versa is okay.

2. Assumes some things about the hardware which is not universally
correct: VGA memory base is at 0xA0000, memory size is 64KB, the
hardware palette is readable, etc.

Any comments welcome.

Tony

PS: Please reverse the early patch I submitted if it was applied --
vgastate.diff





Attachments:
vgastate1.diff (15.22 kB)

2002-12-04 18:34:03

by Petr Vandrovec

[permalink] [raw]
Subject: Re: [PATCH 1/3: FBDEV: VGA State Save/Restore module

On 4 Dec 02 at 22:26, Antonino Daplas wrote:
> +static void vga_cleanup(struct fb_vgastate *state)
> +{
> + if (state->fbbase)
> + iounmap(state->fbbase);
> +}

Nobody is setting state->fbbase to NULL, so
"save_vga(SAVE_FONT); restore_vga(); save_vga(0); restore_vga();"
will badly die. I think that you should change save prototype to
int fb_save_vga(int whattosave, struct fb_vgastate* state);, and at
the beginning of save you should do "memset(state, 0, sizeof(*state));"

And fbbase should not be in state variable, as this value is valid
only during duration of save or restore, not outside of them. Pass it
to function which needs it as an explicit argument.

It looks easier to me to make fb_vgastate larger structure with
crt/seq/cmap fields embeded, instead of kmallocing these portions.
Or at least allocate them together depending on 'whattosave' flag,
doing four kmalloc() to allocate about 64 bytes is waste of resources.

And do not use kmalloc() for > 4KB regions, use vmalloc (in
fonts save).

> + state->attr = kmalloc(state->num_attr, GFP_KERNEL);
> + state->crtc = kmalloc(state->num_crtc, GFP_KERNEL);
> + state->gfx = kmalloc(state->num_gfx, GFP_KERNEL);
> + state->seq = kmalloc(state->num_seq, GFP_KERNEL);
...
> + state->vga_font0 = kmalloc(8192 * 8, GFP_KERNEL);
...
> + state->vga_font1 = kmalloc(8192 * 8, GFP_KERNEL);
...
> + state->vga_text = kmalloc(8192 * 4, GFP_KERNEL);

And if my VGA documentation is correct, you are saving random
data into vga_text: first 8192 chars interleaved with
8192 bytes of garbage, plus attributes from chars 8192-16383 interleaved
with 8192 bytes of garbage.

When you program hardware to get access to planes (vga 16 color
graphics mode), every second byte from plane 0 contains character,
and every second byte from plane 1 contains character attribute
(it is that way because of bit A0 selects plane, while A15-A1.0
selects memory address, so odd in memory addresses are unreachable
in vga text mode).

And if you are using standard hardware, then font data live only in
plane 2, plane 3 is unused on VGA hardware in text mode. I think that
you should either save whole 256KB of memory, without deeper understanding,
or you should just save FONT 0 (first 32*256 bytes from plane 2) if you
want to save memory and you know that console was driven by vgacon in
text mode.
Petr Vandrovec
[email protected]

2002-12-04 22:06:37

by Antonino A. Daplas

[permalink] [raw]
Subject: Re: [PATCH 1/3: FBDEV: VGA State Save/Restore module

On Wed, 2002-12-04 at 23:41, Petr Vandrovec wrote:
> On 4 Dec 02 at 22:26, Antonino Daplas wrote:
> > +static void vga_cleanup(struct fb_vgastate *state)
> > +{
> > + if (state->fbbase)
> > + iounmap(state->fbbase);
> > +}
>
> Nobody is setting state->fbbase to NULL, so
Oops, yes I missed setting fbbase to NULL at the start.

> "save_vga(SAVE_FONT); restore_vga(); save_vga(0); restore_vga();"
> will badly die. I think that you should change save prototype to
> int fb_save_vga(int whattosave, struct fb_vgastate* state);, and at
> the beginning of save you should do "memset(state, 0, sizeof(*state));"
>
> And fbbase should not be in state variable, as this value is valid
> only during duration of save or restore, not outside of them. Pass it
> to function which needs it as an explicit argument.
>
The field fbbase is kind of temporary at the moment, fixed for the
0xa0000 window. I'm currently thinking of having the offset and the
size of the window optionally set by the calling process.

> It looks easier to me to make fb_vgastate larger structure with
> crt/seq/cmap fields embeded, instead of kmallocing these portions.
> Or at least allocate them together depending on 'whattosave' flag,
> doing four kmalloc() to allocate about 64 bytes is waste of resources.

I hate hardwiring fixed numbers :) It's also possible that some drivers
may find it sufficient to use the module to save the standard and its
own set of extended registers.

But you're correct, 4 kmallocs is wasteful, I wasn't thinking straight
:-). I'll just do one and adjust the offsets for each field.

[...]
> And if my VGA documentation is correct, you are saving random
> data into vga_text: first 8192 chars interleaved with
> 8192 bytes of garbage, plus attributes from chars 8192-16383 interleaved
> with 8192 bytes of garbage.
>
Right, I'm not sure about this part too. The docs say that this is true
for EGA compatible hardware. How about non-compliant hardware?

> When you program hardware to get access to planes (vga 16 color
> graphics mode), every second byte from plane 0 contains character,
> and every second byte from plane 1 contains character attribute
> (it is that way because of bit A0 selects plane, while A15-A1.0
> selects memory address, so odd in memory addresses are unreachable
> in vga text mode).
>
> And if you are using standard hardware, then font data live only in
> plane 2, plane 3 is unused on VGA hardware in text mode. I think that
> you should either save whole 256KB of memory, without deeper understanding,
> or you should just save FONT 0 (first 32*256 bytes from plane 2) if you
Only if saving the first character map in plane 2. Hardware can have as
much as 8 character maps per plane, each 8K in size for 64K. The same
setup is true for plane 3 fonts.

> want to save memory and you know that console was driven by vgacon in
> text mode.
To save memory, apps can explicitly choose what to save, but I don't
want to go finer than that, ie. save character maps 2,3,5 of plane 2
and 1,2,3 of plane 3. The current way of saving the text mode map may
be a bit wasteful, but better than being bitten by hardware that's
non-EGA compliant.

Tony



2002-12-04 22:26:54

by Petr Vandrovec

[permalink] [raw]
Subject: Re: [PATCH 1/3: FBDEV: VGA State Save/Restore module

On 5 Dec 02 at 6:05, Antonino Daplas wrote:
> On Wed, 2002-12-04 at 23:41, Petr Vandrovec wrote:
> > On 4 Dec 02 at 22:26, Antonino Daplas wrote:
> [...]
> > And if my VGA documentation is correct, you are saving random
> > data into vga_text: first 8192 chars interleaved with
> > 8192 bytes of garbage, plus attributes from chars 8192-16383 interleaved
> > with 8192 bytes of garbage.
> >
> Right, I'm not sure about this part too. The docs say that this is true
> for EGA compatible hardware. How about non-compliant hardware?

Like non-VGA? Like CGA/MDA? I thought that non-VGA/non-EGA adapters are
out of scope of this document ;-)

> > And if you are using standard hardware, then font data live only in
> > plane 2, plane 3 is unused on VGA hardware in text mode. I think that
> > you should either save whole 256KB of memory, without deeper understanding,
> > or you should just save FONT 0 (first 32*256 bytes from plane 2) if you

> Only if saving the first character map in plane 2. Hardware can have as
> much as 8 character maps per plane, each 8K in size for 64K. The same
> setup is true for plane 3 fonts.

How you select them? Mine doc says that font block 0 begins in plane 2
at offset 0, block 1 at offset 16KB, 2 at 32K, 3 at 48K, 4 at 8K, 5 at 24K,
6 at 40K, and last, 7th, at 56KB, and sequencer has two threebit fields...

> > want to save memory and you know that console was driven by vgacon in
> > text mode.
> To save memory, apps can explicitly choose what to save, but I don't
> want to go finer than that, ie. save character maps 2,3,5 of plane 2
> and 1,2,3 of plane 3. The current way of saving the text mode map may
> be a bit wasteful, but better than being bitten by hardware that's
> non-EGA compliant.

Look at vgacon. Uses font block 0,2,3 from plane 2 when built
without BROKEN_GRAPHICS_PROGRAMS, or 0,1 when built with
BROKEN_GRAPHICS_PROGRAMS. So if you want just restore vgacon environment,
save only these 4 blocks (4*8K = 32K). Or you want to save whole
VGA memory, and then save whole 256KB, without tricks while saving
planes 0 & 1.
Petr Vandrovec
[email protected]

2002-12-04 23:53:46

by Antonino A. Daplas

[permalink] [raw]
Subject: Re: [PATCH 1/3: FBDEV: VGA State Save/Restore module

On Thu, 2002-12-05 at 03:33, Petr Vandrovec wrote:
> On 5 Dec 02 at 6:05, Antonino Daplas wrote:
> > On Wed, 2002-12-04 at 23:41, Petr Vandrovec wrote:
> > > On 4 Dec 02 at 22:26, Antonino Daplas wrote:
> > [...]
> > > And if my VGA documentation is correct, you are saving random
> > > data into vga_text: first 8192 chars interleaved with
> > > 8192 bytes of garbage, plus attributes from chars 8192-16383 interleaved
> > > with 8192 bytes of garbage.
> > >
> > Right, I'm not sure about this part too. The docs say that this is true
> > for EGA compatible hardware. How about non-compliant hardware?
>
> Like non-VGA? Like CGA/MDA? I thought that non-VGA/non-EGA adapters are
> out of scope of this document ;-)
>
Okay :-), as I've said this is the part I'm not sure of. I'll do it
your way then, save 8K at offset 0 and save 8K at offset 16K. That
should be 16K used instead of 32K, right?

> > > And if you are using standard hardware, then font data live only in
> > > plane 2, plane 3 is unused on VGA hardware in text mode. I think that
> > > you should either save whole 256KB of memory, without deeper understanding,
> > > or you should just save FONT 0 (first 32*256 bytes from plane 2) if you
>
> > Only if saving the first character map in plane 2. Hardware can have as
> > much as 8 character maps per plane, each 8K in size for 64K. The same
> > setup is true for plane 3 fonts.
>
> How you select them? Mine doc says that font block 0 begins in plane 2
> at offset 0, block 1 at offset 16KB, 2 at 32K, 3 at 48K, 4 at 8K, 5 at 24K,
> 6 at 40K, and last, 7th, at 56KB, and sequencer has two threebit fields...
>
Selection was not the problem, the code just saves the entire 64K for
character maps 0-7.

> > > want to save memory and you know that console was driven by vgacon in
> > > text mode.
> > To save memory, apps can explicitly choose what to save, but I don't
> > want to go finer than that, ie. save character maps 2,3,5 of plane 2
> > and 1,2,3 of plane 3. The current way of saving the text mode map may
> > be a bit wasteful, but better than being bitten by hardware that's
> > non-EGA compliant.
>
> Look at vgacon. Uses font block 0,2,3 from plane 2 when built
> without BROKEN_GRAPHICS_PROGRAMS, or 0,1 when built with
> BROKEN_GRAPHICS_PROGRAMS. So if you want just restore vgacon environment,
> save only these 4 blocks (4*8K = 32K). Or you want to save whole
> VGA memory, and then save whole 256KB, without tricks while saving
> planes 0 & 1.

Okay, then. My approach was to be non-vgacon specific. I'll do it to
specifically target vgacon then.

To summarize:
plane 0/1 save 8K at offset 0 and 8K at offset 16K;
plane 2 save 32K at offset 0 (covers blocks 0-3),
plane 3 same for plane 2

Drivers can set VGA_SAVE_TEXT | VGA_SAVE_FONT0 to save planes 0-2. If
there are no complaints, I'll proceed doing it this way.

Thanks for the input Petr.

Tony


2002-12-05 02:34:42

by Petr Vandrovec

[permalink] [raw]
Subject: Re: [PATCH 1/3: FBDEV: VGA State Save/Restore module

On Thu, Dec 05, 2002 at 07:53:16AM +0500, Antonino Daplas wrote:
> On Thu, 2002-12-05 at 03:33, Petr Vandrovec wrote:
> > On 5 Dec 02 at 6:05, Antonino Daplas wrote:
> > planes 0 & 1.
>
> Okay, then. My approach was to be non-vgacon specific. I'll do it to
> specifically target vgacon then.
>
> To summarize:
> plane 0/1 save 8K at offset 0 and 8K at offset 16K;

I'm not sure about this one. AFAIK by default vgacon uses first 32KB
completely (every second byte...) from both 0/1, and can be configured
(by enabling VGA_CAN_DO_64KB) to use full 64KB.

> plane 2 save 32K at offset 0 (covers blocks 0-3),
> plane 3 same for plane 2
>
> Drivers can set VGA_SAVE_TEXT | VGA_SAVE_FONT0 to save planes 0-2. If
> there are no complaints, I'll proceed doing it this way.

I have no other problems.
Petr Vandrovec
[email protected]

2002-12-05 17:31:40

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 1/3: FBDEV: VGA State Save/Restore module


> Look at vgacon. Uses font block 0,2,3 from plane 2 when built
> without BROKEN_GRAPHICS_PROGRAMS, or 0,1 when built with
> BROKEN_GRAPHICS_PROGRAMS. So if you want just restore vgacon environment,
> save only these 4 blocks (4*8K = 32K). Or you want to save whole
> VGA memory, and then save whole 256KB, without tricks while saving
> planes 0 & 1.

If you could get ride of the dependance on BROKEN_GRAPHICS_PROGRAM I would
be really happy.


2002-12-05 17:23:54

by James Simmons

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 1/3: FBDEV: VGA State Save/Restore module


> Limitations:
> 1. Restoring the VGA state from high-resolution graphics mode may
> result in a corrupt display which can be corrected by switching
> consoles. May need a screen redraw at this point. Restoring from VGA
> graphics mode to text mode and vice versa is okay.
>
> 2. Assumes some things about the hardware which is not universally
> correct: VGA memory base is at 0xA0000, memory size is 64KB, the
> hardware palette is readable, etc.
>
> Any comments welcome.

One thing I like to suggest. I like to move the vga code in fb.h to vga.h.
Alot of fbdev devices don't have a VGA core.


2002-12-05 21:49:27

by Antonino A. Daplas

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 1/3: FBDEV: VGA State Save/Restore module

On Thu, 2002-12-05 at 22:31, James Simmons wrote:
>
> > Limitations:
> > 1. Restoring the VGA state from high-resolution graphics mode may
> > result in a corrupt display which can be corrected by switching
> > consoles. May need a screen redraw at this point. Restoring from VGA
> > graphics mode to text mode and vice versa is okay.
> >
> > 2. Assumes some things about the hardware which is not universally
> > correct: VGA memory base is at 0xA0000, memory size is 64KB, the
> > hardware palette is readable, etc.
> >
> > Any comments welcome.
>
> One thing I like to suggest. I like to move the vga code in fb.h to vga.h.
> Alot of fbdev devices don't have a VGA core.
>
>

Only the structure definition of fb_vgastate is in fb.h. For drivers
without a vga core, they'll just won't link to it and it won't be
compiled. Plus, vga.h is not a common header (not located in
include/asm or include/linux) and it contains a lot of declarations and
definitions which are irrelevant to most drivers or are already
duplicated. This will be messier, I think.

Maybe we can just enclose it in a macro, something like:

#ifdef FBDEV_HAS_VGACORE
...
#endif

Tony

2002-12-06 00:46:08

by James Simmons

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 1/3: FBDEV: VGA State Save/Restore module


> > One thing I like to suggest. I like to move the vga code in fb.h to vga.h.
> > Alot of fbdev devices don't have a VGA core.
> >
> >
>
> Only the structure definition of fb_vgastate is in fb.h. For drivers
> without a vga core, they'll just won't link to it and it won't be
> compiled. Plus, vga.h is not a common header (not located in
> include/asm or include/linux) and it contains a lot of declarations and
> definitions which are irrelevant to most drivers or are already
> duplicated. This will be messier, I think.

I like to move vga.h to include/video. And yes I like to clean it up. The
reason is I like to implement the function in vga.h and some in vgastate
into vgacon.c. It would be nice if vgacon could support different hardware
states per VC instead of changing every virtual console for everything.
The other dream is I like to see vgacon become firmware independent.

> Maybe we can just enclose it in a macro, something like:
>
> #ifdef FBDEV_HAS_VGACORE
> ...
> #endif

Yuck :-(

2002-12-06 12:27:31

by Antonino A. Daplas

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 1/3: FBDEV: VGA State Save/Restore module

On Fri, 2002-12-06 at 05:53, James Simmons wrote:
> >
> > Only the structure definition of fb_vgastate is in fb.h. For drivers
> > without a vga core, they'll just won't link to it and it won't be
> > compiled. Plus, vga.h is not a common header (not located in
> > include/asm or include/linux) and it contains a lot of declarations and
> > definitions which are irrelevant to most drivers or are already
> > duplicated. This will be messier, I think.
>
> I like to move vga.h to include/video. And yes I like to clean it up. The
> reason is I like to implement the function in vga.h and some in vgastate
> into vgacon.c. It would be nice if vgacon could support different hardware
> states per VC instead of changing every virtual console for everything.
> The other dream is I like to see vgacon become firmware independent.
>
OK.

Tony