All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] uapi: two minor changes
@ 2012-10-12 23:49 Luis R. Rodriguez
  2012-10-12 23:49 ` [PATCH 1/2] uapi: update includes for drm content when no kernel API exists Luis R. Rodriguez
  2012-10-12 23:49 ` [PATCH 2/2] uapi: remove trailing spaces Luis R. Rodriguez
  0 siblings, 2 replies; 9+ messages in thread
From: Luis R. Rodriguez @ 2012-10-12 23:49 UTC (permalink / raw)
  To: dri-devel, linux-kernel
  Cc: devel, backports, rob, arnd, davej, airlied, bskeggs, alan,
	dhowells, tglx, daniel.vetter, jbarnes, alexander.deucher,
	paulmck, gregkh, laurent.pinchart, Luis R. Rodriguez

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

Here are a few minor updates to the UAPI changes [0]. The first
one is to help with the backport effort [1], the second one is
simply space cosmetic change to address my eyes bleeding
while reviewing the changes on next-20121012 with git.

If the changes on the first patch make sense perhaps similar
changes can be done for the other subsystem in similar cases.
In those cases the now kernel API header file no longer exists
so even if userspace were using the files the only way to pick
the header up would have been to provide the new uapi path.

[0] http://lwn.net/Articles/507794/
[1] https://backports.wiki.kernel.org

Luis R. Rodriguez (2):
  uapi: update includes for drm content when no kernel API exists
  uapi: remove trailing spaces

 drivers/gpu/drm/drm_crtc.c            |    2 +-
 drivers/gpu/drm/nouveau/nv04_cursor.c |    2 +-
 drivers/staging/omapdrm/omap_crtc.c   |    2 +-
 include/drm/drmP.h                    |    4 ++--
 include/drm/drm_crtc.h                |    4 ++--
 include/uapi/asm-generic/siginfo.h    |    4 ++--
 include/uapi/asm-generic/statfs.h     |    4 ++--
 include/uapi/drm/drm.h                |    2 +-
 include/uapi/drm/radeon_drm.h         |    2 +-
 9 files changed, 13 insertions(+), 13 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] uapi: update includes for drm content when no kernel API exists
  2012-10-12 23:49 [PATCH 0/2] uapi: two minor changes Luis R. Rodriguez
@ 2012-10-12 23:49 ` Luis R. Rodriguez
  2012-10-13 10:33   ` Laurent Pinchart
  2012-10-12 23:49 ` [PATCH 2/2] uapi: remove trailing spaces Luis R. Rodriguez
  1 sibling, 1 reply; 9+ messages in thread
From: Luis R. Rodriguez @ 2012-10-12 23:49 UTC (permalink / raw)
  To: dri-devel, linux-kernel
  Cc: devel, backports, rob, arnd, davej, airlied, bskeggs, alan,
	dhowells, tglx, daniel.vetter, jbarnes, alexander.deucher,
	paulmck, gregkh, laurent.pinchart, Luis R. Rodriguez

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

The UAPI changes split kernel API and userspace API
content onto two separate header files. The userspace
API drm content was moved to include/uapi/drm/ with the
same file name while kernel specific API content was
kept under include/drm/ with the same file name. When
one file was split into two files the kernel header
includes the uapi header and a UAPI prefix was added to
the uapi header for its header guard. When there was no
kernel API content found the uapi header file was the
only one that was kept and the original guard for the
header file was kept. In this particular case the
original users of this header file were not modified
and the uapi header file is expected to be picked up
by path.

This may work well at compilation on the kernel but when
backporting this creates a few complexities. To help with
backporting [0] lets be explicit about the new uapi path
when there is no respective kernel API header file. For
more details on the UAPI changes see the lwn article on
this [1].

[0] https://backports.wiki.kernel.org
[1] http://lwn.net/Articles/507794/

Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: backports@vger.kernel.org

Cc: Rob Clark <rob@ti.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dave Jones <davej@redhat.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: David Howells <dhowells@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 drivers/gpu/drm/drm_crtc.c            |    2 +-
 drivers/gpu/drm/nouveau/nv04_cursor.c |    2 +-
 drivers/staging/omapdrm/omap_crtc.c   |    2 +-
 include/drm/drmP.h                    |    4 ++--
 include/drm/drm_crtc.h                |    4 ++--
 include/uapi/drm/drm.h                |    2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index ef1b221..6486e89 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -35,7 +35,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
-#include <drm/drm_fourcc.h>
+#include <uapi/drm/drm_fourcc.h>
 
 /* Avoid boilerplate.  I'm tired of typing. */
 #define DRM_ENUM_NAME_FN(fnname, list)				\
diff --git a/drivers/gpu/drm/nouveau/nv04_cursor.c b/drivers/gpu/drm/nouveau/nv04_cursor.c
index fe86f0d..7af590f 100644
--- a/drivers/gpu/drm/nouveau/nv04_cursor.c
+++ b/drivers/gpu/drm/nouveau/nv04_cursor.c
@@ -1,5 +1,5 @@
 #include <drm/drmP.h>
-#include <drm/drm_mode.h>
+#include <uapi/drm/drm_mode.h>
 #include "nouveau_drm.h"
 #include "nouveau_reg.h"
 #include "nouveau_crtc.h"
diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c
index 732f2ad..029c9ec 100644
--- a/drivers/staging/omapdrm/omap_crtc.c
+++ b/drivers/staging/omapdrm/omap_crtc.c
@@ -19,7 +19,7 @@
 
 #include "omap_drv.h"
 
-#include "drm_mode.h"
+#include <uapi/drm/drm_mode.h>
 #include "drm_crtc.h"
 #include "drm_crtc_helper.h"
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3fd8280..9030369 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -72,8 +72,8 @@
 #include <linux/workqueue.h>
 #include <linux/poll.h>
 #include <asm/pgalloc.h>
-#include <drm/drm.h>
-#include <drm/drm_sarea.h>
+#include <uapi/drm/drm.h>
+#include <uapi/drm/drm_sarea.h>
 
 #include <linux/idr.h>
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3fa18b7..1012503 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -30,9 +30,9 @@
 #include <linux/types.h>
 #include <linux/idr.h>
 #include <linux/fb.h>
-#include <drm/drm_mode.h>
 
-#include <drm/drm_fourcc.h>
+#include <uapi/drm/drm_mode.h>
+#include <uapi/drm/drm_fourcc.h>
 
 struct drm_device;
 struct drm_mode_set;
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 1e3481e..51ee504 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -628,7 +628,7 @@ struct drm_prime_handle {
 	__s32 fd;
 };
 
-#include <drm/drm_mode.h>
+#include <uapi/drm/drm_mode.h>
 
 #define DRM_IOCTL_BASE			'd'
 #define DRM_IO(nr)			_IO(DRM_IOCTL_BASE,nr)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] uapi: remove trailing spaces
  2012-10-12 23:49 [PATCH 0/2] uapi: two minor changes Luis R. Rodriguez
  2012-10-12 23:49 ` [PATCH 1/2] uapi: update includes for drm content when no kernel API exists Luis R. Rodriguez
@ 2012-10-12 23:49 ` Luis R. Rodriguez
  1 sibling, 0 replies; 9+ messages in thread
From: Luis R. Rodriguez @ 2012-10-12 23:49 UTC (permalink / raw)
  To: dri-devel, linux-kernel
  Cc: devel, backports, rob, arnd, davej, airlied, bskeggs, alan,
	dhowells, tglx, daniel.vetter, jbarnes, alexander.deucher,
	paulmck, gregkh, laurent.pinchart, Luis R. Rodriguez

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

No functional changes.

Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: backports@vger.kernel.org

Cc: Rob Clark <rob@ti.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dave Jones <davej@redhat.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: David Howells <dhowells@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 include/uapi/asm-generic/siginfo.h |    4 ++--
 include/uapi/asm-generic/statfs.h  |    4 ++--
 include/uapi/drm/radeon_drm.h      |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index ba5be7f..34b941ce6 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -252,8 +252,8 @@ typedef struct siginfo {
 
 /*
  * sigevent definitions
- * 
- * It seems likely that SIGEV_THREAD will have to be handled from 
+ *
+ * It seems likely that SIGEV_THREAD will have to be handled from
  * userspace, libpthread transmuting it to SIGEV_SIGNAL, which the
  * thread manager then catches and does the appropriate nonsense.
  * However, everything is written out here so as to not get lost.
diff --git a/include/uapi/asm-generic/statfs.h b/include/uapi/asm-generic/statfs.h
index 0999647..0d79b2f 100644
--- a/include/uapi/asm-generic/statfs.h
+++ b/include/uapi/asm-generic/statfs.h
@@ -36,7 +36,7 @@ struct statfs {
 
 /*
  * ARM needs to avoid the 32-bit padding at the end, for consistency
- * between EABI and OABI 
+ * between EABI and OABI
  */
 #ifndef ARCH_PACK_STATFS64
 #define ARCH_PACK_STATFS64
@@ -57,7 +57,7 @@ struct statfs64 {
 	__statfs_word f_spare[4];
 } ARCH_PACK_STATFS64;
 
-/* 
+/*
  * IA64 and x86_64 need to avoid the 32-bit padding at the end,
  * to be compatible with the i386 ABI
  */
diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h
index 4766c0f..48b0db3 100644
--- a/include/uapi/drm/radeon_drm.h
+++ b/include/uapi/drm/radeon_drm.h
@@ -229,7 +229,7 @@ typedef union {
 #	define R300_WAIT_3D		0x2
 /* these two defines are DOING IT WRONG - however
  * we have userspace which relies on using these.
- * The wait interface is backwards compat new 
+ * The wait interface is backwards compat new
  * code should use the NEW_WAIT defines below
  * THESE ARE NOT BIT FIELDS
  */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] uapi: update includes for drm content when no kernel API exists
  2012-10-12 23:49 ` [PATCH 1/2] uapi: update includes for drm content when no kernel API exists Luis R. Rodriguez
@ 2012-10-13 10:33   ` Laurent Pinchart
  2012-10-13 17:00     ` Luis R. Rodriguez
  0 siblings, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2012-10-13 10:33 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: dri-devel, linux-kernel, devel, backports, rob, arnd, davej,
	airlied, bskeggs, alan, dhowells, tglx, daniel.vetter, jbarnes,
	alexander.deucher, paulmck, gregkh

Hi Luis,

On Friday 12 October 2012 16:49:31 Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
> 
> The UAPI changes split kernel API and userspace API
> content onto two separate header files. The userspace
> API drm content was moved to include/uapi/drm/ with the
> same file name while kernel specific API content was
> kept under include/drm/ with the same file name. When
> one file was split into two files the kernel header
> includes the uapi header and a UAPI prefix was added to
> the uapi header for its header guard. When there was no
> kernel API content found the uapi header file was the
> only one that was kept and the original guard for the
> header file was kept. In this particular case the
> original users of this header file were not modified
> and the uapi header file is expected to be picked up
> by path.
> 
> This may work well at compilation on the kernel but when
> backporting this creates a few complexities.

Could you please provide more details about those complexities ?

> To help with
> backporting [0] lets be explicit about the new uapi path
> when there is no respective kernel API header file. For
> more details on the UAPI changes see the lwn article on
> this [1].
> 
> [0] https://backports.wiki.kernel.org
> [1] http://lwn.net/Articles/507794/
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Cc: devel@driverdev.osuosl.org
> Cc: backports@vger.kernel.org
> 
> Cc: Rob Clark <rob@ti.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dave Jones <davej@redhat.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] uapi: update includes for drm content when no kernel API exists
  2012-10-13 10:33   ` Laurent Pinchart
@ 2012-10-13 17:00     ` Luis R. Rodriguez
  2012-10-16 12:34       ` Laurent Pinchart
  0 siblings, 1 reply; 9+ messages in thread
From: Luis R. Rodriguez @ 2012-10-13 17:00 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, linux-kernel, devel, backports, rob, arnd, davej,
	airlied, bskeggs, alan, dhowells, tglx, daniel.vetter, jbarnes,
	alexander.deucher, paulmck, gregkh

On Sat, Oct 13, 2012 at 3:33 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Luis,
>
> On Friday 12 October 2012 16:49:31 Luis R. Rodriguez wrote:
>> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
>>
>> The UAPI changes split kernel API and userspace API
>> content onto two separate header files. The userspace
>> API drm content was moved to include/uapi/drm/ with the
>> same file name while kernel specific API content was
>> kept under include/drm/ with the same file name. When
>> one file was split into two files the kernel header
>> includes the uapi header and a UAPI prefix was added to
>> the uapi header for its header guard. When there was no
>> kernel API content found the uapi header file was the
>> only one that was kept and the original guard for the
>> header file was kept. In this particular case the
>> original users of this header file were not modified
>> and the uapi header file is expected to be picked up
>> by path.
>>
>> This may work well at compilation on the kernel but when
>> backporting this creates a few complexities.
>
> Could you please provide more details about those complexities ?

Sure, the backported DRM code is as-is code from linux-next. The
header search path includes a copy of a few select header files from
linux-next, the rest are part of the compat [0] project to help with
backporting and the last set are from your own kernel. In this
particular case the UAPI effort pushed into include/uapi/drm a few
files which are now no longer present in the old include/drm/ location
when there was no respective kernel-only APIs exposed. For DRM code
that did not use the new uapi/drm/ path for old header includes this
means that upon backporting the older kernel's header file would be
used obviously leading to a series of compilation failures. By being
explicit about the path, as was done with a few other header files, we
can suck out DRM code intact from the kernel to be compilable for
older kernels. As of the v3.7-rc1 the compat-drivers project [1] will
be providing DRM modules. The new UAPI changes broke compilation on
compat-drivers when compiling DRM code so to fix this we either patch
code taken from the Linux kernel as I have done, force inclusion of a
few specific headers files, or use include_next tricks. It turns out
that forcing inclusion of -I$(M)/include/uapi as a path search prior
to your own kernel's at compilation time did not help. The
include_next trick can work as well but that'd mean synching the UAPI
files regularly into compat. I'd much prefer to have code intact when
possible when backporting so the option I stuck with then was to patch
the code directly and then as part of compat-drivers to always copy
that day's linux-next UAPI headers into the current directory for
compilation. I see no other driver code using the uapi path explicitly
though, is that by design? Would it be wrong to be explicit about
using a UAPI header alone if no kernel API files exist?

[0] https://backports.wiki.kernel.org/index.php/Documentation/compat
[1] https://backports.wiki.kernel.org/

  Luis

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] uapi: update includes for drm content when no kernel API exists
  2012-10-13 17:00     ` Luis R. Rodriguez
@ 2012-10-16 12:34       ` Laurent Pinchart
  2012-10-16 14:38         ` Luis R. Rodriguez
  2012-10-16 23:27           ` David Howells
  0 siblings, 2 replies; 9+ messages in thread
From: Laurent Pinchart @ 2012-10-16 12:34 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: dri-devel, linux-kernel, devel, backports, rob, arnd, davej,
	airlied, bskeggs, alan, dhowells, tglx, daniel.vetter, jbarnes,
	alexander.deucher, paulmck, gregkh

Hi Luis,

On Saturday 13 October 2012 10:00:42 Luis R. Rodriguez wrote:
> On Sat, Oct 13, 2012 at 3:33 AM, Laurent Pinchart wrote:
> > On Friday 12 October 2012 16:49:31 Luis R. Rodriguez wrote:
> >> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
> >> 
> >> The UAPI changes split kernel API and userspace API
> >> content onto two separate header files. The userspace
> >> API drm content was moved to include/uapi/drm/ with the
> >> same file name while kernel specific API content was
> >> kept under include/drm/ with the same file name. When
> >> one file was split into two files the kernel header
> >> includes the uapi header and a UAPI prefix was added to
> >> the uapi header for its header guard. When there was no
> >> kernel API content found the uapi header file was the
> >> only one that was kept and the original guard for the
> >> header file was kept. In this particular case the
> >> original users of this header file were not modified
> >> and the uapi header file is expected to be picked up
> >> by path.
> >> 
> >> This may work well at compilation on the kernel but when
> >> backporting this creates a few complexities.
> > 
> > Could you please provide more details about those complexities ?
> 
> Sure, the backported DRM code is as-is code from linux-next. The
> header search path includes a copy of a few select header files from
> linux-next, the rest are part of the compat [0] project to help with
> backporting and the last set are from your own kernel. In this
> particular case the UAPI effort pushed into include/uapi/drm a few
> files which are now no longer present in the old include/drm/ location
> when there was no respective kernel-only APIs exposed. For DRM code
> that did not use the new uapi/drm/ path for old header includes this
> means that upon backporting the older kernel's header file would be
> used obviously leading to a series of compilation failures. By being
> explicit about the path, as was done with a few other header files, we
> can suck out DRM code intact from the kernel to be compilable for
> older kernels. As of the v3.7-rc1 the compat-drivers project [1] will
> be providing DRM modules. The new UAPI changes broke compilation on
> compat-drivers when compiling DRM code so to fix this we either patch
> code taken from the Linux kernel as I have done, force inclusion of a
> few specific headers files, or use include_next tricks. It turns out
> that forcing inclusion of -I$(M)/include/uapi as a path search prior
> to your own kernel's at compilation time did not help.

Shouldn't that be added as a search path *after* include/linux/ instead of 
before ?

> The include_next trick can work as well but that'd mean synching the UAPI
> files regularly into compat. I'd much prefer to have code intact when
> possible when backporting so the option I stuck with then was to patch
> the code directly and then as part of compat-drivers to always copy
> that day's linux-next UAPI headers into the current directory for
> compilation. I see no other driver code using the uapi path explicitly
> though, is that by design?

As far as I understand that's by design, yes. Kernel code isn't expected to 
reference uapi/ headers directly.

> Would it be wrong to be explicit about using a UAPI header alone if no
> kernel API files exist?

I personally don't really like including such "features" in a mainline just 
for backporting, especially when they go against the spirit of the 
architecture (the uapi/ split design principles in this case). The way we're 
dealing with this in the V4L backport tree 
(http://git.linuxtv.org/media_build.git) is to play with Makefiles, include 
compatibility headers and, if nothing else can work, maintain backport patches 
for mainline code.

> [0] https://backports.wiki.kernel.org/index.php/Documentation/compat
> [1] https://backports.wiki.kernel.org/

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] uapi: update includes for drm content when no kernel API exists
  2012-10-16 12:34       ` Laurent Pinchart
@ 2012-10-16 14:38         ` Luis R. Rodriguez
  2012-10-16 23:27           ` David Howells
  1 sibling, 0 replies; 9+ messages in thread
From: Luis R. Rodriguez @ 2012-10-16 14:38 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, linux-kernel, devel, backports, rob, arnd, davej,
	airlied, bskeggs, alan, dhowells, tglx, daniel.vetter, jbarnes,
	alexander.deucher, paulmck, gregkh

