[PATCH v1 0/6] drm: make headers self-contained

[PATCH v1 0/6] drm: make headers self-contained

[Sam Ravnborg]

While removing use of drmP.h from files in drm/* I
noticed that I had to add the same include files due to
build errors in the header files.

It is better to let the header files include what is necessary
and let the users pull in only the additional headers files required.
So I went ahead and made the header files self-conatined.
(I did not check if this made any includes redundant in some files,
I do not have tooling in place to do so).

These patches are preparation for removing use of drmP.h in all
the files in drm/*

I have the patchset ready - one patch per file.
It is a lot of small patches (59) and maybe it is better to
squash them all into one big patch. Any opinions on this?

I will await feedback on this patchset before sending the
patch(es) to remove drmP.h in drm/*

	Sam

Sam Ravnborg (6):
      drm: make drm_auth.h self contained
      drm: make drm_legacy.h self-contained
      drm: make drm_crtc_internal.h self-contained
      drm: make drm_internal.h self-contained
      drm: make drm_legacy.h self-contained
      drm: make drm_trace.h self-contained

 drivers/gpu/drm/drm_crtc_internal.h | 24 ++++++++++++++++++++----
 drivers/gpu/drm/drm_internal.h      | 10 +++++++++-
 drivers/gpu/drm/drm_legacy.h        |  4 ++++
 drivers/gpu/drm/drm_trace.h         |  2 ++
 include/drm/drm_auth.h              |  7 +++++++
 include/drm/drm_legacy.h            |  2 ++
 6 files changed, 44 insertions(+), 5 deletions(-)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 01/06] drm: make drm_auth.h self contained

[Sam Ravnborg]

Do not require users of drm_auth.h to include other files just to
let it build.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 include/drm/drm_auth.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index 871008118bab..f34b7b842976 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -28,6 +28,13 @@
 #ifndef _DRM_AUTH_H_
 #define _DRM_AUTH_H_
 
+#include <linux/idr.h>
+#include <linux/kref.h>
+#include <linux/wait.h>
+
+struct drm_file;
+struct drm_hw_lock;
+
 /*
  * Legacy DRI1 locking data structure. Only here instead of in drm_legacy.h for
  * include ordering reasons.
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 02/06] drm: make drm_legacy.h self-contained

[Sam Ravnborg]

Do not require users of drm_legacy.h to include other files
just to let it build.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 include/drm/drm_legacy.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
index 2182a56ac421..1d32acd712de 100644
--- a/include/drm/drm_legacy.h
+++ b/include/drm/drm_legacy.h
@@ -1,10 +1,12 @@
 #ifndef __DRM_DRM_LEGACY_H__
 #define __DRM_DRM_LEGACY_H__
 
+#include <drm/drm.h>
 #include <drm/drm_auth.h>
 #include <drm/drm_hashtab.h>
 
 struct drm_device;
+struct file;
 
 /*
  * Legacy driver interfaces for the Direct Rendering Manager
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 03/06] drm: make drm_crtc_internal.h self-contained

[Sam Ravnborg]

Do not require users of drm_crtc_internal.h to include other
files just to let it build

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_internal.h | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 0719a235d6cc..c78a44fad13d 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -31,14 +31,30 @@
  * and are not exported to drivers.
  */
 
-enum drm_mode_status;
+#include <linux/types.h>
+
+enum drm_color_encoding;
+enum drm_color_range;
 enum drm_connector_force;
+enum drm_mode_status;
 
-struct drm_display_mode;
-struct work_struct;
-struct drm_connector;
+struct drm_atomic_state;
 struct drm_bridge;
+struct drm_connector;
+struct drm_crtc;
+struct drm_device;
+struct drm_display_mode;
+struct drm_file;
+struct drm_framebuffer;
+struct drm_mode_create_dumb;
+struct drm_mode_fb_cmd2;
+struct drm_mode_fb_cmd;
+struct drm_mode_object;
+struct drm_plane;
+struct drm_property;
 struct edid;
+struct kref;
+struct work_struct;
 
 /* drm_crtc.c */
 int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj,
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 04/06] drm: make drm_internal.h self-contained

[Sam Ravnborg]

Do not require users of drm_internal.h to include other
files just to let it build

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/drm_internal.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index e6281d9f9c87..76bd4b81c5b1 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -28,8 +28,16 @@
 
 #define DRM_IF_VERSION(maj, min) (maj << 16 | min)
 
-struct drm_prime_file_private;
+struct dentry;
 struct dma_buf;
+struct drm_connector;
+struct drm_crtc;
+struct drm_framebuffer;
+struct drm_gem_object;
+struct drm_master;
+struct drm_minor;
+struct drm_prime_file_private;
+struct drm_printer;
 
 /* drm_file.c */
 extern struct mutex drm_global_mutex;
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 05/06] drm: make drm_legacy.h self-contained

[Sam Ravnborg]

