| Message ID | 20100802161157.GA11743@morn.localdomain |
|---|---|
| State | New |
| Headers | show |
On 08/02/2010 11:11 AM, Kevin O'Connor wrote: > Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms > directory. > Sorry, I should have provided this in the first response. Does the bootsplash cause a delay in startup time? If so, we'll want to be able to disable this at boot time via the -boot option. If it doesn't, this patch is probably fine as is. Regards, Anthony Liguori > Signed-off-by: Kevin O'Connor<kevin@koconnor.net> > --- > Changes v2->v3: > Fix coding style (missing braces). > Changes v1->v2: > Add signed-off-by line. > --- > hw/fw_cfg.c | 9 +++++++-- > hw/pc.c | 8 ++++++++ > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c > index 72866ae..5a2c0aa 100644 > --- a/hw/fw_cfg.c > +++ b/hw/fw_cfg.c > @@ -304,8 +304,13 @@ int fw_cfg_add_file(FWCfgState *s, const char *dir, const char *filename, > basename = filename; > } > > - snprintf(s->files->f[index].name, sizeof(s->files->f[index].name), > - "%s/%s", dir, basename); > + if (dir&& dir[0]) { > + snprintf(s->files->f[index].name, sizeof(s->files->f[index].name), > + "%s/%s", dir, basename); > + } else { > + strncpy(s->files->f[index].name, basename, > + sizeof(s->files->f[index].name)); > + } > for (i = 0; i< index; i++) { > if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) { > FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__, > diff --git a/hw/pc.c b/hw/pc.c > index 58dea57..6893799 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -54,6 +54,7 @@ > #endif > > #define BIOS_FILENAME "bios.bin" > +#define BOOTSPLASH_FILENAME "bootsplash.jpg" > > #define PC_MAX_BIOS_SIZE (4 * 1024 * 1024) > > @@ -963,6 +964,13 @@ void pc_memory_init(ram_addr_t ram_size, > fw_cfg = bochs_bios_init(); > rom_set_fw(fw_cfg); > > + /* Optional bootsplash file */ > + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BOOTSPLASH_FILENAME); > + if (filename) { > + qemu_free(filename); > + rom_add_file(BOOTSPLASH_FILENAME, "", 0); > + } > + > if (linux_boot) { > load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size); > } >
On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote: > On 08/02/2010 11:11 AM, Kevin O'Connor wrote: > >Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms > >directory. > > Sorry, I should have provided this in the first response. > > Does the bootsplash cause a delay in startup time? If so, we'll > want to be able to disable this at boot time via the -boot option. > If it doesn't, this patch is probably fine as is. That's okay - the patch is intended to start discussion anyway. There is no added delay for the bootsplash. However, it does take time to decompress it. On my machine it can take between 200-300ms for the jpeg code. It may make more sense for SeaBIOS to only bother with the bootsplash if the boot menu is enabled (or some new option) - as otherwise the picture is basically cleared immiediately after it's shown. -Kevin
On 08/02/2010 11:47 AM, Kevin O'Connor wrote: > On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote: > >> On 08/02/2010 11:11 AM, Kevin O'Connor wrote: >> >>> Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms >>> directory. >>> >> Sorry, I should have provided this in the first response. >> >> Does the bootsplash cause a delay in startup time? If so, we'll >> want to be able to disable this at boot time via the -boot option. >> If it doesn't, this patch is probably fine as is. >> > That's okay - the patch is intended to start discussion anyway. > > There is no added delay for the bootsplash. However, it does take > time to decompress it. On my machine it can take between 200-300ms > for the jpeg code. > > It may make more sense for SeaBIOS to only bother with the bootsplash > if the boot menu is enabled (or some new option) - as otherwise the > picture is basically cleared immiediately after it's shown. > Yeah, if there's no boot menu and no boot menu delay, I imagine the boot splash ends up just being a blip on the screen, right? We definitely want a "boot as fast as you possibly can" mode for the BIOS although for non-developer environments, I think a boot splash is a nice touch. BTW, we need to document somewhere any assumptions SeaBIOS has about the JPEG. I see that it expects a 1024x768 image. Any additional restrictions on the jpeg image? Regards, Anthony Liguori > -Kevin >
On Mon, Aug 02, 2010 at 11:52:22AM -0500, Anthony Liguori wrote: > BTW, we need to document somewhere any assumptions SeaBIOS has about > the JPEG. I see that it expects a 1024x768 image. Any additional > restrictions on the jpeg image? I listed some notes in a previous email: >> Some notes: >> >> This uses the qemu "rom" interface for loading the jpeg file. It >> seems to work, but I'm not sure if this is strictly correct. >> >> The jpeg viewer in SeaBIOS will look at the image size and try to find >> a vesa graphics mode that supports that size. So, choose images that >> are exactly 640x480, 1024x768, etc. Also, the SeaBIOS viewer has >> stripped down support for jpegs - not all valid jpegs will work. Some >> quick tests with the netpbm tools worked okay for me. >> >> SeaBIOS only shows the bootsplash during the interval between vgabios >> init and OS execution. This is generally too short a time to be seen. >> Add "-menu boot=on" to the qemu command line to have it shown longer. >> >> Unfortunately, the vgabios doesn't support writing text to the screen >> while in a vesa video mode. So, this means that if a user selects F12 >> for the boot menu, they can't actually see the boot menu. This will >> need to be fixed in SeaBIOS in a follow up patch. -Kevin
On 08/02/2010 01:10 PM, Kevin O'Connor wrote: > On Mon, Aug 02, 2010 at 11:52:22AM -0500, Anthony Liguori wrote: > >> BTW, we need to document somewhere any assumptions SeaBIOS has about >> the JPEG. I see that it expects a 1024x768 image. Any additional >> restrictions on the jpeg image? >> > I listed some notes in a previous email: > Right, we need this in either docs/seabios.txt in qemu.git or in a file in seabios.git I think. Regards, Anthony Liguori >>> Some notes: >>> >>> This uses the qemu "rom" interface for loading the jpeg file. It >>> seems to work, but I'm not sure if this is strictly correct. >>> >>> The jpeg viewer in SeaBIOS will look at the image size and try to find >>> a vesa graphics mode that supports that size. So, choose images that >>> are exactly 640x480, 1024x768, etc. Also, the SeaBIOS viewer has >>> stripped down support for jpegs - not all valid jpegs will work. Some >>> quick tests with the netpbm tools worked okay for me. >>> >>> SeaBIOS only shows the bootsplash during the interval between vgabios >>> init and OS execution. This is generally too short a time to be seen. >>> Add "-menu boot=on" to the qemu command line to have it shown longer. >>> >>> Unfortunately, the vgabios doesn't support writing text to the screen >>> while in a vesa video mode. So, this means that if a user selects F12 >>> for the boot menu, they can't actually see the boot menu. This will >>> need to be fixed in SeaBIOS in a follow up patch. >>> > -Kevin >
On Mon, Aug 02, 2010 at 12:47:53PM -0400, Kevin O'Connor wrote: > On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote: > > On 08/02/2010 11:11 AM, Kevin O'Connor wrote: > > >Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms > > >directory. > > > > Sorry, I should have provided this in the first response. > > > > Does the bootsplash cause a delay in startup time? If so, we'll > > want to be able to disable this at boot time via the -boot option. > > If it doesn't, this patch is probably fine as is. > > That's okay - the patch is intended to start discussion anyway. > > There is no added delay for the bootsplash. However, it does take > time to decompress it. On my machine it can take between 200-300ms > for the jpeg code. Does it detect a "headless" boot and disable itself completely? 300ms is 5% of libguestfs boot time ... Rich.
On 08/03/2010 04:52 AM, Richard W.M. Jones wrote: > On Mon, Aug 02, 2010 at 12:47:53PM -0400, Kevin O'Connor wrote: > >> On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote: >> >>> On 08/02/2010 11:11 AM, Kevin O'Connor wrote: >>> >>>> Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms >>>> directory. >>>> >>> Sorry, I should have provided this in the first response. >>> >>> Does the bootsplash cause a delay in startup time? If so, we'll >>> want to be able to disable this at boot time via the -boot option. >>> If it doesn't, this patch is probably fine as is. >>> >> That's okay - the patch is intended to start discussion anyway. >> >> There is no added delay for the bootsplash. However, it does take >> time to decompress it. On my machine it can take between 200-300ms >> for the jpeg code. >> > Does it detect a "headless" boot and disable itself completely? > > 300ms is 5% of libguestfs boot time ... > If menu=off, it probably just shouldn't display. I assume libguestfs is passing menu=off... Regards, Anthony Liguori > Rich. > >
On Tue, Aug 03, 2010 at 07:51:02AM -0500, Anthony Liguori wrote: > If menu=off, it probably just shouldn't display. I assume > libguestfs is passing menu=off... We don't actually, but it is a tremendously good idea so I'll add it, thanks :-) Rich.
On Tue, Aug 03, 2010 at 01:54:12PM +0100, Richard W.M. Jones wrote: > On Tue, Aug 03, 2010 at 07:51:02AM -0500, Anthony Liguori wrote: > > If menu=off, it probably just shouldn't display. I assume > > libguestfs is passing menu=off... > > We don't actually, but it is a tremendously good idea so I'll add it, > thanks :-) Or maybe I won't. The default is boot_menu = 0 (off), confirmed by examining the code. Rich.
On 08/03/2010 08:04 AM, Richard W.M. Jones wrote: > On Tue, Aug 03, 2010 at 01:54:12PM +0100, Richard W.M. Jones wrote: > >> On Tue, Aug 03, 2010 at 07:51:02AM -0500, Anthony Liguori wrote: >> >>> If menu=off, it probably just shouldn't display. I assume >>> libguestfs is passing menu=off... >>> >> We don't actually, but it is a tremendously good idea so I'll add it, >> thanks :-) >> > Or maybe I won't. The default is boot_menu = 0 (off), confirmed > by examining the code. > You shouldn't rely on defaults though if you want to be future proof. The defaults are "whatever we think is the best for the user" and as such, may change from version to version. Regards, Anthony Liguori > Rich. > >
On 08/02/2010 06:47 PM, Kevin O'Connor wrote: > On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote: >> On 08/02/2010 11:11 AM, Kevin O'Connor wrote: >>> Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms >>> directory. >> >> Sorry, I should have provided this in the first response. >> >> Does the bootsplash cause a delay in startup time? If so, we'll >> want to be able to disable this at boot time via the -boot option. >> If it doesn't, this patch is probably fine as is. > > That's okay - the patch is intended to start discussion anyway. > > There is no added delay for the bootsplash. However, it does take > time to decompress it. On my machine it can take between 200-300ms > for the jpeg code. KVM or TCG? Also, what's the time for a completely black bootsplash? Paolo
On Tue, Aug 03, 2010 at 04:38:48PM +0200, Paolo Bonzini wrote: > On 08/02/2010 06:47 PM, Kevin O'Connor wrote: > >There is no added delay for the bootsplash. However, it does take > >time to decompress it. On my machine it can take between 200-300ms > >for the jpeg code. > > KVM or TCG? TCG. > Also, what's the time for a completely black bootsplash? Here are the timings (times are in seconds measured on the host) for a completely black 640x480 jpeg on my machine: 00.135: Checking for bootsplash 00.139: VESA 2.0 00.140: VENDOR: VGABIOS Cirrus extension 00.143: PRODUCT: VGABIOS Cirrus extension 00.145: Copying bootsplash.jpg 00.146: Decoding bootsplash.jpg 00.154: Finding vesa mode with dimensions 640/480 00.160: mode: 0111 00.162: framebuffer: 0xf0000000 00.164: bytes per scanline: 1280 00.165: bits per pixel: 16 00.166: Decompressing bootsplash.jpg 00.283: Switching to graphics mode 00.322: Showing bootsplash.jpg 00.327: Bootsplash copy complete One can compare this output with the dprintf() calls in the code at: http://git.linuxtogo.org/?p=kevin/seabios.git;a=blob;f=src/bootsplash.c;h=14bdd4cfaa6b931032e30236039b0fd4934f75ac;hb=eb6dc785475e4676de728f99a0fcd638d81c5b68#l144 The times weren't notably different with a real picture instead of just a black picture. As before, SeaBIOS should probably be changed so that it only shows the image if the boot menu has been enabled. -Kevin
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c index 72866ae..5a2c0aa 100644 --- a/hw/fw_cfg.c +++ b/hw/fw_cfg.c @@ -304,8 +304,13 @@ int fw_cfg_add_file(FWCfgState *s, const char *dir, const char *filename, basename = filename; } - snprintf(s->files->f[index].name, sizeof(s->files->f[index].name), - "%s/%s", dir, basename); + if (dir && dir[0]) { + snprintf(s->files->f[index].name, sizeof(s->files->f[index].name), + "%s/%s", dir, basename); + } else { + strncpy(s->files->f[index].name, basename, + sizeof(s->files->f[index].name)); + } for (i = 0; i < index; i++) { if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) { FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__, diff --git a/hw/pc.c b/hw/pc.c index 58dea57..6893799 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -54,6 +54,7 @@ #endif #define BIOS_FILENAME "bios.bin" +#define BOOTSPLASH_FILENAME "bootsplash.jpg" #define PC_MAX_BIOS_SIZE (4 * 1024 * 1024) @@ -963,6 +964,13 @@ void pc_memory_init(ram_addr_t ram_size, fw_cfg = bochs_bios_init(); rom_set_fw(fw_cfg); + /* Optional bootsplash file */ + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BOOTSPLASH_FILENAME); + if (filename) { + qemu_free(filename); + rom_add_file(BOOTSPLASH_FILENAME, "", 0); + } + if (linux_boot) { load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size); }
Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms directory. Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- Changes v2->v3: Fix coding style (missing braces). Changes v1->v2: Add signed-off-by line. --- hw/fw_cfg.c | 9 +++++++-- hw/pc.c | 8 ++++++++ 2 files changed, 15 insertions(+), 2 deletions(-)