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

[Marek Olšák]

On Sat, Aug 20, 2016 at 1:08 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 20 August 2016 at 11:05, Marek Olšák <maraeo at gmail.com> wrote:
>> On Sat, Aug 20, 2016 at 12:54 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> On 19 August 2016 at 15:26, Christian König <deathsimple at vodafone.de> wrote:
>>>> Am 19.08.2016 um 15:50 schrieb Marek Olšák:
>>>>>
>>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>>
>>>>> 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 at amd.com>
>>>>
>>>>
>>>> I completely agree with you that this was a bad move, but I fear that we
>>>> will run into opposition with that.
>>>>
>>> Please check the facts before introducing RATHER ANNOYING AND HARD TO
>>> READ COMMENT IN ALL CAPS.
>>>
>>> Story time:
>>> I was dreaming of a day were we can stop installing these headers,
>>> thus making deprecation a bit easier process.
>>> Yet after failing to convince Dave and Daniel on a number of occasions
>>> I've accepted that those headers _are_ here to stay. And yes they
>>> _are_ the UAPI, even though no applications are meant to use them but
>>> the libdrm 'version'.
>>> Thus any changes to the libdrm ones should be a mirror of the ones
>>> here and libdrm should _not_ differ.
>>>
>>> But let's ignore all that and imagine that those headers are _not_
>>> UAPI. That gives us even greater reason to _not_ use the uintx_t types
>>> but the kernel __uX ones. The series that did these changes had a fair
>>> few references why we want that.
>>>
>>> Yes, I can imagine that the situation isn't ideal, and/or not that
>>> clear. Then again a check with git log should have straightened things
>>> out.
>>> If not _please_ help us improve this (documentation and/or others).
>>>
>>>
>>> And last but not least, please share with up what inspired this -
>>> (build/runtime) regression, attempted sync with libdrm, other ?
>>
>> Syncing with libdrm became difficult.
> Actually it should be easier now. Perhaps the radeon one was always a
> good citizen, but sadly that was not the case for the rest.
>
>> I'd like the diff between kernel
>> and libdrm to be as small as possible.
>>
> I believe we all agree on this one :-)
>
>> We must take into account that the uapi headers can potentially be
>> implemented by a different OS.
> Agreed. Have you looked at the 'compatibility layer' in drm.h ?
>
>> That's why they are in libdrm and
>> that's why nobody should make random changes to them in the kernel
>> tree. Do not think like a kernel developer isolated in Linux and just
>> consider the broader use case. If you do, you'll realize that it
>> simply doesn't make sense to use the __uX types here.
>>
> Ftr, like Rob (and maybe others) I believe that using __uX (in the
> kernel) is a bit odd, and opting for the stdint.h types should happen.
> But until/if that happens we have to live with the __uX ones.
>
> That said, I have poked various BSD people on a number of occasions,
> (hopefully) inspiring them to upstream their changes in a compatible
> way. Thus the whole "don't think like a kernel developer" doesn't
> really apply here :-\
>
> I'm simply one of the few fools^wpeople trying to make things OK for
> most (since one can never please everyone, all the time).
>
> IIRC the FreeBSD/DragonFly people had some issues with their
> compatibility layer since the kernel and userspace drm.h were
> divergent "by design" [1]. To make it even 'better' there's even two
> difference versions of drm.h in their kernel itself [2].
>
> What I am for is a discussion how to resolve things. Although expect
> resistance if you're thinking about applying tape, in order to fix
> somethings that's 'broken' elsewhere.
>
> If you or any !Linux folks are around on XDC we should really sit down
> and untangle some/all of these issues.

It's not 100% certain but it looks like we won't be there.

We need the uapi headers to be the same as libdrm ones to make syncing
easier. There is not much else to discuss here really. We (AMD) are
also the ones who have to work with these headers the most, not you, not Mikko.

While I understand some people want to discuss this further, these
patches must land first in order bring back the compatibility with
libdrm. After that, we can discuss the possible solutions and
everybody interested in a better solution *that will take libdrm into
account* can join. For now, I have to expect that those discussions
might also lead nowhere and I don't wanna be stuck with bad uapi
headers in the kernel forever.

Marek