| Message ID | 20220505214030.4261-1-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-gpu: update done only on the scanout associated with rect | expand |
Hi On Fri, May 6, 2022 at 1:46 AM Dongwon Kim <dongwon.kim@intel.com> wrote: > It only needs to update the scanouts containing the rect area > coming with the resource-flush request from the guest. > > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > --- > hw/display/virtio-gpu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 529b5246b2..165ecafd7a 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -514,6 +514,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g, > for (i = 0; i < g->parent_obj.conf.max_outputs; i++) { > scanout = &g->parent_obj.scanout[i]; > if (scanout->resource_id == res->resource_id && > + rf.r.x >= scanout->x && rf.r.y >= scanout->y && > + rf.r.x + rf.r.width <= scanout->x + scanout->width && > + rf.r.y + rf.r.height <= scanout->y + scanout->height && > That doesn't seem to handle intersections/overlapping, I think it should. > console_has_gl(scanout->con)) { > dpy_gl_update(scanout->con, 0, 0, scanout->width, > scanout->height); > -- > 2.30.2 > > >
On Fri, May 06, 2022 at 11:53:22AM +0400, Marc-André Lureau wrote: > Hi > > On Fri, May 6, 2022 at 1:46 AM Dongwon Kim <dongwon.kim@intel.com> wrote: > > > It only needs to update the scanouts containing the rect area > > coming with the resource-flush request from the guest. > > > > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > --- > > hw/display/virtio-gpu.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > > index 529b5246b2..165ecafd7a 100644 > > --- a/hw/display/virtio-gpu.c > > +++ b/hw/display/virtio-gpu.c > > @@ -514,6 +514,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g, > > for (i = 0; i < g->parent_obj.conf.max_outputs; i++) { > > scanout = &g->parent_obj.scanout[i]; > > if (scanout->resource_id == res->resource_id && > > + rf.r.x >= scanout->x && rf.r.y >= scanout->y && > > + rf.r.x + rf.r.width <= scanout->x + scanout->width && > > + rf.r.y + rf.r.height <= scanout->y + scanout->height && > > > > > That doesn't seem to handle intersections/overlapping, I think it should. so set-scanouts and resource flushes are issued per scanout(CRTC/plane from guest's point of view). In case of intersections/overlapping, there will be two resource flushes (in case there are two scanouts) and each resource flush will take care of updating the scanout that covers partial damaged area. The problem with the original code is that even if there is a resource flush request for a single scanout, it does update on both (or more) as well, which is unnecessary burden. Thanks > > > > console_has_gl(scanout->con)) { > > dpy_gl_update(scanout->con, 0, 0, scanout->width, > > scanout->height); > > -- > > 2.30.2 > > > > > > > > -- > Marc-André Lureau
On Fri, May 06, 2022 at 10:09:30AM -0700, Dongwon Kim wrote: > On Fri, May 06, 2022 at 11:53:22AM +0400, Marc-André Lureau wrote: > > Hi > > > > On Fri, May 6, 2022 at 1:46 AM Dongwon Kim <dongwon.kim@intel.com> wrote: > > > > > It only needs to update the scanouts containing the rect area > > > coming with the resource-flush request from the guest. > > > > > > > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > > --- > > > hw/display/virtio-gpu.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > > > index 529b5246b2..165ecafd7a 100644 > > > --- a/hw/display/virtio-gpu.c > > > +++ b/hw/display/virtio-gpu.c > > > @@ -514,6 +514,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g, > > > for (i = 0; i < g->parent_obj.conf.max_outputs; i++) { > > > scanout = &g->parent_obj.scanout[i]; > > > if (scanout->resource_id == res->resource_id && > > > + rf.r.x >= scanout->x && rf.r.y >= scanout->y && > > > + rf.r.x + rf.r.width <= scanout->x + scanout->width && > > > + rf.r.y + rf.r.height <= scanout->y + scanout->height && > > > > > > > > > That doesn't seem to handle intersections/overlapping, I think it should. > > so set-scanouts and resource flushes are issued per scanout(CRTC/plane > from guest's point of view). In case of intersections/overlapping, there > will be two resource flushes (in case there are two scanouts) and each > resource flush will take care of updating the scanout that covers > partial damaged area. Even though the linux kernel driver sends two flushes, one for each scanout, it is perfectly valid send a single flush for the complete resource. So checking whenever the rectangle is completely within the scanout is not correct. When the scanout is covered partly you must update too. Only when the rectangle is completely outside the scanout it is valid to skip it. take care, Gerd
On Tue, Jul 19, 2022 at 01:15:26PM +0200, Gerd Hoffmann wrote: > On Fri, May 06, 2022 at 10:09:30AM -0700, Dongwon Kim wrote: > > On Fri, May 06, 2022 at 11:53:22AM +0400, Marc-André Lureau wrote: > > > Hi > > > > > > On Fri, May 6, 2022 at 1:46 AM Dongwon Kim <dongwon.kim@intel.com> wrote: > > > > > > > It only needs to update the scanouts containing the rect area > > > > coming with the resource-flush request from the guest. > > > > > > > > > > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > > > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > > > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > > > --- > > > > hw/display/virtio-gpu.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > > > > index 529b5246b2..165ecafd7a 100644 > > > > --- a/hw/display/virtio-gpu.c > > > > +++ b/hw/display/virtio-gpu.c > > > > @@ -514,6 +514,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g, > > > > for (i = 0; i < g->parent_obj.conf.max_outputs; i++) { > > > > scanout = &g->parent_obj.scanout[i]; > > > > if (scanout->resource_id == res->resource_id && > > > > + rf.r.x >= scanout->x && rf.r.y >= scanout->y && > > > > + rf.r.x + rf.r.width <= scanout->x + scanout->width && > > > > + rf.r.y + rf.r.height <= scanout->y + scanout->height && > > > > > > > > > > > > > That doesn't seem to handle intersections/overlapping, I think it should. > > > > so set-scanouts and resource flushes are issued per scanout(CRTC/plane > > from guest's point of view). In case of intersections/overlapping, there > > will be two resource flushes (in case there are two scanouts) and each > > resource flush will take care of updating the scanout that covers > > partial damaged area. > > Even though the linux kernel driver sends two flushes, one for each > scanout, it is perfectly valid send a single flush for the complete > resource. > > So checking whenever the rectangle is completely within the scanout is > not correct. When the scanout is covered partly you must update too. > Only when the rectangle is completely outside the scanout it is valid to > skip it. Gerd, I got your point. I will take a look into it. > > take care, > Gerd >
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 529b5246b2..165ecafd7a 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -514,6 +514,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g, for (i = 0; i < g->parent_obj.conf.max_outputs; i++) { scanout = &g->parent_obj.scanout[i]; if (scanout->resource_id == res->resource_id && + rf.r.x >= scanout->x && rf.r.y >= scanout->y && + rf.r.x + rf.r.width <= scanout->x + scanout->width && + rf.r.y + rf.r.height <= scanout->y + scanout->height && console_has_gl(scanout->con)) { dpy_gl_update(scanout->con, 0, 0, scanout->width, scanout->height);
It only needs to update the scanouts containing the rect area coming with the resource-flush request from the guest. Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> --- hw/display/virtio-gpu.c | 3 +++ 1 file changed, 3 insertions(+)