|
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 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
remove-dup-function.patch
Description: Text Data
move-memarch-comment.patch
Description: Text Data
rename-variables.patch
Description: Text Data
vbe-active-mode-cleanup.patch
Description: Text Data
[Prev in Thread] | Current Thread | [Next in Thread] |