From: Marc-André Lureau <marcandre.lureau@redhat.com>

It looks like the virtio_gpu_load() does not compute and set the offset,
the same way virtio_gpu_set_scanout() does. This probably results in
incorrect display until the scanout/framebuffer is updated again, I
guess we should fix it, although I haven't checked this yet.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/display/virtio-gpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 66ac9b6cc5..66cddd94d9 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1289,6 +1289,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
     /* load & apply scanout state */
     vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1);
     for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
+        /* FIXME: should take scanout.r.{x,y} into account */
         scanout = &g->parent_obj.scanout[i];
         if (!scanout->resource_id) {
             continue;
-- 
2.40.1

On Mon, May 15, 2023 at 05:25:18PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> It looks like the virtio_gpu_load() does not compute and set the offset,
> the same way virtio_gpu_set_scanout() does. This probably results in
> incorrect display until the scanout/framebuffer is updated again, I
> guess we should fix it, although I haven't checked this yet.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I guess it's a way to ping Gerd ;)
Better to just fix it though, no?

> ---
>  hw/display/virtio-gpu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 66ac9b6cc5..66cddd94d9 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1289,6 +1289,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
>      /* load & apply scanout state */
>      vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1);
>      for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
> +        /* FIXME: should take scanout.r.{x,y} into account */
>          scanout = &g->parent_obj.scanout[i];
>          if (!scanout->resource_id) {
>              continue;
> -- 
> 2.40.1

Hi Michael

On Fri, May 19, 2023 at 1:35 AM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Mon, May 15, 2023 at 05:25:18PM +0400, marcandre.lureau@redhat.com
> wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > It looks like the virtio_gpu_load() does not compute and set the offset,
> > the same way virtio_gpu_set_scanout() does. This probably results in
> > incorrect display until the scanout/framebuffer is updated again, I
> > guess we should fix it, although I haven't checked this yet.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> I guess it's a way to ping Gerd ;)
> Better to just fix it though, no?
>

Indeed, it's just lack of time and priority, and also figure out a good way
to test this.

thanks


> > ---
> >  hw/display/virtio-gpu.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index 66ac9b6cc5..66cddd94d9 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -1289,6 +1289,7 @@ static int virtio_gpu_load(QEMUFile *f, void
> *opaque, size_t size,
> >      /* load & apply scanout state */
> >      vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1);
> >      for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
> > +        /* FIXME: should take scanout.r.{x,y} into account */
> >          scanout = &g->parent_obj.scanout[i];
> >          if (!scanout->resource_id) {
> >              continue;
> > --
> > 2.40.1
>
>
>

-- 
Marc-André Lureau