[PATCH v1 1/1] drm/bridge: drop drmP.h usage
Sam Ravnborg
sam at ravnborg.org
Sun May 19 15:25:13 UTC 2019
Hi Laurent
Thanks for the quick feedback.
> > @@ -15,14 +15,17 @@
> > #include <linux/of_device.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/reset.h>
> > -#include <drm/drmP.h>
> > +
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_bridge.h>
> > #include <drm/drm_crtc.h>
> > #include <drm/drm_mipi_dsi.h>
> > #include <drm/drm_of.h>
> > +#include <drm/drm_print.h>
> > #include <drm/drm_probe_helper.h>
> > +
> > #include <drm/bridge/dw_mipi_dsi.h>
>
> If you separate this file fromt he rest of the drm/ includes, you should
> do the same in drivers/gpu/drm/bridge/synopsys/dw-hdmi.c for
> consistency. I'm fine either way.
>
> > index a20e454ddd64..170f162ffa55 100644
> > --- a/drivers/gpu/drm/bridge/tc358764.c
> > +++ b/drivers/gpu/drm/bridge/tc358764.c
> > @@ -7,18 +7,22 @@
> > * Maciej Purski <m.purski at samsung.com>
> > */
> >
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/module.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#include <video/mipi_display.h>
> > +
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_crtc.h>
> > #include <drm/drm_fb_helper.h>
> > #include <drm/drm_mipi_dsi.h>
> > #include <drm/drm_of.h>
> > #include <drm/drm_panel.h>
> > +#include <drm/drm_print.h>
> > #include <drm/drm_probe_helper.h>
> > -#include <drm/drmP.h>
> > -#include <linux/gpio/consumer.h>
> > -#include <linux/of_graph.h>
> > -#include <linux/regulator/consumer.h>
> > -#include <video/mipi_display.h>
>
> Similarly, in drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c you have
> video/ after drm/, while it's the other way around. I think we should
> pick one order and stick with it.
Well done to spot this inconsistency.
I will go through the files and consistently use following order:
<linux/*>
<asm/*>
<video/*>
<drm/*>
""
This seems to match the majority but all variants can be found.
And then the principle is:
- most general includes first, with linux/* first
- drm includes second-last
- local drm includes last
>
> With these small issues fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Thanks. I will post a v2 - where I can also fix so the mail is sent to
the relevant people (my cc: was missing some this time).
Sam
More information about the dri-devel
mailing list