From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mats Petersson Subject: Re: [PATCH v4 3/7] xen: introduce a generic framebuffer driver Date: Wed, 9 Jan 2013 14:51:33 +0000 Message-ID: <50ED83F5.8070302@citrix.com> References: <1357675408-27949-3-git-send-email-stefano.stabellini@eu.citrix.com> <50ED5A4F02000078000B3FDA@nat28.tlf.novell.com> <50ED8281.9070609@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <50ED8281.9070609@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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 wrote: >>>> --- /dev/null >>>> +++ b/xen/drivers/video/lfb.c >>>> @@ -0,0 +1,206 @@ >>>> +/****************************************************************************** >>>> + * lfb.c >>>> + * >>>> + * linear frame buffer handling. >>>> + */ >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#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<>>> + 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. -- Mats > > -- > Mats > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > >