[PATCH 4.19 regression fix 2/2] staging: vboxvideo: Change address of scanout buffer on page-flip

[Hans de Goede]

Hi,

On 11-09-18 01:08, Steve Longerbeam wrote:
> Hi Hans,
> 
> 
> On 09/10/2018 11:30 AM, Hans de Goede wrote:
>> Commit 2408898e3b6c ("staging: vboxvideo: Add page-flip support") only
>> calls vbox_crtc_do_set_base() on page-flips, but despite that function's
>> name it only pins the new fb, unpins the old fb and sets
>> vbox_crtc->fb_offset. It does not program the hardware to scan out at the
>> new vbox_crtc->fb_offset value.
> 
> Has that always been the case of vbox_crtc_do_set_base()? Or was
> there a recent commit that changed that behavior?

I believe that always was the case.

> I tested this patch using a Weston EGL mock navigation test app around
> 4.14 time-frame, that exercises page flip and it was scanning out the
> new fb, but maybe what I was looking at was a scan-out of an old/now stale
> fb from a previous page-flip.

Yes I think that is what you saw, this was not easy to notice, I noticed
that when typing slowly my letters would always appear 2 at a time, then
I deliberately typed: 'a' waited a couple of seconds and the 'a' would
not show up until I also typed 'b' and then both showed up at once.

This was with GNOME3 as Wayland compositor, which I believe stops
sending new frames when nothing changes which makes this very noticeable :)

> In any case thanks for fixing.

You are welcome.

Regards,

Hans



> 
> Steve
> 
>> This was causing only every other frame (assuming page-flipping between 2
>> buffers) to be shown since we kept scanning out of the old (now unpinned!)
>> buffer.
>>
>> This commit fixes this by adding code to vbox_crtc_page_flip() to tell
>> the hardware to scanout from the new fb_offset.
>>
>> Fixes: 2408898e3b6c ("staging: vboxvideo: Add page-flip support")
>> Cc: Steve Longerbeam <steve_longerbeam at mentor.com>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>   drivers/staging/vboxvideo/vbox_mode.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c
>> index a83eac8668d0..79836c8fb909 100644
>> --- a/drivers/staging/vboxvideo/vbox_mode.c
>> +++ b/drivers/staging/vboxvideo/vbox_mode.c
>> @@ -323,6 +323,11 @@ static int vbox_crtc_page_flip(struct drm_crtc *crtc,
>>       if (rc)
>>           return rc;
>> +    mutex_lock(&vbox->hw_mutex);
>> +    vbox_set_view(crtc);
>> +    vbox_do_modeset(crtc, &crtc->mode);
>> +    mutex_unlock(&vbox->hw_mutex);
>> +
>>       spin_lock_irqsave(&drm->event_lock, flags);
>>       if (event)
>