On Tue, Oct 16, 2012 at 5:34 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Luis,
>
> On Saturday 13 October 2012 10:00:42 Luis R. Rodriguez wrote:
>> On Sat, Oct 13, 2012 at 3:33 AM, Laurent Pinchart wrote:
>> > On Friday 12 October 2012 16:49:31 Luis R. Rodriguez wrote:
>> >> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
>> >>
>> >> The UAPI changes split kernel API and userspace API
>> >> content onto two separate header files. The userspace
>> >> API drm content was moved to include/uapi/drm/ with the
>> >> same file name while kernel specific API content was
>> >> kept under include/drm/ with the same file name. When
>> >> one file was split into two files the kernel header
>> >> includes the uapi header and a UAPI prefix was added to
>> >> the uapi header for its header guard. When there was no
>> >> kernel API content found the uapi header file was the
>> >> only one that was kept and the original guard for the
>> >> header file was kept. In this particular case the
>> >> original users of this header file were not modified
>> >> and the uapi header file is expected to be picked up
>> >> by path.
>> >>
>> >> This may work well at compilation on the kernel but when
>> >> backporting this creates a few complexities.
>> >
>> > Could you please provide more details about those complexities ?
>>
>> Sure, the backported DRM code is as-is code from linux-next. The
>> header search path includes a copy of a few select header files from
>> linux-next, the rest are part of the compat [0] project to help with
>> backporting and the last set are from your own kernel. In this
>> particular case the UAPI effort pushed into include/uapi/drm a few
>> files which are now no longer present in the old include/drm/ location
>> when there was no respective kernel-only APIs exposed. For DRM code
>> that did not use the new uapi/drm/ path for old header includes this
>> means that upon backporting the older kernel's header file would be
>> used obviously leading to a series of compilation failures. By being
>> explicit about the path, as was done with a few other header files, we
>> can suck out DRM code intact from the kernel to be compilable for
>> older kernels. As of the v3.7-rc1 the compat-drivers project [1] will
>> be providing DRM modules. The new UAPI changes broke compilation on
>> compat-drivers when compiling DRM code so to fix this we either patch
>> code taken from the Linux kernel as I have done, force inclusion of a
>> few specific headers files, or use include_next tricks. It turns out
>> that forcing inclusion of -I$(M)/include/uapi as a path search prior
>> to your own kernel's at compilation time did not help.
>
> Shouldn't that be added as a search path *after* include/linux/ instead of
> before ?

Ah, therein lies the issue. If backporting no, if backporting you
should seek first for your current directory's include/drm/ dir and
then uapi, and then only later the older kernel's paths:

NOSTDINC_FLAGS := \
        -I$(M)/include/ \
        -I$(M)/include/drm \
        -I$(M)/include/uapi \
        -include $(M)/include/linux/compat-2.6.h \
        $(CFLAGS)

>> The include_next trick can work as well but that'd mean synching the UAPI
>> files regularly into compat. I'd much prefer to have code intact when
>> possible when backporting so the option I stuck with then was to patch
>> the code directly and then as part of compat-drivers to always copy
>> that day's linux-next UAPI headers into the current directory for
>> compilation. I see no other driver code using the uapi path explicitly
>> though, is that by design?
>
> As far as I understand that's by design, yes. Kernel code isn't expected to
> reference uapi/ headers directly.

Did the design consider the case where no respective kernel API header
file would ever exist?

>> Would it be wrong to be explicit about using a UAPI header alone if no
>> kernel API files exist?
>
> I personally don't really like including such "features" in a mainline just
> for backporting,

Sort of ditto. I haven't yet made a strong case for one particular
situation where this would help (but I will in the long run, see
blog), but this example should not be considered one. The patch itself
should be considered in terms of its merit for clarifying usage and
assumptions of uapi. The fact that I came up with it due to issues
when backporting is only secondary.

> especially when they go against the spirit of the
> architecture (the uapi/ split design principles in this case).

The patches, pull request, and even lwn article did not cover the
intent on architecture so if it was the design objective I'd like to
know how that is determined. From the looks of it, this as all
scripted and I do wonder if the case where no kernel API header file
existed was taken into consideration.

> The way we're
> dealing with this in the V4L backport tree
> (http://git.linuxtv.org/media_build.git) is to play with Makefiles, include
> compatibility headers and, if nothing else can work, maintain backport patches
> for mainline code.

Thanks for the heads up, I can certainly keep these patches as
external but when if I see something that typically may be addressed
upstream for non-backport purposes I typically post it for review. I
can't say I'm still satisfied with the answers.

Why would we restrict drivers with direct access to uapi headers,
specially when no kernel API header exists?

  Luis

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] uapi: update includes for drm content when no kernel API exists
  2012-10-16 12:34       ` Laurent Pinchart
@ 2012-10-16 23:27           ` David Howells
  2012-10-16 23:27           ` David Howells
  1 sibling, 0 replies; 9+ messages in thread
From: David Howells @ 2012-10-16 23:27 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: dhowells, Laurent Pinchart, dri-devel, linux-kernel, devel,
	backports, rob, arnd, davej, airlied, bskeggs, alan, tglx,
	daniel.vetter, jbarnes, alexander.deucher, paulmck, gregkh

Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:

> >> The include_next trick can work as well but that'd mean synching the UAPI
> >> files regularly into compat. I'd much prefer to have code intact when
> >> possible when backporting so the option I stuck with then was to patch
> >> the code directly and then as part of compat-drivers to always copy
> >> that day's linux-next UAPI headers into the current directory for
> >> compilation. I see no other driver code using the uapi path explicitly
> >> though, is that by design?
> >
> > As far as I understand that's by design, yes. Kernel code isn't expected to
> > reference uapi/ headers directly.
> 
> Did the design consider the case where no respective kernel API header
> file would ever exist?

I didn't particularly design it such that kernel .c files couldn't access uapi
.h files directly.  I did, however, design it so that my scripts wouldn't have
to touch any .c files where possible, and certainly I didn't want to have to
double up all #includes that refer to KAPI/UAPI split headers.

Ideally, I'd've used #include_next in the KAPI file to refer to the UAPI file
where both exist, but some people have strong objections to that, so I ended
up having to do #include <uapi/...> instead.

I also didn't want to rename the asm/, linux/, etc. prefixes as that would
mandate changing pretty much every #include in the kernel.

For the case where no respective KAPI file exists, it was considered and it is
handled.  This is done by adding extra -I flags, for example:

	-I include
	-I include/uapi

so looking for linux/foo.h, say, will look first for include/linux/foo.h and
then for include/uapi/linux/foo.h.

David

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] uapi: update includes for drm content when no kernel API exists
@ 2012-10-16 23:27           ` David Howells
  0 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2012-10-16 23:27 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: devel, backports, arnd, gregkh, airlied, daniel.vetter,
	linux-kernel, dri-devel, dhowells, jbarnes, Laurent Pinchart,
	davej, alexander.deucher, rob, tglx, paulmck, alan, bskeggs

Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:

> >> The include_next trick can work as well but that'd mean synching the UAPI
> >> files regularly into compat. I'd much prefer to have code intact when
> >> possible when backporting so the option I stuck with then was to patch
> >> the code directly and then as part of compat-drivers to always copy
> >> that day's linux-next UAPI headers into the current directory for
> >> compilation. I see no other driver code using the uapi path explicitly
> >> though, is that by design?
> >
> > As far as I understand that's by design, yes. Kernel code isn't expected to
> > reference uapi/ headers directly.
> 
> Did the design consider the case where no respective kernel API header
> file would ever exist?

I didn't particularly design it such that kernel .c files couldn't access uapi
.h files directly.  I did, however, design it so that my scripts wouldn't have
to touch any .c files where possible, and certainly I didn't want to have to
double up all #includes that refer to KAPI/UAPI split headers.

Ideally, I'd've used #include_next in the KAPI file to refer to the UAPI file
where both exist, but some people have strong objections to that, so I ended
up having to do #include <uapi/...> instead.

I also didn't want to rename the asm/, linux/, etc. prefixes as that would
mandate changing pretty much every #include in the kernel.

For the case where no respective KAPI file exists, it was considered and it is
handled.  This is done by adding extra -I flags, for example:

	-I include
	-I include/uapi

so looking for linux/foo.h, say, will look first for include/linux/foo.h and
then for include/uapi/linux/foo.h.

David

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-10-16 23:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-12 23:49 [PATCH 0/2] uapi: two minor changes Luis R. Rodriguez
2012-10-12 23:49 ` [PATCH 1/2] uapi: update includes for drm content when no kernel API exists Luis R. Rodriguez
2012-10-13 10:33   ` Laurent Pinchart
2012-10-13 17:00     ` Luis R. Rodriguez
2012-10-16 12:34       ` Laurent Pinchart
2012-10-16 14:38         ` Luis R. Rodriguez
2012-10-16 23:27         ` David Howells
2012-10-16 23:27           ` David Howells
2012-10-12 23:49 ` [PATCH 2/2] uapi: remove trailing spaces Luis R. Rodriguez

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.