On Thu, Aug 18, 2016 at 12:11:35PM +0100, Emil Velikov wrote: > Hi Daniel, > > On 17 August 2016 at 21:56, Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > > --- /dev/null > > +++ b/include/drm/drm_property.h > > > +#ifndef __DRM_PROPERTY_H__ > > +#define __DRM_PROPERTY_H__ > > + > > +#include <linux/list.h> > > +#include <linux/ctype.h> > > +#include <drm/drm_mode_object.h> > > + > Add the following fwd declaration since we use a pointer to the said > struct ? From a brief look the other newly introduced headers > could/should use it as well. > > struct drm_device;
tbh this is only a half-useful attempt at untangling the header loop mess. Once drm_crtc.[hc] is fully split I guess we can look into what makes sense. I'll definitely be happy to review patches, but personally I don't mind loops in headers much. It's annoying, but as long as things are reasonably split and it's possible to untangle at least the code/structures and documentation itself I'm happy. Wrt all things drm_device: I'm not sure when (if ever) I'll cough up the courage to split up and untangle drmP.h ;-) > The declarations in include/drm should be the ones meant for drivers > and there's no core drm internal ones ? Where/how does one manage API > that should be kept private between the different core DRM components, > even if there's symbol dependency between the different modules ? drm_crtc_helper_internal.h, drm_crtc_internal.h and drm_internal.h, all in drivers/gpu/drm/ Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
