Re: [Xen-devel] [PATCH v2 4/8] xen/vesa: use the new fb_* functions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.12.12 at 18:54, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 
>>> wrote:
> On Mon, 10 Dec 2012, Jan Beulich wrote:
>> >>> On 07.12.12 at 19:02, Stefano Stabellini 
>> >>> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> > --- a/xen/drivers/video/vesa.c
>> > +++ b/xen/drivers/video/vesa.c
>> > @@ -13,20 +13,15 @@
>> >  #include <asm/io.h>
>> >  #include <asm/page.h>
>> >  #include "font.h"
>> > +#include "fb.h"
>> >  
>> >  #define vlfb_info    vga_console_info.u.vesa_lfb
>> > -#define text_columns (vlfb_info.width / font->width)
>> > -#define text_rows    (vlfb_info.height / font->height)
>> >  
>> > -static void vesa_redraw_puts(const char *s);
>> > -static void vesa_scroll_puts(const char *s);
>> > +static void lfb_flush(void);
>> >  
>> > -static unsigned char *lfb, *lbuf, *text_buf;
>> > -static unsigned int *__initdata line_len;
>> > +static unsigned char *lfb;
>> 
>> What's the point of retaining this, when ...
>> 
>> >  static const struct font_desc *font;
>> >  static bool_t vga_compat;
>> > -static unsigned int pixel_on;
>> > -static unsigned int xpos, ypos;
>> >  
>> >  static unsigned int vram_total;
>> >  integer_param("vesa-ram", vram_total);
>> > @@ -87,29 +82,26 @@ void __init vesa_early_init(void)
>> >  
>> >  void __init vesa_init(void)
>> >  {
>> > -    if ( !font )
>> > -        goto fail;
>> > -
>> > -    lbuf = xmalloc_bytes(vlfb_info.bytes_per_line);
>> > -    if ( !lbuf )
>> > -        goto fail;
>> > +    struct fb_prop fbp;
>> >  
>> > -    text_buf = xzalloc_bytes(text_columns * text_rows);
>> > -    if ( !text_buf )
>> > -        goto fail;
>> > +    if ( !font )
>> > +        return;
>> >  
>> > -    line_len = xzalloc_array(unsigned int, text_columns);
>> > -    if ( !line_len )
>> > -        goto fail;
>> > +    fbp.font = font;
>> > +    fbp.bits_per_pixel = vlfb_info.bits_per_pixel;
>> > +    fbp.bytes_per_line = vlfb_info.bytes_per_line;
>> > +    fbp.width = vlfb_info.width;
>> > +    fbp.height = vlfb_info.height;
>> > +    fbp.flush = lfb_flush;
>> > +    fbp.text_columns = vlfb_info.width / font->width;
>> > +    fbp.text_rows = vlfb_info.height / font->height;
>> >  
>> > -    lfb = ioremap(vlfb_info.lfb_base, vram_remap);
>> > +    fbp.lfb = lfb = ioremap(vlfb_info.lfb_base, vram_remap);
>> 
>> ... you set up the consolidated field at the same time anyway?
> 
> It is used by vesa_mtrr_init and vesa_endboot.
> At the moment I don't store fbp in a static variable so after vesa_init
> returns, vesa.c doesn't have a way to retrieve it.
> Maybe I should introduce a "struct fb_prop fbp" static variable that
> replaces lfb?

Oh, I didn't realize that the static is private to the new fb.c. I'd
think sharing this would make sense (and the specific driver could
then fill the generic fields there directly). But I'll leave that up to
you then which way you prefer - it just looked like unnecessary
redundancy to me.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel