[Top][All Lists]

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

Re: Fwd: [PATCH 1/2] Framebuffer split

From: Vladimir 'phcoder' Serbinenko
Subject: Re: Fwd: [PATCH 1/2] Framebuffer split
Date: Fri, 14 Aug 2009 15:23:24 +0200

Framebuffer patch comitted and I removed framebuf branch on my repo.
> OK, I did not get why fbfill.h of the three private headers.
> Perhaps fbutil.h which already declares blit_info would be a better place 
> then.
I haven't looked in depth how private headers were organised, I just
moved the same layout to fb*. Perhaps we need only one framebuffer
private header now
>>> In grub_video_vbe_setup function in vbe.c the default palette is
>>> loaded before the created render target is set as active. I guess this
>>> would be a problem if the palette was actually needed/supported.
>>> Changing the order to create, set_active, set_palette makes the code
>>> fail for me,though.
>> Works for me. Have you forgotten about 'return' ?
> Yes, perhaps I did something wrong when reordering the calls for the first 
> time.
It's already reordered in comitted version
>>> In this same code struct grub_video_mode_info mode_info is added in
>>> the last round of patches but I cannot find where the changed code
>>> touches the variable. This is somewhat odd.
>> grep is your friend:
>>      framebuffer.mode_info.width = active_mode_info.x_resolution;
>> and follows. Since now vbe.c can't manipulate directly the target.
>> Again, you're the one who requested encapsulation.
> Sorry, I just confused the
> grub_video_mode_info mode_info
> with the
> grub_vbe_mode_info_block mode_info
> Perhaps a more distinctive name for the latter would be helpful, though.
I'm ok with rename.
> This is a patch against the current (as far as git knows) framebuf branch:
This branch doesn't exist anymore since it's upstream ;)
>    * move video_fb_fender_target structure declaration to fbutil.h
Requires me to looks at how three headers are organised.
>    * remove duplicate assignment in create_render_target_from_pointer
>       + quiet warning
Is already done in committed version
>    * rename local variable in grub_video_vbe_setup
>    * remove grub_video_fb_get_video_ptr from video_fb.c (dup in fbutil.c)
Good patch too
> Take which you want
Could you send them as separate patches with changelogs? It's
especially important in your case since you have no copyright
assignment yet and we have to decide which patches can go in without
such. I'm not maintainer so I need explicit consent from them to apply
your patches before you sign copyright assignment
> Thanks
> Michal
> _______________________________________________
> Grub-devel mailing list
> address@hidden

Vladimir 'phcoder' Serbinenko

Personal git repository:

reply via email to

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