[vbox-dev] Additions: linux/drm: Change vbox_*.c to kernel coding style set 2

[Hans de Goede]

Hi,

On 22-06-17 07:46, Michael Thayer wrote:
> Hello Hans,
> 
> Sorry for the long delay.  I am working on too many things in parallel
> (though I suspect you have a few too!)

No problem.

> 14.06.2017 16:52, Michael Thayer wrote:
>> Hello Hans,
>>
>> 14.06.2017 15:36, Hans de Goede wrote:
>> [Discussion of converting vboxvideo to kernel coding style.]
>>> Given Sean Paul's comments on the dri-devel list, I plan to also
>>> convert the files which in the staging submission sit under the
>>> osindepdent dir to the kernel coding style.
>>>
>>> If you want I can submit a patch switching the drm code in svn
>>> over to use the cleaned-up files rather then the shared ones,
>>> that way it will stay relatively close the code going upstream
>>> and it should be easier to keep things in sync.
> [...]
>> I still have to talk to my colleagues to decide what we will do, but a
>> patch would certainly be of interest if that doesn't create too much
>> effort for you.  Let's at least try to see that we get a good logical
>> mapping between the two (not necessarily file for file, but at lease
>> function for function and structure for structure).
> 
> After talking to my colleagues I decided to keep our driver as it is
> (possibly with a compatibility layer analogous to VBoxVideoIPRT.h, but
> in reverse this time) as long as I can do that without the difference to
> the in-kernel version getting too big.  If I ever feel that the
> difference to the in-kernel driver is causing more extra work than we
> save through the shared code I will switch to using the code from the
> kernel (it would be nice if it stayed under MIT licence) and drop our
> dependency on the shared code.

Ok, I'm about to wrap up my moving of the vboxvideo code to the kernel
coding style and to do a v3 submission for staging. One of the things on
my TODO list was to send you a RFC patch to move the vbox svn code to
the new style code including the shared files. So that the svn vboxvideo
would end up using only files under src/VBox/Additions/linux/drm in
essence having a 1:1 copy of the kernel version with just some
#if LINUX_VERSION_CODE ... bits in there, which should make syncing
things really easy.

The amount of changes I ended up doing is quite big since I also had
to convert all the CamelCaseNames to not_camel_case_names for
function, variable and struct member names which ended up touching a
lot of code. I also removed most unused shared code. In some
cases I also replaced shared code which was just tiny wrappers, e.g.
a function consisting of solely calling another function with direct
calls to the wrapped function. So the remaining amount of what once was
shared code used is quite small:

[hans at shalem linux]$ wc -l drivers/staging/vboxvideo/hgsmi_base.c drivers/staging/vboxvideo/modesetting.c drivers/staging/vboxvideo/vbva_base.c drivers/staging/vboxvideo/hgsmi_*.h drivers/staging/vboxvideo/vboxvideo.h
   264 drivers/staging/vboxvideo/hgsmi_base.c
   147 drivers/staging/vboxvideo/modesetting.c
   238 drivers/staging/vboxvideo/vbva_base.c
    70 drivers/staging/vboxvideo/hgsmi_ch_setup.h
    60 drivers/staging/vboxvideo/hgsmi_channels.h
    98 drivers/staging/vboxvideo/hgsmi_defs.h
   495 drivers/staging/vboxvideo/vboxvideo.h
  1372 total

As such I believe you will be better of to just jump to the new cleaned
up code, which will make keeping things in sync much easier. But it is
your call and since for now you plan to not do so I'm not going to
bother with creating a patch against vbox svn for this.

> I am curious to know if you have plans for the RHEL kernel (is that an
> official way to refer to it?)  Do you plan to keep the version of the
> driver there up-to-date with our version or stick to what you was
> current when the kernel was released?

For RHEL kernels drm/kms drivers get synced with the mainline kernel
every .y release, so when we prepared 7.1 we did a sync, again with
7.2, etc. For now I'm focusing on Fedora and we've no plans to add
vbox guest additions to RHEL 7.x, but that could change if there is
customer demand for this. And the vboxvideo driver might get enabled
on a drivers/gpu sync once it has left staging.

> Obviously I would prefer keeping
> it up-to-date (again, you could save yourself porting time by using our
> one, which we test against Red Hat kernels, and in which we will
> integrate fixes from the current kernel version).  If not we will
> recommend that the user switches to our Additions version.
> 
> Regarding your suggestion for providing a script to uninstall
> distribution-provided Additions, so far the reaction from other
> packagers has not been overwhelming, so I am thinking of going back to
> your first suggestion - just making our installer aware of the most
> important (as decided by us) distributions which provide Additions and
> knowing how to deal with them.  With the added twist that we would not
> attempt to install our version on distributions known to keep their
> version up-to-date.

Ok, for now I'm focusing on the kernel side of things, but I will
keep this in mind when I start creating a Fedora package for the
guest additions.

Regards,

Hans