[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Minor framebuffer cleanup patches

From: Michal Suchanek
Subject: Re: Minor framebuffer cleanup patches
Date: Sat, 15 Aug 2009 00:44:03 +0200

2009/8/14 Vladimir 'phcoder' Serbinenko <address@hidden>:
> On Fri, Aug 14, 2009 at 4:04 PM, Michal Suchanek<address@hidden> 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
* video/i386/pc/vbe.c (grub_vbe_set_video_mode): save active mode info
only after setting the mode



Attachment: remove-dup-function.patch
Description: Text Data

Attachment: move-memarch-comment.patch
Description: Text Data

Attachment: rename-variables.patch
Description: Text Data

Attachment: vbe-active-mode-cleanup.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]