Re: [PATCH RFC 01/19] drm: Stop including drm_bridge.h from drm_crtc.h

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	dri-devel@lists.freedesktop.org,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Chris Healy <Chris.Healy@zii.aero>,
	kernel@collabora.com, Sam Ravnborg <sam@ravnborg.org>
Subject: Re: [PATCH RFC 01/19] drm: Stop including drm_bridge.h from drm_crtc.h
Date: Wed, 21 Aug 2019 17:44:06 +0200	[thread overview]
Message-ID: <20190821174406.10c7af2e@collabora.com> (raw)
In-Reply-To: <20190820185315.GO10820@pendragon.ideasonboard.com>

On Tue, 20 Aug 2019 21:53:15 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> Hi Boris,
> 
> Thank you for the patch.
> 
> On Thu, Aug 08, 2019 at 05:11:32PM +0200, Boris Brezillon wrote:
> > We are about to add a drm_bridge_state that inherits from
> > drm_private_state which is defined in drm_atomic.h. Problem is,
> > drm_atomic.h includes drm_crtc.h which in turn includes drm_bridge.h,
> > leading to "drm_private_state has incomplete type" error.
> > 
> > Let's force all users of the drm_bridge API to explicitly include
> > drm_bridge.h.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >  drivers/gpu/drm/arc/arcpgu_hdmi.c                        | 1 +
> >  drivers/gpu/drm/bridge/analogix-anx78xx.c                | 1 +
> >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c       | 1 +
> >  drivers/gpu/drm/bridge/dumb-vga-dac.c                    | 1 +
> >  drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 1 +
> >  drivers/gpu/drm/bridge/nxp-ptn3460.c                     | 1 +
> >  drivers/gpu/drm/bridge/panel.c                           | 1 +
> >  drivers/gpu/drm/bridge/parade-ps8622.c                   | 1 +
> >  drivers/gpu/drm/bridge/sii902x.c                         | 1 +
> >  drivers/gpu/drm/bridge/sii9234.c                         | 1 +
> >  drivers/gpu/drm/bridge/sil-sii8620.c                     | 1 +
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c                | 1 +
> >  drivers/gpu/drm/bridge/tc358764.c                        | 1 +
> >  drivers/gpu/drm/bridge/tc358767.c                        | 1 +
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c                    | 1 +
> >  drivers/gpu/drm/bridge/ti-tfp410.c                       | 1 +
> >  drivers/gpu/drm/drm_atomic_helper.c                      | 1 +
> >  drivers/gpu/drm/drm_crtc_helper.c                        | 1 +
> >  drivers/gpu/drm/drm_encoder.c                            | 1 +
> >  drivers/gpu/drm/drm_probe_helper.c                       | 1 +
> >  drivers/gpu/drm/drm_simple_kms_helper.c                  | 1 +
> >  drivers/gpu/drm/exynos/exynos_dp.c                       | 1 +
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c                  | 1 +
> >  drivers/gpu/drm/exynos/exynos_drm_mic.c                  | 1 +
> >  drivers/gpu/drm/exynos/exynos_hdmi.c                     | 1 +
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c                | 1 +
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c             | 1 +
> >  drivers/gpu/drm/i2c/tda998x_drv.c                        | 1 +
> >  drivers/gpu/drm/imx/imx-ldb.c                            | 1 +
> >  drivers/gpu/drm/imx/parallel-display.c                   | 1 +
> >  drivers/gpu/drm/ingenic/ingenic-drm.c                    | 1 +
> >  drivers/gpu/drm/mediatek/mtk_dpi.c                       | 1 +
> >  drivers/gpu/drm/mediatek/mtk_dsi.c                       | 1 +
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c                      | 1 +
> >  drivers/gpu/drm/msm/dsi/dsi.h                            | 1 +
> >  drivers/gpu/drm/msm/edp/edp.h                            | 1 +
> >  drivers/gpu/drm/msm/hdmi/hdmi.h                          | 2 ++
> >  drivers/gpu/drm/omapdrm/dss/output.c                     | 1 +
> >  drivers/gpu/drm/omapdrm/omap_drv.c                       | 1 +
> >  drivers/gpu/drm/omapdrm/omap_encoder.c                   | 1 +
> >  drivers/gpu/drm/rcar-du/rcar_du_encoder.c                | 1 +
> >  drivers/gpu/drm/rockchip/rockchip_lvds.c                 | 1 +
> >  drivers/gpu/drm/rockchip/rockchip_rgb.c                  | 1 +
> >  drivers/gpu/drm/sti/sti_dvo.c                            | 1 +
> >  drivers/gpu/drm/sti/sti_hda.c                            | 1 +
> >  drivers/gpu/drm/sti/sti_hdmi.c                           | 1 +
> >  drivers/gpu/drm/sun4i/sun4i_lvds.c                       | 1 +
> >  drivers/gpu/drm/sun4i/sun4i_rgb.c                        | 1 +
> >  drivers/gpu/drm/sun4i/sun4i_tcon.c                       | 1 +
> >  drivers/gpu/drm/tilcdc/tilcdc_external.c                 | 1 +
> >  drivers/gpu/drm/vc4/vc4_dsi.c                            | 1 +
> >  include/drm/drm_crtc.h                                   | 1 -
> >  52 files changed, 52 insertions(+), 1 deletion(-)  
> 
> Overall this looks good to me. Before I ack the patch, how have you
> tested this ? Have you compiled all the DRM/KMS drivers ?

git grep to find all drm_bridge users that were not including
drm_bridge.h directly and then I compiled all impacted drivers. Also
pushed to a tree monitored by kbuild bots to make sure I didn't
forget one of them.

> 
> If you wanted to go one step further you could also remove the forward
> declaration of struct drm_bridge from drm_ctrc.h, as it's not needed
> there.
> 

I'll add a patch doing that.

Thanks,

Boris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel