On 11/29/2016 02:34 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@xxxxxxxxxxxxxxxx> --- 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 +++--- include/drm/drm_bridge.h | 3 +- 23 files changed, 83 insertions(+), 103 deletions(-) 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 6e0447f329a2..1835f1fdad19 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 b71088dab268..432e0e3fff72 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;
I think we could derive previous from the encoder itself. Something like: previous = encoder->bridge; while (previous && previous->next) previous = previous->next;
+ + 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; }
<snip> Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project