[v4,3/5] drm: bridge: Link encoder and bridge in core code

Message ID	1481709550-29226-4-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <dri-devel-bounces@lists.freedesktop.org> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> To: dri-devel@lists.freedesktop.org Subject: [PATCH v4 3/5] drm: bridge: Link encoder and bridge in core code Date: Wed, 14 Dec 2016 11:59:08 +0200 Message-Id: <1481709550-29226-4-git-send-email-laurent.pinchart+renesas@ideasonboard.com> In-Reply-To: <1481709550-29226-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> References: <1481709550-29226-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>, Alexey Brodkin <abrodkin@synopsys.com>, Seung-Woo Kim <sw0312.kim@samsung.com>, Xinwei Kong <kong.kongxinwei@hisilicon.com>, Jyri Sarha <jsarha@ti.com>, Kyungmin Park <kyungmin.park@samsung.com>, Tomi Valkeinen <tomi.valkeinen@ti.com>, Chen Feng <puck.chen@hisilicon.com>, Rongrong Zou <zourongrong@gmail.com>, Maxime Ripard <maxime.ripard@free-electrons.com>, Chen-Yu Tsai <wens@csie.org> Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>

Laurent Pinchart Dec. 14, 2016, 9:59 a.m. UTC

Instead of linking encoders and bridges in every driver (and getting it
wrong half of the time, as many drivers forget to set the drm_bridge
encoder pointer), do so in core code. The drm_bridge_attach() function
needs the encoder and optional previous bridge to perform that task,
update all the callers.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Stefan Agner <stefan@agner.ch> # For DCU
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # For atmel-hlcdc
Acked-by: Vincent Abriou <vincent.abriou@st.com> # For STI
---
 drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  5 +--
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
 drivers/gpu/drm/bridge/dw-hdmi.c                   |  3 +-
 drivers/gpu/drm/drm_bridge.c                       | 46 ++++++++++++++++------
 drivers/gpu/drm/drm_simple_kms_helper.c            |  4 +-
 drivers/gpu/drm/exynos/exynos_dp.c                 |  5 +--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  6 +--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  5 +--
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  5 +--
 drivers/gpu/drm/imx/imx-ldb.c                      |  6 +--
 drivers/gpu/drm/imx/parallel-display.c             |  4 +-
 drivers/gpu/drm/mediatek/mtk_dpi.c                 |  8 ++--
 drivers/gpu/drm/mediatek/mtk_dsi.c                 | 24 ++---------
 drivers/gpu/drm/mediatek/mtk_hdmi.c                | 11 +++---
 drivers/gpu/drm/msm/dsi/dsi_manager.c              | 17 +++++---
 drivers/gpu/drm/msm/edp/edp_bridge.c               |  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c             |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c          |  5 +--
 drivers/gpu/drm/sti/sti_dvo.c                      |  3 +-
 drivers/gpu/drm/sti/sti_hda.c                      |  3 +-
 drivers/gpu/drm/sti/sti_hdmi.c                     |  3 +-
 drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 13 +++---
 drivers/gpu/drm/tilcdc/tilcdc_external.c           |  4 +-
 include/drm/drm_bridge.h                           |  3 +-
 25 files changed, 85 insertions(+), 110 deletions(-)

Maxime Ripard Dec. 14, 2016, 10:30 a.m. UTC | #1

On Wed, Dec 14, 2016 at 11:59:08AM +0200, Laurent Pinchart wrote:
> Instead of linking encoders and bridges in every driver (and getting it
> wrong half of the time, as many drivers forget to set the drm_bridge
> encoder pointer), do so in core code. The drm_bridge_attach() function
> needs the encoder and optional previous bridge to perform that task,
> update all the callers.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Stefan Agner <stefan@agner.ch> # For DCU
> Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # For atmel-hlcdc
> Acked-by: Vincent Abriou <vincent.abriou@st.com> # For STI

For sun4i,
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks,
Maxime

xinliang Dec. 16, 2016, 1:12 a.m. UTC | #2

