On 27 January 2016 at 14:47, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
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.
The idea is to avoid jumping back and forth, if that can be avoided. And based on how long and how badly this has been broken I'd go with "the quickest, simplest, idiot proof solution".
Then again... I would love to hear on the more important, imho, topics. Please ?
Thanks Emil