[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: Sat, 1 Aug 2009 17:10:30 +0200

> 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
> 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
> address@hidden

Vladimir 'phcoder' Serbinenko

Personal git repository:

reply via email to

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