On 2016/12/14 17:59, Laurent Pinchart wrote:
>   int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev)
> diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> index 998452ad0fcb..1737e98bc10a 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> @@ -709,10 +709,7 @@ static int dsi_bridge_init(struct drm_device *dev, struct dw_dsi *dsi)
>   	int ret;
>   
>   	/* associate the bridge to dsi encoder */
> -	encoder->bridge = bridge;
> -	bridge->encoder = encoder;
> -
> -	ret = drm_bridge_attach(dev, bridge);
> +	ret = drm_bridge_attach(encoder, bridge, NULL);
>   	if (ret) {
>   		DRM_ERROR("failed to attach external bridge\n");
>   		return ret;

For hisilicon,

Acked-by: Xinliang Liu <z.liuxinliang@hisilicon.com>

Archit Taneja Dec. 16, 2016, 8:35 a.m. UTC | #3

Hi,

On 12/14/2016 03:29 PM, Laurent Pinchart wrote:
> Instead of linking encoders and bridges in every driver (and getting it
> wrong half of the time, as many drivers forget to set the drm_bridge
> encoder pointer), do so in core code. The drm_bridge_attach() function
> needs the encoder and optional previous bridge to perform that task,
> update all the callers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Stefan Agner <stefan@agner.ch> # For DCU
> Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # For atmel-hlcdc
> Acked-by: Vincent Abriou <vincent.abriou@st.com> # For STI

This one needs acks for arcgpu, tilcdc, mediatek and imx. The changes in
those drivers look good to me, though. Will push it in a day or so unless anyone
has any comments on it.

Thanks,
Archit

> ---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  5 +--
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c                   |  3 +-
>  drivers/gpu/drm/drm_bridge.c                       | 46 ++++++++++++++++------
>  drivers/gpu/drm/drm_simple_kms_helper.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  5 +--
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  6 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  5 +--
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  5 +--
>  drivers/gpu/drm/imx/imx-ldb.c                      |  6 +--
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  8 ++--
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 | 24 ++---------
>  drivers/gpu/drm/mediatek/mtk_hdmi.c                | 11 +++---
>  drivers/gpu/drm/msm/dsi/dsi_manager.c              | 17 +++++---
>  drivers/gpu/drm/msm/edp/edp_bridge.c               |  2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c             |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c          |  5 +--
>  drivers/gpu/drm/sti/sti_dvo.c                      |  3 +-
>  drivers/gpu/drm/sti/sti_hda.c                      |  3 +-
>  drivers/gpu/drm/sti/sti_hdmi.c                     |  3 +-
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 13 +++---
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  4 +-
>  include/drm/drm_bridge.h                           |  3 +-
>  25 files changed, 85 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> index b69c66b4897e..0ce7f398bcff 100644
> --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
> +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> @@ -47,10 +47,7 @@ int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
>  		return ret;
>
>  	/* Link drm_bridge to encoder */
> -	bridge->encoder = encoder;
> -	encoder->bridge = bridge;
> -
> -	ret = drm_bridge_attach(drm, bridge);
> +	ret = drm_bridge_attach(encoder, bridge, NULL);
>  	if (ret)
>  		drm_encoder_cleanup(encoder);
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> index 6119b5085501..e7799b6ee829 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> @@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev,
>  	of_node_put(np);
>
>  	if (bridge) {
> -		output->encoder.bridge = bridge;
> -		bridge->encoder = &output->encoder;
> -		ret = drm_bridge_attach(dev, bridge);
> +		ret = drm_bridge_attach(&output->encoder, bridge, NULL);
>  		if (!ret)
>  			return 0;
>  	}
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index eb9bf8786c24..b7494c8d43fe 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct drm_device *drm_dev,
>
>  	dp->bridge = bridge;
>
> -	dp->encoder->bridge = bridge;
>  	bridge->driver_private = dp;
> -	bridge->encoder = dp->encoder;
>  	bridge->funcs = &analogix_dp_bridge_funcs;
>
> -	ret = drm_bridge_attach(drm_dev, bridge);
> +	ret = drm_bridge_attach(dp->encoder, bridge, NULL);
>  	if (ret) {
>  		DRM_ERROR("failed to attach drm bridge\n");
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 235ce7d1583d..f5009ae39b89 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
>  	hdmi->bridge = bridge;
>  	bridge->driver_private = hdmi;
>  	bridge->funcs = &dw_hdmi_bridge_funcs;
> -	ret = drm_bridge_attach(drm, bridge);
> +	ret = drm_bridge_attach(encoder, bridge, NULL);
>  	if (ret) {
>  		DRM_ERROR("Failed to initialize bridge with drm\n");
>  		return -EINVAL;
>  	}
>
> -	encoder->bridge = bridge;
>  	hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
>
>  	drm_connector_helper_add(&hdmi->connector,
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0ee052b7c21a..850bd6509ef1 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -26,6 +26,7 @@
>  #include <linux/mutex.h>
>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_encoder.h>
>
>  /**
>   * DOC: overview
> @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
>  EXPORT_SYMBOL(drm_bridge_remove);
>
>  /**
> - * drm_bridge_attach - associate given bridge to our DRM device
> + * drm_bridge_attach - attach the bridge to an encoder's chain
>   *
> - * @dev: DRM device
> - * @bridge: bridge control structure
> + * @encoder: DRM encoder
> + * @bridge: bridge to attach
> + * @previous: previous bridge in the chain (optional)
>   *
> - * Called by a kms driver to link one of our encoder/bridge to the given
> - * bridge.
> + * Called by a kms driver to link the bridge to an encoder's chain. The previous
> + * argument specifies the previous bridge in the chain. If NULL, the bridge is
> + * linked directly at the encoder's output. Otherwise it is linked at the
> + * previous bridge's output.
>   *
> - * Note that setting up links between the bridge and our encoder/bridge
> - * objects needs to be handled by the kms driver itself.
> + * If non-NULL the previous bridge must be already attached by a call to this
> + * function.
>   *
>   * RETURNS:
>   * Zero on success, error code on failure
>   */
> -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
> +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> +		      struct drm_bridge *previous)
>  {
> -	if (!dev || !bridge)
> +	int ret;
> +
> +	if (!encoder || !bridge)
> +		return -EINVAL;
> +
> +	if (previous && (!previous->dev || previous->encoder != encoder))
>  		return -EINVAL;
>
>  	if (bridge->dev)
>  		return -EBUSY;
>
> -	bridge->dev = dev;
> +	bridge->dev = encoder->dev;
> +	bridge->encoder = encoder;
> +
> +	if (bridge->funcs->attach) {
> +		ret = bridge->funcs->attach(bridge);
> +		if (ret < 0) {
> +			bridge->dev = NULL;
> +			bridge->encoder = NULL;
> +			return ret;
> +		}
> +	}
>
> -	if (bridge->funcs->attach)
> -		return bridge->funcs->attach(bridge);
> +	if (previous)
> +		previous->next = bridge;
> +	else
> +		encoder->bridge = bridge;
>
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 7bae08c2bf0a..ba7be6169339 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -182,9 +182,7 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = {
>  int drm_simple_display_pipe_attach_bridge(struct drm_simple_display_pipe *pipe,
>  					  struct drm_bridge *bridge)
>  {
> -	bridge->encoder = &pipe->encoder;
> -	pipe->encoder.bridge = bridge;
> -	return drm_bridge_attach(pipe->encoder.dev, bridge);
> +	return drm_bridge_attach(&pipe->encoder, bridge, NULL);
>  }
>  EXPORT_SYMBOL(drm_simple_display_pipe_attach_bridge);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> index 528229faffe4..1ef0be338b85 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -99,7 +99,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
>  				   struct drm_connector *connector)
>  {
>  	struct exynos_dp_device *dp = to_dp(plat_data);
> -	struct drm_encoder *encoder = &dp->encoder;
>  	int ret;
>
>  	drm_connector_register(connector);
> @@ -107,9 +106,7 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
>
>  	/* Pre-empt DP connector creation if there's a bridge */
>  	if (dp->ptn_bridge) {
> -		bridge->next = dp->ptn_bridge;
> -		dp->ptn_bridge->encoder = encoder;
> -		ret = drm_bridge_attach(encoder->dev, dp->ptn_bridge);
> +		ret = drm_bridge_attach(&dp->encoder, dp->ptn_bridge, bridge);
>  		if (ret) {
>  			DRM_ERROR("Failed to attach bridge to drm\n");
>  			bridge->next = NULL;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index e07cb1fe4860..812e2ec0761d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1718,10 +1718,8 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>  	}
>
>  	bridge = of_drm_find_bridge(dsi->bridge_node);
> -	if (bridge) {
> -		encoder->bridge = bridge;
> -		drm_bridge_attach(drm_dev, bridge);
> -	}
> +	if (bridge)
> +		drm_bridge_attach(encoder, bridge, NULL);
>
>  	return mipi_dsi_host_register(&dsi->dsi_host);
>  }
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> index 05a8ee106879..c3651456c963 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> @@ -160,10 +160,7 @@ static int fsl_dcu_attach_endpoint(struct fsl_dcu_drm_device *fsl_dev,
>  	if (!bridge)
>  		return -ENODEV;
>
> -	fsl_dev->encoder.bridge = bridge;
> -	bridge->encoder = &fsl_dev->encoder;
> -
> -	return drm_bridge_attach(fsl_dev->drm, bridge);
> +	return drm_bridge_attach(&fsl_dev->encoder, bridge, NULL);
>  }
>
>  int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev)
> diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> index 998452ad0fcb..1737e98bc10a 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> @@ -709,10 +709,7 @@ static int dsi_bridge_init(struct drm_device *dev, struct dw_dsi *dsi)
>  	int ret;
>
>  	/* associate the bridge to dsi encoder */
> -	encoder->bridge = bridge;
> -	bridge->encoder = encoder;
> -
> -	ret = drm_bridge_attach(dev, bridge);
> +	ret = drm_bridge_attach(encoder, bridge, NULL);
>  	if (ret) {
>  		DRM_ERROR("failed to attach external bridge\n");
>  		return ret;
> diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> index 516d06490465..ec49ea3d8e40 100644
> --- a/drivers/gpu/drm/imx/imx-ldb.c
> +++ b/drivers/gpu/drm/imx/imx-ldb.c
> @@ -454,10 +454,8 @@ static int imx_ldb_register(struct drm_device *drm,
>  			 DRM_MODE_ENCODER_LVDS, NULL);
>
>  	if (imx_ldb_ch->bridge) {
> -		imx_ldb_ch->bridge->encoder = encoder;
> -
> -		imx_ldb_ch->encoder.bridge = imx_ldb_ch->bridge;
> -		ret = drm_bridge_attach(drm, imx_ldb_ch->bridge);
> +		ret = drm_bridge_attach(&imx_ldb_ch->encoder,
> +					imx_ldb_ch->bridge, NULL);
>  		if (ret) {
>  			DRM_ERROR("Failed to initialize bridge with drm\n");
>  			return ret;
> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
> index 8582a83c0d9b..51d9f735c358 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -191,9 +191,7 @@ static int imx_pd_register(struct drm_device *drm,
>  		drm_panel_attach(imxpd->panel, &imxpd->connector);
>
>  	if (imxpd->bridge) {
> -		imxpd->bridge->encoder = encoder;
> -		encoder->bridge = imxpd->bridge;
> -		ret = drm_bridge_attach(drm, imxpd->bridge);
> +		ret = drm_bridge_attach(encoder, imxpd->bridge, NULL);
>  		if (ret < 0) {
>  			dev_err(imxpd->dev, "failed to attach bridge: %d\n",
>  				ret);
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 90fb831ef031..3bd3bd688d1a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -63,6 +63,7 @@ enum mtk_dpi_out_color_format {
>  struct mtk_dpi {
>  	struct mtk_ddp_comp ddp_comp;
>  	struct drm_encoder encoder;
> +	struct drm_bridge *bridge;
>  	void __iomem *regs;
>  	struct device *dev;
>  	struct clk *engine_clk;
> @@ -620,8 +621,7 @@ static int mtk_dpi_bind(struct device *dev, struct device *master, void *data)
>  	/* Currently DPI0 is fixed to be driven by OVL1 */
>  	dpi->encoder.possible_crtcs = BIT(1);
>
> -	dpi->encoder.bridge->encoder = &dpi->encoder;
> -	ret = drm_bridge_attach(dpi->encoder.dev, dpi->encoder.bridge);
> +	ret = drm_bridge_attach(&dpi->encoder, dpi->bridge, NULL);
>  	if (ret) {
>  		dev_err(dev, "Failed to attach bridge: %d\n", ret);
>  		goto err_cleanup;
> @@ -718,9 +718,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>
>  	dev_info(dev, "Found bridge node: %s\n", bridge_node->full_name);
>
> -	dpi->encoder.bridge = of_drm_find_bridge(bridge_node);
> +	dpi->bridge = of_drm_find_bridge(bridge_node);
>  	of_node_put(bridge_node);
> -	if (!dpi->encoder.bridge)
> +	if (!dpi->bridge)
>  		return -EPROBE_DEFER;
>
>  	comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DPI);
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 2c42f90809d8..dd71cbb1a622 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -622,26 +622,6 @@ static const struct drm_connector_helper_funcs
>  	.get_modes = mtk_dsi_connector_get_modes,
>  };
>
> -static int mtk_drm_attach_bridge(struct drm_bridge *bridge,
> -				 struct drm_encoder *encoder)
> -{
> -	int ret;
> -
> -	if (!bridge)
> -		return -ENOENT;
> -
> -	encoder->bridge = bridge;
> -	bridge->encoder = encoder;
> -	ret = drm_bridge_attach(encoder->dev, bridge);
> -	if (ret) {
> -		DRM_ERROR("Failed to attach bridge to drm\n");
> -		encoder->bridge = NULL;
> -		bridge->encoder = NULL;
> -	}
> -
> -	return ret;
> -}
> -
>  static int mtk_dsi_create_connector(struct drm_device *drm, struct mtk_dsi *dsi)
>  {
>  	int ret;
> @@ -692,8 +672,10 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi)
>  	dsi->encoder.possible_crtcs = 1;
>
>  	/* If there's a bridge, attach to it and let it create the connector */
> -	ret = mtk_drm_attach_bridge(dsi->bridge, &dsi->encoder);
> +	ret = drm_bridge_attach(&dsi->encoder, dsi->bridge, NULL);
>  	if (ret) {
> +		DRM_ERROR("Failed to attach bridge to drm\n");
> +
>  		/* Otherwise create our own connector and attach to a panel */
>  		ret = mtk_dsi_create_connector(drm, dsi);
>  		if (ret)
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 0e8c4d9af340..c26251260b83 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -149,6 +149,7 @@ struct hdmi_audio_param {
>
>  struct mtk_hdmi {
>  	struct drm_bridge bridge;
> +	struct drm_bridge *next_bridge;
>  	struct drm_connector conn;
>  	struct device *dev;
>  	struct phy *phy;
> @@ -1314,9 +1315,9 @@ static int mtk_hdmi_bridge_attach(struct drm_bridge *bridge)
>  		return ret;
>  	}
>
> -	if (bridge->next) {
> -		bridge->next->encoder = bridge->encoder;
> -		ret = drm_bridge_attach(bridge->encoder->dev, bridge->next);
> +	if (hdmi->next_bridge) {
> +		ret = drm_bridge_attach(bridge->encoder, hdmi->next_bridge,
> +					bridge);
>  		if (ret) {
>  			dev_err(hdmi->dev,
>  				"Failed to attach external bridge: %d\n", ret);
> @@ -1510,8 +1511,8 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
>  	of_node_put(ep);
>
>  	if (!of_device_is_compatible(remote, "hdmi-connector")) {
> -		hdmi->bridge.next = of_drm_find_bridge(remote);
> -		if (!hdmi->bridge.next) {
> +		hdmi->next_bridge = of_drm_find_bridge(remote);
> +		if (!hdmi->next_bridge) {
>  			dev_err(dev, "Waiting for external bridge\n");
>  			of_node_put(remote);
>  			return -EPROBE_DEFER;
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index c8d1f19c9a6d..2bd8dad76105 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -579,6 +579,7 @@ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id)
>  	struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
>  	struct drm_bridge *bridge = NULL;
>  	struct dsi_bridge *dsi_bridge;
> +	struct drm_encoder *encoder;
>  	int ret;
>
>  	dsi_bridge = devm_kzalloc(msm_dsi->dev->dev,
> @@ -590,10 +591,18 @@ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id)
>
>  	dsi_bridge->id = id;
>
> +	/*
> +	 * HACK: we may not know the external DSI bridge device's mode
> +	 * flags here. We'll get to know them only when the device
> +	 * attaches to the dsi host. For now, assume the bridge supports
> +	 * DSI video mode
> +	 */
> +	encoder = msm_dsi->encoders[MSM_DSI_VIDEO_ENCODER_ID];
> +
>  	bridge = &dsi_bridge->base;
>  	bridge->funcs = &dsi_mgr_bridge_funcs;
>
> -	ret = drm_bridge_attach(msm_dsi->dev, bridge);
> +	ret = drm_bridge_attach(encoder, bridge, NULL);
>  	if (ret)
>  		goto fail;
>
> @@ -628,11 +637,7 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id)
>  	encoder = msm_dsi->encoders[MSM_DSI_VIDEO_ENCODER_ID];
>
>  	/* link the internal dsi bridge to the external bridge */
> -	int_bridge->next = ext_bridge;
> -	/* set the external bridge's encoder as dsi's encoder */
> -	ext_bridge->encoder = encoder;
> -
> -	drm_bridge_attach(dev, ext_bridge);
> +	drm_bridge_attach(encoder, ext_bridge, int_bridge);
>
>  	/*
>  	 * we need the drm_connector created by the external bridge
> diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
> index 2bc73f82f3f5..931a5c97cccf 100644
> --- a/drivers/gpu/drm/msm/edp/edp_bridge.c
> +++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
> @@ -106,7 +106,7 @@ struct drm_bridge *msm_edp_bridge_init(struct msm_edp *edp)
>  	bridge = &edp_bridge->base;
>  	bridge->funcs = &edp_bridge_funcs;
>
> -	ret = drm_bridge_attach(edp->dev, bridge);
> +	ret = drm_bridge_attach(edp->encoder, bridge, NULL);
>  	if (ret)
>  		goto fail;
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index bacbd5d8df0e..4e6d1bf27474 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -227,7 +227,7 @@ struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi)
>  	bridge = &hdmi_bridge->base;
>  	bridge->funcs = &msm_hdmi_bridge_funcs;
>
> -	ret = drm_bridge_attach(hdmi->dev, bridge);
> +	ret = drm_bridge_attach(hdmi->encoder, bridge, NULL);
>  	if (ret)
>  		goto fail;
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> index a1a2c5e7822c..933a2547798e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> @@ -124,10 +124,7 @@ int rcar_du_hdmienc_init(struct rcar_du_device *rcdu,
>  	hdmienc->renc = renc;
>
>  	/* Link the bridge to the encoder. */
> -	bridge->encoder = encoder;
> -	encoder->bridge = bridge;
> -
> -	ret = drm_bridge_attach(rcdu->ddev, bridge);
> +	ret = drm_bridge_attach(encoder, bridge, NULL);
>  	if (ret) {
>  		drm_encoder_cleanup(encoder);
>  		return ret;
> diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> index e8c1ed08a9f7..411dc6ec976e 100644
> --- a/drivers/gpu/drm/sti/sti_dvo.c
> +++ b/drivers/gpu/drm/sti/sti_dvo.c
> @@ -478,14 +478,13 @@ static int sti_dvo_bind(struct device *dev, struct device *master, void *data)
>  		return err;
>  	}
>
> -	err = drm_bridge_attach(drm_dev, bridge);
> +	err = drm_bridge_attach(encoder, bridge, NULL);
>  	if (err) {
>  		DRM_ERROR("Failed to attach bridge\n");
>  		return err;
>  	}
>
>  	dvo->bridge = bridge;
> -	encoder->bridge = bridge;
>  	connector->encoder = encoder;
>  	dvo->encoder = encoder;
>
> diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> index 96f336dd0e29..66d37d78152a 100644
> --- a/drivers/gpu/drm/sti/sti_hda.c
> +++ b/drivers/gpu/drm/sti/sti_hda.c
> @@ -707,9 +707,8 @@ static int sti_hda_bind(struct device *dev, struct device *master, void *data)
>
>  	bridge->driver_private = hda;
>  	bridge->funcs = &sti_hda_bridge_funcs;
> -	drm_bridge_attach(drm_dev, bridge);
> +	drm_bridge_attach(encoder, bridge, NULL);
>
> -	encoder->bridge = bridge;
>  	connector->encoder = encoder;
>
>  	drm_connector = (struct drm_connector *)connector;
> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index 376b0763c874..f0af1ae82ee9 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -1308,9 +1308,8 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
>
>  	bridge->driver_private = hdmi;
>  	bridge->funcs = &sti_hdmi_bridge_funcs;
> -	drm_bridge_attach(drm_dev, bridge);
> +	drm_bridge_attach(encoder, bridge, NULL);
>
> -	encoder->bridge = bridge;
>  	connector->encoder = encoder;
>
>  	drm_connector = (struct drm_connector *)connector;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index f5e86fe7750e..757208f51731 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -208,6 +208,7 @@ int sun4i_rgb_init(struct drm_device *drm)
>  	struct sun4i_drv *drv = drm->dev_private;
>  	struct sun4i_tcon *tcon = drv->tcon;
>  	struct drm_encoder *encoder;
> +	struct drm_bridge *bridge;
>  	struct sun4i_rgb *rgb;
>  	int ret;
>
> @@ -218,8 +219,8 @@ int sun4i_rgb_init(struct drm_device *drm)
>  	encoder = &rgb->encoder;
>
>  	tcon->panel = sun4i_tcon_find_panel(tcon->dev->of_node);
> -	encoder->bridge = sun4i_tcon_find_bridge(tcon->dev->of_node);
> -	if (IS_ERR(tcon->panel) && IS_ERR(encoder->bridge)) {
> +	bridge = sun4i_tcon_find_bridge(tcon->dev->of_node);
> +	if (IS_ERR(tcon->panel) && IS_ERR(bridge)) {
>  		dev_info(drm->dev, "No panel or bridge found... RGB output disabled\n");
>  		return 0;
>  	}
> @@ -260,16 +261,12 @@ int sun4i_rgb_init(struct drm_device *drm)
>  		}
>  	}
>
> -	if (!IS_ERR(encoder->bridge)) {
> -		encoder->bridge->encoder = &rgb->encoder;
> -
> -		ret = drm_bridge_attach(drm, encoder->bridge);
> +	if (!IS_ERR(bridge)) {
> +		ret = drm_bridge_attach(encoder, bridge, NULL);
>  		if (ret) {
>  			dev_err(drm->dev, "Couldn't attach our bridge\n");
>  			goto err_cleanup_connector;
>  		}
> -	} else {
> -		encoder->bridge = NULL;
>  	}
>
>  	return 0;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index c67d7cd7d57e..b0dd5e8634ae 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -167,10 +167,8 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
>  	int ret;
>
>  	priv->external_encoder->possible_crtcs = BIT(0);
> -	priv->external_encoder->bridge = bridge;
> -	bridge->encoder = priv->external_encoder;
>
> -	ret = drm_bridge_attach(ddev, bridge);
> +	ret = drm_bridge_attach(priv->external_encoder, bridge, NULL);
>  	if (ret) {
>  		dev_err(ddev->dev, "drm_bridge_attach() failed %d\n", ret);
>  		return ret;
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 530a1d6e8cde..94e5ee96b3b5 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -201,7 +201,8 @@ struct drm_bridge {
>  int drm_bridge_add(struct drm_bridge *bridge);
>  void drm_bridge_remove(struct drm_bridge *bridge);
>  struct drm_bridge *of_drm_find_bridge(struct device_node *np);
> -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
> +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> +		      struct drm_bridge *previous);
>  void drm_bridge_detach(struct drm_bridge *bridge);
>
>  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>

Jyri Sarha Dec. 16, 2016, 9:17 a.m. UTC | #4

On 12/14/16 11:59, Laurent Pinchart wrote:
> Instead of linking encoders and bridges in every driver (and getting it
> wrong half of the time, as many drivers forget to set the drm_bridge
> encoder pointer), do so in core code. The drm_bridge_attach() function
> needs the encoder and optional previous bridge to perform that task,
> update all the callers.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Stefan Agner <stefan@agner.ch> # For DCU
> Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # For atmel-hlcdc
> Acked-by: Vincent Abriou <vincent.abriou@st.com> # For STI

Acked-by: Jyri Sarha <jsarha@ti.com> # For tilcdc

> ---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  5 +--
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c                   |  3 +-
>  drivers/gpu/drm/drm_bridge.c                       | 46 ++++++++++++++++------
>  drivers/gpu/drm/drm_simple_kms_helper.c            |  4 +-
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  5 +--
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  6 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  5 +--
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  5 +--
>  drivers/gpu/drm/imx/imx-ldb.c                      |  6 +--
>  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
>  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  8 ++--
>  drivers/gpu/drm/mediatek/mtk_dsi.c                 | 24 ++---------
>  drivers/gpu/drm/mediatek/mtk_hdmi.c                | 11 +++---
>  drivers/gpu/drm/msm/dsi/dsi_manager.c              | 17 +++++---
>  drivers/gpu/drm/msm/edp/edp_bridge.c               |  2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c             |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c          |  5 +--
>  drivers/gpu/drm/sti/sti_dvo.c                      |  3 +-
>  drivers/gpu/drm/sti/sti_hda.c                      |  3 +-
>  drivers/gpu/drm/sti/sti_hdmi.c                     |  3 +-
>  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 13 +++---
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  4 +-
...
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index c67d7cd7d57e..b0dd5e8634ae 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -167,10 +167,8 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
>  	int ret;
>  
>  	priv->external_encoder->possible_crtcs = BIT(0);
> -	priv->external_encoder->bridge = bridge;
> -	bridge->encoder = priv->external_encoder;
>  
> -	ret = drm_bridge_attach(ddev, bridge);
> +	ret = drm_bridge_attach(priv->external_encoder, bridge, NULL);
>  	if (ret) {
>  		dev_err(ddev->dev, "drm_bridge_attach() failed %d\n", ret);
>  		return ret;
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 530a1d6e8cde..94e5ee96b3b5 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -201,7 +201,8 @@ struct drm_bridge {
>  int drm_bridge_add(struct drm_bridge *bridge);
>  void drm_bridge_remove(struct drm_bridge *bridge);
>  struct drm_bridge *of_drm_find_bridge(struct device_node *np);
> -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
> +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> +		      struct drm_bridge *previous);
>  void drm_bridge_detach(struct drm_bridge *bridge);
>  
>  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>

Daniel Vetter Dec. 16, 2016, 1:29 p.m. UTC | #5

On Fri, Dec 16, 2016 at 02:05:48PM +0530, Archit Taneja wrote:
> Hi,
> 
> On 12/14/2016 03:29 PM, Laurent Pinchart wrote:
> > Instead of linking encoders and bridges in every driver (and getting it
> > wrong half of the time, as many drivers forget to set the drm_bridge
> > encoder pointer), do so in core code. The drm_bridge_attach() function
> > needs the encoder and optional previous bridge to perform that task,
> > update all the callers.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Acked-by: Stefan Agner <stefan@agner.ch> # For DCU
> > Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # For atmel-hlcdc
> > Acked-by: Vincent Abriou <vincent.abriou@st.com> # For STI
> 
> This one needs acks for arcgpu, tilcdc, mediatek and imx. The changes in
> those drivers look good to me, though. Will push it in a day or so unless anyone
> has any comments on it.

Imo big refactorings don't need ack from every driver maintainer, since
some fail to do their jobs timely. Overall review + a few acks should be
good enough. Anyway I double-checked things and looks all good.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Thanks,
> Archit
> 
> > ---
> >  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  5 +--
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
> >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
> >  drivers/gpu/drm/bridge/dw-hdmi.c                   |  3 +-
> >  drivers/gpu/drm/drm_bridge.c                       | 46 ++++++++++++++++------
> >  drivers/gpu/drm/drm_simple_kms_helper.c            |  4 +-
> >  drivers/gpu/drm/exynos/exynos_dp.c                 |  5 +--
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  6 +--
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  5 +--
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  5 +--
> >  drivers/gpu/drm/imx/imx-ldb.c                      |  6 +--
> >  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
> >  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  8 ++--
> >  drivers/gpu/drm/mediatek/mtk_dsi.c                 | 24 ++---------
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c                | 11 +++---
> >  drivers/gpu/drm/msm/dsi/dsi_manager.c              | 17 +++++---
> >  drivers/gpu/drm/msm/edp/edp_bridge.c               |  2 +-
> >  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c             |  2 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c          |  5 +--
> >  drivers/gpu/drm/sti/sti_dvo.c                      |  3 +-
> >  drivers/gpu/drm/sti/sti_hda.c                      |  3 +-
> >  drivers/gpu/drm/sti/sti_hdmi.c                     |  3 +-
> >  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 13 +++---
> >  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  4 +-
> >  include/drm/drm_bridge.h                           |  3 +-
> >  25 files changed, 85 insertions(+), 110 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > index b69c66b4897e..0ce7f398bcff 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > @@ -47,10 +47,7 @@ int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
> >  		return ret;
> > 
> >  	/* Link drm_bridge to encoder */
> > -	bridge->encoder = encoder;
> > -	encoder->bridge = bridge;
> > -
> > -	ret = drm_bridge_attach(drm, bridge);
> > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> >  	if (ret)
> >  		drm_encoder_cleanup(encoder);
> > 
> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> > index 6119b5085501..e7799b6ee829 100644
> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> > @@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev,
> >  	of_node_put(np);
> > 
> >  	if (bridge) {
> > -		output->encoder.bridge = bridge;
> > -		bridge->encoder = &output->encoder;
> > -		ret = drm_bridge_attach(dev, bridge);
> > +		ret = drm_bridge_attach(&output->encoder, bridge, NULL);
> >  		if (!ret)
> >  			return 0;
> >  	}
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > index eb9bf8786c24..b7494c8d43fe 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct drm_device *drm_dev,
> > 
> >  	dp->bridge = bridge;
> > 
> > -	dp->encoder->bridge = bridge;
> >  	bridge->driver_private = dp;
> > -	bridge->encoder = dp->encoder;
> >  	bridge->funcs = &analogix_dp_bridge_funcs;
> > 
> > -	ret = drm_bridge_attach(drm_dev, bridge);
> > +	ret = drm_bridge_attach(dp->encoder, bridge, NULL);
> >  	if (ret) {
> >  		DRM_ERROR("failed to attach drm bridge\n");
> >  		return -EINVAL;
> > diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> > index 235ce7d1583d..f5009ae39b89 100644
> > --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> > @@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
> >  	hdmi->bridge = bridge;
> >  	bridge->driver_private = hdmi;
> >  	bridge->funcs = &dw_hdmi_bridge_funcs;
> > -	ret = drm_bridge_attach(drm, bridge);
> > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> >  	if (ret) {
> >  		DRM_ERROR("Failed to initialize bridge with drm\n");
> >  		return -EINVAL;
> >  	}
> > 
> > -	encoder->bridge = bridge;
> >  	hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
> > 
> >  	drm_connector_helper_add(&hdmi->connector,
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 0ee052b7c21a..850bd6509ef1 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/mutex.h>
> > 
> >  #include <drm/drm_bridge.h>
> > +#include <drm/drm_encoder.h>
> > 
> >  /**
> >   * DOC: overview
> > @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
> >  EXPORT_SYMBOL(drm_bridge_remove);
> > 
> >  /**
> > - * drm_bridge_attach - associate given bridge to our DRM device
> > + * drm_bridge_attach - attach the bridge to an encoder's chain
> >   *
> > - * @dev: DRM device
> > - * @bridge: bridge control structure
> > + * @encoder: DRM encoder
> > + * @bridge: bridge to attach
> > + * @previous: previous bridge in the chain (optional)
> >   *
> > - * Called by a kms driver to link one of our encoder/bridge to the given
> > - * bridge.
> > + * Called by a kms driver to link the bridge to an encoder's chain. The previous
> > + * argument specifies the previous bridge in the chain. If NULL, the bridge is
> > + * linked directly at the encoder's output. Otherwise it is linked at the
> > + * previous bridge's output.
> >   *
> > - * Note that setting up links between the bridge and our encoder/bridge
> > - * objects needs to be handled by the kms driver itself.
> > + * If non-NULL the previous bridge must be already attached by a call to this
> > + * function.
> >   *
> >   * RETURNS:
> >   * Zero on success, error code on failure
> >   */
> > -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
> > +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> > +		      struct drm_bridge *previous)
> >  {
> > -	if (!dev || !bridge)
> > +	int ret;
> > +
> > +	if (!encoder || !bridge)
> > +		return -EINVAL;
> > +
> > +	if (previous && (!previous->dev || previous->encoder != encoder))
> >  		return -EINVAL;
> > 
> >  	if (bridge->dev)
> >  		return -EBUSY;
> > 
> > -	bridge->dev = dev;
> > +	bridge->dev = encoder->dev;
> > +	bridge->encoder = encoder;
> > +
> > +	if (bridge->funcs->attach) {
> > +		ret = bridge->funcs->attach(bridge);
> > +		if (ret < 0) {
> > +			bridge->dev = NULL;
> > +			bridge->encoder = NULL;
> > +			return ret;
> > +		}
> > +	}
> > 
> > -	if (bridge->funcs->attach)
> > -		return bridge->funcs->attach(bridge);
> > +	if (previous)
> > +		previous->next = bridge;
> > +	else
> > +		encoder->bridge = bridge;
> > 
> >  	return 0;
> >  }
> > diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> > index 7bae08c2bf0a..ba7be6169339 100644
> > --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> > @@ -182,9 +182,7 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = {
> >  int drm_simple_display_pipe_attach_bridge(struct drm_simple_display_pipe *pipe,
> >  					  struct drm_bridge *bridge)
> >  {
> > -	bridge->encoder = &pipe->encoder;
> > -	pipe->encoder.bridge = bridge;
> > -	return drm_bridge_attach(pipe->encoder.dev, bridge);
> > +	return drm_bridge_attach(&pipe->encoder, bridge, NULL);
> >  }
> >  EXPORT_SYMBOL(drm_simple_display_pipe_attach_bridge);
> > 
> > diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> > index 528229faffe4..1ef0be338b85 100644
> > --- a/drivers/gpu/drm/exynos/exynos_dp.c
> > +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> > @@ -99,7 +99,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
> >  				   struct drm_connector *connector)
> >  {
> >  	struct exynos_dp_device *dp = to_dp(plat_data);
> > -	struct drm_encoder *encoder = &dp->encoder;
> >  	int ret;
> > 
> >  	drm_connector_register(connector);
> > @@ -107,9 +106,7 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
> > 
> >  	/* Pre-empt DP connector creation if there's a bridge */
> >  	if (dp->ptn_bridge) {
> > -		bridge->next = dp->ptn_bridge;
> > -		dp->ptn_bridge->encoder = encoder;
> > -		ret = drm_bridge_attach(encoder->dev, dp->ptn_bridge);
> > +		ret = drm_bridge_attach(&dp->encoder, dp->ptn_bridge, bridge);
> >  		if (ret) {
> >  			DRM_ERROR("Failed to attach bridge to drm\n");
> >  			bridge->next = NULL;
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > index e07cb1fe4860..812e2ec0761d 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > @@ -1718,10 +1718,8 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
> >  	}
> > 
> >  	bridge = of_drm_find_bridge(dsi->bridge_node);
> > -	if (bridge) {
> > -		encoder->bridge = bridge;
> > -		drm_bridge_attach(drm_dev, bridge);
> > -	}
> > +	if (bridge)
> > +		drm_bridge_attach(encoder, bridge, NULL);
> > 
> >  	return mipi_dsi_host_register(&dsi->dsi_host);
> >  }
> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > index 05a8ee106879..c3651456c963 100644
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > @@ -160,10 +160,7 @@ static int fsl_dcu_attach_endpoint(struct fsl_dcu_drm_device *fsl_dev,
> >  	if (!bridge)
> >  		return -ENODEV;
> > 
> > -	fsl_dev->encoder.bridge = bridge;
> > -	bridge->encoder = &fsl_dev->encoder;
> > -
> > -	return drm_bridge_attach(fsl_dev->drm, bridge);
> > +	return drm_bridge_attach(&fsl_dev->encoder, bridge, NULL);
> >  }
> > 
> >  int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev)
> > diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> > index 998452ad0fcb..1737e98bc10a 100644
> > --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> > +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> > @@ -709,10 +709,7 @@ static int dsi_bridge_init(struct drm_device *dev, struct dw_dsi *dsi)
> >  	int ret;
> > 
> >  	/* associate the bridge to dsi encoder */
> > -	encoder->bridge = bridge;
> > -	bridge->encoder = encoder;
> > -
> > -	ret = drm_bridge_attach(dev, bridge);
> > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> >  	if (ret) {
> >  		DRM_ERROR("failed to attach external bridge\n");
> >  		return ret;
> > diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> > index 516d06490465..ec49ea3d8e40 100644
> > --- a/drivers/gpu/drm/imx/imx-ldb.c
> > +++ b/drivers/gpu/drm/imx/imx-ldb.c
> > @@ -454,10 +454,8 @@ static int imx_ldb_register(struct drm_device *drm,
> >  			 DRM_MODE_ENCODER_LVDS, NULL);
> > 
> >  	if (imx_ldb_ch->bridge) {
> > -		imx_ldb_ch->bridge->encoder = encoder;
> > -
> > -		imx_ldb_ch->encoder.bridge = imx_ldb_ch->bridge;
> > -		ret = drm_bridge_attach(drm, imx_ldb_ch->bridge);
> > +		ret = drm_bridge_attach(&imx_ldb_ch->encoder,
> > +					imx_ldb_ch->bridge, NULL);
> >  		if (ret) {
> >  			DRM_ERROR("Failed to initialize bridge with drm\n");
> >  			return ret;
> > diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
> > index 8582a83c0d9b..51d9f735c358 100644
> > --- a/drivers/gpu/drm/imx/parallel-display.c
> > +++ b/drivers/gpu/drm/imx/parallel-display.c
> > @@ -191,9 +191,7 @@ static int imx_pd_register(struct drm_device *drm,
> >  		drm_panel_attach(imxpd->panel, &imxpd->connector);
> > 
> >  	if (imxpd->bridge) {
> > -		imxpd->bridge->encoder = encoder;
> > -		encoder->bridge = imxpd->bridge;
> > -		ret = drm_bridge_attach(drm, imxpd->bridge);
> > +		ret = drm_bridge_attach(encoder, imxpd->bridge, NULL);
> >  		if (ret < 0) {
> >  			dev_err(imxpd->dev, "failed to attach bridge: %d\n",
> >  				ret);
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > index 90fb831ef031..3bd3bd688d1a 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > @@ -63,6 +63,7 @@ enum mtk_dpi_out_color_format {
> >  struct mtk_dpi {
> >  	struct mtk_ddp_comp ddp_comp;
> >  	struct drm_encoder encoder;
> > +	struct drm_bridge *bridge;
> >  	void __iomem *regs;
> >  	struct device *dev;
> >  	struct clk *engine_clk;
> > @@ -620,8 +621,7 @@ static int mtk_dpi_bind(struct device *dev, struct device *master, void *data)
> >  	/* Currently DPI0 is fixed to be driven by OVL1 */
> >  	dpi->encoder.possible_crtcs = BIT(1);
> > 
> > -	dpi->encoder.bridge->encoder = &dpi->encoder;
> > -	ret = drm_bridge_attach(dpi->encoder.dev, dpi->encoder.bridge);
> > +	ret = drm_bridge_attach(&dpi->encoder, dpi->bridge, NULL);
> >  	if (ret) {
> >  		dev_err(dev, "Failed to attach bridge: %d\n", ret);
> >  		goto err_cleanup;
> > @@ -718,9 +718,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
> > 
> >  	dev_info(dev, "Found bridge node: %s\n", bridge_node->full_name);
> > 
> > -	dpi->encoder.bridge = of_drm_find_bridge(bridge_node);
> > +	dpi->bridge = of_drm_find_bridge(bridge_node);
> >  	of_node_put(bridge_node);
> > -	if (!dpi->encoder.bridge)
> > +	if (!dpi->bridge)
> >  		return -EPROBE_DEFER;
> > 
> >  	comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DPI);
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index 2c42f90809d8..dd71cbb1a622 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -622,26 +622,6 @@ static const struct drm_connector_helper_funcs
> >  	.get_modes = mtk_dsi_connector_get_modes,
> >  };
> > 
> > -static int mtk_drm_attach_bridge(struct drm_bridge *bridge,
> > -				 struct drm_encoder *encoder)
> > -{
> > -	int ret;
> > -
> > -	if (!bridge)
> > -		return -ENOENT;
> > -
> > -	encoder->bridge = bridge;
> > -	bridge->encoder = encoder;
> > -	ret = drm_bridge_attach(encoder->dev, bridge);
> > -	if (ret) {
> > -		DRM_ERROR("Failed to attach bridge to drm\n");
> > -		encoder->bridge = NULL;
> > -		bridge->encoder = NULL;
> > -	}
> > -
> > -	return ret;
> > -}
> > -
> >  static int mtk_dsi_create_connector(struct drm_device *drm, struct mtk_dsi *dsi)
> >  {
> >  	int ret;
> > @@ -692,8 +672,10 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi)
> >  	dsi->encoder.possible_crtcs = 1;
> > 
> >  	/* If there's a bridge, attach to it and let it create the connector */
> > -	ret = mtk_drm_attach_bridge(dsi->bridge, &dsi->encoder);
> > +	ret = drm_bridge_attach(&dsi->encoder, dsi->bridge, NULL);
> >  	if (ret) {
> > +		DRM_ERROR("Failed to attach bridge to drm\n");
> > +
> >  		/* Otherwise create our own connector and attach to a panel */
> >  		ret = mtk_dsi_create_connector(drm, dsi);
> >  		if (ret)
> > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > index 0e8c4d9af340..c26251260b83 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > @@ -149,6 +149,7 @@ struct hdmi_audio_param {
> > 
> >  struct mtk_hdmi {
> >  	struct drm_bridge bridge;
> > +	struct drm_bridge *next_bridge;
> >  	struct drm_connector conn;
> >  	struct device *dev;
> >  	struct phy *phy;
> > @@ -1314,9 +1315,9 @@ static int mtk_hdmi_bridge_attach(struct drm_bridge *bridge)
> >  		return ret;
> >  	}
> > 
> > -	if (bridge->next) {
> > -		bridge->next->encoder = bridge->encoder;
> > -		ret = drm_bridge_attach(bridge->encoder->dev, bridge->next);
> > +	if (hdmi->next_bridge) {
> > +		ret = drm_bridge_attach(bridge->encoder, hdmi->next_bridge,
> > +					bridge);
> >  		if (ret) {
> >  			dev_err(hdmi->dev,
> >  				"Failed to attach external bridge: %d\n", ret);
> > @@ -1510,8 +1511,8 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
> >  	of_node_put(ep);
> > 
> >  	if (!of_device_is_compatible(remote, "hdmi-connector")) {
> > -		hdmi->bridge.next = of_drm_find_bridge(remote);
> > -		if (!hdmi->bridge.next) {
> > +		hdmi->next_bridge = of_drm_find_bridge(remote);
> > +		if (!hdmi->next_bridge) {
> >  			dev_err(dev, "Waiting for external bridge\n");
> >  			of_node_put(remote);
> >  			return -EPROBE_DEFER;
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > index c8d1f19c9a6d..2bd8dad76105 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > @@ -579,6 +579,7 @@ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id)
> >  	struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
> >  	struct drm_bridge *bridge = NULL;
> >  	struct dsi_bridge *dsi_bridge;
> > +	struct drm_encoder *encoder;
> >  	int ret;
> > 
> >  	dsi_bridge = devm_kzalloc(msm_dsi->dev->dev,
> > @@ -590,10 +591,18 @@ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id)
> > 
> >  	dsi_bridge->id = id;
> > 
> > +	/*
> > +	 * HACK: we may not know the external DSI bridge device's mode
> > +	 * flags here. We'll get to know them only when the device
> > +	 * attaches to the dsi host. For now, assume the bridge supports
> > +	 * DSI video mode
> > +	 */
> > +	encoder = msm_dsi->encoders[MSM_DSI_VIDEO_ENCODER_ID];
> > +
> >  	bridge = &dsi_bridge->base;
> >  	bridge->funcs = &dsi_mgr_bridge_funcs;
> > 
> > -	ret = drm_bridge_attach(msm_dsi->dev, bridge);
> > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> >  	if (ret)
> >  		goto fail;
> > 
> > @@ -628,11 +637,7 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id)
> >  	encoder = msm_dsi->encoders[MSM_DSI_VIDEO_ENCODER_ID];
> > 
> >  	/* link the internal dsi bridge to the external bridge */
> > -	int_bridge->next = ext_bridge;
> > -	/* set the external bridge's encoder as dsi's encoder */
> > -	ext_bridge->encoder = encoder;
> > -
> > -	drm_bridge_attach(dev, ext_bridge);
> > +	drm_bridge_attach(encoder, ext_bridge, int_bridge);
> > 
> >  	/*
> >  	 * we need the drm_connector created by the external bridge
> > diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
> > index 2bc73f82f3f5..931a5c97cccf 100644
> > --- a/drivers/gpu/drm/msm/edp/edp_bridge.c
> > +++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
> > @@ -106,7 +106,7 @@ struct drm_bridge *msm_edp_bridge_init(struct msm_edp *edp)
> >  	bridge = &edp_bridge->base;
> >  	bridge->funcs = &edp_bridge_funcs;
> > 
> > -	ret = drm_bridge_attach(edp->dev, bridge);
> > +	ret = drm_bridge_attach(edp->encoder, bridge, NULL);
> >  	if (ret)
> >  		goto fail;
> > 
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > index bacbd5d8df0e..4e6d1bf27474 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > @@ -227,7 +227,7 @@ struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi)
> >  	bridge = &hdmi_bridge->base;
> >  	bridge->funcs = &msm_hdmi_bridge_funcs;
> > 
> > -	ret = drm_bridge_attach(hdmi->dev, bridge);
> > +	ret = drm_bridge_attach(hdmi->encoder, bridge, NULL);
> >  	if (ret)
> >  		goto fail;
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> > index a1a2c5e7822c..933a2547798e 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> > @@ -124,10 +124,7 @@ int rcar_du_hdmienc_init(struct rcar_du_device *rcdu,
> >  	hdmienc->renc = renc;
> > 
> >  	/* Link the bridge to the encoder. */
> > -	bridge->encoder = encoder;
> > -	encoder->bridge = bridge;
> > -
> > -	ret = drm_bridge_attach(rcdu->ddev, bridge);
> > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> >  	if (ret) {
> >  		drm_encoder_cleanup(encoder);
> >  		return ret;
> > diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> > index e8c1ed08a9f7..411dc6ec976e 100644
> > --- a/drivers/gpu/drm/sti/sti_dvo.c
> > +++ b/drivers/gpu/drm/sti/sti_dvo.c
> > @@ -478,14 +478,13 @@ static int sti_dvo_bind(struct device *dev, struct device *master, void *data)
> >  		return err;
> >  	}
> > 
> > -	err = drm_bridge_attach(drm_dev, bridge);
> > +	err = drm_bridge_attach(encoder, bridge, NULL);
> >  	if (err) {
> >  		DRM_ERROR("Failed to attach bridge\n");
> >  		return err;
> >  	}
> > 
> >  	dvo->bridge = bridge;
> > -	encoder->bridge = bridge;
> >  	connector->encoder = encoder;
> >  	dvo->encoder = encoder;
> > 
> > diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> > index 96f336dd0e29..66d37d78152a 100644
> > --- a/drivers/gpu/drm/sti/sti_hda.c
> > +++ b/drivers/gpu/drm/sti/sti_hda.c
> > @@ -707,9 +707,8 @@ static int sti_hda_bind(struct device *dev, struct device *master, void *data)
> > 
> >  	bridge->driver_private = hda;
> >  	bridge->funcs = &sti_hda_bridge_funcs;
> > -	drm_bridge_attach(drm_dev, bridge);
> > +	drm_bridge_attach(encoder, bridge, NULL);
> > 
> > -	encoder->bridge = bridge;
> >  	connector->encoder = encoder;
> > 
> >  	drm_connector = (struct drm_connector *)connector;
> > diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> > index 376b0763c874..f0af1ae82ee9 100644
> > --- a/drivers/gpu/drm/sti/sti_hdmi.c
> > +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> > @@ -1308,9 +1308,8 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
> > 
> >  	bridge->driver_private = hdmi;
> >  	bridge->funcs = &sti_hdmi_bridge_funcs;
> > -	drm_bridge_attach(drm_dev, bridge);
> > +	drm_bridge_attach(encoder, bridge, NULL);
> > 
> > -	encoder->bridge = bridge;
> >  	connector->encoder = encoder;
> > 
> >  	drm_connector = (struct drm_connector *)connector;
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > index f5e86fe7750e..757208f51731 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > @@ -208,6 +208,7 @@ int sun4i_rgb_init(struct drm_device *drm)
> >  	struct sun4i_drv *drv = drm->dev_private;
> >  	struct sun4i_tcon *tcon = drv->tcon;
> >  	struct drm_encoder *encoder;
> > +	struct drm_bridge *bridge;
> >  	struct sun4i_rgb *rgb;
> >  	int ret;
> > 
> > @@ -218,8 +219,8 @@ int sun4i_rgb_init(struct drm_device *drm)
> >  	encoder = &rgb->encoder;
> > 
> >  	tcon->panel = sun4i_tcon_find_panel(tcon->dev->of_node);
> > -	encoder->bridge = sun4i_tcon_find_bridge(tcon->dev->of_node);
> > -	if (IS_ERR(tcon->panel) && IS_ERR(encoder->bridge)) {
> > +	bridge = sun4i_tcon_find_bridge(tcon->dev->of_node);
> > +	if (IS_ERR(tcon->panel) && IS_ERR(bridge)) {
> >  		dev_info(drm->dev, "No panel or bridge found... RGB output disabled\n");
> >  		return 0;
> >  	}
> > @@ -260,16 +261,12 @@ int sun4i_rgb_init(struct drm_device *drm)
> >  		}
> >  	}
> > 
> > -	if (!IS_ERR(encoder->bridge)) {
> > -		encoder->bridge->encoder = &rgb->encoder;
> > -
> > -		ret = drm_bridge_attach(drm, encoder->bridge);
> > +	if (!IS_ERR(bridge)) {
> > +		ret = drm_bridge_attach(encoder, bridge, NULL);
> >  		if (ret) {
> >  			dev_err(drm->dev, "Couldn't attach our bridge\n");
> >  			goto err_cleanup_connector;
> >  		}
> > -	} else {
> > -		encoder->bridge = NULL;
> >  	}
> > 
> >  	return 0;
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > index c67d7cd7d57e..b0dd5e8634ae 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > @@ -167,10 +167,8 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
> >  	int ret;
> > 
> >  	priv->external_encoder->possible_crtcs = BIT(0);
> > -	priv->external_encoder->bridge = bridge;
> > -	bridge->encoder = priv->external_encoder;
> > 
> > -	ret = drm_bridge_attach(ddev, bridge);
> > +	ret = drm_bridge_attach(priv->external_encoder, bridge, NULL);
> >  	if (ret) {
> >  		dev_err(ddev->dev, "drm_bridge_attach() failed %d\n", ret);
> >  		return ret;
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index 530a1d6e8cde..94e5ee96b3b5 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -201,7 +201,8 @@ struct drm_bridge {
> >  int drm_bridge_add(struct drm_bridge *bridge);
> >  void drm_bridge_remove(struct drm_bridge *bridge);
> >  struct drm_bridge *of_drm_find_bridge(struct device_node *np);
> > -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
> > +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> > +		      struct drm_bridge *previous);
> >  void drm_bridge_detach(struct drm_bridge *bridge);
> > 
> >  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
> > 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Daniel Vetter Dec. 16, 2016, 1:29 p.m. UTC | #6

On Fri, Dec 16, 2016 at 02:29:00PM +0100, Daniel Vetter wrote:
> On Fri, Dec 16, 2016 at 02:05:48PM +0530, Archit Taneja wrote:
> > Hi,
> > 
> > On 12/14/2016 03:29 PM, Laurent Pinchart wrote:
> > > Instead of linking encoders and bridges in every driver (and getting it
> > > wrong half of the time, as many drivers forget to set the drm_bridge
> > > encoder pointer), do so in core code. The drm_bridge_attach() function
> > > needs the encoder and optional previous bridge to perform that task,
> > > update all the callers.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > Acked-by: Stefan Agner <stefan@agner.ch> # For DCU
> > > Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # For atmel-hlcdc
> > > Acked-by: Vincent Abriou <vincent.abriou@st.com> # For STI
> > 
> > This one needs acks for arcgpu, tilcdc, mediatek and imx. The changes in
> > those drivers look good to me, though. Will push it in a day or so unless anyone
> > has any comments on it.
> 
> Imo big refactorings don't need ack from every driver maintainer, since
> some fail to do their jobs timely. Overall review + a few acks should be
> good enough. Anyway I double-checked things and looks all good.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Argh, wrong macro ;-)

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> > 
> > Thanks,
> > Archit
> > 
> > > ---
> > >  drivers/gpu/drm/arc/arcpgu_hdmi.c                  |  5 +--
> > >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
> > >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
> > >  drivers/gpu/drm/bridge/dw-hdmi.c                   |  3 +-
> > >  drivers/gpu/drm/drm_bridge.c                       | 46 ++++++++++++++++------
> > >  drivers/gpu/drm/drm_simple_kms_helper.c            |  4 +-
> > >  drivers/gpu/drm/exynos/exynos_dp.c                 |  5 +--
> > >  drivers/gpu/drm/exynos/exynos_drm_dsi.c            |  6 +--
> > >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          |  5 +--
> > >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c       |  5 +--
> > >  drivers/gpu/drm/imx/imx-ldb.c                      |  6 +--
> > >  drivers/gpu/drm/imx/parallel-display.c             |  4 +-
> > >  drivers/gpu/drm/mediatek/mtk_dpi.c                 |  8 ++--
> > >  drivers/gpu/drm/mediatek/mtk_dsi.c                 | 24 ++---------
> > >  drivers/gpu/drm/mediatek/mtk_hdmi.c                | 11 +++---
> > >  drivers/gpu/drm/msm/dsi/dsi_manager.c              | 17 +++++---
> > >  drivers/gpu/drm/msm/edp/edp_bridge.c               |  2 +-
> > >  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c             |  2 +-
> > >  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c          |  5 +--
> > >  drivers/gpu/drm/sti/sti_dvo.c                      |  3 +-
> > >  drivers/gpu/drm/sti/sti_hda.c                      |  3 +-
> > >  drivers/gpu/drm/sti/sti_hdmi.c                     |  3 +-
> > >  drivers/gpu/drm/sun4i/sun4i_rgb.c                  | 13 +++---
> > >  drivers/gpu/drm/tilcdc/tilcdc_external.c           |  4 +-
> > >  include/drm/drm_bridge.h                           |  3 +-
> > >  25 files changed, 85 insertions(+), 110 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > > index b69c66b4897e..0ce7f398bcff 100644
> > > --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > > +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > > @@ -47,10 +47,7 @@ int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
> > >  		return ret;
> > > 
> > >  	/* Link drm_bridge to encoder */
> > > -	bridge->encoder = encoder;
> > > -	encoder->bridge = bridge;
> > > -
> > > -	ret = drm_bridge_attach(drm, bridge);
> > > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> > >  	if (ret)
> > >  		drm_encoder_cleanup(encoder);
> > > 
> > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> > > index 6119b5085501..e7799b6ee829 100644
> > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> > > @@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev,
> > >  	of_node_put(np);
> > > 
> > >  	if (bridge) {
> > > -		output->encoder.bridge = bridge;
> > > -		bridge->encoder = &output->encoder;
> > > -		ret = drm_bridge_attach(dev, bridge);
> > > +		ret = drm_bridge_attach(&output->encoder, bridge, NULL);
> > >  		if (!ret)
> > >  			return 0;
> > >  	}
> > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > index eb9bf8786c24..b7494c8d43fe 100644
> > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > @@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct drm_device *drm_dev,
> > > 
> > >  	dp->bridge = bridge;
> > > 
> > > -	dp->encoder->bridge = bridge;
> > >  	bridge->driver_private = dp;
> > > -	bridge->encoder = dp->encoder;
> > >  	bridge->funcs = &analogix_dp_bridge_funcs;
> > > 
> > > -	ret = drm_bridge_attach(drm_dev, bridge);
> > > +	ret = drm_bridge_attach(dp->encoder, bridge, NULL);
> > >  	if (ret) {
> > >  		DRM_ERROR("failed to attach drm bridge\n");
> > >  		return -EINVAL;
> > > diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> > > index 235ce7d1583d..f5009ae39b89 100644
> > > --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> > > +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> > > @@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
> > >  	hdmi->bridge = bridge;
> > >  	bridge->driver_private = hdmi;
> > >  	bridge->funcs = &dw_hdmi_bridge_funcs;
> > > -	ret = drm_bridge_attach(drm, bridge);
> > > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> > >  	if (ret) {
> > >  		DRM_ERROR("Failed to initialize bridge with drm\n");
> > >  		return -EINVAL;
> > >  	}
> > > 
> > > -	encoder->bridge = bridge;
> > >  	hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
> > > 
> > >  	drm_connector_helper_add(&hdmi->connector,
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index 0ee052b7c21a..850bd6509ef1 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -26,6 +26,7 @@
> > >  #include <linux/mutex.h>
> > > 
> > >  #include <drm/drm_bridge.h>
> > > +#include <drm/drm_encoder.h>
> > > 
> > >  /**
> > >   * DOC: overview
> > > @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
> > >  EXPORT_SYMBOL(drm_bridge_remove);
> > > 
> > >  /**
> > > - * drm_bridge_attach - associate given bridge to our DRM device
> > > + * drm_bridge_attach - attach the bridge to an encoder's chain
> > >   *
> > > - * @dev: DRM device
> > > - * @bridge: bridge control structure
> > > + * @encoder: DRM encoder
> > > + * @bridge: bridge to attach
> > > + * @previous: previous bridge in the chain (optional)
> > >   *
> > > - * Called by a kms driver to link one of our encoder/bridge to the given
> > > - * bridge.
> > > + * Called by a kms driver to link the bridge to an encoder's chain. The previous
> > > + * argument specifies the previous bridge in the chain. If NULL, the bridge is
> > > + * linked directly at the encoder's output. Otherwise it is linked at the
> > > + * previous bridge's output.
> > >   *
> > > - * Note that setting up links between the bridge and our encoder/bridge
> > > - * objects needs to be handled by the kms driver itself.
> > > + * If non-NULL the previous bridge must be already attached by a call to this
> > > + * function.
> > >   *
> > >   * RETURNS:
> > >   * Zero on success, error code on failure
> > >   */
> > > -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
> > > +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> > > +		      struct drm_bridge *previous)
> > >  {
> > > -	if (!dev || !bridge)
> > > +	int ret;
> > > +
> > > +	if (!encoder || !bridge)
> > > +		return -EINVAL;
> > > +
> > > +	if (previous && (!previous->dev || previous->encoder != encoder))
> > >  		return -EINVAL;
> > > 
> > >  	if (bridge->dev)
> > >  		return -EBUSY;
> > > 
> > > -	bridge->dev = dev;
> > > +	bridge->dev = encoder->dev;
> > > +	bridge->encoder = encoder;
> > > +
> > > +	if (bridge->funcs->attach) {
> > > +		ret = bridge->funcs->attach(bridge);
> > > +		if (ret < 0) {
> > > +			bridge->dev = NULL;
> > > +			bridge->encoder = NULL;
> > > +			return ret;
> > > +		}
> > > +	}
> > > 
> > > -	if (bridge->funcs->attach)
> > > -		return bridge->funcs->attach(bridge);
> > > +	if (previous)
> > > +		previous->next = bridge;
> > > +	else
> > > +		encoder->bridge = bridge;
> > > 
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> > > index 7bae08c2bf0a..ba7be6169339 100644
> > > --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> > > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> > > @@ -182,9 +182,7 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = {
> > >  int drm_simple_display_pipe_attach_bridge(struct drm_simple_display_pipe *pipe,
> > >  					  struct drm_bridge *bridge)
> > >  {
> > > -	bridge->encoder = &pipe->encoder;
> > > -	pipe->encoder.bridge = bridge;
> > > -	return drm_bridge_attach(pipe->encoder.dev, bridge);
> > > +	return drm_bridge_attach(&pipe->encoder, bridge, NULL);
> > >  }
> > >  EXPORT_SYMBOL(drm_simple_display_pipe_attach_bridge);
> > > 
> > > diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> > > index 528229faffe4..1ef0be338b85 100644
> > > --- a/drivers/gpu/drm/exynos/exynos_dp.c
> > > +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> > > @@ -99,7 +99,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
> > >  				   struct drm_connector *connector)
> > >  {
> > >  	struct exynos_dp_device *dp = to_dp(plat_data);
> > > -	struct drm_encoder *encoder = &dp->encoder;
> > >  	int ret;
> > > 
> > >  	drm_connector_register(connector);
> > > @@ -107,9 +106,7 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
> > > 
> > >  	/* Pre-empt DP connector creation if there's a bridge */
> > >  	if (dp->ptn_bridge) {
> > > -		bridge->next = dp->ptn_bridge;
> > > -		dp->ptn_bridge->encoder = encoder;
> > > -		ret = drm_bridge_attach(encoder->dev, dp->ptn_bridge);
> > > +		ret = drm_bridge_attach(&dp->encoder, dp->ptn_bridge, bridge);
> > >  		if (ret) {
> > >  			DRM_ERROR("Failed to attach bridge to drm\n");
> > >  			bridge->next = NULL;
> > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > index e07cb1fe4860..812e2ec0761d 100644
> > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > > @@ -1718,10 +1718,8 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
> > >  	}
> > > 
> > >  	bridge = of_drm_find_bridge(dsi->bridge_node);
> > > -	if (bridge) {
> > > -		encoder->bridge = bridge;
> > > -		drm_bridge_attach(drm_dev, bridge);
> > > -	}
> > > +	if (bridge)
> > > +		drm_bridge_attach(encoder, bridge, NULL);
> > > 
> > >  	return mipi_dsi_host_register(&dsi->dsi_host);
> > >  }
> > > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > > index 05a8ee106879..c3651456c963 100644
> > > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > > @@ -160,10 +160,7 @@ static int fsl_dcu_attach_endpoint(struct fsl_dcu_drm_device *fsl_dev,
> > >  	if (!bridge)
> > >  		return -ENODEV;
> > > 
> > > -	fsl_dev->encoder.bridge = bridge;
> > > -	bridge->encoder = &fsl_dev->encoder;
> > > -
> > > -	return drm_bridge_attach(fsl_dev->drm, bridge);
> > > +	return drm_bridge_attach(&fsl_dev->encoder, bridge, NULL);
> > >  }
> > > 
> > >  int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev)
> > > diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> > > index 998452ad0fcb..1737e98bc10a 100644
> > > --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> > > +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> > > @@ -709,10 +709,7 @@ static int dsi_bridge_init(struct drm_device *dev, struct dw_dsi *dsi)
> > >  	int ret;
> > > 
> > >  	/* associate the bridge to dsi encoder */
> > > -	encoder->bridge = bridge;
> > > -	bridge->encoder = encoder;
> > > -
> > > -	ret = drm_bridge_attach(dev, bridge);
> > > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> > >  	if (ret) {
> > >  		DRM_ERROR("failed to attach external bridge\n");
> > >  		return ret;
> > > diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> > > index 516d06490465..ec49ea3d8e40 100644
> > > --- a/drivers/gpu/drm/imx/imx-ldb.c
> > > +++ b/drivers/gpu/drm/imx/imx-ldb.c
> > > @@ -454,10 +454,8 @@ static int imx_ldb_register(struct drm_device *drm,
> > >  			 DRM_MODE_ENCODER_LVDS, NULL);
> > > 
> > >  	if (imx_ldb_ch->bridge) {
> > > -		imx_ldb_ch->bridge->encoder = encoder;
> > > -
> > > -		imx_ldb_ch->encoder.bridge = imx_ldb_ch->bridge;
> > > -		ret = drm_bridge_attach(drm, imx_ldb_ch->bridge);
> > > +		ret = drm_bridge_attach(&imx_ldb_ch->encoder,
> > > +					imx_ldb_ch->bridge, NULL);
> > >  		if (ret) {
> > >  			DRM_ERROR("Failed to initialize bridge with drm\n");
> > >  			return ret;
> > > diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
> > > index 8582a83c0d9b..51d9f735c358 100644
> > > --- a/drivers/gpu/drm/imx/parallel-display.c
> > > +++ b/drivers/gpu/drm/imx/parallel-display.c
> > > @@ -191,9 +191,7 @@ static int imx_pd_register(struct drm_device *drm,
> > >  		drm_panel_attach(imxpd->panel, &imxpd->connector);
> > > 
> > >  	if (imxpd->bridge) {
> > > -		imxpd->bridge->encoder = encoder;
> > > -		encoder->bridge = imxpd->bridge;
> > > -		ret = drm_bridge_attach(drm, imxpd->bridge);
> > > +		ret = drm_bridge_attach(encoder, imxpd->bridge, NULL);
> > >  		if (ret < 0) {
> > >  			dev_err(imxpd->dev, "failed to attach bridge: %d\n",
> > >  				ret);
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > index 90fb831ef031..3bd3bd688d1a 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > @@ -63,6 +63,7 @@ enum mtk_dpi_out_color_format {
> > >  struct mtk_dpi {
> > >  	struct mtk_ddp_comp ddp_comp;
> > >  	struct drm_encoder encoder;
> > > +	struct drm_bridge *bridge;
> > >  	void __iomem *regs;
> > >  	struct device *dev;
> > >  	struct clk *engine_clk;
> > > @@ -620,8 +621,7 @@ static int mtk_dpi_bind(struct device *dev, struct device *master, void *data)
> > >  	/* Currently DPI0 is fixed to be driven by OVL1 */
> > >  	dpi->encoder.possible_crtcs = BIT(1);
> > > 
> > > -	dpi->encoder.bridge->encoder = &dpi->encoder;
> > > -	ret = drm_bridge_attach(dpi->encoder.dev, dpi->encoder.bridge);
> > > +	ret = drm_bridge_attach(&dpi->encoder, dpi->bridge, NULL);
> > >  	if (ret) {
> > >  		dev_err(dev, "Failed to attach bridge: %d\n", ret);
> > >  		goto err_cleanup;
> > > @@ -718,9 +718,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
> > > 
> > >  	dev_info(dev, "Found bridge node: %s\n", bridge_node->full_name);
> > > 
> > > -	dpi->encoder.bridge = of_drm_find_bridge(bridge_node);
> > > +	dpi->bridge = of_drm_find_bridge(bridge_node);
> > >  	of_node_put(bridge_node);
> > > -	if (!dpi->encoder.bridge)
> > > +	if (!dpi->bridge)
> > >  		return -EPROBE_DEFER;
> > > 
> > >  	comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DPI);
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > index 2c42f90809d8..dd71cbb1a622 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > @@ -622,26 +622,6 @@ static const struct drm_connector_helper_funcs
> > >  	.get_modes = mtk_dsi_connector_get_modes,
> > >  };
> > > 
> > > -static int mtk_drm_attach_bridge(struct drm_bridge *bridge,
> > > -				 struct drm_encoder *encoder)
> > > -{
> > > -	int ret;
> > > -
> > > -	if (!bridge)
> > > -		return -ENOENT;
> > > -
> > > -	encoder->bridge = bridge;
> > > -	bridge->encoder = encoder;
> > > -	ret = drm_bridge_attach(encoder->dev, bridge);
> > > -	if (ret) {
> > > -		DRM_ERROR("Failed to attach bridge to drm\n");
> > > -		encoder->bridge = NULL;
> > > -		bridge->encoder = NULL;
> > > -	}
> > > -
> > > -	return ret;
> > > -}
> > > -
> > >  static int mtk_dsi_create_connector(struct drm_device *drm, struct mtk_dsi *dsi)
> > >  {
> > >  	int ret;
> > > @@ -692,8 +672,10 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi)
> > >  	dsi->encoder.possible_crtcs = 1;
> > > 
> > >  	/* If there's a bridge, attach to it and let it create the connector */
> > > -	ret = mtk_drm_attach_bridge(dsi->bridge, &dsi->encoder);
> > > +	ret = drm_bridge_attach(&dsi->encoder, dsi->bridge, NULL);
> > >  	if (ret) {
> > > +		DRM_ERROR("Failed to attach bridge to drm\n");
> > > +
> > >  		/* Otherwise create our own connector and attach to a panel */
> > >  		ret = mtk_dsi_create_connector(drm, dsi);
> > >  		if (ret)
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > index 0e8c4d9af340..c26251260b83 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > @@ -149,6 +149,7 @@ struct hdmi_audio_param {
> > > 
> > >  struct mtk_hdmi {
> > >  	struct drm_bridge bridge;
> > > +	struct drm_bridge *next_bridge;
> > >  	struct drm_connector conn;
> > >  	struct device *dev;
> > >  	struct phy *phy;
> > > @@ -1314,9 +1315,9 @@ static int mtk_hdmi_bridge_attach(struct drm_bridge *bridge)
> > >  		return ret;
> > >  	}
> > > 
> > > -	if (bridge->next) {
> > > -		bridge->next->encoder = bridge->encoder;
> > > -		ret = drm_bridge_attach(bridge->encoder->dev, bridge->next);
> > > +	if (hdmi->next_bridge) {
> > > +		ret = drm_bridge_attach(bridge->encoder, hdmi->next_bridge,
> > > +					bridge);
> > >  		if (ret) {
> > >  			dev_err(hdmi->dev,
> > >  				"Failed to attach external bridge: %d\n", ret);
> > > @@ -1510,8 +1511,8 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
> > >  	of_node_put(ep);
> > > 
> > >  	if (!of_device_is_compatible(remote, "hdmi-connector")) {
> > > -		hdmi->bridge.next = of_drm_find_bridge(remote);
> > > -		if (!hdmi->bridge.next) {
> > > +		hdmi->next_bridge = of_drm_find_bridge(remote);
> > > +		if (!hdmi->next_bridge) {
> > >  			dev_err(dev, "Waiting for external bridge\n");
> > >  			of_node_put(remote);
> > >  			return -EPROBE_DEFER;
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > index c8d1f19c9a6d..2bd8dad76105 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > @@ -579,6 +579,7 @@ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id)
> > >  	struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
> > >  	struct drm_bridge *bridge = NULL;
> > >  	struct dsi_bridge *dsi_bridge;
> > > +	struct drm_encoder *encoder;
> > >  	int ret;
> > > 
> > >  	dsi_bridge = devm_kzalloc(msm_dsi->dev->dev,
> > > @@ -590,10 +591,18 @@ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id)
> > > 
> > >  	dsi_bridge->id = id;
> > > 
> > > +	/*
> > > +	 * HACK: we may not know the external DSI bridge device's mode
> > > +	 * flags here. We'll get to know them only when the device
> > > +	 * attaches to the dsi host. For now, assume the bridge supports
> > > +	 * DSI video mode
> > > +	 */
> > > +	encoder = msm_dsi->encoders[MSM_DSI_VIDEO_ENCODER_ID];
> > > +
> > >  	bridge = &dsi_bridge->base;
> > >  	bridge->funcs = &dsi_mgr_bridge_funcs;
> > > 
> > > -	ret = drm_bridge_attach(msm_dsi->dev, bridge);
> > > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> > >  	if (ret)
> > >  		goto fail;
> > > 
> > > @@ -628,11 +637,7 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id)
> > >  	encoder = msm_dsi->encoders[MSM_DSI_VIDEO_ENCODER_ID];
> > > 
> > >  	/* link the internal dsi bridge to the external bridge */
> > > -	int_bridge->next = ext_bridge;
> > > -	/* set the external bridge's encoder as dsi's encoder */
> > > -	ext_bridge->encoder = encoder;
> > > -
> > > -	drm_bridge_attach(dev, ext_bridge);
> > > +	drm_bridge_attach(encoder, ext_bridge, int_bridge);
> > > 
> > >  	/*
> > >  	 * we need the drm_connector created by the external bridge
> > > diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
> > > index 2bc73f82f3f5..931a5c97cccf 100644
> > > --- a/drivers/gpu/drm/msm/edp/edp_bridge.c
> > > +++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
> > > @@ -106,7 +106,7 @@ struct drm_bridge *msm_edp_bridge_init(struct msm_edp *edp)
> > >  	bridge = &edp_bridge->base;
> > >  	bridge->funcs = &edp_bridge_funcs;
> > > 
> > > -	ret = drm_bridge_attach(edp->dev, bridge);
> > > +	ret = drm_bridge_attach(edp->encoder, bridge, NULL);
> > >  	if (ret)
> > >  		goto fail;
> > > 
> > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > index bacbd5d8df0e..4e6d1bf27474 100644
> > > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > > @@ -227,7 +227,7 @@ struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi)
> > >  	bridge = &hdmi_bridge->base;
> > >  	bridge->funcs = &msm_hdmi_bridge_funcs;
> > > 
> > > -	ret = drm_bridge_attach(hdmi->dev, bridge);
> > > +	ret = drm_bridge_attach(hdmi->encoder, bridge, NULL);
> > >  	if (ret)
> > >  		goto fail;
> > > 
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> > > index a1a2c5e7822c..933a2547798e 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> > > @@ -124,10 +124,7 @@ int rcar_du_hdmienc_init(struct rcar_du_device *rcdu,
> > >  	hdmienc->renc = renc;
> > > 
> > >  	/* Link the bridge to the encoder. */
> > > -	bridge->encoder = encoder;
> > > -	encoder->bridge = bridge;
> > > -
> > > -	ret = drm_bridge_attach(rcdu->ddev, bridge);
> > > +	ret = drm_bridge_attach(encoder, bridge, NULL);
> > >  	if (ret) {
> > >  		drm_encoder_cleanup(encoder);
> > >  		return ret;
> > > diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> > > index e8c1ed08a9f7..411dc6ec976e 100644
> > > --- a/drivers/gpu/drm/sti/sti_dvo.c
> > > +++ b/drivers/gpu/drm/sti/sti_dvo.c
> > > @@ -478,14 +478,13 @@ static int sti_dvo_bind(struct device *dev, struct device *master, void *data)
> > >  		return err;
> > >  	}
> > > 
> > > -	err = drm_bridge_attach(drm_dev, bridge);
> > > +	err = drm_bridge_attach(encoder, bridge, NULL);
> > >  	if (err) {
> > >  		DRM_ERROR("Failed to attach bridge\n");
> > >  		return err;
> > >  	}
> > > 
> > >  	dvo->bridge = bridge;
> > > -	encoder->bridge = bridge;
> > >  	connector->encoder = encoder;
> > >  	dvo->encoder = encoder;
> > > 
> > > diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> > > index 96f336dd0e29..66d37d78152a 100644
> > > --- a/drivers/gpu/drm/sti/sti_hda.c
> > > +++ b/drivers/gpu/drm/sti/sti_hda.c
> > > @@ -707,9 +707,8 @@ static int sti_hda_bind(struct device *dev, struct device *master, void *data)
> > > 
> > >  	bridge->driver_private = hda;
> > >  	bridge->funcs = &sti_hda_bridge_funcs;
> > > -	drm_bridge_attach(drm_dev, bridge);
> > > +	drm_bridge_attach(encoder, bridge, NULL);
> > > 
> > > -	encoder->bridge = bridge;
> > >  	connector->encoder = encoder;
> > > 
> > >  	drm_connector = (struct drm_connector *)connector;
> > > diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> > > index 376b0763c874..f0af1ae82ee9 100644
> > > --- a/drivers/gpu/drm/sti/sti_hdmi.c
> > > +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> > > @@ -1308,9 +1308,8 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
> > > 
> > >  	bridge->driver_private = hdmi;
> > >  	bridge->funcs = &sti_hdmi_bridge_funcs;
> > > -	drm_bridge_attach(drm_dev, bridge);
> > > +	drm_bridge_attach(encoder, bridge, NULL);
> > > 
> > > -	encoder->bridge = bridge;
> > >  	connector->encoder = encoder;
> > > 
> > >  	drm_connector = (struct drm_connector *)connector;
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > > index f5e86fe7750e..757208f51731 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > > @@ -208,6 +208,7 @@ int sun4i_rgb_init(struct drm_device *drm)
> > >  	struct sun4i_drv *drv = drm->dev_private;
> > >  	struct sun4i_tcon *tcon = drv->tcon;
> > >  	struct drm_encoder *encoder;
> > > +	struct drm_bridge *bridge;
> > >  	struct sun4i_rgb *rgb;
> > >  	int ret;
> > > 
> > > @@ -218,8 +219,8 @@ int sun4i_rgb_init(struct drm_device *drm)
> > >  	encoder = &rgb->encoder;
> > > 
> > >  	tcon->panel = sun4i_tcon_find_panel(tcon->dev->of_node);
> > > -	encoder->bridge = sun4i_tcon_find_bridge(tcon->dev->of_node);
> > > -	if (IS_ERR(tcon->panel) && IS_ERR(encoder->bridge)) {
> > > +	bridge = sun4i_tcon_find_bridge(tcon->dev->of_node);
> > > +	if (IS_ERR(tcon->panel) && IS_ERR(bridge)) {
> > >  		dev_info(drm->dev, "No panel or bridge found... RGB output disabled\n");
> > >  		return 0;
> > >  	}
> > > @@ -260,16 +261,12 @@ int sun4i_rgb_init(struct drm_device *drm)
> > >  		}
> > >  	}
> > > 
> > > -	if (!IS_ERR(encoder->bridge)) {
> > > -		encoder->bridge->encoder = &rgb->encoder;
> > > -
> > > -		ret = drm_bridge_attach(drm, encoder->bridge);
> > > +	if (!IS_ERR(bridge)) {
> > > +		ret = drm_bridge_attach(encoder, bridge, NULL);
> > >  		if (ret) {
> > >  			dev_err(drm->dev, "Couldn't attach our bridge\n");
> > >  			goto err_cleanup_connector;
> > >  		}
> > > -	} else {
> > > -		encoder->bridge = NULL;
> > >  	}
> > > 
> > >  	return 0;
> > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > > index c67d7cd7d57e..b0dd5e8634ae 100644
> > > --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > > +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > > @@ -167,10 +167,8 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
> > >  	int ret;
> > > 
> > >  	priv->external_encoder->possible_crtcs = BIT(0);
> > > -	priv->external_encoder->bridge = bridge;
> > > -	bridge->encoder = priv->external_encoder;
> > > 
> > > -	ret = drm_bridge_attach(ddev, bridge);
> > > +	ret = drm_bridge_attach(priv->external_encoder, bridge, NULL);
> > >  	if (ret) {
> > >  		dev_err(ddev->dev, "drm_bridge_attach() failed %d\n", ret);
> > >  		return ret;
> > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > > index 530a1d6e8cde..94e5ee96b3b5 100644
> > > --- a/include/drm/drm_bridge.h
> > > +++ b/include/drm/drm_bridge.h
> > > @@ -201,7 +201,8 @@ struct drm_bridge {
> > >  int drm_bridge_add(struct drm_bridge *bridge);
> > >  void drm_bridge_remove(struct drm_bridge *bridge);
> > >  struct drm_bridge *of_drm_find_bridge(struct device_node *np);
> > > -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
> > > +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> > > +		      struct drm_bridge *previous);
> > >  void drm_bridge_detach(struct drm_bridge *bridge);
> > > 
> > >  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
> > > 
> > 
> > -- 
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Philipp Zabel Dec. 23, 2016, 11:10 a.m. UTC | #7

Am Freitag, den 16.12.2016, 14:05 +0530 schrieb Archit Taneja:
> Hi,
> 
> On 12/14/2016 03:29 PM, Laurent Pinchart wrote:
> > Instead of linking encoders and bridges in every driver (and getting it
> > wrong half of the time, as many drivers forget to set the drm_bridge
> > encoder pointer), do so in core code. The drm_bridge_attach() function
> > needs the encoder and optional previous bridge to perform that task,
> > update all the callers.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Acked-by: Stefan Agner <stefan@agner.ch> # For DCU
> > Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> # For atmel-hlcdc
> > Acked-by: Vincent Abriou <vincent.abriou@st.com> # For STI
> 
> This one needs acks for arcgpu, tilcdc, mediatek and imx. The changes in
> those drivers look good to me, though. Will push it in a day or so unless anyone
> has any comments on it.

for mediatek and imx
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

[v4,3/5] drm: bridge: Link encoder and bridge in core code

Commit Message

Comments

Patch