Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>"

From: Daniel Vetter
Date: Fri Aug 19 2016 - 13:11:41 EST


On Fri, Aug 19, 2016 at 5:22 PM, Marek OlÅÃk <maraeo@xxxxxxxxx> wrote:
> On Fri, Aug 19, 2016 at 4:52 PM, Mikko Rapeli <mikko.rapeli@xxxxxx> wrote:
>> On Fri, Aug 19, 2016 at 04:26:40PM +0200, Christian KÃnig wrote:
>>> Am 19.08.2016 um 15:50 schrieb Marek OlÅÃk:
>>> >From: Marek OlÅÃk <marek.olsak@xxxxxxx>
>>> >
>>> >This reverts commit 2ce9dde0d47f2f94ab25c73a30596a7328bcdf1f.
>>> >
>>> >See the comment in the code. Basically, don't do cleanups in this header.
>>> >
>>> >Signed-off-by: Marek OlÅÃk <marek.olsak@xxxxxxx>
>>>
>>> I completely agree with you that this was a bad move, but I fear that we
>>> will run into opposition with that.
>>>
>>> Adding Mikko Rapeli who made the reverted patch to comment.
>>
>> But this header is part of Linux kernel uapi. Remove it from there too then.
>
> That's a good idea, but it really is a uapi header in the sense that
> it defines the kernel driver interface for a specific kernel version.
> However, it is not a header that the userspace stack should include,
> because userspace should get it from libdrm. (it makes userspace more
> independent from the currently running kernel)

Please don't remove it from the kernel side if it's included in
libdrm. Emil&I just spent a bit of time making sure those two sets of
headers match when we produce them using the make headers_install
target.

drm is indeed special, as in our headers aren't shipped with the
kernel-headers package but libdrm. But otherwise they are supposed to
work exactly like any other uapi headers. No need to move them out of
the uapi headers - I'd even say that would be bad since it removes the
visibility and clear marker that this header must be considered uapi!

Also the very loud comment at the top is definitely misleading,
there's nothing that guarnatees that the libdrm and kernel copy are in
sync when you build or run userspace. Also maybe for context, but what
exactly is the problem with the __ types?

Adding Emil.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch