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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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@redhat.com>
> ---
>  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?

thanks,

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