On 7/4/2023 9:07 AM, Marc-André Lureau wrote:
Hi
On Mon, Jun 26, 2023 at 9:49 PM Kim, Dongwon <dongwon....@intel.com>
wrote:
Hi Marc-André Lureau,
On 6/26/2023 4:56 AM, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jun 21, 2023 at 11:53 PM Dongwon Kim
<dongwon....@intel.com>
> wrote:
>
> x and y offsets and width and height of the scanout texture
> is not correctly configured in case guest scanout frame is
> dmabuf.
>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasire...@intel.com>
> Signed-off-by: Dongwon Kim <dongwon....@intel.com>
>
>
> I find this a bit confusing, and I don't know how to actually
test it.
>
> The only place where scanout_{width, height} are set is
> virtio_gpu_create_dmabuf() and there, they have the same values as
> width and height. it's too easy to get confused with the values
imho.
Yes, scanout_width/height are same as width/height as far as there is
only one guest display exist. But they will be different in case
there
multiple displays on the guest side, configured in extended mode
(when
the guest is running Xorg).
In this case, blob for the guest display is same for scanout 1 and
2 but
each scanout will have different offset and
scanout_width/scanout_height
to reference a sub region in the same blob(dmabuf).
I added x/y/scanout_width/scanout_height with a previous commit:
commit e86a93f55463c088aa0b5260e915ffbf9f86c62b
Author: Dongwon Kim <dongwon....@intel.com>
Date: Wed Nov 3 23:51:52 2021 -0700
virtio-gpu: splitting one extended mode guest fb into n-scanouts
> I find the terminology we use for ScanoutTexture much clearer.
It uses
> backing_{width, height} instead, which indicates quite clearly that
> the usual x/y/w/h are for the sub-region to be shown.
yeah agreed. Then dmabuf->width/height should be changed to
dmabuf->backing_width/height and dmabuf->width/height will be
replacing
dmabuf->scanout_width/scanout_height. I guess this is what you
meant, right?
right, can you send a new patch?
thanks
https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg01081.html
Thanks!
> I think we should have a preliminary commit that renames
> scanout_{width, height}.
>
> Please give some help/hints on how to actually test this code too.
So this patch is just to make things look consistent in the code
level.
Having offset (0,0) in this function call for all different scanouts
didn't look right to me. This code change won't make anything done
differently though. So no test is applicable.
>
> Thanks!
>
>
> ---
> ui/gtk-egl.c | 3 ++-
> ui/gtk-gl-area.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> index 19130041bc..e99e3b0d8c 100644
> --- a/ui/gtk-egl.c
> +++ b/ui/gtk-egl.c
> @@ -257,7 +257,8 @@ void
> gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
>
> gd_egl_scanout_texture(dcl, dmabuf->texture,
> dmabuf->y0_top, dmabuf->width,
> dmabuf->height,
> - 0, 0, dmabuf->width,
dmabuf->height);
> + dmabuf->x, dmabuf->y,
> dmabuf->scanout_width,
> + dmabuf->scanout_height);
>
> if (dmabuf->allow_fences) {
> vc->gfx.guest_fb.dmabuf = dmabuf;
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index c384a1516b..1605818bd1 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -299,7 +299,8 @@ void
> gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl,
>
> gd_gl_area_scanout_texture(dcl, dmabuf->texture,
> dmabuf->y0_top, dmabuf->width,
> dmabuf->height,
> - 0, 0, dmabuf->width,
dmabuf->height);
> + dmabuf->x, dmabuf->y,
> dmabuf->scanout_width,
> + dmabuf->scanout_height);
>
> if (dmabuf->allow_fences) {
> vc->gfx.guest_fb.dmabuf = dmabuf;
> --
> 2.34.1
>
>
>
>
> --
> Marc-André Lureau
--
Marc-André Lureau
