Re: [PATCH 1/4] staging: vboxvideo: Fix modeset / page_flip error handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 11-09-18 20:46, Greg Kroah-Hartman wrote:
On Tue, Sep 11, 2018 at 09:15:41AM +0200, Hans de Goede wrote:
The default settings for Linux vms created in VirtualBox allocate only
16M of videomem. When running fullscreen on a 1920x1080 (or bigger) monitor
this is not a lot.

When using GNOME3 on Wayland we have already been seeing out of video
memory errors for a while now. After commit 2408898e3b6c ("staging:
vboxvideo: Add page-flip support") this has become much worse as now
multiple buffers are used.

There is nothing we can do about there not being enough video-mem, but
we should handle running out of video-mem properly, currently there are
2 problems with this:

1) vbox_crtc_mode_set() does not check if vbox_crtc_mode_set_base() fails
at all and does not properly propagate the oom error.

2) vbox_crtc_do_set_base() unpins the old fb too soon:

2.1) It unpins it before pinning the new fb, so if the pinning of the new
fb fails (which it will when we run out of video-mem), then we also cannot
fall back to the old-fb as it has been already unpinned. We could try to
re-pin it but there is no guarantee that will succeed.

2.2) It unpins it before reprogramming the hardware to scan out from the
new-fb, which could lead to some ugliness where the hw is scanning out the
oldfb while it is being replaced with something else.

Fixing this requires to do things in this order:
1) Pin the new fb
2) Program the hw
3) Unpin the oldfb

This needs to be done for both a mode_set and for a page_flip so this
commit re-writes vbox_crtc_do_set_base() into vbox_crtc_set_base_and_mode()
which does this in the correct order, putting the hardware programming
which was duplicated between the mode_set and page_flip code inside the
new function.

Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
  drivers/staging/vboxvideo/vbox_mode.c | 121 +++++++++++++-------------
  1 file changed, 62 insertions(+), 59 deletions(-)

This doesn't apply to my tree.  Does it need to go on top of the
previous patches you sent for 4.19-final?

Yes this goes on top of the fixes for 4.19-final.

Regards,

Hans

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux