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

[Daniel Vetter]

On Fri, Aug 19, 2016 at 7:11 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> 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.

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