[PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging

[Michael Thayer]

Hello Dave,

12.06.2017 09:27, Dave Airlie wrote:
> On 12 June 2017 at 16:56, Hans de Goede <hdegoede at redhat.com> wrote:
>> This commit adds the vboxvideo drm/kms driver for the virtual graphics
>> card used in Virtual Box virtual machines to drivers/staging.
>>
>> Why drivers/staging? This driver is already being patched into the kernel
>> by several distros, thus it is good to get this driver upstream soon, so
>> that work on the driver can be easily shared.
>>
>> At the same time we want to take our time to get this driver properly
>> cleaned up before submitting it as a normal driver under drivers/gpu/drm,
>> putting this driver in staging for now allows both.
>>
>> Note this driver has already been significantly cleaned up, when I started
>> working on this the files under /usr/src/vboxguest/vboxvideo as installed
>> by Virtual Box 5.1.18 Guest Additions had a total linecount of 52681
>> lines. The version in this commit has 7275 lines.
>>
>> Of those 7275 lines 3980 lines are in an osindependent directory which
>> contains portable code which is shared with other guest platforms such as
>> C-structure definitions for the virtual graphics card protocol and helper
>> functions for using these structures to communicate with the card. Since
>> these files are shared they are in the standard Virtual Box upstream code
>> style and not in the kernel coding style. All files outside of the
>> osindependent directory fully adhere to the kernel coding style.
>>
> 
> Just some questions,
> 
> Does the hw have acceleration support of some kind? this driver seems
> be a modesetting only one, which is fine, just wondering if there are
> plans to do more.

The device does support acceleration - what you looked at before 
starting on Virgl.  Currently it is only implemented for X11 and goes 
through the guest device without touching the graphics device at all, 
but it is also possible to pass the stream through the graphics device, 
which is what our Windows drivers do.  However, at this point we are not 
sure whether to implement support for this in the drm driver, or to do 
something new.

> We haven't used staging for drm drivers for a short while for a few
> reasons, the speed of iterating the kms APIs esp in the atomic area
> means nobody remembers to build staging, then you end up with broken
> staging, I'd like to wait for Daniel to get back from holidays to
> consult on whether we should just put this in non-staging so we don't
> lose it down the cracks.
> 
> The most important thing is for the driver to be atomic if it's KMS
> only, and it would be good to have someone review that properly.

How big a change would atomic support be?  The reason I am asking is 
that our out-of-kernel version of the driver builds on all kernels from 
3.11 to 4.11 using ifdefs, and for obvious reasons - reducing the risk 
of breaking a particular kernel version - we try to minimise the 
differences.  We would continue maintaining our version even with the 
driver upstream, to be able to update the driver without updating the 
whole kernel, which is often desirable in virtual machine use cases. 
But we would also want our version to be as close as possible to the 
most recent kernel version so that we can pull fixes as easily as 
possible: generally the people making those fixes will know the drm 
subsystem much better than we do.

To be clear, I am not trying to argue here, just to get an idea for my 
own planning.

Regards
Michael

> 
> Dave.
>-- 
Michael Thayer | VirtualBox engineer
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister 
der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher