Re: [PATCH v3] staging: vboxvideo: Add vboxvideo to drivers/staging

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 26-06-17 18:24, Daniel Vetter wrote:
> On Mon, Jun 26, 2017 at 06:06:19PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 23-06-17 11:31, Daniel Vetter wrote:
>>> On Thu, Jun 22, 2017 at 11:11:37AM +0200, Hans de Goede 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 (mainly converted to the new atomic modesetting APIs) 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 4874 lines.
>>>>
>>>> Cc: vbox-dev@virtualbox.org
>>>> Cc: Michael Thayer <michael.thayer@oracle.com>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> Signed-off-by: Michael Thayer <michael.thayer@oracle.com>
>>>> ---
>>>> Changes in v2:
>>>> -Add Michael's S-o-b
>>>> -Improve Kconfig help text
>>>> -Remove .config changes which accidentally got added to the commit
>>>>
>>>> Changes in v3:
>>>> -Convert the files shared with the video-driver for other operating-systems
>>>>    to kernel coding style too
>>>> -Remove unused code from these files
>>>
>>> In the end it's up to you, but our experience in drm with -staging has
>>> been that's both a pain (separate tree, which makes coordination harder
>>> for drm drivers) and a ghetto (no one from drm will look at your patches).
>>>
>>> Especially for small drivers (and this is one, and I expect if you use all
>>> the latest and greates helpers atomic will provide it'll shrink
>>> considerably) it's just not worth the pain to polish stuff twice.
>>>
>>> 0toh I see the benefit of putting stuff into staging so in the end it's up
>>> to you, just beware pls.
>>
>> Thanks for the heads up Daniel, for now I would like to move forward with
>> getting this in staging, but I do agree that it would be good to get it
>> into drivers/gpu soon. Michael do you think you will have some time soon
>> to look at moving this to the atomic helpers ?
> 
> To make it clear what I think is the minimal path towards drivers/gpu:
> - switch to atomic helpers (well probably simple_display_pipe if it fits)
> - delete half the driver code by dropping optional hooks and using helpers
> - make sure you don't use any callback/functions where the kerneldoc says
>    it's deprecated (e.g. load/unload)
> - merge

Thank you for this list, that is very helpful.

> We can do the checkpatch/style bikeshedding once merged into drivers/gpu/
> imo, that's real good fodder for newbies anyway :-)

Actually for v3 I've already converted everything to kernel coding style,
so that is already done :)

Regards,

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