On Sat, Aug 15, 2009 at 12:44 AM, Michal Suchanek<hramr...@centrum.cz> wrote: > 2009/8/14 Vladimir 'phcoder' Serbinenko <phco...@gmail.com>: >> On Fri, Aug 14, 2009 at 4:04 PM, Michal Suchanek<hramr...@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 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
