Hello,
Damien Zammit, le dim. 27 oct. 2024 09:38:01 +0000, a ecrit:
> When bootloader sets a linear framebuffer mode and passes
> the required info to Hurd via multiboot info table, we
> can use this framebuffer as is.
> Otherwise, fall back to EGA text mode as before.
Great :)
> +static void
> +blit_glyph(bdf_font_t font, char *mem, int ch, uint32_t rgb, int width, int
> bpp)
> +{
> + int w, h;
> +
> + struct bdf_glyph *gl;
> +
> + gl = bdf_find_glyph (font, ch, 0);
> + if (!gl)
> + gl = bdf_find_glyph (font, -1, ch);
> + if (!gl)
> + gl = bdf_find_glyph (font, '?', 0);
> +
> + assert_backtrace(gl != 0);
> +
> + for (h = 0; h < vga_hc; h++)
> + {
> + for (w = 0; w < vga_wc; w++)
> + {
> + char *pixel = mem + bpp/8 * (w + width * h);
> + uint32_t colour = (gl->bitmap[h] & (1 << (7-w))) ? rgb : 0;
You don't want to hardcode 0 as background, otherwise various
applications that use black on color will be unreadable.
You want to pass to such functions both fg_rgb and bg_rgb (as gotten from
attr.bgcol)
> + pixel[0] = (colour >> 16) & 0xff;
> + pixel[1] = (colour >> 8) & 0xff;
> + pixel[2] = colour & 0xff;
> + }
> + }
> +}
> +
> +static void
> +blit_glyph_xor(bdf_font_t font, char *mem, int ch, uint32_t rgb, int width,
> int bpp)
> +{
> + int w, h;
> +
> + struct bdf_glyph *gl;
> + gl = bdf_find_glyph (font, ch, 0);
> + if (!gl)
> + gl = bdf_find_glyph (font, -1, ch);
> + if (!gl)
> + gl = bdf_find_glyph (font, '?', 0);
Rather create a function that looks for these alternatives, that we'll
be able to use in all the existing places that are already trying -1 as
alternative (and could then be using '?' as well like you did).
> + assert_backtrace(gl != 0);
That function could return a hardcoded fallback, see
console-client/vga-dynafont.c:594
> + for (h = 0; h < vga_hc; h++)
> + {
> + for (w = 0; w < vga_wc; w++)
> + {
> + char *pixel = mem + bpp/8 * (w + width * h);
> + uint32_t colour = (gl->bitmap[h] & (1 << (7-w))) ? rgb : 0;
> + pixel[0] ^= (colour >> 16) & 0xff;
> + pixel[1] ^= (colour >> 8) & 0xff;
> + pixel[2] ^= colour & 0xff;
> + }
> + }
> +}
> +
> +static error_t
> +fb_init(void)
> +{
> + error_t err;
> + int fd;
> +
> + fd = open ("/dev/mem", O_RDWR);
> + if (fd >= 0)
Better invert that test, that'll avoid the indentation.
> + {
> + vga_videomem = mmap (0, vga_width * vga_height * vga_bpp/8, PROT_READ
> | PROT_WRITE,
> + MAP_SHARED, fd, vga_fb);
> + err = errno;
> + close (fd);
> + if (vga_videomem == (void *) -1)
Rather use MMAP_FAILED.
> + return err;
> + }
> + else
> + return errno;
> +
> + /* Clear screen */
> + memset (vga_videomem, 0, vga_width * vga_height * vga_bpp/8);
> + return 0;
> +}
[...]
> +/* First 8 colours of EGA palette */
> +uint32_t console_rgb[8] = {
> + 0x000000, /* black */
> + 0x0000aa, /* blue */
> + 0x00aa00, /* green */
> + 0x00aaaa, /* cyan */
> + 0xaa0000, /* red */
> + 0xaa00aa, /* magenta */
> + 0xffff55, /* yellow */
That's rather aa5500.
> + 0xaaaaaa /* white */
> +};
Also, we don't want to use the EGA palette, but the ANSI palette:
\x1b[30m foreground black
\x1b[31m foreground red
\x1b[32m foreground green
\x1b[33m foreground yellow
\x1b[34m foreground blue
\x1b[35m foreground magenta
\x1b[36m foreground cyan
\x1b[37m foreground white
otherwise the colors will be wrong.
And while at it, you can hardcode the 8 bright colors:
0x555555 bright black
0xff5555 bright red
0x55ff55 bright green
0xffff55 bright yellow
0x5555ff bright blue
0xff55ff bright magenta
0x55ffff bright cyan
0xffffff bright white
to be used when attr.intensity == CONS_ATTR_INTENSITY_BOLD
> +static void
> +hide_mousecursor (struct fb_display *disp)
> +{
> + char *oldpos = vga_videomem + vga_bpp/8 * ((int) disp->mousecursor.posy *
> vga_hc * disp->width
> + + (int) disp->mousecursor.posx * vga_wc);
Make this computation a macro, you have a lot of such.
> + if (!disp->mousecursor.visible)
> + return;
> +
> + /* First remove the old cursor. */
> + blit_glyph_xor (disp->font, oldpos, 'X', 0x00ff00, disp->width, vga_bpp);
> + disp->mousecursor.visible = 0;
> +}
> +static void
> +hide_cursor(struct fb_display *disp)
> +{
> + int x, y;
> + char *curpos;
> +
> + if (cursor_hidden)
> + return;
> +
> + /* Remove old cursor */
> + x = cursor_pos % (disp->width/vga_wc);
> + y = cursor_pos / (disp->width/vga_wc);
> + curpos = vga_videomem + vga_bpp/8 * (y * vga_hc * disp->width + x *
> vga_wc);
> + blit_glyph_xor (disp->font, curpos, '_', 0xffffff, disp->width, vga_bpp);
> + cursor_hidden = 1;
> +}
Rather use 0x2581, otherwise the cursor will be invisible over '_'.
> +/* Set the cursor's position on display HANDLE to column COL and row
> + ROW. */
> +static error_t
> +fb_display_set_cursor_pos (void *handle, uint32_t col, uint32_t row)
> +{
> + struct fb_display *disp = handle;
> +
> + /* If the cursor did not move from the character position, don't
> + bother about updating the cursor position. */
> + if (cursor_state && (col == (cursor_pos % (disp->width/vga_wc)))
> + && (row == (cursor_pos / (disp->width/vga_wc))))
> + return 0;
> +
> + hide_cursor (disp);
Since hide_cursor will xor, we'd want an if (cursor_state) here too.
> + cursor_pos = col + disp->width/vga_wc * row;
It doesn't seem useful to keep cursor_pos as just one number? Better
have cursor_pos_x and _y.
> + if (cursor_state)
> + draw_cursor (disp);
> +
> + return 0;
> +}
> +
> +/* Deallocate any scarce resources occupied by the LENGTH characters
> + from column COL and row ROW. */
You don't have anything to do here. See the clear field comment:
«
If there are no scarce resources, the caller might do nothing.
»
> +static error_t
> +fb_display_clear (void *handle, size_t length, uint32_t col, uint32_t row)
> +{
> + int len;
> + struct fb_display *disp = handle;
> + struct fbchr *refpos = &disp->refmatrix[row][0];
> + char *pos = vga_videomem;
> +
> +/* Scroll the display by the desired number of lines. The area that becomes
> + free will be filled in a subsequent write call. */
> +static error_t
> +fb_display_scroll (void *handle, int delta)
> +{
> + struct fb_display *disp = handle;
> + int pixels, chars, nextdelta = 0;
> + char *pos = vga_videomem;
> + uint32_t r;
> +
> + if (delta > disp->height/vga_hc)
> + {
> + nextdelta = delta - disp->height/vga_hc;
> + delta = disp->height/vga_hc;
> + }
> + else if (delta < -(disp->height/vga_hc))
> + {
> + nextdelta = delta + disp->height/vga_hc;
> + delta = -(disp->height/vga_hc);
> + }
There is no use trying to wrap around like this: if delta is larger than
the height, we just don't copy anything, and the caller will actually
write the whole screen. See the comment of the scroll field.
> + pixels = abs(delta)*vga_hc * disp->width;
> + chars = abs(delta) * disp->width/vga_wc;
> +
> + hide_mousecursor (disp);
> + hide_cursor (disp);
> +
> + /* XXX: If the virtual console is bigger than the physical console it is
> + impossible to scroll because the data to scroll is not in memory. */
> + if (current_height > disp->height/vga_hc)
> + return ENOTSUP;
> +
> + if (delta > 0)
> + {
> + memmove (vga_videomem, vga_videomem + vga_bpp/8 * pixels,
> + vga_bpp/8 * disp->width * (disp->height - delta*vga_hc));
> + }
> + else
> + {
> + memmove (vga_videomem + vga_bpp/8 * pixels, vga_videomem,
> + vga_bpp/8 * disp->width * (disp->height + delta*vga_hc));
> + }
> +
> + if (delta > 0)
> + {
> + r = disp->height/vga_hc - delta;
> + memmove (&disp->refmatrix[0][0], &disp->refmatrix[0][0] + chars,
> + sizeof (struct fbchr) * disp->width/vga_wc * r);
> + pos += vga_bpp/8 * (r*vga_hc * disp->width);
> + }
> + else
> + {
> + r = 0;
> + memmove (&disp->refmatrix[0][0] + chars, &disp->refmatrix[0][0],
> + sizeof (struct fbchr) * disp->width/vga_wc *
> (disp->height/vga_hc + delta));
> + }
> +
> + fb_display_clear (disp, chars, 0, r);
We don't need to clear, we have nothing there to do.
> + if (nextdelta)
> + fb_display_scroll (handle, nextdelta);
> +
> + return 0;
> +}
> +
> +
> +
> +/* Write the text STR with LENGTH characters to column COL and row
> + ROW. */
> +static error_t
> +fb_display_write (void *handle, conchar_t *str, size_t length,
> + uint32_t col, uint32_t row)
> +{
> + struct fb_display *disp = handle;
> + char *pos;
> + struct fbchr *refpos = &disp->refmatrix[row][col];
> + char *mouse_cursor_pos;
> +
> + hide_mousecursor (disp);
> + hide_cursor (disp);
> +
> + /* The starting column is outside the physical screen. */
> + if (disp->width/vga_wc < current_width && col >= disp->width/vga_wc)
> + {
> + size_t skip = current_width - disp->width/vga_wc;
> + str += skip;
> + length -= skip;
> + col = 0;
> + row += skip / current_width + 1;
Why skip/current_width? By construction, skip < current_width. We only
ever go to the next line, we have no reason to skip several lines.
> + }
> +
> + mouse_cursor_pos = vga_videomem + vga_bpp/8
> + * ((int) disp->mousecursor.posy*vga_hc
> + * disp->width + (int) disp->mousecursor.posx*vga_wc);
> +
> + while (length--)
> + {
> + int charval = str->chr;
You changed the col++ position. Better do the same in the vga driver, to
avoid divergence between both.
> + /* The virtual console is smaller than the physical screen. */
> + if (col >= current_width)
> + {
> + size_t skip = disp->width/vga_wc - current_width;
> + refpos += skip;
> + col = 0;
> + row += skip / disp->width/vga_wc + 1;
ditto
> + }
> + /* The virtual console is bigger than the physical console. */
> + else if (disp->width/vga_wc < current_width && col ==
> disp->width/vga_wc)
> + {
> + size_t skip = current_width - disp->width/vga_wc;
> + str += skip;
> + length -= skip;
> + col = skip % current_width;
> + row += skip / current_width;
ditto
> + }
> +
> + /* The screen is filled until the bottom of the screen. */
> + if (row >= disp->height/vga_hc)
Why /vga_hc??
> + return 0;
> +
> + pos = vga_videomem + (col*vga_wc + (row*vga_hc * disp->width)) *
> vga_bpp/8;
> +
> + /* blit glyph to screen */
> + blit_glyph(disp->font, pos, charval, console_rgb[str->attr.fgcol],
> disp->width, vga_bpp);
> +
> + if (pos == mouse_cursor_pos)
> + disp->mousecursor.visible = 0;
> +
> + /* Perform reference counting. */
That comment was for code that you removed.
> + refpos->used = 1;
> + refpos->chr = charval;
> + refpos->fgcol = str->attr.fgcol;
> + refpos++;
> + col++;
> +
> + /* Go to next character. */
> + str++;
> + }
> + return 0;
> +}
> +
> +static error_t
> +fb_set_mousecursor_pos (void *handle, float x, float y)
> +{
> + struct fb_display *disp = handle;
> +
> + /* If the mouse did not move from the character position, don't
> + bother about updating the cursor position. */
> + if (disp->mousecursor.visible && x == (int) disp->mousecursor.posx
> + && y == (int) disp->mousecursor.posy)
> + return 0;
> +
> + hide_mousecursor (disp);
Since hide_mousecursor will xor, we'd want an if (cursor_state) here too.
> + disp->mousecursor.posx = x;
> + disp->mousecursor.posy = y;
> +
> + if (disp->mousecursor.enabled)
> + draw_mousecursor (disp);
> +
> + return 0;
> +}
> +
> +
> diff --git a/console-client/fb.h b/console-client/fb.h
> new file mode 100644
> index 00000000..23e0a51a
> --- /dev/null
> +++ b/console-client/fb.h
> @@ -0,0 +1,74 @@
> +struct fbchr
> +{
> + unsigned int used : 1;
> + unsigned int chr : 9;
You want a whole wchar_t, we don't have the 512-glyph limitation
> + unsigned int fgcol: 3;
> +};
> +
> +typedef struct fb_mousecursor
> +{
> + float posx;
> + float posy;
> + int visible;
> + int enabled;
> +} fb_mousecursor_t;
> +
> +struct fb_display
> +{
> + /* The font for this display. */
> + bdf_font_t font;
> +
> + int df_size;
> + int df_width;
> +
> + /* The color palette. */
> + dynacolor_t dc;
This is unused (no hardware palette)
> + int width;
> + int height;
> +
> + /* The state of the mouse cursor. */
> + fb_mousecursor_t mousecursor;
> +
> + /* The position of the cursor, x_pos + y_pos * width (in characters) */
> + int cursor_pos;
> +
> + /* Remember for each cell on the display the glyph written to it and
> + the colours assigned. 0 means unassigned. */
> +
> + struct fbchr refmatrix[VGA_VIDEO_MEM_MAX_H /
> VGA_PIXELS_H][VGA_VIDEO_MEM_MAX_W / VGA_PIXELS_W];
> +};
> +
> +#endif
> diff --git a/console-client/vga-hw.h b/console-client/vga-hw.h
> index b4212e62..71ffb721 100644
> --- a/console-client/vga-hw.h
> +++ b/console-client/vga-hw.h
> @@ -24,6 +24,13 @@
> #define VGA_VIDEO_MEM_BASE_ADDR 0x0b8000
> #define VGA_VIDEO_MEM_LENGTH 0x004000
>
> +#define VGA_VIDEO_MEM_MAX_W 1920
> +#define VGA_VIDEO_MEM_MAX_H 1080
> +#define VGA_VIDEO_MEM_MAX_BPP 32
> +
> +#define VGA_PIXELS_W 8
> +#define VGA_PIXELS_H 16
This is not related to hardware conditions, better put them fb.h.
> #define VGA_FONT_BUFFER 8
> #define VGA_FONT_SIZE 256
> #define VGA_FONT_HEIGHT 32
> diff --git a/console-client/vga-support.c b/console-client/vga-support.c
> index 32ede46d..0fa229d4 100644
> --- a/console-client/vga-support.c
> +++ b/console-client/vga-support.c
> @@ -25,6 +25,9 @@
> #include <sys/io.h>
> #include <sys/mman.h>
> #include <sys/types.h>
> +#include <device/device.h>
> +#include <hurd.h>
> +#include <mach.h>
> #include <string.h>
> #include <stdlib.h>
>
> @@ -32,8 +35,15 @@
> #include "vga-support.h"
>
>
> -/* The base of the video memory mapping. */
> +/* The parameters of the video memory mapping. */
> char *vga_videomem;
> +size_t vga_fb;
That should rather be an off_t
> +int vga_pitch;
> +int vga_width;
> +int vga_height;
> +int vga_bpp;
> +int vga_wc;
> +int vga_hc;
>
> /* The saved state of the VGA card. */
> struct vga_state
> @@ -56,14 +66,65 @@ struct vga_state
> unsigned char attr_mode;
>
> /* Alignment is required by some "hardware", and optimizes transfers. */
> - char videomem[2 * 80 * 25]
> + char videomem[VGA_VIDEO_MEM_MAX_W * VGA_VIDEO_MEM_MAX_H *
> VGA_VIDEO_MEM_MAX_BPP / 8]
? No need to tinker with the vga driver videomem, and it's text cells
there, not pixels.
> __attribute__ ((aligned (__BIGGEST_ALIGNMENT__)));
> - unsigned char fontmem[2 * VGA_FONT_SIZE * VGA_FONT_HEIGHT]
> + unsigned char fontmem[VGA_VIDEO_MEM_MAX_BPP / 8 * VGA_FONT_SIZE *
> VGA_FONT_HEIGHT]
> __attribute__ ((aligned (__BIGGEST_ALIGNMENT__)));
Even more so: VGA fonts is really always 2*256 glyphs.
> };
>
> static struct vga_state *vga_state;
>
> +error_t
> +vga_get_multiboot_params (void)
> +{
> + error_t ret = 0;
> + mach_port_t master_device, mbinfo_dev;
> + struct multiboot_raw_info mbi;
> + char buf[sizeof(struct multiboot_raw_info)] = {0};
? No need to initialize the array.
> + char *bufptr = &buf[0];
> + uint32_t bytes = sizeof(struct multiboot_raw_info);
> + uint32_t bytes_read = 0;
> +
> + ret = get_privileged_ports (NULL, &master_device);
> + if (ret)
> + goto fail;
> +
> + ret = device_open (master_device, D_READ, "mbinfo", &mbinfo_dev);
> + mach_port_deallocate (mach_task_self (), master_device);
> + if (ret)
> + goto fail;
> +
> + ret = device_read (mbinfo_dev, D_READ, 0, bytes, &bufptr, &bytes_read);
> + mach_port_deallocate (mach_task_self (), mbinfo_dev);
> + if (ret)
> + goto fail;
> +
> + if (bytes_read != bytes)
> + goto fail;
> +
> + memcpy((void *)&mbi, (void *)bufptr, sizeof(struct multiboot_raw_info));
> +
> + vga_fb = mbi.fb_info.framebuffer_addr;
> + vga_pitch = mbi.fb_info.framebuffer_pitch;
> + vga_width = mbi.fb_info.framebuffer_width;
> + vga_height = mbi.fb_info.framebuffer_height;
> + vga_bpp = mbi.fb_info.framebuffer_bpp;
> + vga_wc = VGA_PIXELS_W;
> + vga_hc = VGA_PIXELS_H;
These two should be taken from the font. The bdf format unfortunately
doesn't really provide the information (glyphs 0-31 are 16x16 in
vga-system.bdf for instance). As a workaround you can take the bbox of
the glyph for U+0020.
> + return ret;
> +
> +fail:
> + /* Fall back to EGA text mode */
> + vga_fb = VGA_VIDEO_MEM_BASE_ADDR;
> + vga_pitch = 160;
> + vga_width = 80;
> + vga_height = 25;
> + vga_bpp = 16;
> + vga_wc = 1;
> + vga_hc = 1;
I don't think you need to initialize anything, provided you do not change
the vga driver. And you can then call these fb_*, that'll mean something
for non-vga hardware.
> + return ret;
> +}
> +
>
> error_t
> vga_init (void)
> @@ -87,8 +148,8 @@ vga_init (void)
> fd = open ("/dev/mem", O_RDWR);
> if (fd >= 0)
> {
> - vga_videomem = mmap (0, VGA_VIDEO_MEM_LENGTH, PROT_READ | PROT_WRITE,
> - MAP_SHARED, fd, VGA_VIDEO_MEM_BASE_ADDR);
> + vga_videomem = mmap (0, vga_width * vga_height * vga_bpp, PROT_READ |
> PROT_WRITE,
> + MAP_SHARED, fd, vga_fb);
In vga, bpp is always 2, and the video memory is always
VGA_VIDEO_MEM_LENGTH (which allows to have some scrollback history
actually). I don't think it's useful to mess with the vga driver, this
is unrelated work.
> err = errno;
> close (fd);
> if (vga_videomem == (void *) -1)
> @@ -147,13 +208,13 @@ vga_init (void)
> outb (VGA_GFX_MODE_ADDR, VGA_GFX_ADDR_REG);
> outb (VGA_GFX_MODE_HOSTOE, VGA_GFX_DATA_REG);
>
> - memcpy (vga_state->videomem, vga_videomem, 2 * 80 * 25);
> + memcpy (vga_state->videomem, vga_videomem, vga_width * vga_height *
> vga_bpp / 8);
> vga_read_font_buffer (0, 0, vga_state->fontmem,
> - 2 * VGA_FONT_SIZE * VGA_FONT_HEIGHT);
> + vga_bpp/8 * VGA_FONT_SIZE * VGA_FONT_HEIGHT);
>
> - /* 80 cols, 25 rows, two bytes per cell and twice because with lower
> + /* X cols, Y rows, bpp/8 bytes per cell and twice because with lower
> max scan line we get more lines on the screen. */
> - memset (vga_videomem, 0, 80 * 25 * 2 * 2);
> + memset (vga_videomem, 0, vga_width * vga_height * vga_bpp / 4);
>
> return 0;
> }
> @@ -166,8 +227,8 @@ vga_fini (void)
> {
> /* Recover the saved state. */
> vga_write_font_buffer (0, 0, vga_state->fontmem,
> - 2 * VGA_FONT_SIZE * VGA_FONT_HEIGHT);
> - memcpy (vga_videomem, vga_state->videomem, 2 * 80 * 25);
> + vga_bpp/8 * VGA_FONT_SIZE * VGA_FONT_HEIGHT);
> + memcpy (vga_videomem, vga_state->videomem, vga_width * vga_height *
> vga_bpp / 8);
>
> /* Restore the registers. */
> outb (VGA_SEQ_CLOCK_MODE_ADDR, VGA_SEQ_ADDR_REG);
> @@ -207,7 +268,7 @@ vga_fini (void)
> outb (0x00, VGA_ATTR_ADDR_DATA_REG);
>
> ioperm (VGA_MIN_REG, VGA_MAX_REG - VGA_MIN_REG + 1, 0);
> - munmap (vga_videomem, VGA_VIDEO_MEM_LENGTH);
> + munmap (vga_videomem, vga_width * vga_height * vga_bpp);
> }
>
>
> @@ -225,7 +286,7 @@ vga_read_write_font_buffer (int write, int buffer, int
> index,
> char saved_gfx_misc;
>
> int offset = buffer * VGA_FONT_SIZE + index * VGA_FONT_HEIGHT;
> - assert_backtrace (offset >= 0 && offset + datalen <= VGA_VIDEO_MEM_LENGTH);
> + assert_backtrace (offset >= 0 && offset + datalen <= vga_width *
> vga_height * vga_bpp);
>
> /* Select plane 2 for sequential writing. You might think it is not
> necessary for reading, but it is. Likewise for read settings
> diff --git a/console-client/vga-support.h b/console-client/vga-support.h
> index 17c0b5fd..a3de5aea 100644
> --- a/console-client/vga-support.h
> +++ b/console-client/vga-support.h
> @@ -23,6 +23,57 @@
>
> #include <errno.h>
> #include <sys/types.h>
> +#include <stdint.h>
> +
> +struct multiboot_framebuffer_info {
> + uint64_t framebuffer_addr;
> + uint32_t framebuffer_pitch;
> + uint32_t framebuffer_width;
> + uint32_t framebuffer_height;
> + uint8_t framebuffer_bpp;
> +#define MULTIBOOT_FRAMEBUFFER_TYPE_INDEXED 0
> +#define MULTIBOOT_FRAMEBUFFER_TYPE_RGB 1
> +#define MULTIBOOT_FRAMEBUFFER_TYPE_EGA_TEXT 2
> + uint8_t framebuffer_type;
> + union
> + {
> + struct
> + {
> + uint32_t framebuffer_palette_addr;
> + uint16_t framebuffer_palette_num_colors;
> + };
> + struct
> + {
> + uint8_t framebuffer_red_field_position;
> + uint8_t framebuffer_red_mask_size;
> + uint8_t framebuffer_green_field_position;
> + uint8_t framebuffer_green_mask_size;
> + uint8_t framebuffer_blue_field_position;
> + uint8_t framebuffer_blue_mask_size;
> + };
> + };
> +} __attribute__((packed));
> +
> +/*
> + * Multiboot information structure as passed by the boot loader.
> + */
> +struct multiboot_raw_info {
> + uint32_t flags;
> + uint32_t mem_lower;
> + uint32_t mem_upper;
> + uint32_t unused0;
> + uint32_t cmdline;
> + uint32_t mods_count;
> + uint32_t mods_addr;
> + uint32_t shdr_num;
> + uint32_t shdr_size;
> + uint32_t shdr_addr;
> + uint32_t shdr_strndx;
> + uint32_t mmap_length;
> + uint32_t mmap_addr;
> + uint32_t unused1[9];
> + struct multiboot_framebuffer_info fb_info;
> +} __attribute__((packed));
Rather put them in a separate header, it's unrelated to vga.
> @@ -32,6 +83,15 @@
>
> /* The mapped video memory. */
> extern char *vga_videomem;
> +extern size_t vga_fb;
> +extern int vga_width;
> +extern int vga_height;
> +extern int vga_bpp;
> +extern int vga_wc;
> +extern int vga_hc;
> +
> +/* Get the multiboot video parameters */
> +error_t vga_get_multiboot_params (void);
>
> /* Initialize the VGA hardware and set up the permissions and memory
> mappings. */
> diff --git a/console-client/vga.c b/console-client/vga.c
> index e954013d..67e7e3f2 100644
> --- a/console-client/vga.c
> +++ b/console-client/vga.c
> @@ -37,6 +37,7 @@
> #include "driver.h"
> #include "timer.h"
>
> +#include "fb.h"
> #include "vga-hw.h"
> #include "vga-support.h"
> #include "bdf.h"
> @@ -45,8 +46,6 @@
> #include "unicode.h"
>
>
> -#define VGA_DISP_WIDTH 80
> -#define VGA_DISP_HEIGHT 25
>
> /* The font file. */
> #define DEFAULT_VGA_FONT DEFAULT_VGA_FONT_DIR "vga-system.bdf"
> @@ -87,15 +86,19 @@ static int cursor_state;
> /* Is set to 1 if the cursor moved out of the physical screen and the
> cursor state should be hidden. */
> static int cursor_hidden;
> +
> +/* Forward declaration */
> +struct driver_ops driver_vga_ops;
> +
>
> +
> struct refchr
> {
> unsigned int used : 1;
> unsigned int chr : 9;
> - unsigned int attr : 8;
> + unsigned int attr: 8;
Avoid unrelated changes.
> };
>
> -
> typedef struct vga_mousecursor
> {
> float posx;
> @@ -129,7 +132,7 @@ struct vga_display
> /* Remember for each cell on the display the glyph written to it and
> the colors (in the upper byte) assigned. 0 means unassigned. */
>
> - struct refchr refmatrix[VGA_DISP_HEIGHT][VGA_DISP_WIDTH];
> + struct refchr refmatrix[VGA_VIDEO_MEM_MAX_H /
> VGA_PIXELS_H][VGA_VIDEO_MEM_MAX_W / VGA_PIXELS_W];
You don't need to change this, the vga driver is really stuck with
hardware vga constraints, which are completely beneath fb capabilities.
> };
>
>
> @@ -279,9 +282,12 @@ vga_display_init (void **handle, int no_exit, int argc,
> char *argv[],
> int *next)
> {
> error_t err;
> - struct vga_display *disp;
> + struct vga_display *vgadisp;
> + struct fb_display *fbdisp;
> int pos = 1;
>
> + vga_get_multiboot_params();
> +
> /* XXX Assert that we are called only once. */
> pthread_mutex_init (&vga_display_lock, NULL);
> timer_clear (&vga_display_timer);
> @@ -294,22 +300,43 @@ vga_display_init (void **handle, int no_exit, int argc,
> char *argv[],
> if (err && err != EINVAL)
> return err;
>
> - /* Create and initialize the display structure as much as
> - possible. */
> - disp = calloc (1, sizeof *disp);
> - if (!disp)
> - return ENOMEM;
> + if (vga_fb == VGA_VIDEO_MEM_BASE_ADDR)
Should we not rather use
framebuffer_type == MULTIBOOT_FRAMEBUFFER_TYPE_EGA_TEXT?
in theory a framebuffer could be at VGA_VIDEO_MEM_BASE_ADDR
> + {
> + /* EGA text mode */
> + vgadisp = calloc (1, sizeof *vgadisp);
> + if (!vgadisp)
> + return ENOMEM;
> +
> + vgadisp->df_size = vga_display_max_glyphs ? 512 : 256;
> + vgadisp->df_width = vga_display_font_width;
> + vgadisp->width = vga_width;
> + vgadisp->height = vga_height;
>
> - disp->df_size = vga_display_max_glyphs ? 512 : 256;
> - disp->df_width = vga_display_font_width;
> - disp->width = VGA_DISP_WIDTH;
> - disp->height = VGA_DISP_HEIGHT;
> + *handle = vgadisp;
> + }
> + else
> + {
> + /* Linear framebuffer! */
> + fbdisp = calloc (1, sizeof *fbdisp);
> + if (!fbdisp)
> + return ENOMEM;
> +
> + fbdisp->df_size = vga_display_max_glyphs ? 512 : 256;
> + fbdisp->df_width = vga_display_font_width;
> + fbdisp->width = vga_width;
> + fbdisp->height = vga_height;
> +
> + /* dynamically switch drivers */
> + driver_vga_ops.start = fb_display_start;
> + driver_vga_ops.fini = fb_display_fini;
> + driver_vga_ops.restore_status = NULL;
> +
> + *handle = fbdisp;
> + }
>
> - *handle = disp;
> return 0;
> }
>
> -
> /* Start the driver. */
> static error_t
> vga_display_start (void *handle)
> --
> 2.45.2
>
>
>
