From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1E4inU-0006AB-69 for mharc-grub-devel@gnu.org; Mon, 15 Aug 2005 13:30:52 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1E4inQ-000694-HV for grub-devel@gnu.org; Mon, 15 Aug 2005 13:30:49 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1E4inO-000684-Ac for grub-devel@gnu.org; Mon, 15 Aug 2005 13:30:46 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1E4inM-00063l-Bm for grub-devel@gnu.org; Mon, 15 Aug 2005 13:30:45 -0400 Received: from [145.74.66.11] (helo=mail-cn.han.nl) by monty-python.gnu.org with esmtp (Exim 4.34) id 1E4iwQ-0000QD-Gd for grub-devel@gnu.org; Mon, 15 Aug 2005 13:40:06 -0400 Received: from vscan-cn.han.nl (venus.han.nl [145.74.65.6]) by mail-cn.han.nl (Postfix) with ESMTP id D014986FF for ; Mon, 15 Aug 2005 19:24:16 +0200 (CEST) Received: from mail-cn.han.nl ([145.74.66.11]) by vscan-cn.han.nl (venus.han.nl [145.74.65.6]) (amavisd-new, port 10024) with ESMTP id 16376-04 for ; Mon, 15 Aug 2005 19:24:14 +0200 (CEST) Received: from mail1.han.nl (mail1.han.nl [145.74.103.11]) by mail-cn.han.nl (Postfix) with ESMTP id D26CB87B9 for ; Mon, 15 Aug 2005 19:24:13 +0200 (CEST) Received: from localhost.localdomain (mgerards.xs4all.nl [82.92.27.129]) by mail1.han.nl (Postfix) with ESMTP id 8F9FEC047 for ; Mon, 15 Aug 2005 19:24:13 +0200 (CEST) Mail-Copies-To: metgerards@student.han.nl To: The development of GRUB 2 References: <42FF3097.7030904@nic.fi> From: Marco Gerards Date: Mon, 15 Aug 2005 19:24:19 +0200 In-Reply-To: <42FF3097.7030904@nic.fi> ( =?iso-8859-1?q?Vesa_J=E4=E4skel=E4inen's_message_of?= "Sun, 14 Aug 2005 14:52:55 +0300") Message-ID: <87zmrjxge4.fsf@student.han.nl> User-Agent: Gnus/5.1007 (Gnus v5.10.7) Emacs/21.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Virus-Scanned: by amavisd-new (2.2.0) at vscan-cn.han.nl Subject: Re: vesafb terminal for testing. X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Aug 2005 17:30:49 -0000 Vesa J=E4=E4skel=E4inen writes: Hi Vesa, > I have attached patch here that adds simple terminal that uses VESA BIOS > Extension 2.0+ for rendering terminal. It is not meant to be included as > is in GRUB 2, but I would hope that people would test it, so I could try > to improve it for greater compatibility (even though I have tried to > follow the standards, there might be some glitches between implementation= s). Nice! In this email I will propose some suggestions and comments on the patch itself. > There are couple of things that I do not like in this patch. > > First one is that there should be common virtual terminal for all arch's > and then separate graphics drivers that will do the actual drawing. In > this patch this is all implemented in one module and it is not as pretty > as it could be. Right, the video driver should be a new kind of interface. It should have functions to get/put pixels, boxes, whatever to the screen. I am not sure about video interfaces, but they should be simple and generic. In that case I can easily write a framebuffer for the PPC, for example. You should keep in mind that there are many depths (1, 4, 8, 16, 24 and 32 bits) that should be supported. They all work differently. > Second one is that this patch uses VGA BIOS fonts to draw characters and > it might cause some problems in some cards. We should use the font manager and nothing else IMO. > Now how to get started on testing: > - insmod vesafb > - Use vbe_list_modes to find mode number (eg. 0x111) that you want to use. > - set vbe_mode varible ('set vbe_mode=3D0x111') I will test it soon. Some comments on the patch: > 2005-08-14 Vesa Jaaskelainen >=20=20 > + * DISTLIST: Added term/i386/pc/vesafb.c > + * term/i386/pc/vesafb: New file. Can you please split this up to the generic framebuffer terminal? The terminal for framebuffers should be generic and the videodrivers should be arch specific. Or do you want to do that later? You forgot the ".c" on the last line. > + > +2005-08-14 Vesa Jaaskelainen > + > * DISTLIST: Added include/grub/i386/pc/vbe.h. Why two changelog entries? I do not see this change anywhere in the patch. =20=20 > +static void > +*real2pm(grub_vbe_farptr_t ptr) The * is a part of the type, can you put it on the previous line? There are some long lines, can you please make them shorter so the maximal line length is 78 characters? You sometimes use C++ style comments. Can you change your comments so they match the style described in the GCS: http://www.gnu.org/prep/standards/html_node/Comments.html#Comments Thanks, Marco