| Message ID | 20210706235255.7575-2-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts | expand |
Hi DW, > eb_fb_blit needs more parameters which describe x and y offsets and width > and height of the actual scanout to specify the size and cordination of > partial image to blit in the guest fb in case the guest fb contains multiple > display outputs. > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > --- > hw/display/virtio-gpu-udmabuf.c | 4 ++-- > include/ui/egl-helpers.h | 2 +- > ui/egl-headless.c | 2 +- > ui/egl-helpers.c | 10 ++++++---- > ui/gtk-egl.c | 7 ++++--- > ui/sdl2-gl.c | 2 +- > 6 files changed, 15 insertions(+), 12 deletions(-) > > diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c [Kasireddy, Vivek] You might not want to mix virtio-gpu and UI changes in the same patch. > index a64194c6de..3ea6e76371 100644 > --- a/hw/display/virtio-gpu-udmabuf.c > +++ b/hw/display/virtio-gpu-udmabuf.c > @@ -186,8 +186,8 @@ static VGPUDMABuf > dmabuf->buf.stride = fb->stride; > dmabuf->buf.x = r->x; > dmabuf->buf.y = r->y; > - dmabuf->buf.scanout_width; > - dmabuf->buf.scanout_height; > + dmabuf->buf.scanout_width = r->width; > + dmabuf->buf.scanout_height = r->height; > dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format); > dmabuf->buf.fd = res->dmabuf_fd; > > diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h > index f1bf8f97fc..e21118501e 100644 > --- a/include/ui/egl-helpers.h > +++ b/include/ui/egl-helpers.h > @@ -26,7 +26,7 @@ void egl_fb_setup_default(egl_fb *fb, int width, int height); > void egl_fb_setup_for_tex(egl_fb *fb, int width, int height, > GLuint texture, bool delete); > void egl_fb_setup_new_tex(egl_fb *fb, int width, int height); > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip); > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip); > void egl_fb_read(DisplaySurface *dst, egl_fb *src); > > void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip); > diff --git a/ui/egl-headless.c b/ui/egl-headless.c > index da377a74af..bdf10fec84 100644 > --- a/ui/egl-headless.c > +++ b/ui/egl-headless.c > @@ -144,7 +144,7 @@ static void egl_scanout_flush(DisplayChangeListener *dcl, > 1.0, 1.0); > } else { > /* no cursor -> use simple framebuffer blit */ > - egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top); > + egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, x, y, w, h, edpy->y_0_top); > } > > egl_fb_read(edpy->ds, &edpy->blit_fb); > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c > index 6d0cb2b5cb..2af3dcc0a6 100644 > --- a/ui/egl-helpers.c > +++ b/ui/egl-helpers.c > @@ -88,16 +88,18 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height) > egl_fb_setup_for_tex(fb, width, height, texture, true); > } > > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip) > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip) [Kasireddy, Vivek] Instead of explicitly passing x, y, w, h to egl_fb_blit, would you be not be able to use the dmabuf member that would be added to egl_fb that would contain x, y, w and h: https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg06746.html > { > GLuint y1, y2; > > glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer); > glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer); > glViewport(0, 0, dst->width, dst->height); > - y1 = flip ? src->height : 0; > - y2 = flip ? 0 : src->height; > - glBlitFramebuffer(0, y1, src->width, y2, > + w = (x + w) > src->width ? src->width - x : w; > + h = (y + h) > src->height ? src->height - y : h; > + y1 = flip ? h + y : y; > + y2 = flip ? y : h + y; > + glBlitFramebuffer(x, y1, x + w, y2, [Kasireddy, Vivek] While you are at it, could you please create new local variables x1, y1, x2, y2 to store the above values and pass them to glBlitFramebuffer to improve the readability of this code? Thanks, Vivek > 0, 0, dst->width, dst->height, > GL_COLOR_BUFFER_BIT, GL_LINEAR); > } > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c > index 2a2e6d3a17..ceb52b1045 100644 > --- a/ui/gtk-egl.c > +++ b/ui/gtk-egl.c > @@ -73,7 +73,7 @@ void gd_egl_draw(VirtualConsole *vc) > wh = gdk_window_get_height(window); > > if (vc->gfx.scanout_mode) { > - gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h); > + gd_egl_scanout_flush(&vc->gfx.dcl, vc->gfx.x, vc->gfx.y, vc->gfx.w, vc->gfx.h); > > vc->gfx.scale_x = (double)ww / vc->gfx.w; > vc->gfx.scale_y = (double)wh / vc->gfx.h; > @@ -216,7 +216,8 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl, > > gd_egl_scanout_texture(dcl, dmabuf->texture, > false, dmabuf->width, dmabuf->height, > - 0, 0, dmabuf->width, dmabuf->height); > + dmabuf->x, dmabuf->y, dmabuf->scanout_width, > + dmabuf->scanout_height); > #endif > } > > @@ -286,7 +287,7 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl, > vc->gfx.cursor_x, vc->gfx.cursor_y, > vc->gfx.scale_x, vc->gfx.scale_y); > } else { > - egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top); > + egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, x, y, w, h, !vc->gfx.y0_top); > } > > eglSwapBuffers(qemu_egl_display, vc->gfx.esurface); > diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c > index a21d2deed9..67bc8b5f4e 100644 > --- a/ui/sdl2-gl.c > +++ b/ui/sdl2-gl.c > @@ -238,7 +238,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl, > > SDL_GetWindowSize(scon->real_window, &ww, &wh); > egl_fb_setup_default(&scon->win_fb, ww, wh); > - egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top); > + egl_fb_blit(&scon->win_fb, &scon->guest_fb, x, y, w, h, !scon->y0_top); > > SDL_GL_SwapWindow(scon->real_window); > graphic_hw_gl_flushed(dcl->con); > -- > 2.17.1 >
On Sun, Jul 18, 2021 at 11:35:35PM -0700, Kasireddy, Vivek wrote: > Hi DW, > > > eb_fb_blit needs more parameters which describe x and y offsets and width > > and height of the actual scanout to specify the size and cordination of > > partial image to blit in the guest fb in case the guest fb contains multiple > > display outputs. > > > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > --- > > hw/display/virtio-gpu-udmabuf.c | 4 ++-- > > include/ui/egl-helpers.h | 2 +- > > ui/egl-headless.c | 2 +- > > ui/egl-helpers.c | 10 ++++++---- > > ui/gtk-egl.c | 7 ++++--- > > ui/sdl2-gl.c | 2 +- > > 6 files changed, 15 insertions(+), 12 deletions(-) > > > > diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c > [Kasireddy, Vivek] You might not want to mix virtio-gpu and UI changes in the same patch. [DW] Yeah, I will split it. > > > index a64194c6de..3ea6e76371 100644 > > --- a/hw/display/virtio-gpu-udmabuf.c > > +++ b/hw/display/virtio-gpu-udmabuf.c > > @@ -186,8 +186,8 @@ static VGPUDMABuf > > dmabuf->buf.stride = fb->stride; > > dmabuf->buf.x = r->x; > > dmabuf->buf.y = r->y; > > - dmabuf->buf.scanout_width; > > - dmabuf->buf.scanout_height; > > + dmabuf->buf.scanout_width = r->width; > > + dmabuf->buf.scanout_height = r->height; > > dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format); > > dmabuf->buf.fd = res->dmabuf_fd; > > > > diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h > > index f1bf8f97fc..e21118501e 100644 > > --- a/include/ui/egl-helpers.h > > +++ b/include/ui/egl-helpers.h > > @@ -26,7 +26,7 @@ void egl_fb_setup_default(egl_fb *fb, int width, int height); > > void egl_fb_setup_for_tex(egl_fb *fb, int width, int height, > > GLuint texture, bool delete); > > void egl_fb_setup_new_tex(egl_fb *fb, int width, int height); > > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip); > > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip); > > void egl_fb_read(DisplaySurface *dst, egl_fb *src); > > > > void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip); > > diff --git a/ui/egl-headless.c b/ui/egl-headless.c > > index da377a74af..bdf10fec84 100644 > > --- a/ui/egl-headless.c > > +++ b/ui/egl-headless.c > > @@ -144,7 +144,7 @@ static void egl_scanout_flush(DisplayChangeListener *dcl, > > 1.0, 1.0); > > } else { > > /* no cursor -> use simple framebuffer blit */ > > - egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top); > > + egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, x, y, w, h, edpy->y_0_top); > > } > > > > egl_fb_read(edpy->ds, &edpy->blit_fb); > > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c > > index 6d0cb2b5cb..2af3dcc0a6 100644 > > --- a/ui/egl-helpers.c > > +++ b/ui/egl-helpers.c > > @@ -88,16 +88,18 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height) > > egl_fb_setup_for_tex(fb, width, height, texture, true); > > } > > > > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip) > > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip) > [Kasireddy, Vivek] Instead of explicitly passing x, y, w, h to egl_fb_blit, would you be not > be able to use the dmabuf member that would be added to egl_fb that would contain x, y, w and h: > https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg06746.html [DW] sounds like a valid idea but wouldn't it be making this function too specific? I think it is reasonable to specify the offset and the size of blitted area.. although I agree that having too many parameters don't look good. > > > > { > > GLuint y1, y2; > > > > glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer); > > glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer); > > glViewport(0, 0, dst->width, dst->height); > > - y1 = flip ? src->height : 0; > > - y2 = flip ? 0 : src->height; > > - glBlitFramebuffer(0, y1, src->width, y2, > > + w = (x + w) > src->width ? src->width - x : w; > > + h = (y + h) > src->height ? src->height - y : h; > > + y1 = flip ? h + y : y; > > + y2 = flip ? y : h + y; > > + glBlitFramebuffer(x, y1, x + w, y2, > [Kasireddy, Vivek] While you are at it, could you please create new local variables x1, y1, x2, y2 > to store the above values and pass them to glBlitFramebuffer to improve the readability of this code? [DW] I will think about making this look more undertandable. > > Thanks, > Vivek > > 0, 0, dst->width, dst->height, > > GL_COLOR_BUFFER_BIT, GL_LINEAR); > > } > > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c > > index 2a2e6d3a17..ceb52b1045 100644 > > --- a/ui/gtk-egl.c > > +++ b/ui/gtk-egl.c > > @@ -73,7 +73,7 @@ void gd_egl_draw(VirtualConsole *vc) > > wh = gdk_window_get_height(window); > > > > if (vc->gfx.scanout_mode) { > > - gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h); > > + gd_egl_scanout_flush(&vc->gfx.dcl, vc->gfx.x, vc->gfx.y, vc->gfx.w, vc->gfx.h); > > > > vc->gfx.scale_x = (double)ww / vc->gfx.w; > > vc->gfx.scale_y = (double)wh / vc->gfx.h; > > @@ -216,7 +216,8 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl, > > > > gd_egl_scanout_texture(dcl, dmabuf->texture, > > false, dmabuf->width, dmabuf->height, > > - 0, 0, dmabuf->width, dmabuf->height); > > + dmabuf->x, dmabuf->y, dmabuf->scanout_width, > > + dmabuf->scanout_height); > > #endif > > } > > > > @@ -286,7 +287,7 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl, > > vc->gfx.cursor_x, vc->gfx.cursor_y, > > vc->gfx.scale_x, vc->gfx.scale_y); > > } else { > > - egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top); > > + egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, x, y, w, h, !vc->gfx.y0_top); > > } > > > > eglSwapBuffers(qemu_egl_display, vc->gfx.esurface); > > diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c > > index a21d2deed9..67bc8b5f4e 100644 > > --- a/ui/sdl2-gl.c > > +++ b/ui/sdl2-gl.c > > @@ -238,7 +238,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl, > > > > SDL_GetWindowSize(scon->real_window, &ww, &wh); > > egl_fb_setup_default(&scon->win_fb, ww, wh); > > - egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top); > > + egl_fb_blit(&scon->win_fb, &scon->guest_fb, x, y, w, h, !scon->y0_top); > > > > SDL_GL_SwapWindow(scon->real_window); > > graphic_hw_gl_flushed(dcl->con); > > -- > > 2.17.1 > > >
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c index a64194c6de..3ea6e76371 100644 --- a/hw/display/virtio-gpu-udmabuf.c +++ b/hw/display/virtio-gpu-udmabuf.c @@ -186,8 +186,8 @@ static VGPUDMABuf dmabuf->buf.stride = fb->stride; dmabuf->buf.x = r->x; dmabuf->buf.y = r->y; - dmabuf->buf.scanout_width; - dmabuf->buf.scanout_height; + dmabuf->buf.scanout_width = r->width; + dmabuf->buf.scanout_height = r->height; dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format); dmabuf->buf.fd = res->dmabuf_fd; diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h index f1bf8f97fc..e21118501e 100644 --- a/include/ui/egl-helpers.h +++ b/include/ui/egl-helpers.h @@ -26,7 +26,7 @@ void egl_fb_setup_default(egl_fb *fb, int width, int height); void egl_fb_setup_for_tex(egl_fb *fb, int width, int height, GLuint texture, bool delete); void egl_fb_setup_new_tex(egl_fb *fb, int width, int height); -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip); +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip); void egl_fb_read(DisplaySurface *dst, egl_fb *src); void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip); diff --git a/ui/egl-headless.c b/ui/egl-headless.c index da377a74af..bdf10fec84 100644 --- a/ui/egl-headless.c +++ b/ui/egl-headless.c @@ -144,7 +144,7 @@ static void egl_scanout_flush(DisplayChangeListener *dcl, 1.0, 1.0); } else { /* no cursor -> use simple framebuffer blit */ - egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top); + egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, x, y, w, h, edpy->y_0_top); } egl_fb_read(edpy->ds, &edpy->blit_fb); diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c index 6d0cb2b5cb..2af3dcc0a6 100644 --- a/ui/egl-helpers.c +++ b/ui/egl-helpers.c @@ -88,16 +88,18 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int height) egl_fb_setup_for_tex(fb, width, height, texture, true); } -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip) +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, bool flip) { GLuint y1, y2; glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer); glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer); glViewport(0, 0, dst->width, dst->height); - y1 = flip ? src->height : 0; - y2 = flip ? 0 : src->height; - glBlitFramebuffer(0, y1, src->width, y2, + w = (x + w) > src->width ? src->width - x : w; + h = (y + h) > src->height ? src->height - y : h; + y1 = flip ? h + y : y; + y2 = flip ? y : h + y; + glBlitFramebuffer(x, y1, x + w, y2, 0, 0, dst->width, dst->height, GL_COLOR_BUFFER_BIT, GL_LINEAR); } diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c index 2a2e6d3a17..ceb52b1045 100644 --- a/ui/gtk-egl.c +++ b/ui/gtk-egl.c @@ -73,7 +73,7 @@ void gd_egl_draw(VirtualConsole *vc) wh = gdk_window_get_height(window); if (vc->gfx.scanout_mode) { - gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h); + gd_egl_scanout_flush(&vc->gfx.dcl, vc->gfx.x, vc->gfx.y, vc->gfx.w, vc->gfx.h); vc->gfx.scale_x = (double)ww / vc->gfx.w; vc->gfx.scale_y = (double)wh / vc->gfx.h; @@ -216,7 +216,8 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl, gd_egl_scanout_texture(dcl, dmabuf->texture, false, dmabuf->width, dmabuf->height, - 0, 0, dmabuf->width, dmabuf->height); + dmabuf->x, dmabuf->y, dmabuf->scanout_width, + dmabuf->scanout_height); #endif } @@ -286,7 +287,7 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl, vc->gfx.cursor_x, vc->gfx.cursor_y, vc->gfx.scale_x, vc->gfx.scale_y); } else { - egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top); + egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, x, y, w, h, !vc->gfx.y0_top); } eglSwapBuffers(qemu_egl_display, vc->gfx.esurface); diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c index a21d2deed9..67bc8b5f4e 100644 --- a/ui/sdl2-gl.c +++ b/ui/sdl2-gl.c @@ -238,7 +238,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl, SDL_GetWindowSize(scon->real_window, &ww, &wh); egl_fb_setup_default(&scon->win_fb, ww, wh); - egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top); + egl_fb_blit(&scon->win_fb, &scon->guest_fb, x, y, w, h, !scon->y0_top); SDL_GL_SwapWindow(scon->real_window); graphic_hw_gl_flushed(dcl->con);
eb_fb_blit needs more parameters which describe x and y offsets and width and height of the actual scanout to specify the size and cordination of partial image to blit in the guest fb in case the guest fb contains multiple display outputs. Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> --- hw/display/virtio-gpu-udmabuf.c | 4 ++-- include/ui/egl-helpers.h | 2 +- ui/egl-headless.c | 2 +- ui/egl-helpers.c | 10 ++++++---- ui/gtk-egl.c | 7 ++++--- ui/sdl2-gl.c | 2 +- 6 files changed, 15 insertions(+), 12 deletions(-)