+Anatolij Hi Vladimir,
On Sun, Apr 7, 2013 at 8:36 AM, Vladimir 'φ-coder/phcoder' Serbinenko <[email protected]> wrote: > Most coreboot users use just VGA text console, not graphics. Support it. > Remaining problem is that software and hardware cursor blinking superimpose. > I disabled software blinking locally but it's not part of this patch since > it's not done properly. > Further to the checkpatch errors (try using patman!) see below: > diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c > index 61e1058..0c3c26d 100644 > --- a/drivers/video/cfb_console.c > +++ b/drivers/video/cfb_console.c > @@ -180,11 +180,10 @@ > /* > * some Macros > */ > -#define VIDEO_VISIBLE_COLS (pGD->winSizeX) > -#define VIDEO_VISIBLE_ROWS (pGD->winSizeY) > -#define VIDEO_PIXEL_SIZE (pGD->gdfBytesPP) > +static int VIDEO_VISIBLE_COLS; > +static int VIDEO_VISIBLE_ROWS; > +static int VIDEO_PIXEL_SIZE; > #define VIDEO_DATA_FORMAT (pGD->gdfIndex) > -#define VIDEO_FB_ADRS (pGD->frameAdrs) > > /* > * Console device defines with i8042 keyboard controller > @@ -384,6 +383,7 @@ static int console_row; /* cursor row */ > static u32 eorx, fgx, bgx; /* color pats */ > > static int cfb_do_flush_cache; > +static int vga_text; > > #ifdef CONFIG_CFB_CONSOLE_ANSI > static char ansi_buf[10]; > @@ -450,6 +450,18 @@ static void video_drawchars(int xx, int yy, unsigned > char *s, int count) > u8 *cdat, *dest, *dest0; > int rows, offset, c; > > + if (vga_text) I wonder if 'if (!vga_graphics)' might be clearer? But this is OK with me if you prefer it. > + { > + xx /= VIDEO_FONT_WIDTH; > + yy /= VIDEO_FONT_HEIGHT; > + while (count--) > + { > + ((uint16_t *)video_fb_address) [yy * 80 + xx] = 0x700 > | *s++; > + xx++; > + } > + return; > + } > + > offset = yy * VIDEO_LINE_LEN + xx * VIDEO_PIXEL_SIZE; > dest0 = video_fb_address + offset; > > @@ -644,6 +656,20 @@ static void video_invertchar(int xx, int yy) > } > } > > +static inline void > +cr_write (uint8_t val, uint8_t addr) > +{ > + outb (addr, 0x3d4); > + outb (val, 0x3d5); > +} > + > +static inline uint8_t > +cr_read (uint8_t addr) > +{ > + outb (addr, 0x3d4); > + return inb (0x3d5); > +} > + > void console_cursor(int state) > { > #ifdef CONFIG_CONSOLE_TIME > @@ -667,6 +693,22 @@ void console_cursor(int state) > } > #endif > > + if (vga_text) > + { > + uint8_t old; > + unsigned int pos = console_row * 80 + console_col; > + old = cr_read (0x0a); > + if (state) > + cr_write (old & ~0x20, 0x0a); > + else > + cr_write (old | 0x20, 0x0a); > + > + cr_write (pos >> 8, 0x0e); > + cr_write (pos & 0xFF, 0x0f); > + > + return; > + } > + > if (cursor_state != state) { > if (cursor_state) { > /* turn off the cursor */ > @@ -704,6 +746,18 @@ static void memcpyl(int *d, int *s, int c) > > static void console_clear_line(int line, int begin, int end) > { > + if (vga_text) > + { > + begin /= VIDEO_FONT_WIDTH; > + end /= VIDEO_FONT_WIDTH; > + line /= VIDEO_FONT_HEIGHT; > + for (; begin < end; begin++) > + { > + ((uint16_t *)video_fb_address) [line * 80 + begin] = > 0x700 | ' '; Well looking at the number of times you repeat this code below, it seems like this should go in its own function. > + } > + return; > + } > + > #ifdef VIDEO_HW_RECTFILL > video_hw_rectfill(VIDEO_PIXEL_SIZE, /* bytes per pixel */ > VIDEO_FONT_WIDTH * begin, /* dest pos x */ > @@ -742,6 +796,15 @@ static void console_clear_line(int line, int begin, int > end) > static void console_scrollup(void) > { > /* copy up rows ignoring the first one */ > + if (vga_text) > + { > + int i; > + memcpyl(video_fb_address, ((int32_t *)video_fb_address) + 80 > / 2, > + (80 * 24 * 2) >> 2); This is a big ugly. I think you should cast the video_fb_address to char if you need to, and use * 2 instead of / 2. Also I don't understand the number of bytes you are copying. It seems like it should be 80 * (24 - 1) * 2? > + for (i = 0; i < 80; i++) > + ((uint16_t *)video_fb_address) [24 * 80 + i] = 0x700 > | ' '; > + return; > + } > > #ifdef VIDEO_HW_BITBLT > video_hw_bitblt(VIDEO_PIXEL_SIZE, /* bytes per pixel */ > @@ -779,6 +842,15 @@ static void console_back(void) > > static void console_clear(void) > { > + if (vga_text) > + { > + int i; > + for (i = 0; i < 80 * 25; i++) > + ((uint16_t *)video_fb_address) [i] = 0x700 | ' '; How about creating a uint16_t pointer as a local variable? > + > + return; > + } > + > #ifdef VIDEO_HW_RECTFILL > video_hw_rectfill(VIDEO_PIXEL_SIZE, /* bytes per pixel */ > 0, /* dest pos x */ > @@ -1446,6 +1518,9 @@ int video_display_bitmap(ulong bmp_image, int x, int y) > > WATCHDOG_RESET(); > > + if (vga_text) > + return 0; > + > if (!((bmp->header.signature[0] == 'B') && > (bmp->header.signature[1] == 'M'))) { > > @@ -1846,6 +1921,9 @@ static void plot_logo_or_black(void *screen, int width, > int x, int y, int black) > unsigned char *source; > unsigned char *dest; > > + if (vga_text) > + return; > + > #ifdef CONFIG_SPLASH_SCREEN_ALIGN > if (x == BMP_ALIGN_CENTER) > x = max(0, (VIDEO_VISIBLE_COLS - VIDEO_LOGO_WIDTH) / 2); > @@ -2113,9 +2191,24 @@ static int video_init(void) > > pGD = video_hw_init(); > if (pGD == NULL) > - return -1; Please add a comment here about what pGD == NULL means. > + { > + vga_text = 1; > + video_fb_address = (void *) 0xb8000; > + cfb_do_flush_cache = 0; > + console_col = 0; > + console_row = 0; > + video_logo_height = 0; > + VIDEO_VISIBLE_ROWS = 25 * VIDEO_FONT_HEIGHT; > + VIDEO_VISIBLE_COLS = 80 * VIDEO_FONT_WIDTH; > + VIDEO_PIXEL_SIZE = 2; > + return 0; > + } > + > + video_fb_address = (void *) pGD->frameAdrs; > + VIDEO_VISIBLE_ROWS = pGD->winSizeY; > + VIDEO_VISIBLE_COLS = pGD->winSizeX; > + VIDEO_PIXEL_SIZE = (pGD->gdfBytesPP); > > - video_fb_address = (void *) VIDEO_FB_ADRS; > #ifdef CONFIG_VIDEO_HW_CURSOR > video_init_hw_cursor(VIDEO_FONT_WIDTH, VIDEO_FONT_HEIGHT); > #endif > @@ -2295,6 +2388,15 @@ void video_clear(void) > { > if (!video_fb_address) > return; > + > + if (vga_text) > + { > + int i; > + for (i = 0; i < 80 * 25; i++) Is the display always 80 * 25, or can it sometimes be different? Call console_clear() here instead of repeating the code? Suggest a #define/enum constant for these values. > + ((uint16_t *)video_fb_address) [i] = 0x700 | ' '; > + return; > + } > + > #ifdef VIDEO_HW_RECTFILL > video_hw_rectfill(VIDEO_PIXEL_SIZE, /* bytes per pixel */ > 0, /* dest pos x */ > > > _______________________________________________ > U-Boot mailing list > [email protected] > http://lists.denx.de/mailman/listinfo/u-boot > Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
