Minor framebuffer cleanup patches

Minor framebuffer cleanup patches

[Michal Suchanek]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

Hello

I am sending the rest of framebuffer patches that were not included in
the split by Vladimir.

remove-dup-function.patch

* video_fb.c: remove grub_video_fb_get_video_ptr which is duplicated in fbutil.c
* fbutil.c: copy the note from fbfill.c

rename-var.patch

vbe.c: grub_video_vbe_setup rename local variable
 - this should make the distinction between grub_vbe_mode_info_block
and grub_video_mode_info clearer in this function

Thanks

Michal

[-- Attachment #2: remove-dup-function.patch --]
[-- Type: text/x-diff, Size: 11534 bytes --]

Index: video/fb/fbblit.c
===================================================================
--- video/fb/fbblit.c	(revision 2496)
+++ video/fb/fbblit.c	(working copy)
@@ -83,8 +83,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint32_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       grub_memmove (dstptr, srcptr, width * bpp);
     }
@@ -110,8 +110,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint8_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -153,8 +153,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint8_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -197,8 +197,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint32_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint32_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -245,8 +245,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint8_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -285,8 +285,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint8_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint32_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
@@ -321,8 +321,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
 	{
@@ -358,8 +358,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
 	{
@@ -394,8 +394,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint8_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
@@ -491,8 +491,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint32_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint32_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint32_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint32_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -572,8 +572,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint32_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint32_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -656,8 +656,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint32_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
@@ -721,8 +721,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
@@ -788,8 +788,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
Index: video/fb/fbutil.c
===================================================================
--- video/fb/fbutil.c	(revision 2496)
+++ video/fb/fbutil.c	(working copy)
@@ -16,12 +16,23 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* SPECIAL NOTES!
+
+   Please note following when reading the code below:
+
+   - In this driver we assume that every memory can be accessed by same memory
+     bus.  If there are different address spaces do not use this code as a base
+     code for other archs.
+
+   - Every function in this code assumes that bounds checking has been done in
+     previous phase and they are opted out in here.  */
+
 #include <grub/fbutil.h>
 #include <grub/types.h>
 #include <grub/video.h>
 
 grub_uint8_t *
-get_data_ptr (struct grub_video_fbblit_info *source,
+grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source,
               unsigned int x, unsigned int y)
 {
   grub_uint8_t *ptr = 0;
@@ -72,24 +83,24 @@
   switch (source->mode_info->bpp)
     {
     case 32:
-      color = *(grub_uint32_t *)get_data_ptr (source, x, y);
+      color = *(grub_uint32_t *)grub_video_fb_get_video_ptr (source, x, y);
       break;
 
     case 24:
       {
         grub_uint8_t *ptr;
-        ptr = get_data_ptr (source, x, y);
+        ptr = grub_video_fb_get_video_ptr (source, x, y);
         color = ptr[0] | (ptr[1] << 8) | (ptr[2] << 16);
       }
       break;
 
     case 16:
     case 15:
-      color = *(grub_uint16_t *)get_data_ptr (source, x, y);
+      color = *(grub_uint16_t *)grub_video_fb_get_video_ptr (source, x, y);
       break;
 
     case 8:
-      color = *(grub_uint8_t *)get_data_ptr (source, x, y);
+      color = *(grub_uint8_t *)grub_video_fb_get_video_ptr (source, x, y);
       break;
 
     case 1:
@@ -120,7 +131,7 @@
       {
         grub_uint32_t *ptr;
 
-        ptr = (grub_uint32_t *)get_data_ptr (source, x, y);
+        ptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (source, x, y);
 
         *ptr = color;
       }
@@ -131,7 +142,7 @@
         grub_uint8_t *ptr;
         grub_uint8_t *colorptr = (grub_uint8_t *)&color;
 
-        ptr = get_data_ptr (source, x, y);
+        ptr = grub_video_fb_get_video_ptr (source, x, y);
 
         ptr[0] = colorptr[0];
         ptr[1] = colorptr[1];
@@ -144,7 +155,7 @@
       {
         grub_uint16_t *ptr;
 
-        ptr = (grub_uint16_t *)get_data_ptr (source, x, y);
+        ptr = (grub_uint16_t *)grub_video_fb_get_video_ptr (source, x, y);
 
         *ptr = (grub_uint16_t) (color & 0xFFFF);
       }
@@ -154,7 +165,7 @@
       {
         grub_uint8_t *ptr;
 
-        ptr = (grub_uint8_t *)get_data_ptr (source, x, y);
+        ptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (source, x, y);
 
         *ptr = (grub_uint8_t) (color & 0xFF);
       }
Index: video/fb/video_fb.c
===================================================================
--- video/fb/video_fb.c	(revision 2496)
+++ video/fb/video_fb.c	(working copy)
@@ -83,44 +83,6 @@
   return GRUB_ERR_NONE;
 }
 
-
-grub_uint8_t *
-grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source,
-			     grub_uint32_t x, grub_uint32_t y)
-{
-  grub_uint8_t *ptr = 0;
-
-  switch (source->mode_info->bpp)
-    {
-    case 32:
-      ptr = (grub_uint8_t *)source->data
-            + y * source->mode_info->pitch
-            + x * 4;
-      break;
-
-    case 24:
-      ptr = (grub_uint8_t *)source->data
-            + y * source->mode_info->pitch
-            + x * 3;
-      break;
-
-    case 16:
-    case 15:
-      ptr = (grub_uint8_t *)source->data
-            + y * source->mode_info->pitch
-            + x * 2;
-      break;
-
-    case 8:
-      ptr = (grub_uint8_t *)source->data
-            + y * source->mode_info->pitch
-            + x;
-      break;
-    }
-
-  return ptr;
-}
-
 grub_err_t
 grub_video_fb_get_palette (unsigned int start, unsigned int count,
 			   struct grub_video_palette_data *palette_data)
Index: include/grub/fbutil.h
===================================================================
--- include/grub/fbutil.h	(revision 2496)
+++ include/grub/fbutil.h	(working copy)
@@ -31,7 +31,7 @@
   void *data;
 };
 
-grub_uint8_t *get_data_ptr (struct grub_video_fbblit_info *source,
+grub_uint8_t *grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source,
                             unsigned int x, unsigned int y);
 
 grub_video_color_t get_pixel (struct grub_video_fbblit_info *source,
Index: include/grub/video_fb.h
===================================================================
--- include/grub/video_fb.h	(revision 2496)
+++ include/grub/video_fb.h	(working copy)
@@ -33,9 +33,6 @@
 #define GRUB_VIDEO_FBSTD_NUMCOLORS 16
 extern struct grub_video_palette_data grub_video_fbstd_colors[GRUB_VIDEO_FBSTD_NUMCOLORS];
 
-grub_uint8_t * grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source,
-                                             grub_uint32_t x, grub_uint32_t y);
-
 grub_err_t
 grub_video_fb_init (void);
 

[-- Attachment #3: rename-var.patch --]
[-- Type: text/x-diff, Size: 3813 bytes --]

Index: video/i386/pc/vbe.c
===================================================================
--- video/i386/pc/vbe.c	(revision 2496)
+++ video/i386/pc/vbe.c	(working copy)
@@ -382,7 +382,7 @@
                       unsigned int mode_type)
 {
   grub_uint16_t *p;
-  struct grub_vbe_mode_info_block mode_info;
+  struct grub_vbe_mode_info_block vbe_mode_info;
   struct grub_vbe_mode_info_block best_mode_info;
   grub_uint32_t best_mode = 0;
   int depth;
@@ -396,7 +396,7 @@
     {
       grub_uint32_t mode = *p;
 
-      grub_vbe_get_video_mode_info (mode, &mode_info);
+      grub_vbe_get_video_mode_info (mode, &vbe_mode_info);
       if (grub_errno != GRUB_ERR_NONE)
         {
           /* Could not retrieve mode info, retreat.  */
@@ -404,33 +404,33 @@
           break;
         }
 
-      if ((mode_info.mode_attributes & 0x001) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x001) == 0)
         /* If not available, skip it.  */
         continue;
 
-      if ((mode_info.mode_attributes & 0x002) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x002) == 0)
         /* Not enough information.  */
         continue;
 
-      if ((mode_info.mode_attributes & 0x008) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x008) == 0)
         /* Monochrome is unusable.  */
         continue;
 
-      if ((mode_info.mode_attributes & 0x080) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x080) == 0)
         /* We support only linear frame buffer modes.  */
         continue;
 
-      if ((mode_info.mode_attributes & 0x010) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x010) == 0)
         /* We allow only graphical modes.  */
         continue;
 
-      if ((mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL)
-          && (mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR))
+      if ((vbe_mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL)
+          && (vbe_mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR))
         /* Not compatible memory model.  */
         continue;
 
-      if ((mode_info.x_resolution != width)
-          || (mode_info.y_resolution != height))
+      if ((vbe_mode_info.x_resolution != width)
+          || (vbe_mode_info.y_resolution != height))
         /* Non matching resolution.  */
         continue;
 
@@ -438,28 +438,28 @@
       if ((mode_type & GRUB_VIDEO_MODE_TYPE_COLOR_MASK) != 0)
         {
           if (((mode_type & GRUB_VIDEO_MODE_TYPE_INDEX_COLOR) != 0)
-              && (mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL))
+              && (vbe_mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL))
             /* Requested only index color modes.  */
             continue;
 
           if (((mode_type & GRUB_VIDEO_MODE_TYPE_RGB) != 0)
-              && (mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR))
+              && (vbe_mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR))
             /* Requested only RGB modes.  */
             continue;
         }
 
       /* If there is a request for specific depth, ignore others.  */
-      if ((depth != 0) && (mode_info.bits_per_pixel != depth))
+      if ((depth != 0) && (vbe_mode_info.bits_per_pixel != depth))
         continue;
 
       /* Select mode with most number of bits per pixel.  */
       if (best_mode != 0)
-        if (mode_info.bits_per_pixel < best_mode_info.bits_per_pixel)
+        if (vbe_mode_info.bits_per_pixel < best_mode_info.bits_per_pixel)
           continue;
 
       /* Save so far best mode information for later use.  */
       best_mode = mode;
-      grub_memcpy (&best_mode_info, &mode_info, sizeof (mode_info));
+      grub_memcpy (&best_mode_info, &vbe_mode_info, sizeof (vbe_mode_info));
     }
 
   /* Try to initialize best mode found.  */

Re: Minor framebuffer cleanup patches

[Vladimir 'phcoder' Serbinenko]

On Fri, Aug 14, 2009 at 4:04 PM, Michal Suchanek<hramrach@centrum.cz> wrote:
> Hello
>
> I am sending the rest of framebuffer patches that were not included in
> the split by Vladimir.
>
> remove-dup-function.patch
>
> * video_fb.c: remove grub_video_fb_get_video_ptr which is duplicated in fbutil.c
> * fbutil.c: copy the note from fbfill.c
I think the comment should go to video_fb.h to instruct future video
driver writers.
Could you create entries in the same format as Changelog
>
> rename-var.patch
>
> vbe.c: grub_video_vbe_setup rename local variable
>  - this should make the distinction between grub_vbe_mode_info_block
> and grub_video_mode_info clearer in this function
>
Perhaps best_mode_info should be renamed too, for consistency.
Other than this I'm ok with both patches.
> Thanks
>
> Michal
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: Minor framebuffer cleanup patches

[Michal Suchanek]

