* [PATCH] drm: headers: Add neccessary include files and guards
@ 2019-03-26 23:23 Ahmed S. Darwish
2019-03-27 8:30 ` Daniel Vetter
0 siblings, 1 reply; 3+ messages in thread
From: Ahmed S. Darwish @ 2019-03-26 23:23 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter
Otherwise gcc will complain about unknown types, and declarations
inside parameter lists, if "drm_internal.h" is used in C files with
less headers than what's now typically done under drivers/gpu/drm/.
Signed-off-by: Ahmed S. Darwish <darwish.07@gmail.com>
---
Notes:
This was triggered by the in-development drm-panic code.
drivers/gpu/drm/drm_crtc_helper_internal.h | 5 +++++
drivers/gpu/drm/drm_crtc_internal.h | 5 +++++
drivers/gpu/drm/drm_internal.h | 12 ++++++++++++
include/drm/drm_auth.h | 9 +++++++++
4 files changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h
index b5ac1581e623..ee8d8682db09 100644
--- a/drivers/gpu/drm/drm_crtc_helper_internal.h
+++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
@@ -20,6 +20,9 @@
* OTHER DEALINGS IN THE SOFTWARE.
*/
+#ifndef _DRM_CRTC_HELPER_INTERNAL_H
+#define _DRM_CRTC_HELPER_INTERNAL_H
+
/*
* This header file contains mode setting related functions and definitions
* which are only used within the drm kms helper module as internal
@@ -75,3 +78,5 @@ enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
const struct drm_display_mode *mode);
enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode);
+
+#endif /* _DRM_CRTC_HELPER_INTERNAL_H */
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 216f2a9ee3d4..840c1cb2eb7b 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -25,6 +25,9 @@
* OTHER DEALINGS IN THE SOFTWARE.
*/
+#ifndef _DRM_CRTC_INTERNAL_H
+#define _DRM_CRTC_INTERNAL_H
+
/*
* This header file contains mode setting related functions and definitions
* which are only used within the drm module as internal implementation details
@@ -252,3 +255,5 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
void drm_mode_fixup_1366x768(struct drm_display_mode *mode);
void drm_reset_display_info(struct drm_connector *connector);
u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid);
+
+#endif /* _DRM_CRTC_INTERNAL_H */
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 251d67e04c2d..a1b68836b048 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -21,7 +21,17 @@
* OTHER DEALINGS IN THE SOFTWARE.
*/
+#ifndef _DRM_INTERNAL_H
+#define _DRM_INTERNAL_H
+
+#include <linux/mutex.h>
+
+#include <drm/drm_auth.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_device.h>
+#include <drm/drm_file.h>
#include <drm/drm_ioctl.h>
+#include <drm/drm_print.h>
#define DRM_IF_MAJOR 1
#define DRM_IF_MINOR 4
@@ -191,3 +201,5 @@ int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
const struct drm_framebuffer *fb);
int drm_framebuffer_debugfs_init(struct drm_minor *minor);
+
+#endif /* _DRM_INTERNAL_H */
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index 86bff9841b54..a1a59fbf26b1 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -28,6 +28,15 @@
#ifndef _DRM_AUTH_H_
#define _DRM_AUTH_H_
+#include <linux/kref.h>
+#include <linux/idr.h>
+#include <linux/wait.h>
+
+#include <uapi/drm/drm.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_file.h>
+
/*
* Legacy DRI1 locking data structure. Only here instead of in drm_legacy.h for
* include ordering reasons.
--
darwi
http://darwish.chasingpointers.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm: headers: Add neccessary include files and guards
2019-03-26 23:23 [PATCH] drm: headers: Add neccessary include files and guards Ahmed S. Darwish
@ 2019-03-27 8:30 ` Daniel Vetter
2019-03-27 17:17 ` Ahmed S. Darwish
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2019-03-27 8:30 UTC (permalink / raw)
To: Ahmed S. Darwish; +Cc: Daniel Vetter, DRI Development
On Wed, Mar 27, 2019 at 12:23:43AM +0100, Ahmed S. Darwish wrote:
> Otherwise gcc will complain about unknown types, and declarations
> inside parameter lists, if "drm_internal.h" is used in C files with
> less headers than what's now typically done under drivers/gpu/drm/.
>
> Signed-off-by: Ahmed S. Darwish <darwish.07@gmail.com>
> ---
>
> Notes:
> This was triggered by the in-development drm-panic code.
>
> drivers/gpu/drm/drm_crtc_helper_internal.h | 5 +++++
> drivers/gpu/drm/drm_crtc_internal.h | 5 +++++
> drivers/gpu/drm/drm_internal.h | 12 ++++++++++++
> include/drm/drm_auth.h | 9 +++++++++
> 4 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h
> index b5ac1581e623..ee8d8682db09 100644
> --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> @@ -20,6 +20,9 @@
> * OTHER DEALINGS IN THE SOFTWARE.
> */
>
> +#ifndef _DRM_CRTC_HELPER_INTERNAL_H
> +#define _DRM_CRTC_HELPER_INTERNAL_H
I'm kinda wondering how you managed to include these multiple times?
> +
> /*
> * This header file contains mode setting related functions and definitions
> * which are only used within the drm kms helper module as internal
> @@ -75,3 +78,5 @@ enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
> const struct drm_display_mode *mode);
> enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode);
> +
> +#endif /* _DRM_CRTC_HELPER_INTERNAL_H */
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index 216f2a9ee3d4..840c1cb2eb7b 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -25,6 +25,9 @@
> * OTHER DEALINGS IN THE SOFTWARE.
> */
>
> +#ifndef _DRM_CRTC_INTERNAL_H
> +#define _DRM_CRTC_INTERNAL_H
> +
> /*
> * This header file contains mode setting related functions and definitions
> * which are only used within the drm module as internal implementation details
> @@ -252,3 +255,5 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> void drm_mode_fixup_1366x768(struct drm_display_mode *mode);
> void drm_reset_display_info(struct drm_connector *connector);
> u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid);
> +
> +#endif /* _DRM_CRTC_INTERNAL_H */
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 251d67e04c2d..a1b68836b048 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -21,7 +21,17 @@
> * OTHER DEALINGS IN THE SOFTWARE.
> */
>
> +#ifndef _DRM_INTERNAL_H
> +#define _DRM_INTERNAL_H
> +
> +#include <linux/mutex.h>
> +
> +#include <drm/drm_auth.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_file.h>
> #include <drm/drm_ioctl.h>
> +#include <drm/drm_print.h>
If you only need these for the structures used in pointers, then please
use forward declarations, don't pull in the entire thing.
I'm also wondering whether we actually need drm_ioctl.h, or whether a few
forward decls would be good enough.
>
> #define DRM_IF_MAJOR 1
> #define DRM_IF_MINOR 4
> @@ -191,3 +201,5 @@ int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
> void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
> const struct drm_framebuffer *fb);
> int drm_framebuffer_debugfs_init(struct drm_minor *minor);
> +
> +#endif /* _DRM_INTERNAL_H */
> diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
> index 86bff9841b54..a1a59fbf26b1 100644
> --- a/include/drm/drm_auth.h
> +++ b/include/drm/drm_auth.h
> @@ -28,6 +28,15 @@
> #ifndef _DRM_AUTH_H_
> #define _DRM_AUTH_H_
>
> +#include <linux/kref.h>
> +#include <linux/idr.h>
> +#include <linux/wait.h>
> +
> +#include <uapi/drm/drm.h>
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_file.h>
Same comment about forward decls. Plus it would be good to also clean up
drm_auth.c, i.e. drop the drmP.h include and put the drm_auth.h include as
the very first one. To make sure the header is now working correctly
stand-alone.
-Daniel
> +
> /*
> * Legacy DRI1 locking data structure. Only here instead of in drm_legacy.h for
> * include ordering reasons.
>
> --
> darwi
> http://darwish.chasingpointers.com
--
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm: headers: Add neccessary include files and guards
2019-03-27 8:30 ` Daniel Vetter
@ 2019-03-27 17:17 ` Ahmed S. Darwish
0 siblings, 0 replies; 3+ messages in thread
From: Ahmed S. Darwish @ 2019-03-27 17:17 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, DRI Development
Hi,
On Wed, Mar 27, 2019 at 09:30:59AM +0100, Daniel Vetter wrote:
> On Wed, Mar 27, 2019 at 12:23:43AM +0100, Ahmed S. Darwish wrote:
> > Otherwise gcc will complain about unknown types, and declarations
> > inside parameter lists, if "drm_internal.h" is used in C files with
> > less headers than what's now typically done under drivers/gpu/drm/.
> >
> > Signed-off-by: Ahmed S. Darwish <darwish.07@gmail.com>
> > ---
> >
> > Notes:
> > This was triggered by the in-development drm-panic code.
> >
> > drivers/gpu/drm/drm_crtc_helper_internal.h | 5 +++++
> > drivers/gpu/drm/drm_crtc_internal.h | 5 +++++
> > drivers/gpu/drm/drm_internal.h | 12 ++++++++++++
> > include/drm/drm_auth.h | 9 +++++++++
> > 4 files changed, 31 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h
> > index b5ac1581e623..ee8d8682db09 100644
> > --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> > +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> > @@ -20,6 +20,9 @@
> > * OTHER DEALINGS IN THE SOFTWARE.
> > */
> >
> > +#ifndef _DRM_CRTC_HELPER_INTERNAL_H
> > +#define _DRM_CRTC_HELPER_INTERNAL_H
>
> I'm kinda wondering how you managed to include these multiple times?
>
Oh, this was only added for completeness / consistency. The actual
errors triggered are the missing declarations.
> > +
> > /*
> > * This header file contains mode setting related functions and definitions
> > * which are only used within the drm kms helper module as internal
> > @@ -75,3 +78,5 @@ enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
> > const struct drm_display_mode *mode);
> > enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *mode);
> > +
> > +#endif /* _DRM_CRTC_HELPER_INTERNAL_H */
> > diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> > index 216f2a9ee3d4..840c1cb2eb7b 100644
> > --- a/drivers/gpu/drm/drm_crtc_internal.h
> > +++ b/drivers/gpu/drm/drm_crtc_internal.h
> > @@ -25,6 +25,9 @@
> > * OTHER DEALINGS IN THE SOFTWARE.
> > */
> >
> > +#ifndef _DRM_CRTC_INTERNAL_H
> > +#define _DRM_CRTC_INTERNAL_H
> > +
> > /*
> > * This header file contains mode setting related functions and definitions
> > * which are only used within the drm module as internal implementation details
> > @@ -252,3 +255,5 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> > void drm_mode_fixup_1366x768(struct drm_display_mode *mode);
> > void drm_reset_display_info(struct drm_connector *connector);
> > u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid);
> > +
> > +#endif /* _DRM_CRTC_INTERNAL_H */
> > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> > index 251d67e04c2d..a1b68836b048 100644
> > --- a/drivers/gpu/drm/drm_internal.h
> > +++ b/drivers/gpu/drm/drm_internal.h
> > @@ -21,7 +21,17 @@
> > * OTHER DEALINGS IN THE SOFTWARE.
> > */
> >
> > +#ifndef _DRM_INTERNAL_H
> > +#define _DRM_INTERNAL_H
> > +
> > +#include <linux/mutex.h>
> > +
> > +#include <drm/drm_auth.h>
> > +#include <drm/drm_connector.h>
> > +#include <drm/drm_device.h>
> > +#include <drm/drm_file.h>
> > #include <drm/drm_ioctl.h>
> > +#include <drm/drm_print.h>
>
> If you only need these for the structures used in pointers, then please
> use forward declarations, don't pull in the entire thing.
>
> I'm also wondering whether we actually need drm_ioctl.h, or whether a few
> forward decls would be good enough.
>
Oh, I see, forward decls then .. let's drop this.
> >
> > #define DRM_IF_MAJOR 1
> > #define DRM_IF_MINOR 4
> > @@ -191,3 +201,5 @@ int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
> > void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
> > const struct drm_framebuffer *fb);
> > int drm_framebuffer_debugfs_init(struct drm_minor *minor);
> > +
> > +#endif /* _DRM_INTERNAL_H */
> > diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
> > index 86bff9841b54..a1a59fbf26b1 100644
> > --- a/include/drm/drm_auth.h
> > +++ b/include/drm/drm_auth.h
> > @@ -28,6 +28,15 @@
> > #ifndef _DRM_AUTH_H_
> > #define _DRM_AUTH_H_
> >
> > +#include <linux/kref.h>
> > +#include <linux/idr.h>
> > +#include <linux/wait.h>
> > +
> > +#include <uapi/drm/drm.h>
> > +
> > +#include <drm/drm_device.h>
> > +#include <drm/drm_file.h>
>
> Same comment about forward decls. Plus it would be good to also clean up
> drm_auth.c, i.e. drop the drmP.h include and put the drm_auth.h include as
> the very first one. To make sure the header is now working correctly
> stand-alone.
>
will do.
> -Daniel
>
thanks,
--darwi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-27 17:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-26 23:23 [PATCH] drm: headers: Add neccessary include files and guards Ahmed S. Darwish
2019-03-27 8:30 ` Daniel Vetter
2019-03-27 17:17 ` Ahmed S. Darwish
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).