Re: [PATCH v3 12/12] ui/console: call gfx_switch() even if the current scanout is GL

[Akihiko Odaki <akihiko.odaki@gmail.com>]

On 2022/03/08 23:26, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Mar 7, 2022 at 4:49 PM Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>>
> 
> (taking notes, mostly for myself)
> 
>>> Could you provide a failing test case or a more concrete suggestion on
>>> what to do instead? I am all ears :)
>>>
>>> thanks
>>>
>>
>> Let me describe a more concrete case. Think that egl-headless and vnc
>> are enabled. The guest devices are serial virtio-gpu-gl. vnc selects
>> serial at first.
> 
> (I am not sure how you would select serial first, but let's assume you did)
> 
>>
>> vnc uses register_displaychangelistener and console_select to switch
>> consoles.
>>
>> displaychangelistener_display_console does the actual switching, and
>> calls dpy_gfx_switch and dpy_gfx_update if con->scanout.kind is
>> SCANOUT_SURFACE. It calls dpy_gl_scanout_texture or
>> dpy_gl_scanout_dmabuf if con->scanout.kind is SCANOUT_TEXTURE or
>> SCANOUT_DMABUF. It works for a OpenGL display, but it doesn't vnc in
>> combination with egl-headless.
> 
> (hmm, what doesn't work? With this patch, the DisplaySurface is always
> switched, no matter what con->scanout.kind is)
> 
>>
>> virtio-gpu-gl starts scanning out texture. It would happen in the
>> following sequence:
>> 1. virtio-gpu-gl calls qemu_console_resize. con->scanout.kind will be
>> SCANOUT_SURFACE.
>> 2. qemu_console_resize calls dpy_gfx_switch, delivering DisplaySurface
>> to egl-headless. con->scanout.kind will be SCANOUT_TEXTURE.
> 
> (qemu_console_resize calls dpy_gfx_replace_surface. con.scanout.kind
> is still SCANOUT_SURFACE)
> 
> (It calls displaychangelistener_gfx_switch() which will call
> egl_gfx_switch() and update the current surface)
> 
>> 3. virtio-gpu-gl calls dpy_gl_scanout_texture. egl-headless starts
>> rendering the texture to the DisplaySurface.
> 
> (now con.scanout.kind becomes SCANOUT_TEXTURE)
> 
>>
>> In the end, the DisplaySurface will have the image rendered, and
>> con->scanout.kind will be SCANOUT_TEXTURE.
> 
> (so far it works as expected, right?)
> 
> 
> 
>>
>> Now vnc switches to virtio-gpu-gl.
>>
>> 4. vnc calls console_select
>> 5. displaychangelistener_display_console finds con->scanout.kind is
>> SCANOUT_TEXTURE so it tries to scanout texture, but vnc is not an OpenGL
>> display. It essentially becomes no-op and the display would be blank.
>>
>> This patch will change it to call dpy_gfx_switch but not to call
>> dpy_gfx_update.
> 
> Alright, I think I see what you mean. egl-headless is associated with
> a specific con, and thus is not involved during vnc console switching.
> 
> However, dpy_gfx_switch on vnc is not a no-op. It updates the surface,
> resize the client and mark the area dirty. Because the con.surface is
> kept updated by egl-headless, the client gets the updated content.
> Iow, it still works.
> 
> If we always called dpy_gfx_update() during
> displaychangelistener_display_console(), it would mean for the
> listener to display the surface content, even when the scanout kind is
> set for a texture. Or we need to change the behaviour of
> dpy_gfx_update() to depend on the current scanout kind.

vnc has to display the surface content so dpy_gfx_update should be 
called for vnc, but not for OpenGL displays.

Regards,
Akihiko Odaki

> 
> Imho, it's acceptable in the current proposed form, since there is no
> apparent bug (unless I miss something). However, I can try to
> formulate a FIXME comment and add some documentation around the gfx
> callback to improve the situation.
> 
> thanks
>