[-- Attachment #1: Type: text/plain, Size: 2462 bytes --]

2009/8/14 Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>:
> On Fri, Aug 14, 2009 at 4:04 PM, Michal Suchanek<hramrach@centrum.cz> wrote:
>> Hello
>>
>> I am sending the rest of framebuffer patches that were not included in
>> the split by Vladimir.
>>
>> remove-dup-function.patch
>>
>> * video_fb.c: remove grub_video_fb_get_video_ptr which is duplicated in fbutil.c
>> * fbutil.c: copy the note from fbfill.c
> I think the comment should go to video_fb.h to instruct future video
> driver writers.
> Could you create entries in the same format as Changelog

* video/fb/video_fb.c (grub_video_fb_get_video_ptr): remove function
(duplicated in fbutil.c)
* video/fb/fbutil.c: copy the note from fbfill.c

Attaching additional patch that moves the note about memory
architecture in video_fb.h

* video_fb.h: move the note about memory architecture from fb*.c here

>>
>> rename-var.patch
>>
>> vbe.c: grub_video_vbe_setup rename local variable
>>  - this should make the distinction between grub_vbe_mode_info_block
>> and grub_video_mode_info clearer in this function
>>
> Perhaps best_mode_info should be renamed too, for consistency.
> Other than this I'm ok with both patches.

I meant this only as a reminder that the mode_info that is used all
the time really refers to a vbe mode (as opposed to framebuffer mode)
bu it is certainly possible to rename

Attaching an alternate patch that renames all the vbe mode related locals.


    * video/i386/pc/vbe.c: rename local variables to make clearer
      distinction between vbe modes and framebuffer modes


While making this patch I noticed that

- grub_vbe_set_video_mode sets the mode in framebuffer.active_mode
and grub_video_vbe_setup in vbe_mode_in_use. Either this is redundant
or any call to grub_vbe_set_video_mode without going through
grub_video_vbe_setup makes the mode_in_use value obsolete - invalid.

- grub_vbe_set_video_mode changes active_mode_info before setting the
mode (and before checking it can be used)

- grub_vbe_setup calls grub_video_set_video_mode with pointer to
active_mode_info as argument which causes call to memcpy with this
pointer as both src and dest

This is resolved with additional patch:

* video/i386/pc/vbe.c: remove vbe_mode_in_use (duplicated in
framebuffer.active_vbe_mode)
* video/i386/pc/vbe.c (grub_vbe_set_video_mode): save active mode info
only after setting the mode

Thanks

Michal

[-- Attachment #2: remove-dup-function.patch --]
[-- Type: text/x-diff, Size: 11534 bytes --]

Index: video/fb/fbblit.c
===================================================================
--- video/fb/fbblit.c	(revision 2496)
+++ video/fb/fbblit.c	(working copy)
@@ -83,8 +83,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint32_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       grub_memmove (dstptr, srcptr, width * bpp);
     }
@@ -110,8 +110,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint8_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -153,8 +153,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint8_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -197,8 +197,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint32_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint32_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -245,8 +245,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint8_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -285,8 +285,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint8_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint32_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
@@ -321,8 +321,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
 	{
@@ -358,8 +358,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
 	{
@@ -394,8 +394,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint8_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
@@ -491,8 +491,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint32_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint32_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint32_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint32_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -572,8 +572,8 @@
   srcrowskip = src->mode_info->pitch - src->mode_info->bytes_per_pixel * width;
   dstrowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width;
 
-  srcptr = (grub_uint32_t *) get_data_ptr (src, offset_x, offset_y);
-  dstptr = (grub_uint8_t *) get_data_ptr (dst, x, y);
+  srcptr = (grub_uint32_t *) grub_video_fb_get_video_ptr (src, offset_x, offset_y);
+  dstptr = (grub_uint8_t *) grub_video_fb_get_video_ptr (dst, x, y);
 
   for (j = 0; j < height; j++)
     {
@@ -656,8 +656,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint32_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
@@ -721,8 +721,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
@@ -788,8 +788,8 @@
 
   for (j = 0; j < height; j++)
     {
-      srcptr = (grub_uint32_t *)get_data_ptr (src, offset_x, j + offset_y);
-      dstptr = (grub_uint8_t *)get_data_ptr (dst, x, y + j);
+      srcptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (src, offset_x, j + offset_y);
+      dstptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (dst, x, y + j);
 
       for (i = 0; i < width; i++)
         {
Index: video/fb/fbutil.c
===================================================================
--- video/fb/fbutil.c	(revision 2496)
+++ video/fb/fbutil.c	(working copy)
@@ -16,12 +16,23 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* SPECIAL NOTES!
+
+   Please note following when reading the code below:
+
+   - In this driver we assume that every memory can be accessed by same memory
+     bus.  If there are different address spaces do not use this code as a base
+     code for other archs.
+
+   - Every function in this code assumes that bounds checking has been done in
+     previous phase and they are opted out in here.  */
+
 #include <grub/fbutil.h>
 #include <grub/types.h>
 #include <grub/video.h>
 
 grub_uint8_t *
-get_data_ptr (struct grub_video_fbblit_info *source,
+grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source,
               unsigned int x, unsigned int y)
 {
   grub_uint8_t *ptr = 0;
@@ -72,24 +83,24 @@
   switch (source->mode_info->bpp)
     {
     case 32:
-      color = *(grub_uint32_t *)get_data_ptr (source, x, y);
+      color = *(grub_uint32_t *)grub_video_fb_get_video_ptr (source, x, y);
       break;
 
     case 24:
       {
         grub_uint8_t *ptr;
-        ptr = get_data_ptr (source, x, y);
+        ptr = grub_video_fb_get_video_ptr (source, x, y);
         color = ptr[0] | (ptr[1] << 8) | (ptr[2] << 16);
       }
       break;
 
     case 16:
     case 15:
-      color = *(grub_uint16_t *)get_data_ptr (source, x, y);
+      color = *(grub_uint16_t *)grub_video_fb_get_video_ptr (source, x, y);
       break;
 
     case 8:
-      color = *(grub_uint8_t *)get_data_ptr (source, x, y);
+      color = *(grub_uint8_t *)grub_video_fb_get_video_ptr (source, x, y);
       break;
 
     case 1:
@@ -120,7 +131,7 @@
       {
         grub_uint32_t *ptr;
 
-        ptr = (grub_uint32_t *)get_data_ptr (source, x, y);
+        ptr = (grub_uint32_t *)grub_video_fb_get_video_ptr (source, x, y);
 
         *ptr = color;
       }
@@ -131,7 +142,7 @@
         grub_uint8_t *ptr;
         grub_uint8_t *colorptr = (grub_uint8_t *)&color;
 
-        ptr = get_data_ptr (source, x, y);
+        ptr = grub_video_fb_get_video_ptr (source, x, y);
 
         ptr[0] = colorptr[0];
         ptr[1] = colorptr[1];
@@ -144,7 +155,7 @@
       {
         grub_uint16_t *ptr;
 
-        ptr = (grub_uint16_t *)get_data_ptr (source, x, y);
+        ptr = (grub_uint16_t *)grub_video_fb_get_video_ptr (source, x, y);
 
         *ptr = (grub_uint16_t) (color & 0xFFFF);
       }
@@ -154,7 +165,7 @@
       {
         grub_uint8_t *ptr;
 
-        ptr = (grub_uint8_t *)get_data_ptr (source, x, y);
+        ptr = (grub_uint8_t *)grub_video_fb_get_video_ptr (source, x, y);
 
         *ptr = (grub_uint8_t) (color & 0xFF);
       }
Index: video/fb/video_fb.c
===================================================================
--- video/fb/video_fb.c	(revision 2496)
+++ video/fb/video_fb.c	(working copy)
@@ -83,44 +83,6 @@
   return GRUB_ERR_NONE;
 }
 
-
-grub_uint8_t *
-grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source,
-			     grub_uint32_t x, grub_uint32_t y)
-{
-  grub_uint8_t *ptr = 0;
-
-  switch (source->mode_info->bpp)
-    {
-    case 32:
-      ptr = (grub_uint8_t *)source->data
-            + y * source->mode_info->pitch
-            + x * 4;
-      break;
-
-    case 24:
-      ptr = (grub_uint8_t *)source->data
-            + y * source->mode_info->pitch
-            + x * 3;
-      break;
-
-    case 16:
-    case 15:
-      ptr = (grub_uint8_t *)source->data
-            + y * source->mode_info->pitch
-            + x * 2;
-      break;
-
-    case 8:
-      ptr = (grub_uint8_t *)source->data
-            + y * source->mode_info->pitch
-            + x;
-      break;
-    }
-
-  return ptr;
-}
-
 grub_err_t
 grub_video_fb_get_palette (unsigned int start, unsigned int count,
 			   struct grub_video_palette_data *palette_data)
Index: include/grub/fbutil.h
===================================================================
--- include/grub/fbutil.h	(revision 2496)
+++ include/grub/fbutil.h	(working copy)
@@ -31,7 +31,7 @@
   void *data;
 };
 
-grub_uint8_t *get_data_ptr (struct grub_video_fbblit_info *source,
+grub_uint8_t *grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source,
                             unsigned int x, unsigned int y);
 
 grub_video_color_t get_pixel (struct grub_video_fbblit_info *source,
Index: include/grub/video_fb.h
===================================================================
--- include/grub/video_fb.h	(revision 2496)
+++ include/grub/video_fb.h	(working copy)
@@ -33,9 +33,6 @@
 #define GRUB_VIDEO_FBSTD_NUMCOLORS 16
 extern struct grub_video_palette_data grub_video_fbstd_colors[GRUB_VIDEO_FBSTD_NUMCOLORS];
 
-grub_uint8_t * grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source,
-                                             grub_uint32_t x, grub_uint32_t y);
-
 grub_err_t
 grub_video_fb_init (void);
 

[-- Attachment #3: move-memarch-comment.patch --]
[-- Type: text/x-diff, Size: 2393 bytes --]

diff --git include/gruvideo_fb.h include/gruvideo_fb.h
index 17debd6..850946d 100644
--- include/gruvideo_fb.h
+++ include/gruvideo_fb.h
@@ -16,6 +16,12 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* SPECIAL NOTE!
+
+   - In this driver we assume that every memory can be accessed by same memory
+     bus.  If there are different address spaces do not use this code as a base
+     code for other archs.  */
+
 #ifndef GRUB_VIDEO_FB_HEADER
 #define GRUB_VIDEO_FB_HEADER	1
 
diff --git video/ffbblit.c video/ffbblit.c
index 5b613bc..705da83 100644
--- video/ffbblit.c
+++ video/ffbblit.c
@@ -16,13 +16,7 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* SPECIAL NOTES!
-
-   Please note following when reading the code below:
-
-   - In this driver we assume that every memory can be accessed by same memory
-   bus.  If there are different address spaces do not use this code as a base
-   code for other archs.
+/* SPECIAL NOTE!
 
    - Every function in this code assumes that bounds checking has been done in
    previous phase and they are opted out in here.  */
diff --git video/ffbfill.c video/ffbfill.c
index a4ca7c2..dba8ceb 100644
--- video/ffbfill.c
+++ video/ffbfill.c
@@ -16,13 +16,7 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* SPECIAL NOTES!
-
-   Please note following when reading the code below:
-
-   - In this driver we assume that every memory can be accessed by same memory
-     bus.  If there are different address spaces do not use this code as a base
-     code for other archs.
+/* SPECIAL NOTE!
 
    - Every function in this code assumes that bounds checking has been done in
      previous phase and they are opted out in here.  */
diff --git video/ffbutil.c video/ffbutil.c
index 1fa48ec..db28e97 100644
--- video/ffbutil.c
+++ video/ffbutil.c
@@ -16,13 +16,7 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* SPECIAL NOTES!
-
-   Please note following when reading the code below:
-
-   - In this driver we assume that every memory can be accessed by same memory
-     bus.  If there are different address spaces do not use this code as a base
-     code for other archs.
+/* SPECIAL NOTE!
 
    - Every function in this code assumes that bounds checking has been done in
      previous phase and they are opted out in here.  */

[-- Attachment #4: rename-variables.patch --]
[-- Type: text/x-diff, Size: 14949 bytes --]

diff --git video/i386/pc/vbe.c video/i386/pc/vbe.c
index 0244b90..34885d6 100644
--- video/i386/pc/vbe.c
+++ video/i386/pc/vbe.c
@@ -31,7 +31,7 @@
 static int vbe_detected = -1;
 
 static struct grub_vbe_info_block controller_info;
-static struct grub_vbe_mode_info_block active_mode_info;
+static struct grub_vbe_mode_info_block active_vbe_mode_info;
 
 static struct
 {
@@ -40,14 +40,14 @@ static struct
 
   unsigned int bytes_per_scan_line;
   unsigned int bytes_per_pixel;
-  grub_uint32_t active_mode;
+  grub_uint32_t active_vbe_mode;
   grub_uint8_t *ptr;
   int index_color_mode;
 } framebuffer;
 
-static grub_uint32_t initial_mode;
-static grub_uint32_t mode_in_use = 0x55aa;
-static grub_uint16_t *mode_list;
+static grub_uint32_t initial_vbe_mode;
+static grub_uint32_t vbe_mode_in_use = 0x55aa;
+static grub_uint16_t *vbe_mode_list;
 
 static void *
 real2pm (grub_vbe_farptr_t ptr)
@@ -110,11 +110,11 @@ grub_vbe_probe (struct grub_vbe_info_block *info_block)
 }
 
 grub_err_t
-grub_vbe_set_video_mode (grub_uint32_t mode,
-                         struct grub_vbe_mode_info_block *mode_info)
+grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
+                         struct grub_vbe_mode_info_block *vbe_mode_info)
 {
   grub_vbe_status_t status;
-  grub_uint32_t old_mode;
+  grub_uint32_t old_vbe_mode;
   grub_err_t err;
 
   /* Make sure that VBE is supported.  */
@@ -123,18 +123,18 @@ grub_vbe_set_video_mode (grub_uint32_t mode,
     return grub_errno;
 
   /* Try to get mode info.  */
-  grub_vbe_get_video_mode_info (mode, &active_mode_info);
+  grub_vbe_get_video_mode_info (vbe_mode, &active_vbe_mode_info);
   if (grub_errno != GRUB_ERR_NONE)
     return grub_errno;
 
   /* For all VESA BIOS modes, force linear frame buffer.  */
-  if (mode >= 0x100)
+  if (vbe_mode >= 0x100)
     {
       /* We only want linear frame buffer modes.  */
-      mode |= 1 << 14;
+      vbe_mode |= 1 << 14;
 
       /* Determine frame buffer pixel format.  */
-      switch (active_mode_info.memory_model)
+      switch (active_vbe_mode_info.memory_model)
         {
         case GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL:
           framebuffer.index_color_mode = 1;
@@ -147,24 +147,24 @@ grub_vbe_set_video_mode (grub_uint32_t mode,
         default:
           return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
                              "unsupported pixel format 0x%x",
-                             active_mode_info.memory_model);
+                             active_vbe_mode_info.memory_model);
         }
     }
 
   /* Get current mode.  */
-  grub_vbe_get_video_mode (&old_mode);
+  grub_vbe_get_video_mode (&old_vbe_mode);
   if (grub_errno != GRUB_ERR_NONE)
     return grub_errno;
 
   /* Try to set video mode.  */
-  status = grub_vbe_bios_set_mode (mode, 0);
+  status = grub_vbe_bios_set_mode (vbe_mode, 0);
   if (status != GRUB_VBE_STATUS_OK)
-    return grub_error (GRUB_ERR_BAD_DEVICE, "cannot set VBE mode %x", mode);
+    return grub_error (GRUB_ERR_BAD_DEVICE, "cannot set VBE mode %x", vbe_mode);
 
   /* Save information for later usage.  */
-  framebuffer.active_mode = mode;
+  framebuffer.active_vbe_mode = vbe_mode;
 
-  if (mode < 0x100)
+  if (vbe_mode < 0x100)
     {
       /* If this is not a VESA mode, guess address.  */
       framebuffer.ptr = (grub_uint8_t *) GRUB_MEMORY_MACHINE_VGA_ADDR;
@@ -172,16 +172,16 @@ grub_vbe_set_video_mode (grub_uint32_t mode,
     }
   else
     {
-      framebuffer.ptr = (grub_uint8_t *) active_mode_info.phys_base_addr;
+      framebuffer.ptr = (grub_uint8_t *) active_vbe_mode_info.phys_base_addr;
 
       if (controller_info.version >= 0x300)
-        framebuffer.bytes_per_scan_line = active_mode_info.lin_bytes_per_scan_line;
+        framebuffer.bytes_per_scan_line = active_vbe_mode_info.lin_bytes_per_scan_line;
       else
-        framebuffer.bytes_per_scan_line = active_mode_info.bytes_per_scan_line;
+        framebuffer.bytes_per_scan_line = active_vbe_mode_info.bytes_per_scan_line;
     }
 
   /* Check whether mode is text mode or graphics mode.  */
-  if (active_mode_info.memory_model == GRUB_VBE_MEMORY_MODEL_TEXT)
+  if (active_vbe_mode_info.memory_model == GRUB_VBE_MEMORY_MODEL_TEXT)
     {
       /* Text mode.  */
 
@@ -193,7 +193,7 @@ grub_vbe_set_video_mode (grub_uint32_t mode,
       /* Graphics mode.  */
 
       /* Calculate bytes_per_pixel value.  */
-      switch(active_mode_info.bits_per_pixel)
+      switch(active_vbe_mode_info.bits_per_pixel)
 	{
 	case 32: framebuffer.bytes_per_pixel = 4; break;
 	case 24: framebuffer.bytes_per_pixel = 3; break;
@@ -201,10 +201,10 @@ grub_vbe_set_video_mode (grub_uint32_t mode,
 	case 15: framebuffer.bytes_per_pixel = 2; break;
 	case 8: framebuffer.bytes_per_pixel = 1; break;
 	default:
-	  grub_vbe_bios_set_mode (old_mode, 0);
+	  grub_vbe_bios_set_mode (old_vbe_mode, 0);
 	  return grub_error (GRUB_ERR_BAD_DEVICE,
 			     "cannot set VBE mode %x",
-			     mode);
+			     vbe_mode);
 	  break;
 	}
 
@@ -237,8 +237,8 @@ grub_vbe_set_video_mode (grub_uint32_t mode,
     }
 
   /* Copy mode info for caller.  */
-  if (mode_info)
-    grub_memcpy (mode_info, &active_mode_info, sizeof (*mode_info));
+  if (vbe_mode_info)
+    grub_memcpy (vbe_mode_info, &active_vbe_mode_info, sizeof (*vbe_mode_info));
 
   return GRUB_ERR_NONE;
 }
@@ -314,9 +314,9 @@ grub_vbe_get_video_mode_info (grub_uint32_t mode,
 static grub_err_t
 grub_video_vbe_init (void)
 {
-  grub_uint16_t *rm_mode_list;
+  grub_uint16_t *rm_vbe_mode_list;
   grub_uint16_t *p;
-  grub_size_t mode_list_size;
+  grub_size_t vbe_mode_list_size;
   struct grub_vbe_info_block info_block;
 
   /* Check if there is adapter present.
@@ -330,23 +330,23 @@ grub_video_vbe_init (void)
     return grub_errno;
 
   /* Copy modelist to local memory.  */
-  p = rm_mode_list = real2pm (info_block.video_mode_ptr);
+  p = rm_vbe_mode_list = real2pm (info_block.video_mode_ptr);
   while(*p++ != 0xFFFF)
     ;
 
-  mode_list_size = (grub_addr_t) p - (grub_addr_t) rm_mode_list;
-  mode_list = grub_malloc (mode_list_size);
-  if (! mode_list)
+  vbe_mode_list_size = (grub_addr_t) p - (grub_addr_t) rm_vbe_mode_list;
+  vbe_mode_list = grub_malloc (vbe_mode_list_size);
+  if (! vbe_mode_list)
     return grub_errno;
-  grub_memcpy (mode_list, rm_mode_list, mode_list_size);
+  grub_memcpy (vbe_mode_list, rm_vbe_mode_list, vbe_mode_list_size);
 
   /* Adapter could be found, figure out initial video mode.  */
-  grub_vbe_get_video_mode (&initial_mode);
+  grub_vbe_get_video_mode (&initial_vbe_mode);
   if (grub_errno != GRUB_ERR_NONE)
     {
       /* Free allocated resources.  */
-      grub_free (mode_list);
-      mode_list = NULL;
+      grub_free (vbe_mode_list);
+      vbe_mode_list = NULL;
 
       return grub_errno;
     }
@@ -363,14 +363,14 @@ grub_video_vbe_fini (void)
   grub_vbe_status_t status;
 
   /* Restore old video mode.  */
-  status = grub_vbe_bios_set_mode (initial_mode, 0);
+  status = grub_vbe_bios_set_mode (initial_vbe_mode, 0);
   if (status != GRUB_VBE_STATUS_OK)
     /* TODO: Decide, is this something we want to do.  */
     return grub_errno;
 
   /* TODO: Free any resources allocated by driver.  */
-  grub_free (mode_list);
-  mode_list = NULL;
+  grub_free (vbe_mode_list);
+  vbe_mode_list = NULL;
 
   /* TODO: destroy render targets.  */
 
@@ -382,9 +382,9 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
                       unsigned int mode_type)
 {
   grub_uint16_t *p;
-  struct grub_vbe_mode_info_block mode_info;
-  struct grub_vbe_mode_info_block best_mode_info;
-  grub_uint32_t best_mode = 0;
+  struct grub_vbe_mode_info_block vbe_mode_info;
+  struct grub_vbe_mode_info_block best_vbe_mode_info;
+  grub_uint32_t best_vbe_mode = 0;
   int depth;
 
   /* Decode depth from mode_type.  If it is zero, then autodetect.  */
@@ -392,11 +392,11 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
           >> GRUB_VIDEO_MODE_TYPE_DEPTH_POS;
 
   /* Walk thru mode list and try to find matching mode.  */
-  for (p = mode_list; *p != 0xFFFF; p++)
+  for (p = vbe_mode_list; *p != 0xFFFF; p++)
     {
-      grub_uint32_t mode = *p;
+      grub_uint32_t vbe_mode = *p;
 
-      grub_vbe_get_video_mode_info (mode, &mode_info);
+      grub_vbe_get_video_mode_info (vbe_mode, &vbe_mode_info);
       if (grub_errno != GRUB_ERR_NONE)
         {
           /* Could not retrieve mode info, retreat.  */
@@ -404,33 +404,33 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
           break;
         }
 
-      if ((mode_info.mode_attributes & 0x001) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x001) == 0)
         /* If not available, skip it.  */
         continue;
 
-      if ((mode_info.mode_attributes & 0x002) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x002) == 0)
         /* Not enough information.  */
         continue;
 
-      if ((mode_info.mode_attributes & 0x008) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x008) == 0)
         /* Monochrome is unusable.  */
         continue;
 
-      if ((mode_info.mode_attributes & 0x080) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x080) == 0)
         /* We support only linear frame buffer modes.  */
         continue;
 
-      if ((mode_info.mode_attributes & 0x010) == 0)
+      if ((vbe_mode_info.mode_attributes & 0x010) == 0)
         /* We allow only graphical modes.  */
         continue;
 
-      if ((mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL)
-          && (mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR))
+      if ((vbe_mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL)
+          && (vbe_mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR))
         /* Not compatible memory model.  */
         continue;
 
-      if ((mode_info.x_resolution != width)
-          || (mode_info.y_resolution != height))
+      if ((vbe_mode_info.x_resolution != width)
+          || (vbe_mode_info.y_resolution != height))
         /* Non matching resolution.  */
         continue;
 
@@ -438,65 +438,65 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
       if ((mode_type & GRUB_VIDEO_MODE_TYPE_COLOR_MASK) != 0)
         {
           if (((mode_type & GRUB_VIDEO_MODE_TYPE_INDEX_COLOR) != 0)
-              && (mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL))
+              && (vbe_mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL))
             /* Requested only index color modes.  */
             continue;
 
           if (((mode_type & GRUB_VIDEO_MODE_TYPE_RGB) != 0)
-              && (mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR))
+              && (vbe_mode_info.memory_model != GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR))
             /* Requested only RGB modes.  */
             continue;
         }
 
       /* If there is a request for specific depth, ignore others.  */
-      if ((depth != 0) && (mode_info.bits_per_pixel != depth))
+      if ((depth != 0) && (vbe_mode_info.bits_per_pixel != depth))
         continue;
 
       /* Select mode with most number of bits per pixel.  */
-      if (best_mode != 0)
-        if (mode_info.bits_per_pixel < best_mode_info.bits_per_pixel)
+      if (best_vbe_mode != 0)
+        if (vbe_mode_info.bits_per_pixel < best_vbe_mode_info.bits_per_pixel)
           continue;
 
       /* Save so far best mode information for later use.  */
-      best_mode = mode;
-      grub_memcpy (&best_mode_info, &mode_info, sizeof (mode_info));
+      best_vbe_mode = vbe_mode;
+      grub_memcpy (&best_vbe_mode_info, &vbe_mode_info, sizeof (vbe_mode_info));
     }
 
   /* Try to initialize best mode found.  */
-  if (best_mode != 0)
+  if (best_vbe_mode != 0)
     {
       grub_err_t err;
       /* If this fails, then we have mode selection heuristics problem,
          or adapter failure.  */
-      grub_vbe_set_video_mode (best_mode, &active_mode_info);
+      grub_vbe_set_video_mode (best_vbe_mode, &active_vbe_mode_info);
       if (grub_errno != GRUB_ERR_NONE)
         return grub_errno;
 
       /* Now we are happily in requested video mode.  Cache some info
          in order to fasten later operations.  */
-      mode_in_use = best_mode;
+      vbe_mode_in_use = best_vbe_mode;
 
       /* Fill mode info details.  */
-      framebuffer.mode_info.width = active_mode_info.x_resolution;
-      framebuffer.mode_info.height = active_mode_info.y_resolution;
+      framebuffer.mode_info.width = active_vbe_mode_info.x_resolution;
+      framebuffer.mode_info.height = active_vbe_mode_info.y_resolution;
 
       if (framebuffer.index_color_mode)
         framebuffer.mode_info.mode_type = GRUB_VIDEO_MODE_TYPE_INDEX_COLOR;
       else
         framebuffer.mode_info.mode_type = GRUB_VIDEO_MODE_TYPE_RGB;
 
-      framebuffer.mode_info.bpp = active_mode_info.bits_per_pixel;
+      framebuffer.mode_info.bpp = active_vbe_mode_info.bits_per_pixel;
       framebuffer.mode_info.bytes_per_pixel = framebuffer.bytes_per_pixel;
       framebuffer.mode_info.pitch = framebuffer.bytes_per_scan_line;
       framebuffer.mode_info.number_of_colors = 256; /* TODO: fix me.  */
-      framebuffer.mode_info.red_mask_size = active_mode_info.red_mask_size;
-      framebuffer.mode_info.red_field_pos = active_mode_info.red_field_position;
-      framebuffer.mode_info.green_mask_size = active_mode_info.green_mask_size;
-      framebuffer.mode_info.green_field_pos = active_mode_info.green_field_position;
-      framebuffer.mode_info.blue_mask_size = active_mode_info.blue_mask_size;
-      framebuffer.mode_info.blue_field_pos = active_mode_info.blue_field_position;
-      framebuffer.mode_info.reserved_mask_size = active_mode_info.rsvd_mask_size;
-      framebuffer.mode_info.reserved_field_pos = active_mode_info.rsvd_field_position;
+      framebuffer.mode_info.red_mask_size = active_vbe_mode_info.red_mask_size;
+      framebuffer.mode_info.red_field_pos = active_vbe_mode_info.red_field_position;
+      framebuffer.mode_info.green_mask_size = active_vbe_mode_info.green_mask_size;
+      framebuffer.mode_info.green_field_pos = active_vbe_mode_info.green_field_position;
+      framebuffer.mode_info.blue_mask_size = active_vbe_mode_info.blue_mask_size;
+      framebuffer.mode_info.blue_field_pos = active_vbe_mode_info.blue_field_position;
+      framebuffer.mode_info.reserved_mask_size = active_vbe_mode_info.rsvd_mask_size;
+      framebuffer.mode_info.reserved_field_pos = active_vbe_mode_info.rsvd_field_position;
 
       framebuffer.mode_info.blit_format = grub_video_get_blit_format (&framebuffer.mode_info);
 
@@ -562,8 +562,8 @@ grub_video_vbe_get_info_and_fini (struct grub_video_mode_info *mode_info,
   grub_memcpy (mode_info, &(framebuffer.mode_info), sizeof (*mode_info));
   *framebuf = (char *) framebuffer.ptr;
 
-  grub_free (mode_list);
-  mode_list = NULL;
+  grub_free (vbe_mode_list);
+  vbe_mode_list = NULL;
 
   grub_video_fb_fini ();
 

[-- Attachment #5: vbe-active-mode-cleanup.patch --]
[-- Type: text/x-diff, Size: 4282 bytes --]

diff --git video/i386/pc/vbe.c video/i386/pc/vbe.c
index 34885d6..1bcbf8e 100644
--- video/i386/pc/vbe.c
+++ video/i386/pc/vbe.c
@@ -46,7 +46,6 @@ static struct
 } framebuffer;
 
 static grub_uint32_t initial_vbe_mode;
-static grub_uint32_t vbe_mode_in_use = 0x55aa;
 static grub_uint16_t *vbe_mode_list;
 
 static void *
@@ -115,6 +114,7 @@ grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
 {
   grub_vbe_status_t status;
   grub_uint32_t old_vbe_mode;
+  struct grub_vbe_mode_info_block new_vbe_mode_info;
   grub_err_t err;
 
   /* Make sure that VBE is supported.  */
@@ -123,7 +123,7 @@ grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
     return grub_errno;
 
   /* Try to get mode info.  */
-  grub_vbe_get_video_mode_info (vbe_mode, &active_vbe_mode_info);
+  grub_vbe_get_video_mode_info (vbe_mode, &new_vbe_mode_info);
   if (grub_errno != GRUB_ERR_NONE)
     return grub_errno;
 
@@ -134,7 +134,7 @@ grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
       vbe_mode |= 1 << 14;
 
       /* Determine frame buffer pixel format.  */
-      switch (active_vbe_mode_info.memory_model)
+      switch (new_vbe_mode_info.memory_model)
         {
         case GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL:
           framebuffer.index_color_mode = 1;
@@ -147,7 +147,7 @@ grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
         default:
           return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
                              "unsupported pixel format 0x%x",
-                             active_vbe_mode_info.memory_model);
+                             new_vbe_mode_info.memory_model);
         }
     }
 
@@ -163,6 +163,7 @@ grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
 
   /* Save information for later usage.  */
   framebuffer.active_vbe_mode = vbe_mode;
+  grub_memcpy (&active_vbe_mode_info, &new_vbe_mode_info, sizeof (active_vbe_mode_info));
 
   if (vbe_mode < 0x100)
     {
@@ -172,16 +173,16 @@ grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
     }
   else
     {
-      framebuffer.ptr = (grub_uint8_t *) active_vbe_mode_info.phys_base_addr;
+      framebuffer.ptr = (grub_uint8_t *) new_vbe_mode_info.phys_base_addr;
 
       if (controller_info.version >= 0x300)
-        framebuffer.bytes_per_scan_line = active_vbe_mode_info.lin_bytes_per_scan_line;
+        framebuffer.bytes_per_scan_line = new_vbe_mode_info.lin_bytes_per_scan_line;
       else
-        framebuffer.bytes_per_scan_line = active_vbe_mode_info.bytes_per_scan_line;
+        framebuffer.bytes_per_scan_line = new_vbe_mode_info.bytes_per_scan_line;
     }
 
   /* Check whether mode is text mode or graphics mode.  */
-  if (active_vbe_mode_info.memory_model == GRUB_VBE_MEMORY_MODEL_TEXT)
+  if (new_vbe_mode_info.memory_model == GRUB_VBE_MEMORY_MODEL_TEXT)
     {
       /* Text mode.  */
 
@@ -193,7 +194,7 @@ grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
       /* Graphics mode.  */
 
       /* Calculate bytes_per_pixel value.  */
-      switch(active_vbe_mode_info.bits_per_pixel)
+      switch(new_vbe_mode_info.bits_per_pixel)
 	{
 	case 32: framebuffer.bytes_per_pixel = 4; break;
 	case 24: framebuffer.bytes_per_pixel = 3; break;
@@ -238,7 +239,7 @@ grub_vbe_set_video_mode (grub_uint32_t vbe_mode,
 
   /* Copy mode info for caller.  */
   if (vbe_mode_info)
-    grub_memcpy (vbe_mode_info, &active_vbe_mode_info, sizeof (*vbe_mode_info));
+    grub_memcpy (vbe_mode_info, &new_vbe_mode_info, sizeof (*vbe_mode_info));
 
   return GRUB_ERR_NONE;
 }
@@ -468,14 +469,11 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
       grub_err_t err;
       /* If this fails, then we have mode selection heuristics problem,
          or adapter failure.  */
-      grub_vbe_set_video_mode (best_vbe_mode, &active_vbe_mode_info);
+      /* grub_vbe_set_video_mode already sets active_vbe_mode_info. */
+      grub_vbe_set_video_mode (best_vbe_mode, NULL);
       if (grub_errno != GRUB_ERR_NONE)
         return grub_errno;
 
-      /* Now we are happily in requested video mode.  Cache some info
-         in order to fasten later operations.  */
-      vbe_mode_in_use = best_vbe_mode;
-
       /* Fill mode info details.  */
       framebuffer.mode_info.width = active_vbe_mode_info.x_resolution;
       framebuffer.mode_info.height = active_vbe_mode_info.y_resolution;

Re: Minor framebuffer cleanup patches

[Vladimir 'phcoder' Serbinenko]

On Sat, Aug 15, 2009 at 12:44 AM, Michal Suchanek<hramrach@centrum.cz> wrote:
> 2009/8/14 Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>:
>> On Fri, Aug 14, 2009 at 4:04 PM, Michal Suchanek<hramrach@centrum.cz> wrote:
>>> Hello
>>>
>>> I am sending the rest of framebuffer patches that were not included in
>>> the split by Vladimir.
>>>
>>> remove-dup-function.patch
>>>
>>> * video_fb.c: remove grub_video_fb_get_video_ptr which is duplicated in fbutil.c
>>> * fbutil.c: copy the note from fbfill.c
>> I think the comment should go to video_fb.h to instruct future video
>> driver writers.
>> Could you create entries in the same format as Changelog
>
> * video/fb/video_fb.c (grub_video_fb_get_video_ptr): remove function
> (duplicated in fbutil.c)
> * video/fb/fbutil.c: copy the note from fbfill.c
>
> Attaching additional patch that moves the note about memory
> architecture in video_fb.h
>
> * video_fb.h: move the note about memory architecture from fb*.c here
>
>>>
>>> rename-var.patch
>>>
>>> vbe.c: grub_video_vbe_setup rename local variable
>>>  - this should make the distinction between grub_vbe_mode_info_block
>>> and grub_video_mode_info clearer in this function
>>>
>> Perhaps best_mode_info should be renamed too, for consistency.
>> Other than this I'm ok with both patches.
>
> I meant this only as a reminder that the mode_info that is used all
> the time really refers to a vbe mode (as opposed to framebuffer mode)
> bu it is certainly possible to rename
>
> Attaching an alternate patch that renames all the vbe mode related locals.
>
>
>    * video/i386/pc/vbe.c: rename local variables to make clearer
>      distinction between vbe modes and framebuffer modes
>
>
> While making this patch I noticed that
>
> - grub_vbe_set_video_mode sets the mode in framebuffer.active_mode
> and grub_video_vbe_setup in vbe_mode_in_use. Either this is redundant
> or any call to grub_vbe_set_video_mode without going through
> grub_video_vbe_setup makes the mode_in_use value obsolete - invalid.
>
> - grub_vbe_set_video_mode changes active_mode_info before setting the
> mode (and before checking it can be used)
>
> - grub_vbe_setup calls grub_video_set_video_mode with pointer to
> active_mode_info as argument which causes call to memcpy with this
> pointer as both src and dest
>
> This is resolved with additional patch:
>
> * video/i386/pc/vbe.c: remove vbe_mode_in_use (duplicated in
> framebuffer.active_vbe_mode)
> * video/i386/pc/vbe.c (grub_vbe_set_video_mode): save active mode info
> only after setting the mode
I comitted three patches to branches michal[123] on my git. If
maintainer confirms it's ok for inclusion (legal reasons) I'll commit
to mainstream. You can see on my git how Changelog entries for you
changes should look like
Patch move-memarch-comment.patch is garbled


-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: Minor framebuffer cleanup patches

[Michal Suchanek]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]

2009/8/16 Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>:

> I comitted three patches to branches michal[123] on my git. If
> maintainer confirms it's ok for inclusion (legal reasons) I'll commit
> to mainstream. You can see on my git how Changelog entries for you
> changes should look like

Looking at these changelog entries makes the format quite clear.

> Patch move-memarch-comment.patch is garbled
Sorry, attaching un-garbled patch.

Thanks

Michal

[-- Attachment #2: move-memarch-comment.patch --]
[-- Type: text/x-diff, Size: 2409 bytes --]

diff --git include/gruvideo_fb.h include/gruvideo_fb.h
index 17debd6..850946d 100644
--- include/grub/video_fb.h
+++ include/grub/video_fb.h
@@ -16,6 +16,12 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* SPECIAL NOTE!
+
+   - In this driver we assume that every memory can be accessed by same memory
+     bus.  If there are different address spaces do not use this code as a base
+     code for other archs.  */
+
 #ifndef GRUB_VIDEO_FB_HEADER
 #define GRUB_VIDEO_FB_HEADER	1
 
diff --git video/ffbblit.c video/ffbblit.c
index 5b613bc..705da83 100644
--- video/fb/fbblit.c
+++ video/fb/fbblit.c
@@ -16,13 +16,7 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* SPECIAL NOTES!
-
-   Please note following when reading the code below:
-
-   - In this driver we assume that every memory can be accessed by same memory
-   bus.  If there are different address spaces do not use this code as a base
-   code for other archs.
+/* SPECIAL NOTE!
 
    - Every function in this code assumes that bounds checking has been done in
    previous phase and they are opted out in here.  */
diff --git video/ffbfill.c video/ffbfill.c
index a4ca7c2..dba8ceb 100644
--- video/fb/fbfill.c
+++ video/fb/fbfill.c
@@ -16,13 +16,7 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* SPECIAL NOTES!
-
-   Please note following when reading the code below:
-
-   - In this driver we assume that every memory can be accessed by same memory
-     bus.  If there are different address spaces do not use this code as a base
-     code for other archs.
+/* SPECIAL NOTE!
 
    - Every function in this code assumes that bounds checking has been done in
      previous phase and they are opted out in here.  */
diff --git video/ffbutil.c video/ffbutil.c
index 1fa48ec..db28e97 100644
--- video/fb/fbutil.c
+++ video/fb/fbutil.c
@@ -16,13 +16,7 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* SPECIAL NOTES!
-
-   Please note following when reading the code below:
-
-   - In this driver we assume that every memory can be accessed by same memory
-     bus.  If there are different address spaces do not use this code as a base
-     code for other archs.
+/* SPECIAL NOTE!
 
    - Every function in this code assumes that bounds checking has been done in
      previous phase and they are opted out in here.  */

Re: Minor framebuffer cleanup patches

[Robert Millan]

On Sun, Aug 16, 2009 at 05:59:18PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> I comitted three patches to branches michal[123] on my git. If
> maintainer confirms it's ok for inclusion (legal reasons) I'll commit
> to mainstream.

Please go ahead.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: Minor framebuffer cleanup patches

[Vladimir 'phcoder' Serbinenko]

On Mon, Aug 17, 2009 at 3:28 PM, Robert Millan<rmh@aybabtu.com> wrote:
> On Sun, Aug 16, 2009 at 05:59:18PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>> I comitted three patches to branches michal[123] on my git. If
>> maintainer confirms it's ok for inclusion (legal reasons) I'll commit
>> to mainstream.
>
> Please go ahead.
>
Done
> --
> Robert Millan
>
>  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>  how) you may access your data; but nobody's threatening your freedom: we
>  still allow you to remove your data and not access it at all."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git