Re: [PATCH 00/25] drm/i915: the great header refactoring, part one

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

Quoting Chris Wilson (2019-04-05 00:32:13)
> Quoting Jani Nikula (2019-04-04 22:14:24)
> > intel_drv.h has grown out of proportions, and turned into a dumping
> > ground. Way back when it was useful to have only a handful of headers,
> > but we're long past that.
> > 
> > Start splitting off per-module headers. The basic principles:
> > 
> > * Make the new headers self-contained (i.e. can be compiled without
> >   including other headers first), and test this using the new infra for
> >   that.
> > 
> > * Use minimal includes for making the headers self-contained. Use
> >   forward declarations for structs where applicable, and e.g. include
> >   <linux/types.h> instead of <linux/kernel.h>.
> > 
> > * Only split off the headers, and mostly refrain from doing other
> >   refactoring while at it. (There are a few minor things.)
> > 
> > * Mostly only split off function declarations. Splitting off types is
> >   left for follow-up work.
> > 
> > * Include the new headers only where needed. This leads to a lot of
> >   includes here and there, but on the other hand increases the clarity
> >   of the relationships between the modules. (And already raises a bunch
> >   of questions about the split and cross-calls between some
> >   modules. It'll be easier to analyze this.)
> > 
> > * Wherever adding new includes, group the includes by <linux/...> first,
> >   then <drm/...>, then "...", and sort the groups alphabetically.
> > 
> > * Choice of what to extract first here is purely arbitrary.
> > 
> > * Follow-up work should consider renaming functions according to the
> >   module, i.e. functions in intel_foo.c should be prefixed
> >   intel_foo_. Better naming will be helpful in further organizing the
> >   driver, as well as grasping the structure to begin with.
> 
> I wholeheartedly agree. Recent experience shows that with a small number
> of very large headers, getting the types defined at the right point is
> much harder than with small scoped headers. (And don't get me started on
> trying to avoid circular definitions for inlines.)
> 
> The counterpoint is that it can be harder to find the right header. But
> it's no harder than trying to find the right c-file, and if we keep to
> the rule that object-name.c is accompanied by object-name.h, once you
> know object name, it is easy to find.

I'm also very much in favor of this split.

Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

> > Jani Nikula (25):
> >   drm/i915: make intel_frontbuffer.h self-contained
> >   drm/i915: extract intel_audio.h from intel_drv.h
> >   drm/i915: extract intel_crt.h from intel_drv.h
> >   drm/i915: extract intel_ddi.h from intel_drv.h
> >   drm/i915: extract intel_connector.h from intel_drv.h
> >   drm/i915: extract intel_csr.h from intel_drv.h
> >   drm/i915: extract intel_fbc.h from intel_drv.h
> >   drm/i915: extract intel_psr.h from intel_drv.h
> >   drm/i915: extract intel_color.h from intel_drv.h
> >   drm/i915: extract intel_lspcon.h from intel_drv.h
> >   drm/i915: extract intel_sdvo.h from intel_drv.h
> >   drm/i915: extract intel_hdcp.h from intel_drv.h
> >   drm/i915: extract intel_panel.h from intel_drv.h
> >   drm/i915: extract intel_pm.h from intel_drv.h
> >   drm/i915: extract intel_fbdev.h from intel_drv.h
> >   drm/i915: extract intel_dp.h from intel_drv.h
> >   drm/i915: extract intel_hdmi.h from intel_drv.h
> >   drm/i915: extract intel_atomic_plane.h from intel_drv.h
> >   drm/i915: extract intel_pipe_crc.h from intel_drv.h
> >   drm/i915: extract intel_tv.h from intel_drv.h
> >   drm/i915: extract intel_lvds.h from intel_drv.h
> >   drm/i915: extract intel_dvo.h from intel_drv.h
> >   drm/i915: extract intel_sprite.h from intel_drv.h
> >   drm/i915: extract intel_cdclk.h from intel_drv.h
> >   drm/i915/cdclk: have only one init/uninit function
> 
> Read through the patches, and they are just code motion.
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> The real work is done by the compiler.
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx