Re: [Spice-devel] [PATCH] drm/qxl: workaround broken qxl hw primary setting.

[Gerd Hoffmann <kraxel@redhat.com>]

On Thu, 2017-10-19 at 07:28 -0400, Frediano Ziglio wrote:

> > This tries to work out when the commit is likely just a pageflip
> > and avoid touching the primary surface, this might go wrong at
> > some point but I believe it's the same level as wrong as the old
> > code
> > base.
> > 
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> > ---
> >  drivers/gpu/drm/qxl/qxl_display.c | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> > b/drivers/gpu/drm/qxl/qxl_display.c
> > index afbf50d..3b702d1 100644
> > --- a/drivers/gpu/drm/qxl/qxl_display.c
> > +++ b/drivers/gpu/drm/qxl/qxl_display.c
> > @@ -502,6 +502,7 @@ static void qxl_primary_atomic_update(struct
> > drm_plane
> > *plane,
> >  	struct qxl_framebuffer *qfb_old;
> >  	struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj);
> >  	struct qxl_bo *bo_old;
> > +	bool update_primary = true;
> >  	struct drm_clip_rect norect = {
> >  	    .x1 = 0,
> >  	    .y1 = 0,
> > @@ -519,15 +520,31 @@ static void qxl_primary_atomic_update(struct
> > drm_plane
> > *plane,
> >  	if (bo == bo_old)
> >  		return;
> >  
> > +	if (bo && bo_old &&
> > +	    plane->state->crtc == old_state->crtc &&
> > +	    plane->state->crtc_w == old_state->crtc_w &&
> > +	    plane->state->crtc_h == old_state->crtc_h &&
> > +	    plane->state->src_x == old_state->src_x &&
> > +	    plane->state->src_y == old_state->src_y &&
> > +	    plane->state->src_w == old_state->src_w &&
> > +	    plane->state->src_h == old_state->src_h &&
> > +	    plane->state->rotation == old_state->rotation &&
> > +	    plane->state->zpos == old_state->zpos)
> > +		/* this is likely a pageflip */
> > +		update_primary = false;
> > +
> >  	if (bo_old && bo_old->is_primary) {
> > -		qxl_io_destroy_primary(qdev);
> > +		if (update_primary)
> > +			qxl_io_destroy_primary(qdev);
> >  		bo_old->is_primary = false;
> >  	}
> >  
> >  	if (!bo->is_primary) {
> > -		qxl_io_create_primary(qdev, 0, bo);
> > +		if (update_primary)
> > +			qxl_io_create_primary(qdev, 0, bo);
> >  		bo->is_primary = true;
> 
> Here the primary is still the old object but you are setting the
> new.
> Looking around the old is unpinned and the new one pinned which
> is now wrong as QXL device suppose the memory pointer by the
> primary surface (after your patch bo_old object) remains
> available.

Yes.

So it is bug compatible with commit 058e9f5c82, as the commit message
claims ;)

> If we are not able to avoid the copy and we need to keep the old
> surface in place maybe instead of creating the new object as SURFACE
> we could just use for source for the Drawable for the DRAW_COPY
> operation.
> In this way when release is received the object can be unpinned being
> free to be moved.

https://lists.freedesktop.org/archives/dri-devel/2017-October/155541.ht
ml

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel