[igt-dev] [PATCH i-g-t v2 1/8] igt/vmwgfx: Add generated headers for svga device

[Kamil Konieczny]

Hi Zack,

On 2023-03-17 at 19:34:04 +0000, Zack Rusin wrote:
> On Fri, 2023-03-17 at 17:52 +0100, Kamil Konieczny wrote:
> > !! External Email
> > 
> > Hi Zack,
> > 
> > On 2023-03-15 at 22:26:52 -0400, Zack Rusin wrote:
> > > From: Maaz Mombasawala <mombasawalam at vmware.com>
> > > 
> > > Add generated headers for svga device which will be used
> > > to interface with the vmwgfx driver.
> > > 
> > > Signed-off-by: Roye Eshed <reshed at vmware.com>
> > > Signed-off-by: Zack Rusin <zackr at vmware.com>
> > > Signed-off-by: Maaz Mombasawala <mombasawalam at vmware.com>
> > > ---
> > >  lib/svga/svga3d_cmd.h         | 1511 +++++++++++++++++++++++++++++
> > >  lib/svga/svga3d_devcaps.h     |  369 +++++++
> > >  lib/svga/svga3d_dx.h          | 1722 +++++++++++++++++++++++++++++++++
> > >  lib/svga/svga3d_limits.h      |   83 ++
> > >  lib/svga/svga3d_reg.h         |   44 +
> > >  lib/svga/svga3d_surfacedefs.h | 1559 +++++++++++++++++++++++++++++
> > >  lib/svga/svga3d_types.h       | 1554 +++++++++++++++++++++++++++++
> > >  lib/svga/svga_escape.h        |   54 ++
> > >  lib/svga/svga_overlay.h       |  115 +++
> > >  lib/svga/svga_reg.h           |  897 +++++++++++++++++
> > >  lib/svga/vm_basic_types.h     |  161 +++
> > >  11 files changed, 8069 insertions(+)
> > >  create mode 100644 lib/svga/svga3d_cmd.h
> > >  create mode 100644 lib/svga/svga3d_devcaps.h
> > >  create mode 100644 lib/svga/svga3d_dx.h
> > >  create mode 100644 lib/svga/svga3d_limits.h
> > >  create mode 100644 lib/svga/svga3d_reg.h
> > >  create mode 100644 lib/svga/svga3d_surfacedefs.h
> > >  create mode 100644 lib/svga/svga3d_types.h
> > >  create mode 100644 lib/svga/svga_escape.h
> > >  create mode 100644 lib/svga/svga_overlay.h
> > >  create mode 100644 lib/svga/svga_reg.h
> > >  create mode 100644 lib/svga/vm_basic_types.h
> > > 
> > > diff --git a/lib/svga/svga3d_cmd.h b/lib/svga/svga3d_cmd.h
> > > new file mode 100644
> > > index 00000000..5a64d40b
> > > --- /dev/null
> > > +++ b/lib/svga/svga3d_cmd.h
> > > @@ -0,0 +1,1511 @@
> > > +/**********************************************************
> > > + * Copyright 2012-2021 VMware, Inc.
> > > + * SPDX-License-Identifier: GPL-2.0 OR MIT
> > 
> > imho SPDX should be first, (c) in line below. Did you check it
> > with checkpatch.pl from Linux kernel ?
> 
> Those headers are from the kernel. They're auto-generated so I'd prefer to keep them
> matching the kernel otherwise it's impossible to keep them up to date and the next
> time someone updates them they revert back. Having said that the spdx line being
> first is something that we did change in the script autogenerating them, it's just
> that we haven't updated those headers in IGT for a while. I was planning to update
> the kernel and igt next month, but I can do that sooner.
> 
> As to the deleting the license text after the spdx header, VMware has standard open
> source contributing header template that we need to use, while we can ask for an
> exception to change it for IGT, it might be a lot more effective is someone other
> than us does it after contribution lands in particular because it seems a few other
> files in IGT do the same thing (e.g. amd_basic.c, amd_pci_unplug.c,
> amd_deadlock.c...) so it might be a worthy project wide cleanup.
> 
> z
> 
> 
No problem if that is from kernel it can go that way. I checked
drm-tip file

./drivers/gpu/drm/vmwgfx/device_include/svga3d_cmd.h

and there is SPDX at first line, then copyright and licence text
so it can be copied into igt libs. I have one more ask, please
change subject lines in pacthes to begin with relevant information.

For example in first patch use lib/sgva: , so instead of:

Subject: [PATCH i-g-t v2 1/8] igt/vmwgfx: Add generated headers for svga device
----------------------------- ^^^^^^^^^^
this should be:

Subject: [PATCH i-g-t v2 1/8] lib/svga: Add generated headers for svga device

It will help if you write in commit from where those files was copied.

Similary, when you add tests in patches 3..7, instead of:
igt/vmwgfx: Add triangle test
^^^

write:
tests/vmwgfx: Add triangle test

Finally, in 2nd patch,

igt/vmwgfx: Add vmwgfx support
^^^^^^^^^^

write

lib: Add vmwgfx support

Regards,
Kamil