On Wed, Jan 27, 2016 at 02:08:21PM +0000, Emil Velikov wrote:
On 27 January 2016 at 13:50, Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Jan 27, 2016 at 01:23:11PM +0000, Emil Velikov wrote:
As some headers do not reside in include/drm we need to tweak our rules, and exclude headers that shouldn't be distributed [XXX: clarify why ?].
To avoid the extra magic of diving into the kernel tree running `make headers_install', just sed out the only reason why we need it - __user.
Cc: Alex Deucher alexander.deucher@amd.com Cc: Michel Dänzer michel.daenzer@amd.com Cc: Ben Skeggs bskeggs@redhat.com Cc: Dave Airlie airlied@redhat.com Cc: Daniel Stone daniels@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Inki Dae inki.dae@samsung.com Cc: Rob Clark robclark@freedesktop.org Cc: Tomi Valkeinen tomi.valkeinen@ti.com Cc: Daniel Kurtz djkurtz@chromium.org Signed-off-by: Emil Velikov emil.l.velikov@gmail.com
Gents, As one runs `make copy-headers' we get a massive diff stat (+1500/-800) and a handful of issues gets pointed out. Please let me know of your prefered solution (regardless if one will get to it soon) and if we should consider it a blocker (B) or not (N).
Thanks Emil
- (N) Header license miss-match - omap, msm, exynos. Update the kernel
ones ?
- (N) Broken compat ioctls - exynos (and the UMS drivers) - using
unsigned int as opposed to _u32/64. Considering they're 32bit only, we can get away with 'breaking' the ABI and using the proper ones ?
- (N, keep local for now) C++ compat - libdrm has a hack/workaround
(virtual is a keyword in C++), which I'd like us to upstream plus some extern C wrappers.
(?) Missing UMS symbols - see _DRM_GEM
(?) Non C89 compliant (see DRM_DRAWABLE_CLIPRECTS) - do we still
want/need that ?
- (B) Using include <drm/...> as opposed to include "..." - drm.h,
nouveau_drm.h. Should be fixed in kernel.
- (N) ABI 'break'
- drm - struct drm_mode_get_connector extra pad
- tegra - struct drm_tegra_gem_mmap extra pad
- (B) API break
- drm - missing DRM_MODE_OBJECT_*
- nouveau - missing (gs)etparam - both structs and macros. everything
else is fine/unused.
- radeon - RADEON_TILING_R600_NO_SCANOUT, CIK_TILE_MODE_COLOR_2D* and
CIK_TILE_MODE_DEPTH_STENCIL_2D_TILESPLIT_* - quick grep shows no users
- omap - struct drm_omap_get_base, DRM_OMAP_GET_BASE + IOCTL
(N) (unneeded?) API additions - nouveau's DRM_IOCTL_NOUVEAU_GEM_*
(N) __KERNEL__ condiditionals. Is it really an issue - sure if looks
a bit spurious but that's about it.
Makefile.am | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Makefile.am b/Makefile.am index ca41508..6c71d3a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -126,7 +126,14 @@ endif
copy-headers : cp -r $(kernel_source)/include/uapi/drm/*.h $(top_srcdir)/include/drm/
sed -i "s/__user //g" $(top_srcdir)/include/drm/*.hmv $(top_srcdir)/include/drm/exynos_drm.h $(top_srcdir)/exynos/mv $(top_srcdir)/include/drm/msm_drm.h $(top_srcdir)/freedreno/msm/mv $(top_srcdir)/include/drm/omap_drm.h $(top_srcdir)/omap/rm $(top_srcdir)/include/drm/armada_drm.hrm $(top_srcdir)/include/drm/etnaviv_drm.hrm $(top_srcdir)/include/drm/i810_drm.hrm $(top_srcdir)/include/drm/vc4_drm.hYou can't copy headers directly. First you need to generate them in the kernel sources:
$ make headers_install
Then you can copy them from usr/include/*
That should at least get rid of the __KERNEL__ stuff. Then any serious issues should be fixed in the upstream kernel first.
I agree that copying the headers isn't that good of an idea. Then again as that causes zero problems - it brings in __KERNEL__ and __user, latter of which sed out. So wouldn't it be better to focus on the serious issues first ? Of course, that doesn't mean we have to forget about this one :-P
-Emil P.S. Diving into other projects and executing make foo/bar is frowned upon, normally.
I doubt anyone was suggesting that you would do that.
I would probably just nuke the copy-headers rule. But if people want to keep it, I'd make it look at the INSTALL_HDR_PATH variable (which is how the kernel decides where to install the headers), bail if it's not set (or maybe default to /usr, like the kernel), and copy the headers from there. Then the user can go and do the 'make INSTALL_HDR_PATH=... headers_install' himself in the kernel tree, and then do the 'make INSTALL_HDR_PATH=... copy-headers' in libdrm. Or you could use some other variable name for that in libdrm, but reusing the kernel one would seem like the easiest solution.