[PATCH] qxl: fix framebuffer unpinning

[Gabriel Krisman Bertazi]

Gerd Hoffmann <kraxel at redhat.com> writes:

> On Tue, 2017-09-19 at 11:35 -0300, Gabriel Krisman Bertazi wrote:
>> Gerd Hoffmann <kraxel at redhat.com> writes:
>> 
>> > qxl_plane_cleanup_fb() unpins the just activated framebuffer
>> > instead of the old one.  Oops.  Fix it.
>> > 
>> > Cc: Gabriel Krisman Bertazi <krisman at collabora.co.uk>
>> > Fixes: 1277eed5fecb8830c8cc414ad70c1ef640464bc0
>> > Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
>> > ---
>> >  drivers/gpu/drm/qxl/qxl_display.c | 7 ++++---
>> >  1 file changed, 4 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/qxl/qxl_display.c
>> > b/drivers/gpu/drm/qxl/qxl_display.c
>> > index e1dd05423e..afbf50d0c0 100644
>> > --- a/drivers/gpu/drm/qxl/qxl_display.c
>> > +++ b/drivers/gpu/drm/qxl/qxl_display.c
>> > @@ -702,14 +702,15 @@ static void qxl_plane_cleanup_fb(struct
>> > drm_plane *plane,
>> >  	struct drm_gem_object *obj;
>> >  	struct qxl_bo *user_bo;
>> >  
>> > -	if (!plane->state->fb) {
>> > -		/* we never executed prepare_fb, so there's
>> > nothing to
>> > +	if (!old_state->fb) {
>> > +		/*
>> > +		 * we never executed prepare_fb, so there's
>> > nothing to
>> >  		 * unpin.
>> >  		 */
>> >  		return;
>> >  	}
>> >  
>> > -	obj = to_qxl_framebuffer(plane->state->fb)->obj;
>> > +	obj = to_qxl_framebuffer(old_state->fb)->obj;
>> 
>> I am not sure about this patch.  My idea was to pin/unpin the current
>> buffer on prepare/cleanup, and only keep it pinned during the atomic
>> commit.  This was the goal of 37235451c6990683a8, for instance.
>
> I'm pretty sure the buffer must remain pinned while it is displayed.
>

ok, then it makes sense indeed :)

>> Anyway, if that goal is wrong, I think that, with your change, we
>> forget to
>> unpin the final plane before removing the device.
>
> "removing the device"?  qxl can't be hotplugged ...
> Or do you mean "rmmod qxl"?

rmmod qxl

>
> cheers,
>   Gerd
>

-- 
Gabriel Krisman Bertazi