[PATCH 0/9] uapi: drm: fixes for userspace compilation

[Emil Velikov]

On 17 November 2015 at 20:13, Dave Airlie <airlied at gmail.com> wrote:
>>
>> The kernel UAPI headers:
>>  - Used by the kernel modules and userspace(?)
>>  - Installed in /usr/include/drm
>>  - Many distributions do _not_ ship them
>>  - Broken for years (mostly fixed with Mikko's earlier patches)
>>  - For the above two reasons - most (all?) userspace uses the libdrm ones.
>
> No userspace uses them at all. Hence why we tend not to care less.
>
> Userspace has never used them, and I'm not sure why userspace would
> need to use them at this point.
>
>>
>> The libdrm headers:
>>  - Mostly preprocessed version of the above, used solely by userspace
>>  - Installed in /usr/include/libdrm
>>  - All distros ship them
>>  - Desperately out of sync wrt the above
>
> I don't think they are that out of sync, it would be impossible for userspace
> to use new kernel features if they were that out of sync.
>
I beg to differ. And the following diff stat seems to confirm it. Note
that we have a massive hunk of #ifndef-erry in mesa
(src/gallium/winsys/drm/radeon) and a fair bit in IGT (some of which
to keep libdrm requirement low). There is also a ton in the intel ddx,
but that's mostly because they don't like to zero initialise (use c99
initializers for) drm structs. Last time I've looked weston also had a
few.

 exynos/exynos_drm.h       | 236 ++++++++++++++--
 freedreno/msm/msm_drm.h   | 103 ++++---
 include/drm/amdgpu_drm.h  |   2 +-
 include/drm/drm.h         |  66 +++--
 include/drm/drm_fourcc.h  |  17 +-
 include/drm/drm_mode.h    | 165 +++++++-----
 include/drm/drm_sarea.h   |   6 +-
 include/drm/i915_drm.h    |  46 +++-
 include/drm/mga_drm.h     |   6 +-
 include/drm/nouveau_drm.h |  99 ++-----
 include/drm/qxl_drm.h     |   2 +-
 include/drm/r128_drm.h    |   2 +
 include/drm/radeon_drm.h  |  44 ++-
 include/drm/savage_drm.h  |  12 +-
 include/drm/sis_drm.h     |   8 +-
 include/drm/tegra_drm.h   |   5 +-
 include/drm/via_drm.h     |   6 +-
 include/drm/vmwgfx_drm.h  | 674 +++++++++++++++++++++++++++++++++++++++-------
 omap/omap_drm.h           |  46 ++--
 19 files changed, 1125 insertions(+), 420 deletions(-)

Not to mention the missing {armada,i810,virtgpu}_drm.h. Fwiw I'm the
one guilty for i810 one - there haven't been any users for it so I
nuked it. Can bring it back if people prefer. On the armada front I'm
leaning towards that we should nuke the header as it's for a DC only
device.

>> With the above said:
>>  - I was thinking about hiding the UAPI ones, although Dave suggested
>> against it.
>
> They are a legitimate kernel API, and show be exposed, probably
> should be made more useable outside the kernel, but for now
> using them inside the kernel is their primary use case.
>
I never suggested that there should be no way for userspace to get
access to them. libdrm seems like a good enough candidate imho. It
takes care of any specific that may or may not be needed, it's used
(almost?) everywhere and existing userspace just works :)

>>  - Doing s|drm/drm.h|drm.h| will break compilation:
>>    + for the kernel - as we don't add the foo/drm/ to the include directive,
>>    + for any hacked up userspace - very unlikely that they have the
>> include, considering that things work without it.
>>  - Differences wrt libdrm userspace - all of which can be handled with
>> simple sed
>>    + __KERNEL__ never defined so we can just ignore it
>>    + __user
>>    + drm/drm.h
>>    + others ?
>
> The kernel header geneartion script should strip a lot of stuff out,
> please look at the headers post kernel install not the ones in the
> kernel tree for comparison.
>
> or ones in usr/include/drm inside a built kernel tree.
>
People are like everything else in nature - seeking the path or least
resistance. If you ask them to do 2 steps they will either do them
once in a blue moon, or simply ignore the whole thing, sadly :-(

If we have/need anything fancy - sure lets use the said infra.
Otherwise let's add one line of sed script in libdrm and be done with
it :-P

Cheers,
Emil