[fbcon] Remove userptr_t from framebuffer console drivers

Simplify the framebuffer console drivers by assuming that the raw
framebuffer, character cell array, background picture, and glyph data
are all directly accessible via pointer dereferences.

In particular, this avoids the need to copy each glyph during drawing:
the VESA framebuffer driver can simply return a pointer to the glyph
data stored in the video ROM.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/arch/x86/interface/pcbios/vesafb.c b/src/arch/x86/interface/pcbios/vesafb.c
index cfa935126006d59166a843209ab9e6cd4b7bc57d..961160ff8a56c60d6ce515d8bd63e43521f7ffa1 100644 (file)
--- a/src/arch/x86/interface/pcbios/vesafb.c
+++ b/src/arch/x86/interface/pcbios/vesafb.c
@@ -103,7 +103,7 @@ struct vesafb {
        /** Font definition */
        struct fbcon_font font;
        /** Character glyphs */
-       struct segoff glyphs;
+       const uint8_t *glyphs;
        /** Saved VGA mode */
        uint8_t saved_mode;
 };
@@ -140,11 +140,10 @@ static int vesafb_rc ( unsigned int status ) {
  * Get character glyph
  *
  * @v character                Unicode character
- * @v glyph            Character glyph to fill in
+ * @ret glyph          Character glyph
  */
-static void vesafb_glyph ( unsigned int character, uint8_t *glyph ) {
+static const uint8_t * vesafb_glyph ( unsigned int character ) {
        unsigned int index;
-       size_t offset;
 
        /* Identify glyph */
        if ( character < VESAFB_ASCII ) {
@@ -155,10 +154,8 @@ static void vesafb_glyph ( unsigned int character, uint8_t *glyph ) {
                index = VESAFB_UNKNOWN;
        }
 
-       /* Copy glyph from BIOS font table */
-       offset = ( index * VESAFB_CHAR_HEIGHT );
-       copy_from_real ( glyph, vesafb.glyphs.segment,
-                        ( vesafb.glyphs.offset + offset ), VESAFB_CHAR_HEIGHT);
+       /* Return glyph in BIOS font table */
+       return &vesafb.glyphs[ index * VESAFB_CHAR_HEIGHT ];
 }
 
 /**
@@ -166,6 +163,7 @@ static void vesafb_glyph ( unsigned int character, uint8_t *glyph ) {
  *
  */
 static void vesafb_font ( void ) {
+       struct segoff glyphs;
 
        /* Get font information
         *
@@ -186,12 +184,13 @@ static void vesafb_font ( void ) {
                                           "movw %%es, %%cx\n\t"
                                           "movw %%bp, %%dx\n\t"
                                           "popw %%bp\n\t" /* gcc bug */ )
-                              : "=c" ( vesafb.glyphs.segment ),
-                                "=d" ( vesafb.glyphs.offset )
+                              : "=c" ( glyphs.segment ),
+                                "=d" ( glyphs.offset )
                               : "a" ( VBE_GET_FONT ),
                                 "b" ( VESAFB_FONT ) );
        DBGC ( &vbe_buf, "VESAFB has font %04x at %04x:%04x\n",
-              VESAFB_FONT, vesafb.glyphs.segment, vesafb.glyphs.offset );
+              VESAFB_FONT, glyphs.segment, glyphs.offset );
+       vesafb.glyphs = real_to_virt ( glyphs.segment, glyphs.offset );
        vesafb.font.height = VESAFB_CHAR_HEIGHT;
        vesafb.font.glyph = vesafb_glyph;
 }
@@ -206,8 +205,8 @@ static void vesafb_font ( void ) {
  */
 static int vesafb_mode_list ( uint16_t **mode_numbers ) {
        struct vbe_controller_info *controller = &vbe_buf.controller;
-       userptr_t video_mode_ptr;
-       uint16_t mode_number;
+       const uint16_t *video_mode_ptr;
+       const uint16_t *mode_number;
        uint16_t status;
        size_t len;
        int rc;
@@ -245,18 +244,16 @@ static int vesafb_mode_list ( uint16_t **mode_numbers ) {
        /* Calculate length of mode list */
        video_mode_ptr = real_to_virt ( controller->video_mode_ptr.segment,
                                        controller->video_mode_ptr.offset );
-       len = 0;
-       do {
-               copy_from_user ( &mode_number, video_mode_ptr, len,
-                                sizeof ( mode_number ) );
-               len += sizeof ( mode_number );
-       } while ( mode_number != VBE_MODE_END );
+       mode_number = video_mode_ptr;
+       while ( *(mode_number++) != VBE_MODE_END ) {}
+       len = ( ( ( const void * ) mode_number ) -
+               ( ( const void * ) video_mode_ptr ) );
 
        /* Allocate and fill mode list */
        *mode_numbers = malloc ( len );
        if ( ! *mode_numbers )
                return -ENOMEM;
-       copy_from_user ( *mode_numbers, video_mode_ptr, 0, len );
+       memcpy ( *mode_numbers, video_mode_ptr, len );
 
        return 0;
 }

diff --git a/src/core/fbcon.c b/src/core/fbcon.c
index 6d08ac419ef14bac8523c10e298090b1bafaa003..f2b2ea566c7a3e0bf750af2a0f0546e9ce4b763e 100644 (file)
--- a/src/core/fbcon.c
+++ b/src/core/fbcon.c
@@ -102,6 +102,23 @@ static void fbcon_set_default_background ( struct fbcon *fbcon ) {
        fbcon->background = FBCON_TRANSPARENT;
 }
 
+/**
+ * Get character cell
+ *
+ * @v fbcon            Frame buffer console
+ * @v xpos             X position
+ * @v ypos             Y position
+ * @ret cell           Text cell
+ */
+static inline struct fbcon_text_cell * fbcon_cell ( struct fbcon *fbcon,
+                                                   unsigned int xpos,
+                                                   unsigned int ypos ) {
+       unsigned int index;
+
+       index = ( ( ypos * fbcon->character.width ) + xpos );
+       return &fbcon->text.cells[index];
+}
+
 /**
  * Clear rows of characters
  *
@@ -109,43 +126,21 @@ static void fbcon_set_default_background ( struct fbcon *fbcon ) {
  * @v ypos             Starting Y position
  */
 static void fbcon_clear ( struct fbcon *fbcon, unsigned int ypos ) {
-       struct fbcon_text_cell cell = {
-               .foreground = fbcon->foreground,
-               .background = fbcon->background,
-               .character = ' ',
-       };
-       size_t offset;
+       struct fbcon_text_cell *cell;
        unsigned int xpos;
 
        /* Clear stored character array */
+       cell = fbcon_cell ( fbcon, 0, ypos );
        for ( ; ypos < fbcon->character.height ; ypos++ ) {
-               offset = ( ypos * fbcon->character.width * sizeof ( cell ) );
                for ( xpos = 0 ; xpos < fbcon->character.width ; xpos++ ) {
-                       copy_to_user ( fbcon->text.start, offset, &cell,
-                                      sizeof ( cell ) );
-                       offset += sizeof ( cell );
+                       cell->foreground = fbcon->foreground;
+                       cell->background = fbcon->background;
+                       cell->character = ' ';
+                       cell++;
                }
        }
 }
 
-/**
- * Store character at specified position
- *
- * @v fbcon            Frame buffer console
- * @v cell             Text cell
- * @v xpos             X position
- * @v ypos             Y position
- */
-static void fbcon_store ( struct fbcon *fbcon, struct fbcon_text_cell *cell,
-                         unsigned int xpos, unsigned int ypos ) {
-       size_t offset;
-
-       /* Store cell */
-       offset = ( ( ( ypos * fbcon->character.width ) + xpos ) *
-                  sizeof ( *cell ) );
-       copy_to_user ( fbcon->text.start, offset, cell, sizeof ( *cell ) );
-}
-
 /**
  * Draw character at specified position
  *
@@ -156,7 +151,7 @@ static void fbcon_store ( struct fbcon *fbcon, struct fbcon_text_cell *cell,
  */
 static void fbcon_draw ( struct fbcon *fbcon, struct fbcon_text_cell *cell,
                         unsigned int xpos, unsigned int ypos ) {
-       uint8_t glyph[fbcon->font->height];
+       const uint8_t *glyph;
        size_t offset;
        size_t pixel_len;
        size_t skip_len;
@@ -164,10 +159,10 @@ static void fbcon_draw ( struct fbcon *fbcon, struct fbcon_text_cell *cell,
        unsigned int column;
        uint8_t bitmask;
        int transparent;
-       void *src;
+       const void *src;
 
        /* Get font character */
-       fbcon->font->glyph ( cell->character, glyph );
+       glyph = fbcon->font->glyph ( cell->character );
 
        /* Calculate pixel geometry */
        offset = ( fbcon->indent +
@@ -204,7 +199,7 @@ static void fbcon_draw ( struct fbcon *fbcon, struct fbcon_text_cell *cell,
                        } else {
                                continue;
                        }
-                       copy_to_user ( fbcon->start, offset, src, pixel_len );
+                       memcpy ( ( fbcon->start + offset ), src, pixel_len );
                }
 
                /* Move to next row */
@@ -218,18 +213,16 @@ static void fbcon_draw ( struct fbcon *fbcon, struct fbcon_text_cell *cell,
  * @v fbcon            Frame buffer console
  */
 static void fbcon_redraw ( struct fbcon *fbcon ) {
-       struct fbcon_text_cell cell;
-       size_t offset = 0;
+       struct fbcon_text_cell *cell;
        unsigned int xpos;
        unsigned int ypos;
 
        /* Redraw characters */
+       cell = fbcon_cell ( fbcon, 0, 0 );
        for ( ypos = 0 ; ypos < fbcon->character.height ; ypos++ ) {
                for ( xpos = 0 ; xpos < fbcon->character.width ; xpos++ ) {
-                       copy_from_user ( &cell, fbcon->text.start, offset,
-                                        sizeof ( cell ) );
-                       fbcon_draw ( fbcon, &cell, xpos, ypos );
-                       offset += sizeof ( cell );
+                       fbcon_draw ( fbcon, cell, xpos, ypos );
+                       cell++;
                }
        }
 }
@@ -246,8 +239,8 @@ static void fbcon_scroll ( struct fbcon *fbcon ) {
        assert ( fbcon->ypos == fbcon->character.height );
 
        /* Scroll up character array */
-       row_len = ( fbcon->character.width * sizeof ( struct fbcon_text_cell ));
-       memmove ( fbcon->text.start, ( fbcon->text.start + row_len ),
+       row_len = ( fbcon->character.width * sizeof ( fbcon->text.cells[0] ) );
+       memmove ( fbcon_cell ( fbcon, 0, 0 ), fbcon_cell ( fbcon, 0, 1 ),
                  ( row_len * ( fbcon->character.height - 1 ) ) );
        fbcon_clear ( fbcon, ( fbcon->character.height - 1 ) );
 
@@ -265,18 +258,19 @@ static void fbcon_scroll ( struct fbcon *fbcon ) {
  * @v show_cursor      Show cursor
  */
 static void fbcon_draw_cursor ( struct fbcon *fbcon, int show_cursor ) {
-       struct fbcon_text_cell cell;
-       size_t offset;
+       struct fbcon_text_cell *cell;
+       struct fbcon_text_cell cursor;
 
-       offset = ( ( ( fbcon->ypos * fbcon->character.width ) + fbcon->xpos ) *
-                  sizeof ( cell ) );
-       copy_from_user ( &cell, fbcon->text.start, offset, sizeof ( cell ) );
+       cell = fbcon_cell ( fbcon, fbcon->xpos, fbcon->ypos );
        if ( show_cursor ) {
-               cell.background = fbcon->foreground;
-               cell.foreground = ( ( fbcon->background == FBCON_TRANSPARENT ) ?
-                                   0 : fbcon->background );
+               cursor.background = fbcon->foreground;
+               cursor.foreground =
+                       ( ( fbcon->background == FBCON_TRANSPARENT ) ?
+                         0 : fbcon->background );
+               cursor.character = cell->character;
+               cell = &cursor;
        }
-       fbcon_draw ( fbcon, &cell, fbcon->xpos, fbcon->ypos );
+       fbcon_draw ( fbcon, cell, fbcon->xpos, fbcon->ypos );
 }
 
 /**
@@ -439,7 +433,7 @@ static struct ansiesc_handler fbcon_ansiesc_handlers[] = {
  * @v character                Character
  */
 void fbcon_putchar ( struct fbcon *fbcon, int character ) {
-       struct fbcon_text_cell cell;
+       struct fbcon_text_cell *cell;
 
        /* Intercept ANSI escape sequences */
        character = ansiesc_process ( &fbcon->ctx, character );
@@ -473,11 +467,11 @@ void fbcon_putchar ( struct fbcon *fbcon, int character ) {
                break;
        default:
                /* Print character at current cursor position */
-               cell.foreground = ( fbcon->foreground | fbcon->bold );
-               cell.background = fbcon->background;
-               cell.character = character;
-               fbcon_store ( fbcon, &cell, fbcon->xpos, fbcon->ypos );
-               fbcon_draw ( fbcon, &cell, fbcon->xpos, fbcon->ypos );
+               cell = fbcon_cell ( fbcon, fbcon->xpos, fbcon->ypos );
+               cell->foreground = ( fbcon->foreground | fbcon->bold );
+               cell->background = fbcon->background;
+               cell->character = character;
+               fbcon_draw ( fbcon, cell, fbcon->xpos, fbcon->ypos );
 
                /* Advance cursor */
                fbcon->xpos++;
@@ -508,12 +502,9 @@ static int fbcon_picture_init ( struct fbcon *fbcon,
        struct fbcon_geometry *pixel = fbcon->pixel;
        struct fbcon_picture *picture = &fbcon->picture;
        size_t len;
-       size_t pixbuf_stride;
        size_t indent;
-       size_t pixbuf_indent;
        size_t offset;
-       size_t pixbuf_offset;
-       uint32_t rgb;
+       const uint32_t *rgb;
        uint32_t raw;
        unsigned int x;
        unsigned int y;
@@ -534,13 +525,10 @@ static int fbcon_picture_init ( struct fbcon *fbcon,
        }
 
        /* Centre picture on console */
-       pixbuf_stride = ( pixbuf->width * sizeof ( rgb ) );
        xgap = ( ( ( int ) ( pixel->width - pixbuf->width ) ) / 2 );
        ygap = ( ( ( int ) ( pixel->height - pixbuf->height ) ) / 2 );
        indent = ( ( ( ( ygap >= 0 ) ? ygap : 0 ) * pixel->stride ) +
                   ( ( ( xgap >= 0 ) ? xgap : 0 ) * pixel->len ) );
-       pixbuf_indent = ( ( ( ( ygap < 0 ) ? -ygap : 0 ) * pixbuf_stride ) +
-                         ( ( ( xgap < 0 ) ? -xgap : 0 ) * sizeof ( rgb ) ) );
        width = pixbuf->width;
        if ( width > pixel->width )
                width = pixel->width;
@@ -555,15 +543,14 @@ static int fbcon_picture_init ( struct fbcon *fbcon,
        memset ( picture->start, 0, len );
        for ( y = 0 ; y < height ; y++ ) {
                offset = ( indent + ( y * pixel->stride ) );
-               pixbuf_offset = ( pixbuf_indent + ( y * pixbuf_stride ) );
+               rgb = pixbuf_pixel ( pixbuf, ( ( xgap < 0 ) ? -xgap : 0 ),
+                                    ( ( ( ygap < 0 ) ? -ygap : 0 ) + y ) );
                for ( x = 0 ; x < width ; x++ ) {
-                       copy_from_user ( &rgb, pixbuf->data, pixbuf_offset,
-                                        sizeof ( rgb ) );
-                       raw = fbcon_colour ( fbcon, rgb );
-                       copy_to_user ( picture->start, offset, &raw,
-                                      pixel->len );
+                       raw = fbcon_colour ( fbcon, *rgb );
+                       memcpy ( ( picture->start + offset ), &raw,
+                                pixel->len );
                        offset += pixel->len;
-                       pixbuf_offset += sizeof ( rgb );
+                       rgb++;
                }
        }
 
@@ -585,7 +572,7 @@ static int fbcon_picture_init ( struct fbcon *fbcon,
  * @v config           Console configuration
  * @ret rc             Return status code
  */
-int fbcon_init ( struct fbcon *fbcon, userptr_t start,
+int fbcon_init ( struct fbcon *fbcon, void *start,
                 struct fbcon_geometry *pixel,
                 struct fbcon_colour_map *map,
                 struct fbcon_font *font,
@@ -674,10 +661,10 @@ int fbcon_init ( struct fbcon *fbcon, userptr_t start,
        fbcon_set_default_background ( fbcon );
 
        /* Allocate and initialise stored character array */
-       fbcon->text.start = umalloc ( fbcon->character.width *
+       fbcon->text.cells = umalloc ( fbcon->character.width *
                                      fbcon->character.height *
-                                     sizeof ( struct fbcon_text_cell ) );
-       if ( ! fbcon->text.start ) {
+                                     sizeof ( fbcon->text.cells[0] ) );
+       if ( ! fbcon->text.cells ) {
                rc = -ENOMEM;
                goto err_text;
        }
@@ -702,7 +689,7 @@ int fbcon_init ( struct fbcon *fbcon, userptr_t start,
 
        ufree ( fbcon->picture.start );
  err_picture:
-       ufree ( fbcon->text.start );
+       ufree ( fbcon->text.cells );
  err_text:
  err_margin:
        return rc;
@@ -715,6 +702,6 @@ int fbcon_init ( struct fbcon *fbcon, userptr_t start,
  */
 void fbcon_fini ( struct fbcon *fbcon ) {
 
-       ufree ( fbcon->text.start );
+       ufree ( fbcon->text.cells );
        ufree ( fbcon->picture.start );
 }

diff --git a/src/include/ipxe/fbcon.h b/src/include/ipxe/fbcon.h
index a4c7a9ab3e4a2b581e046d9724a472d4afa2ce7a..5233b4d0efeb63a4522cebab20aeab061a64e594 100644 (file)
--- a/src/include/ipxe/fbcon.h
+++ b/src/include/ipxe/fbcon.h
@@ -12,7 +12,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <stdint.h>
 #include <ipxe/ansiesc.h>
 #include <ipxe/utf8.h>
-#include <ipxe/uaccess.h>
 #include <ipxe/console.h>
 
 /** Character width, in pixels */
@@ -38,9 +37,9 @@ struct fbcon_font {
         * Get character glyph
         *
         * @v character         Unicode character
-        * @v glyph             Character glyph to fill in
+        * @ret glyph           Character glyph
         */
-       void ( * glyph ) ( unsigned int character, uint8_t *glyph );
+       const uint8_t * ( * glyph ) ( unsigned int character );
 };
 
 /** A frame buffer geometry
@@ -100,19 +99,19 @@ struct fbcon_text_cell {
 /** A frame buffer text array */
 struct fbcon_text {
        /** Stored text cells */
-       userptr_t start;
+       struct fbcon_text_cell *cells;
 };
 
 /** A frame buffer background picture */
 struct fbcon_picture {
        /** Start address */
-       userptr_t start;
+       void *start;
 };
 
 /** A frame buffer console */
 struct fbcon {
        /** Start address */
-       userptr_t start;
+       void *start;
        /** Length of one complete displayed screen */
        size_t len;
        /** Pixel geometry */
@@ -149,7 +148,7 @@ struct fbcon {
        int show_cursor;
 };
 
-extern int fbcon_init ( struct fbcon *fbcon, userptr_t start,
+extern int fbcon_init ( struct fbcon *fbcon, void *start,
                        struct fbcon_geometry *pixel,
                        struct fbcon_colour_map *map,
                        struct fbcon_font *font,

diff --git a/src/include/ipxe/pixbuf.h b/src/include/ipxe/pixbuf.h
index 6157448122616e936abc4e28a5240fcc9a6a5140..6c70c1c058ecc2ded0f4876f5d545339a5bf7a40 100644 (file)
--- a/src/include/ipxe/pixbuf.h
+++ b/src/include/ipxe/pixbuf.h
@@ -11,7 +11,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 
 #include <stddef.h>
 #include <ipxe/refcnt.h>
-#include <ipxe/uaccess.h>
 
 /** A pixel buffer */
 struct pixel_buffer {
@@ -22,7 +21,7 @@ struct pixel_buffer {
        /** Height */
        unsigned int height;
        /** 32-bit (8:8:8:8) xRGB pixel data, in host-endian order */
-       userptr_t data;
+       uint32_t *data;
        /** Total length */
        size_t len;
 };
@@ -49,6 +48,22 @@ pixbuf_put ( struct pixel_buffer *pixbuf ) {
        ref_put ( &pixbuf->refcnt );
 }
 
+/**
+ * Get pixel
+ *
+ * @v pixbuf           Pixel buffer
+ * @v x                        X position
+ * @v y                        Y position
+ * @ret pixel          Pixel
+ */
+static inline __attribute__ (( always_inline )) uint32_t *
+pixbuf_pixel ( struct pixel_buffer *pixbuf, unsigned int x, unsigned int y ) {
+       unsigned int index;
+
+       index = ( ( y * pixbuf->width ) + x );
+       return &pixbuf->data[index];
+}
+
 extern struct pixel_buffer * alloc_pixbuf ( unsigned int width,
                                            unsigned int height );
 

diff --git a/src/interface/efi/efi_fbcon.c b/src/interface/efi/efi_fbcon.c
index 09cfde4c2934a0fd186089d35833b61bab67ae8e..9c5d7063dc6d58fbd7f5cef9101843f9c6a960cf 100644 (file)
--- a/src/interface/efi/efi_fbcon.c
+++ b/src/interface/efi/efi_fbcon.c
@@ -42,6 +42,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <ipxe/ansicol.h>
 #include <ipxe/fbcon.h>
 #include <ipxe/console.h>
+#include <ipxe/uaccess.h>
 #include <ipxe/umalloc.h>
 #include <ipxe/rotate.h>
 #include <config/console.h>
@@ -91,7 +92,7 @@ struct efifb {
        /** Font definition */
        struct fbcon_font font;
        /** Character glyph cache */
-       userptr_t glyphs;
+       uint8_t *glyphs;
        /** Dynamic characters in cache */
        unsigned int dynamic[EFIFB_DYNAMIC];
        /** Next dynamic character cache entry to evict */
@@ -117,14 +118,14 @@ static int efifb_draw ( unsigned int character, unsigned int index,
        unsigned int height;
        unsigned int x;
        unsigned int y;
+       uint8_t *glyph;
        uint8_t bitmask;
-       size_t offset;
        EFI_STATUS efirc;
        int rc;
 
        /* Clear existing glyph */
-       offset = ( index * efifb.font.height );
-       memset ( ( efifb.glyphs + offset ), 0, efifb.font.height );
+       glyph = &efifb.glyphs[ index * efifb.font.height ];
+       memset ( glyph, 0, efifb.font.height );
 
        /* Get glyph */
        blt = NULL;
@@ -157,8 +158,7 @@ static int efifb_draw ( unsigned int character, unsigned int index,
                        pixel++;
                }
                bitmask ^= toggle;
-               copy_to_user ( efifb.glyphs, offset++, &bitmask,
-                              sizeof ( bitmask ) );
+               *(glyph++) = bitmask;
        }
 
        /* Free glyph */
@@ -223,11 +223,10 @@ static unsigned int efifb_dynamic ( unsigned int character ) {
  * Get character glyph
  *
  * @v character                Unicode character
- * @v glyph            Character glyph to fill in
+ * @ret glyph          Character glyph to fill in
  */
-static void efifb_glyph ( unsigned int character, uint8_t *glyph ) {
+static const uint8_t * efifb_glyph ( unsigned int character ) {
        unsigned int index;
-       size_t offset;
 
        /* Identify glyph */
        if ( character < EFIFB_ASCII ) {
@@ -241,9 +240,8 @@ static void efifb_glyph ( unsigned int character, uint8_t *glyph ) {
                index = efifb_dynamic ( character );
        }
 
-       /* Copy cached glyph */
-       offset = ( index * efifb.font.height );
-       copy_from_user ( glyph, efifb.glyphs, offset, efifb.font.height );
+       /* Return cached glyph */
+       return &efifb.glyphs[ index * efifb.font.height ];
 }
 
 /**