Hi On Wed, Mar 9, 2022 at 12:05 PM Akihiko Odaki <akihiko.od...@gmail.com> wrote:
> On 2022/03/09 17:02, Marc-André Lureau wrote: > > Hi > > > > On Tue, Mar 8, 2022 at 6:43 PM Akihiko Odaki <akihiko.od...@gmail.com > > <mailto:akihiko.od...@gmail.com>> wrote: > > > > On 2022/03/08 23:26, Marc-André Lureau wrote: > > > Hi > > > > > > On Mon, Mar 7, 2022 at 4:49 PM Akihiko Odaki > > <akihiko.od...@gmail.com <mailto:akihiko.od...@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. > > > > > > VNC already marks the whole surface as dirty during vnc_dpy_switch(). > > This is like calling dpy_gfx_update(). > > Then why does it call dpy_gfx_update for SCANOUT_SURFACE first place? It > shouldn't call the function if it is not required to display the surface > content. > This is a pre-existing discrepancy. vnc handles switch with an implicit update. Imho, we need to improve the code so that updates/display become explicit. But this is extra, not necessary to fix the regressions you pointed out. I'd rather not mix concerns, and proceed step by step. -- Marc-André Lureau