Do not require users of drm_ilegacy.h to include other
files just to let it build

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/drm_legacy.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
index 013ccdfd90be..1be3ea320474 100644
--- a/drivers/gpu/drm/drm_legacy.h
+++ b/drivers/gpu/drm/drm_legacy.h
@@ -29,11 +29,15 @@
  * drivers use them, and removing them are API breaks.
  */
 #include <linux/list.h>
+
+#include <drm/drm.h>
+#include <drm/drm_device.h>
 #include <drm/drm_legacy.h>
 
 struct agp_memory;
 struct drm_device;
 struct drm_file;
+struct drm_buf_desc;
 
 /*
  * Generic DRM Contexts
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 06/06] drm: make drm_trace.h self-contained

[Sam Ravnborg]

Do not require users of drm_trace.h to include other
files just to let it build

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/drm_trace.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_trace.h b/drivers/gpu/drm/drm_trace.h
index baccc63db106..471eb927474b 100644
--- a/drivers/gpu/drm/drm_trace.h
+++ b/drivers/gpu/drm/drm_trace.h
@@ -6,6 +6,8 @@
 #include <linux/types.h>
 #include <linux/tracepoint.h>
 
+struct drm_file;
+
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM drm
 #define TRACE_INCLUDE_FILE drm_trace
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 0/6] drm: make headers self-contained

[Daniel Vetter]

On Sun, May 19, 2019 at 04:20:30PM +0200, Sam Ravnborg wrote:
> While removing use of drmP.h from files in drm/* I
> noticed that I had to add the same include files due to
> build errors in the header files.
> 
> It is better to let the header files include what is necessary
> and let the users pull in only the additional headers files required.
> So I went ahead and made the header files self-conatined.
> (I did not check if this made any includes redundant in some files,
> I do not have tooling in place to do so).

I think it'd be great to make sure this keeps being the case. Jani Nikula
just pointed me at some neat stuff in drm/i915/Makefile.header-test.

I think it'd be nice to have something similar for drm headers in
drivers/gpu/drm, behind a Kconfig option perhaps so that it doesn't upset
people, but 0day will still spot issues. That would also make the series
easier to review, since each patch would add the header it fixes to that
build target.
 
> These patches are preparation for removing use of drmP.h in all
> the files in drm/*
> 
> I have the patchset ready - one patch per file.
> It is a lot of small patches (59) and maybe it is better to
> squash them all into one big patch. Any opinions on this?

Imo fine to have a huge patch pile for this.

> I will await feedback on this patchset before sending the
> patch(es) to remove drmP.h in drm/*

Sounds good.

Cheers, Daniel

> 
> 	Sam
> 
> Sam Ravnborg (6):
>       drm: make drm_auth.h self contained
>       drm: make drm_legacy.h self-contained
>       drm: make drm_crtc_internal.h self-contained
>       drm: make drm_internal.h self-contained
>       drm: make drm_legacy.h self-contained
>       drm: make drm_trace.h self-contained
> 
>  drivers/gpu/drm/drm_crtc_internal.h | 24 ++++++++++++++++++++----
>  drivers/gpu/drm/drm_internal.h      | 10 +++++++++-
>  drivers/gpu/drm/drm_legacy.h        |  4 ++++
>  drivers/gpu/drm/drm_trace.h         |  2 ++
>  include/drm/drm_auth.h              |  7 +++++++
>  include/drm/drm_legacy.h            |  2 ++
>  6 files changed, 44 insertions(+), 5 deletions(-)
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 0/6] drm: make headers self-contained

[Sam Ravnborg]

Hi Daniel.

On Mon, May 20, 2019 at 08:45:26PM +0200, Daniel Vetter wrote:
> On Sun, May 19, 2019 at 04:20:30PM +0200, Sam Ravnborg wrote:
> > While removing use of drmP.h from files in drm/* I
> > noticed that I had to add the same include files due to
> > build errors in the header files.
> > 
> > It is better to let the header files include what is necessary
> > and let the users pull in only the additional headers files required.
> > So I went ahead and made the header files self-conatined.
> > (I did not check if this made any includes redundant in some files,
> > I do not have tooling in place to do so).
> 
> I think it'd be great to make sure this keeps being the case. Jani Nikula
> just pointed me at some neat stuff in drm/i915/Makefile.header-test.
> 

> I think it'd be nice to have something similar for drm headers in
> drivers/gpu/drm, behind a Kconfig option perhaps so that it doesn't upset
> people, but 0day will still spot issues. That would also make the series
> easier to review, since each patch would add the header it fixes to that
> build target.
I would like to wait with this until Jani's general solution:
https://www.spinics.net/lists/linux-kbuild/msg21839.html
hits mainline.

I can also duplicate what Jani already did and then migrate to the
general solution when it is ready.
Hmm. I think this is the best way forward.
So we can have all the other functionality in place.
Let me give this a shot and get back with a few patches.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel