>
> I see that returning framebuffer address and finishing the video subsystem
> must be together, but is there a reason to couple this with getting mode_info
> ?
When designing this part I thought of accelerated graphics. Before
loading the OS they would switch to standard framebuffer mode this may
also affect the formats used so the info may be different from
get_info
>
> Btw, if I understand correctly, we have a race condition right now. As a
> bugfix it'd be better to merge this separately from the interface redesign if
> possible.
race condition? We don't even have threads
>
> Also, does finishing the video subsystem only affect GRUB internally, or
> result in any effect at the display level? I want to avoid having visual
> glitches when the payload is loaded without switching video mode.
>
Other than addition of double buffering changes will not result in any
visible effect.
>> - modevar = grub_env_get ("gfxpayload");
>> -
>> - /* Now all graphical modes are acceptable.
>> - May change in future if we have modes without framebuffer. */
>> [...]
>> - {
>> - params->have_vga = GRUB_VIDEO_TYPE_TEXT;
>> - params->video_width = 80;
>> - params->video_height = 25;
>> - }
>> -
>
> Why is this chunk of code moved down? AFAICS, this change only involves
> adding an additional layer between it and the video backend. Does this
> make it conflict with something else?
>
I wanted to keep normal grub_printf as long as possible and after
get_mode_and_fini grub_printf may be unfunctional.
>> +#define grub_video_render_target grub_video_fbrender_target
>
> If we want to rename this function, I'd rather do it all the way than
> keeping a compatibility macro. But then, I'd also prefer if this is
> done separately from the rest (either before or after).
>
It's not about renaming but to inform includes that
grub_video_render_target is in fact grub_video_fbrender_target and so
avoid warnings and casts.
>> +/* Select the best double buffering mode available. */
>> +static void
>> +double_buffering_init (int enable)
>
> This patch also seems to add double buffering support, which is a feature
> that wasn't yet implemented. I suppose it's good to have (I'm not very
> clued about graphics), but I'd prefer if we can merge this separately
> from the redesign changes.
>
Ok, I will try to split while keeping patch functional.
> --
> 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
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
