[PATCH libdrm] Update/fix the {copy,commit}-headers targets
On 27 January 2016 at 14:47, Ville Syrjälä wrote: > On Wed, Jan 27, 2016 at 02:08:21PM +, Emil Velikov wrote: >> On 27 January 2016 at 13:50, Daniel Vetter wrote: >> > On Wed, Jan 27, 2016 at 01:23:11PM +, 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 >> >> Cc: Michel Dänzer >> >> Cc: Ben Skeggs >> >> Cc: Dave Airlie >> >> Cc: Daniel Stone >> >> Cc: Daniel Vetter >> >> Cc: Inki Dae >> >> Cc: Rob Clark >> >> Cc: Tomi Valkeinen >> >> Cc: Daniel Kurtz >> >> Signed-off-by: Emil Velikov >> >> --- >> >> >> >> 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 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/*.h >> >> + mv $(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.h >> >> + rm $(top_srcdir)/include/drm/etnaviv_drm.h >> >> + rm $(top_srcdir)/include/drm/i810_drm.h >> >> + rm $(top_srcdir)/include/drm/vc4_drm.h >> > >> > You 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.
[PATCH libdrm] Update/fix the {copy,commit}-headers targets
On Wed, Jan 27, 2016 at 02:08:21PM +, Emil Velikov wrote: > On 27 January 2016 at 13:50, Daniel Vetter wrote: > > On Wed, Jan 27, 2016 at 01:23:11PM +, 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 > >> Cc: Michel Dänzer > >> Cc: Ben Skeggs > >> Cc: Dave Airlie > >> Cc: Daniel Stone > >> Cc: Daniel Vetter > >> Cc: Inki Dae > >> Cc: Rob Clark > >> Cc: Tomi Valkeinen > >> Cc: Daniel Kurtz > >> Signed-off-by: Emil Velikov > >> --- > >> > >> 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 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/*.h > >> + mv $(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.h > >> + rm $(top_srcdir)/include/drm/etnaviv_drm.h > >> + rm $(top_srcdir)/include/drm/i810_drm.h > >> + rm $(top_srcdir)/include/drm/vc4_drm.h > > > > You 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. -- Ville Syrjälä Intel OTC
[PATCH libdrm] Update/fix the {copy,commit}-headers targets
On Wed, Jan 27, 2016 at 01:23:11PM +, 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 > Cc: Michel Dänzer > Cc: Ben Skeggs > Cc: Dave Airlie > Cc: Daniel Stone > Cc: Daniel Vetter > Cc: Inki Dae > Cc: Rob Clark > Cc: Tomi Valkeinen > Cc: Daniel Kurtz > Signed-off-by: Emil Velikov > --- > > 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 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/*.h > + mv $(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.h > + rm $(top_srcdir)/include/drm/etnaviv_drm.h > + rm $(top_srcdir)/include/drm/i810_drm.h > + rm $(top_srcdir)/include/drm/vc4_drm.h You 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. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH libdrm] Update/fix the {copy,commit}-headers targets
On 27 January 2016 at 13:50, Daniel Vetter wrote: > On Wed, Jan 27, 2016 at 01:23:11PM +, 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 >> Cc: Michel Dänzer >> Cc: Ben Skeggs >> Cc: Dave Airlie >> Cc: Daniel Stone >> Cc: Daniel Vetter >> Cc: Inki Dae >> Cc: Rob Clark >> Cc: Tomi Valkeinen >> Cc: Daniel Kurtz >> Signed-off-by: Emil Velikov >> --- >> >> 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 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/*.h >> + mv $(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.h >> + rm $(top_srcdir)/include/drm/etnaviv_drm.h >> + rm $(top_srcdir)/include/drm/i810_drm.h >> + rm $(top_srcdir)/include/drm/vc4_drm.h > > You 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.
[PATCH libdrm] Update/fix the {copy,commit}-headers targets
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 Cc: Michel Dänzer Cc: Ben Skeggs Cc: Dave Airlie Cc: Daniel Stone Cc: Daniel Vetter Cc: Inki Dae Cc: Rob Clark Cc: Tomi Valkeinen Cc: Daniel Kurtz Signed-off-by: Emil Velikov --- 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 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/*.h + mv $(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.h + rm $(top_srcdir)/include/drm/etnaviv_drm.h + rm $(top_srcdir)/include/drm/i810_drm.h + rm $(top_srcdir)/include/drm/vc4_drm.h commit-headers : copy-headers - git add include/drm/*.h git commit -am "Copy headers from kernel $$(GIT_DIR=$(kernel_source)/.git git describe)" -- 2.6.2
