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

Chris Wilson chris at chris-wilson.co.uk
Thu Apr 4 21:32:13 UTC 2019


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.

> 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 at chris-wilson.co.uk>

The real work is done by the compiler.
-Chris


More information about the Intel-gfx mailing list