[v1,11/11] drm: drop uapi dependency from drm_file.h

Submitted by Sam Ravnborg on July 18, 2019, 4:15 p.m.

Details

Message ID 20190718161507.2047-12-sam@ravnborg.org
State New
Headers show
Series "drm: header maintenance" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Sam Ravnborg July 18, 2019, 4:15 p.m.
drm_file used drm_magic_t from uapi/drm/drm.h.
This is a simple unsigned int.
Just opencode it as such to break the dependency from this header file
to uapi.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Eric Anholt <eric@anholt.net>
---
 include/drm/drm_file.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 67af60bb527a..046cd1bf91eb 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -34,8 +34,6 @@ 
 #include <linux/completion.h>
 #include <linux/idr.h>
 
-#include <uapi/drm/drm.h>
-
 #include <drm/drm_prime.h>
 
 struct dma_fence;
@@ -227,7 +225,7 @@  struct drm_file {
 	struct pid *pid;
 
 	/** @magic: Authentication magic, see @authenticated. */
-	drm_magic_t magic;
+	unsigned int magic;
 
 	/**
 	 * @lhead:

Comments

On Thu, Jul 18, 2019 at 06:15:07PM +0200, Sam Ravnborg wrote:
> drm_file used drm_magic_t from uapi/drm/drm.h.
> This is a simple unsigned int.
> Just opencode it as such to break the dependency from this header file
> to uapi.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>

Passes my build tests, thanks for the clean-ups!

Reviewed-by: Sean Paul <sean@poorly.run>

> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Eric Anholt <eric@anholt.net>
> ---
>  include/drm/drm_file.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 67af60bb527a..046cd1bf91eb 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -34,8 +34,6 @@
>  #include <linux/completion.h>
>  #include <linux/idr.h>
>  
> -#include <uapi/drm/drm.h>
> -
>  #include <drm/drm_prime.h>
>  
>  struct dma_fence;
> @@ -227,7 +225,7 @@ struct drm_file {
>  	struct pid *pid;
>  
>  	/** @magic: Authentication magic, see @authenticated. */
> -	drm_magic_t magic;
> +	unsigned int magic;
>  
>  	/**
>  	 * @lhead:
> -- 
> 2.20.1
>
Am 18.07.19 um 18:15 schrieb Sam Ravnborg:
> drm_file used drm_magic_t from uapi/drm/drm.h.

> This is a simple unsigned int.

> Just opencode it as such to break the dependency from this header file

> to uapi.


Mhm, why do you want to remove UAPI dependency here in the first place?

I mean the type can't change because it is UAPI, but it is rather bad 
for a documentation point of view.

Christian.

>

> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>

> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> Cc: Sean Paul <seanpaul@chromium.org>

> Cc: Liviu Dudau <Liviu.Dudau@arm.com>

> Cc: Chris Wilson <chris@chris-wilson.co.uk>

> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> Cc: Maxime Ripard <maxime.ripard@bootlin.com>

> Cc: David Airlie <airlied@linux.ie>

> Cc: Daniel Vetter <daniel@ffwll.ch>

> Cc: Jani Nikula <jani.nikula@intel.com>

> Cc: Eric Anholt <eric@anholt.net>

> ---

>   include/drm/drm_file.h | 4 +---

>   1 file changed, 1 insertion(+), 3 deletions(-)

>

> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h

> index 67af60bb527a..046cd1bf91eb 100644

> --- a/include/drm/drm_file.h

> +++ b/include/drm/drm_file.h

> @@ -34,8 +34,6 @@

>   #include <linux/completion.h>

>   #include <linux/idr.h>

>   

> -#include <uapi/drm/drm.h>

> -

>   #include <drm/drm_prime.h>

>   

>   struct dma_fence;

> @@ -227,7 +225,7 @@ struct drm_file {

>   	struct pid *pid;

>   

>   	/** @magic: Authentication magic, see @authenticated. */

> -	drm_magic_t magic;

> +	unsigned int magic;

>   

>   	/**

>   	 * @lhead:
Hi Christian.

Thanks for your comments and very valid question.

On Fri, Jul 19, 2019 at 06:56:47AM +0000, Koenig, Christian wrote:
> Am 18.07.19 um 18:15 schrieb Sam Ravnborg:
> > drm_file used drm_magic_t from uapi/drm/drm.h.
> > This is a simple unsigned int.
> > Just opencode it as such to break the dependency from this header file
> > to uapi.
> 
> Mhm, why do you want to remove UAPI dependency here in the first place?

The idea is to make include/drm/* independent of uapi/drm/* so the
header files are less tangled up thus easier to read and comprehend.

.c files that requires uapi can then include the uapi headers.

For now my focus was solely on uapi/drm/drm.h - so I dunno if this
is an achievable goal for include/drm/*.

For uapi/drm/* headers things are more clear. They shall be
independent of include/drm/* as they are exported.

> I mean the type can't change because it is UAPI, but it is rather bad 
> for a documentation point of view.

For a widely used type I would agree.
For struct auth, that is ony used in drm_auth.c then the documentation
impact is minor. But your point is indeed valid.
(struct auth has one field of type magic_t).

I will await further feedback before we decide to apply this patch or
not.
The patches that pushes include of drm/drm.h to the respective .c
files are legit as we drop the dependency on an indirectly included
header file. I will process these patches.

	Sam