mbox series

[v1,0/2] drm: drop uapi dependencies from include/drm

Message ID 20190622121156.17217-1-sam@ravnborg.org (mailing list archive)
Headers show
Series drm: drop uapi dependencies from include/drm | expand

Message

Sam Ravnborg June 22, 2019, 12:11 p.m. UTC
include/drm/* shall have no or at least minimal dependencies
to include/uapi/drm/*.
Following two patches do a small effort to drop such dependencies.

After these patches there are two users
of uapi/drm/drm.h left in include/drm:

drm_file.h:
- needs drm_magic_t
  drm_magic_t is a simple typedef, a simple unsigned int would do the trick

drm_legacy.h
- needs drm_map_type and drm_map_flags. Seems legit.

I did not spend time to analyze further dependencies.

Patches are build tested.

	Sam

Sam Ravnborg (2):
      drm: drop uapi dependency from drm_print.h
      drm: drop uapi dependency from drm_vblank.h

 include/drm/drm_print.h  | 4 +---
 include/drm/drm_vblank.h | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

Comments

Daniel Vetter June 24, 2019, 8:21 a.m. UTC | #1
On Sat, Jun 22, 2019 at 02:11:54PM +0200, Sam Ravnborg wrote:
> include/drm/* shall have no or at least minimal dependencies
> to include/uapi/drm/*.
> Following two patches do a small effort to drop such dependencies.
> 
> After these patches there are two users
> of uapi/drm/drm.h left in include/drm:
> 
> drm_file.h:
> - needs drm_magic_t
>   drm_magic_t is a simple typedef, a simple unsigned int would do the trick

I think we could switch over to unsigned int for this.

> drm_legacy.h
> - needs drm_map_type and drm_map_flags. Seems legit.

enum in uapi, sweet (never do this, it's not portable across compilers,
#defines all the way). But yeah seems legit, plus we don't care about
drm_legacy.h

> 
> I did not spend time to analyze further dependencies.
> 
> Patches are build tested.
> 
> 	Sam
> 
> Sam Ravnborg (2):
>       drm: drop uapi dependency from drm_print.h
>       drm: drop uapi dependency from drm_vblank.h

On both Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
>  include/drm/drm_print.h  | 4 +---
>  include/drm/drm_vblank.h | 1 -
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
>
Emil Velikov June 24, 2019, 10:07 a.m. UTC | #2
On Mon, 24 Jun 2019 at 09:21, Daniel Vetter <daniel@ffwll.ch> wrote:

> > drm_legacy.h
> > - needs drm_map_type and drm_map_flags. Seems legit.
>
> enum in uapi, sweet (never do this, it's not portable across compilers,
> #defines all the way).

Don't look too closely then
$ git grep enum -- include/uapi/drm/ | wc -l
96

A handful of those are legacy stuff, yet quite a few are not :-\

HTH
-Emil
Jani Nikula June 24, 2019, 10:55 a.m. UTC | #3
On Mon, 24 Jun 2019, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On Mon, 24 Jun 2019 at 09:21, Daniel Vetter <daniel@ffwll.ch> wrote:
>
>> > drm_legacy.h
>> > - needs drm_map_type and drm_map_flags. Seems legit.
>>
>> enum in uapi, sweet (never do this, it's not portable across compilers,
>> #defines all the way).
>
> Don't look too closely then
> $ git grep enum -- include/uapi/drm/ | wc -l
> 96
>
> A handful of those are legacy stuff, yet quite a few are not :-\

Enums are just fine for *values* as long as they're not used as types in
struct members or function parameters or return types, right?

So we do have some we've screwed up, but it's not nearly as bad as the
git grep above would lead you to believe.

The ones in question are used in struct drm_map which is used in
DRM_IOCTL_GET_MAP, DRM_IOCTL_ADD_MAP, and DRM_IOCTL_RM_MAP.

BR,
Jani.
Sam Ravnborg June 24, 2019, 8:39 p.m. UTC | #4
Hi Daniel.

> > drm_file.h:
> > - needs drm_magic_t
> >   drm_magic_t is a simple typedef, a simple unsigned int would do the trick
> 
> I think we could switch over to unsigned int for this.

OK, will send a v2 with this addressed.
There is some preparatory patches to fix build breakage when the include
file is dropped.
> 
> > drm_legacy.h
> > - needs drm_map_type and drm_map_flags. Seems legit.
> 
> enum in uapi, sweet (never do this, it's not portable across compilers,
> #defines all the way). But yeah seems legit, plus we don't care about
> drm_legacy.h
And this part I will happily ignore.
Maybe one day I will check what other dependencies there are.
On a rainy day or when there are no more users of drmP.h or so.

	Sam