|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 3/7] xen: introduce a generic framebuffer driver
On Wed, 9 Jan 2013, Mats Petersson wrote:
> On 09/01/13 14:45, Mats Petersson wrote:
> > On 09/01/13 14:05, Stefano Stabellini wrote:
> >> On Wed, 9 Jan 2013, Jan Beulich wrote:
> >>>>>> On 08.01.13 at 21:03, Stefano Stabellini
> >>>>>> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> >>>> --- /dev/null
> >>>> +++ b/xen/drivers/video/lfb.c
> >>>> @@ -0,0 +1,206 @@
> >>>> +/******************************************************************************
> >>>> + * lfb.c
> >>>> + *
> >>>> + * linear frame buffer handling.
> >>>> + */
> >>>> +
> >>>> +#include <xen/config.h>
> >>>> +#include <xen/kernel.h>
> >>>> +#include <xen/lib.h>
> >>>> +#include <xen/errno.h>
> >>>> +#include "lfb.h"
> >>>> +#include "font.h"
> >>>> +
> >>>> +#define MAX_XRES 1900
> >>>> +#define MAX_YRES 1200
> >>>> +#define MAX_BPP 4
> >>>> +#define MAX_FONT_W 8
> >>>> +#define MAX_FONT_H 16
> >>>> +static __initdata unsigned int line_len[MAX_XRES / MAX_FONT_W];
> >>>> +static __initdata unsigned char lbuf[MAX_XRES * MAX_BPP];
> >>>> +static __initdata unsigned char text_buf[(MAX_XRES / MAX_FONT_W) * \
> >>>> + (MAX_YRES / MAX_FONT_H)];
> >>> Imo it would be better to move all these __initdata definitions
> >>> do to where the using __init functions actually live, to make
> >>> sure unintended use of them in non-__init ones is noticed.
> >> OK
> >>
> >>
> >>>> +
> >>>> +struct lfb_status {
> >>>> + struct lfb_prop lfbp;
> >>>> +
> >>>> + unsigned char *lbuf, *text_buf;
> >>>> + unsigned int *line_len;
> >>>> + unsigned int xpos, ypos;
> >>>> +};
> >>>> +static struct lfb_status lfb;
> >>>> +
> >>>> +static void lfb_show_line(
> >>>> + const unsigned char *text_line,
> >>>> + unsigned char *video_line,
> >>>> + unsigned int nr_chars,
> >>>> + unsigned int nr_cells)
> >>>> +{
> >>>> + unsigned int i, j, b, bpp, pixel;
> >>>> +
> >>>> + bpp = (lfb.lfbp.bits_per_pixel + 7) >> 3;
> >>>> +
> >>>> + for ( i = 0; i < lfb.lfbp.font->height; i++ )
> >>>> + {
> >>>> + unsigned char *ptr = lfb.lbuf;
> >>>> +
> >>>> + for ( j = 0; j < nr_chars; j++ )
> >>>> + {
> >>>> + const unsigned char *bits = lfb.lfbp.font->data;
> >>>> + bits += ((text_line[j] * lfb.lfbp.font->height + i) *
> >>>> + ((lfb.lfbp.font->width + 7) >> 3));
> >>>> + for ( b = lfb.lfbp.font->width; b--; )
> >>>> + {
> >>>> + pixel = (*bits & (1u<<b)) ? lfb.lfbp.pixel_on : 0;
> >>>> + memcpy(ptr, &pixel, bpp);
> >>>> + ptr += bpp;
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + memset(ptr, 0, (lfb.lfbp.width - nr_chars *
> >>>> lfb.lfbp.font->width) * bpp);
> >>>> + memcpy(video_line, lfb.lbuf, nr_cells * lfb.lfbp.font->width *
> >>>> bpp);
> >>>> + video_line += lfb.lfbp.bytes_per_line;
> >>>> + }
> >>>> +}
> >>>> +
> >>>> +/* Fast mode which redraws all modified parts of a 2D text buffer. */
> >>>> +void lfb_redraw_puts(const char *s)
> >>>> +{
> >>>> + unsigned int i, min_redraw_y = lfb.ypos;
> >>>> + char c;
> >>>> +
> >>>> + /* Paste characters into text buffer. */
> >>>> + while ( (c = *s++) != '\0' )
> >>>> + {
> >>>> + if ( (c == '\n') || (lfb.xpos >= lfb.lfbp.text_columns) )
> >>>> + {
> >>>> + if ( ++lfb.ypos >= lfb.lfbp.text_rows )
> >>>> + {
> >>>> + min_redraw_y = 0;
> >>>> + lfb.ypos = lfb.lfbp.text_rows - 1;
> >>>> + memmove(lfb.text_buf, lfb.text_buf +
> >>>> lfb.lfbp.text_columns,
> >>>> + lfb.ypos * lfb.lfbp.text_columns);
> >>>> + memset(lfb.text_buf + lfb.ypos * lfb.lfbp.text_columns,
> >>>> 0, lfb.xpos);
> >>>> + }
> >>>> + lfb.xpos = 0;
> >>>> + }
> >>>> +
> >>>> + if ( c != '\n' )
> >>>> + lfb.text_buf[lfb.xpos++ + lfb.ypos * lfb.lfbp.text_columns]
> >>>> = c;
> >>>> + }
> >>>> +
> >>>> + /* Render modified section of text buffer to VESA linear
> >>>> framebuffer. */
> >>>> + for ( i = min_redraw_y; i <= lfb.ypos; i++ )
> >>>> + {
> >>>> + const unsigned char *line = lfb.text_buf + i *
> >>>> lfb.lfbp.text_columns;
> >>>> + unsigned int width;
> >>>> +
> >>>> + for ( width = lfb.lfbp.text_columns; width; --width )
> >>>> + if ( line[width - 1] )
> >>>> + break;
> >>>> + lfb_show_line(line,
> >>>> + lfb.lfbp.lfb + i * lfb.lfbp.font->height *
> >>>> lfb.lfbp.bytes_per_line,
> >>>> + width, max(lfb.line_len[i], width));
> >>>> + lfb.line_len[i] = width;
> >>>> + }
> >>>> +
> >>>> + lfb.lfbp.flush();
> >>>> +}
> >>>> +
> >>>> +/* Slower line-based scroll mode which interacts better with dom0. */
> >>>> +void lfb_scroll_puts(const char *s)
> >>>> +{
> >>>> + unsigned int i;
> >>>> + char c;
> >>>> +
> >>>> + while ( (c = *s++) != '\0' )
> >>>> + {
> >>>> + if ( (c == '\n') || (lfb.xpos >= lfb.lfbp.text_columns) )
> >>>> + {
> >>>> + unsigned int bytes = (lfb.lfbp.width *
> >>>> + ((lfb.lfbp.bits_per_pixel + 7) >> 3));
> >>>> + unsigned char *src = lfb.lfbp.lfb + lfb.lfbp.font->height *
> >>>> lfb.lfbp.bytes_per_line;
> >>>> + unsigned char *dst = lfb.lfbp.lfb;
> >>>> +
> >>>> + /* New line: scroll all previous rows up one line. */
> >>>> + for ( i = lfb.lfbp.font->height; i < lfb.lfbp.height; i++ )
> >>>> + {
> >>>> + memcpy(dst, src, bytes);
> >>>> + src += lfb.lfbp.bytes_per_line;
> >>>> + dst += lfb.lfbp.bytes_per_line;
> >>>> + }
> >>>> +
> >>>> + /* Render new line. */
> >>>> + lfb_show_line(
> >>>> + lfb.text_buf,
> >>>> + lfb.lfbp.lfb + (lfb.lfbp.text_rows-1) *
> >>>> lfb.lfbp.font->height * lfb.lfbp.bytes_per_line,
> >>> Long line?
> >> Right
> >>
> > The calculation of
> >
> > lfb.lfbp.lfb + lfb.lfbp.font->height * lfb.lfbp.bytes_per_line;
> >
> > is done twice in the above section - surely that's constant over time, so
> > could be stored in another local variable - thus making the line shorter
> > and at the same time more obvious that "it's the same thing".
> No, it isn't! I started out with
>
> lfb.lfbp.font->height * lfb.lfbp.bytes_per_line
>
> being calculated twice. Then mistakenly thought that it was MORE of the same,
> missing out that the second calculation is different. Sorry for that. But the
> font->hight * bytes_per_line calculation is definitely the same in both
> places.
Thanks for the review!
lfb_scroll_puts and lfb_show_line are just vesa_scroll_puts and
vesa_show_line renamed and with some basic variable name changes.
I would be reticent to change them in this patch unless necessary.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |