Re: [PATCH] virtio-gpu: update done only on the scanout associated with rect

[Dongwon Kim]

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
>