Re: [Qemu-devel] [PATCH 4/9] fbdev: add linux framebuffer display driver.

[Gerd Hoffmann <kraxel@redhat.com>]

On 09/18/12 17:01, Stefano Stabellini wrote:
> On Tue, 18 Sep 2012, Gerd Hoffmann wrote:
>> Display works, requires truecolor framebuffer with 16 or 32 bpp on the
>> host.  32bpp is recommended.  The framebuffer is used as-is, qemu
>> doesn't try to switch modes.  With LCD displays mode switching is pretty
>> pointless IMHO, also it wouldn't work anyway with the most common
>> fbdev drivers (vesafb, KMS).  Guest display is centered on the host
>> screen.
>>
>> Mouse works, uses /dev/input/mice.
>>
>> Keyboard works.  Guest screen has whatever keymap you load inside
>> the guest.  Text windows (monitor, serial, ...) have a simple en-us
>> keymap.  Good enough to type monitor commands.  Not goot enough to
>> work seriously on a serial terminal.  But the qemu terminal emulation
>> isn't good enough for that anyway ;)
>>
>> Hot keys:
>>   Ctrl-Alt-F<nr>  -> host console switching.
>>   Ctrl-Alt-<nr>   -> qemu console switching.
>>   Ctrl-Alt-ESC    -> exit qemu.
>>
>> Special feature:  Sane console switching.  Switching away stops screen
>> updates.  Switching back redraws the screen.  When started from the
>> linux console qemu uses the vt you've started it from (requires just
>> read/write access to /dev/fb0).  When starting from somewhere else qemu
>> tries to open a unused virtual terminal and switch to it (usually
>> requires root privileges to open /dev/tty<nr>).
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> This series is not bisectable: this patch references functions and
> variables only defined in later patches (surface,
> pixman_from_displaystate). Please make it bisectable.

Oops, that wasn't intentionally, squashed bugfix into the wrong patch,
will fixup.

>> +static void fbdev_setdata(DisplayState *ds)
>> +{
>> +    if (surface) {
>             ^ where is this coming from?
> 

See above ;)

>> +int fbdev_display_init(DisplayState *ds, const char *device)
>> +{
>> +    if (dcl != NULL) {
>> +        return 0;
>> +    }
> 
> is it actually possible that fbdev_display_init gets called multiple
> times?

With the next patch (enable/disable fbdev via monitor) yes.

>> +    fbdev_resize(ds);
>> +    dcl = g_new0(DisplayChangeListener, 1);
>> +    dcl->dpy_update  = fbdev_update;
>> +    dcl->dpy_resize  = fbdev_resize;
>> +    dcl->dpy_setdata = fbdev_setdata;
>> +    dcl->dpy_refresh = fbdev_refresh;
>> +    register_displaychangelistener(ds, dcl);
> 
> The fbdev driver could benefit from registering a DisplayAllocator (see
> sdl.c).

I don't think so, at least not once the whole series is applied and
support for scaling & cursor rendering is there.

cheers,
  Gerd