| Message ID | 20190326232308.GA8155@darwi-home-pc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: headers: Add neccessary include files and guards | expand |
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
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
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.
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(+) -- darwi http://darwish.chasingpointers.com