Commit b4027536 authored by Luca Ceresoli's avatar Luca Ceresoli
Browse files

Revert "drm/display: bridge_connector: get/put the stored bridges" 



This reverts commit 2be300f9.

The commit being reverted moved all the bridge_connector->bridge_*
assignments to just before the final successful return in order to handle
the bridge refcounting in a clean way.

This introduced a bug, because a bit before the successful return
drmm_connector_hdmi_cec_register() is called, which calls funcs->init()
which is drm_bridge_connector_hdmi_cec_init() which needs
bridge_connector->bridge_hdmi_cec to be set.

The reported bug may be fixed in a relatively simple way, but other similar
patterns are potentially present, so just revert the offending commit. A
different approach will be implemented.

Fixes: 2be300f9 ("drm/display: bridge_connector: get/put the stored bridges")
Reported-by: default avatarMarek Szyprowski <m.szyprowski@samsung.com>
Closes: https://lore.kernel.org/all/336fbfdd-c424-490e-b5d1-8ee84043dc80@samsung.com/


Reported-by: default avatarNaresh Kamboju <naresh.kamboju@linaro.org>
Closes: https://lore.kernel.org/r/CA+G9fYuKHp3QgPKjgFY3TfkDdh5Vf=Ae5pCW+eU41Bu=D7th2g@mail.gmail.com


Reviewed-by: default avatarLouis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: default avatarDmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> # db410c
Tested-by: default avatarGeert Uytterhoeven <geert+renesas@glider.be>
Tested-by: default avatarNicolas Frattaroli <nicolas.frattaroli@collabora.com>
Tested-by: default avatarTommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Tested-by: default avatarMarek Szyprowski <m.szyprowski@samsung.com>
Link: https://patch.msgid.link/20251017-drm-bridge-alloc-getput-bridge-connector-fix-hdmi_cec-v2-1-667abf6d47c0@bootlin.com


Signed-off-by: default avatarLuca Ceresoli <luca.ceresoli@bootlin.com>
parent 8cde1c9b

drivers/gpu/drm/display/drm_bridge_connector.c

+36 −78
Original line number Diff line number Diff line
@@ -618,20 +618,6 @@ static const struct drm_connector_hdmi_cec_funcs drm_bridge_connector_hdmi_cec_f 
 * Bridge Connector Initialisation

 */



static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)

{

	struct drm_bridge_connector *bridge_connector = (struct drm_bridge_connector *)data;



	drm_bridge_put(bridge_connector->bridge_edid);

	drm_bridge_put(bridge_connector->bridge_hpd);

	drm_bridge_put(bridge_connector->bridge_detect);

	drm_bridge_put(bridge_connector->bridge_modes);

	drm_bridge_put(bridge_connector->bridge_hdmi);

	drm_bridge_put(bridge_connector->bridge_hdmi_audio);

	drm_bridge_put(bridge_connector->bridge_dp_audio);

	drm_bridge_put(bridge_connector->bridge_hdmi_cec);

}



/**

 * drm_bridge_connector_init - Initialise a connector for a chain of bridges

 * @drm: the DRM device
@@ -652,15 +638,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
	struct drm_bridge_connector *bridge_connector;

	struct drm_connector *connector;

	struct i2c_adapter *ddc = NULL;

	struct drm_bridge *panel_bridge      __free(drm_bridge_put) = NULL;

	struct drm_bridge *bridge_edid       __free(drm_bridge_put) = NULL;

	struct drm_bridge *bridge_hpd        __free(drm_bridge_put) = NULL;

	struct drm_bridge *bridge_detect     __free(drm_bridge_put) = NULL;

	struct drm_bridge *bridge_modes      __free(drm_bridge_put) = NULL;

	struct drm_bridge *bridge_hdmi       __free(drm_bridge_put) = NULL;

	struct drm_bridge *bridge_hdmi_audio __free(drm_bridge_put) = NULL;

	struct drm_bridge *bridge_dp_audio   __free(drm_bridge_put) = NULL;

	struct drm_bridge *bridge_hdmi_cec   __free(drm_bridge_put) = NULL;

	struct drm_bridge *panel_bridge = NULL;

	unsigned int supported_formats = BIT(HDMI_COLORSPACE_RGB);

	unsigned int max_bpc = 8;

	bool support_hdcp = false;
@@ -671,10 +649,6 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
	if (!bridge_connector)

		return ERR_PTR(-ENOMEM);



	ret = drmm_add_action(drm, drm_bridge_connector_put_bridges, bridge_connector);

	if (ret)

		return ERR_PTR(ret);



	bridge_connector->encoder = encoder;



	/*
@@ -698,30 +672,22 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
		if (!bridge->ycbcr_420_allowed)

			connector->ycbcr_420_allowed = false;



		if (bridge->ops & DRM_BRIDGE_OP_EDID) {

			drm_bridge_put(bridge_edid);

			bridge_edid = drm_bridge_get(bridge);

		}

		if (bridge->ops & DRM_BRIDGE_OP_HPD) {

			drm_bridge_put(bridge_hpd);

			bridge_hpd = drm_bridge_get(bridge);

		}

		if (bridge->ops & DRM_BRIDGE_OP_DETECT) {

			drm_bridge_put(bridge_detect);

			bridge_detect = drm_bridge_get(bridge);

		}

		if (bridge->ops & DRM_BRIDGE_OP_MODES) {

			drm_bridge_put(bridge_modes);

			bridge_modes = drm_bridge_get(bridge);

		}

		if (bridge->ops & DRM_BRIDGE_OP_EDID)

			bridge_connector->bridge_edid = bridge;

		if (bridge->ops & DRM_BRIDGE_OP_HPD)

			bridge_connector->bridge_hpd = bridge;

		if (bridge->ops & DRM_BRIDGE_OP_DETECT)

			bridge_connector->bridge_detect = bridge;

		if (bridge->ops & DRM_BRIDGE_OP_MODES)

			bridge_connector->bridge_modes = bridge;

		if (bridge->ops & DRM_BRIDGE_OP_HDMI) {

			if (bridge_hdmi)

			if (bridge_connector->bridge_hdmi)

				return ERR_PTR(-EBUSY);

			if (!bridge->funcs->hdmi_write_infoframe ||

			    !bridge->funcs->hdmi_clear_infoframe)

				return ERR_PTR(-EINVAL);



			bridge_hdmi = drm_bridge_get(bridge);

			bridge_connector->bridge_hdmi = bridge;



			if (bridge->supported_formats)

				supported_formats = bridge->supported_formats;
@@ -730,10 +696,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
		}



		if (bridge->ops & DRM_BRIDGE_OP_HDMI_AUDIO) {

			if (bridge_hdmi_audio)

			if (bridge_connector->bridge_hdmi_audio)

				return ERR_PTR(-EBUSY);



			if (bridge_dp_audio)

			if (bridge_connector->bridge_dp_audio)

				return ERR_PTR(-EBUSY);



			if (!bridge->hdmi_audio_max_i2s_playback_channels &&
@@ -744,14 +710,14 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
			    !bridge->funcs->hdmi_audio_shutdown)

				return ERR_PTR(-EINVAL);



			bridge_hdmi_audio = drm_bridge_get(bridge);

			bridge_connector->bridge_hdmi_audio = bridge;

		}



		if (bridge->ops & DRM_BRIDGE_OP_DP_AUDIO) {

			if (bridge_dp_audio)

			if (bridge_connector->bridge_dp_audio)

				return ERR_PTR(-EBUSY);



			if (bridge_hdmi_audio)

			if (bridge_connector->bridge_hdmi_audio)

				return ERR_PTR(-EBUSY);



			if (!bridge->hdmi_audio_max_i2s_playback_channels &&
@@ -762,7 +728,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
			    !bridge->funcs->dp_audio_shutdown)

				return ERR_PTR(-EINVAL);



			bridge_dp_audio = drm_bridge_get(bridge);

			bridge_connector->bridge_dp_audio = bridge;

		}



		if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
@@ -773,10 +739,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
		}



		if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {

			if (bridge_hdmi_cec)

			if (bridge_connector->bridge_hdmi_cec)

				return ERR_PTR(-EBUSY);



			bridge_hdmi_cec = drm_bridge_get(bridge);

			bridge_connector->bridge_hdmi_cec = bridge;



			if (!bridge->funcs->hdmi_cec_enable ||

			    !bridge->funcs->hdmi_cec_log_addr ||
@@ -796,7 +762,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
			ddc = bridge->ddc;



		if (drm_bridge_is_panel(bridge))

			panel_bridge = drm_bridge_get(bridge);

			panel_bridge = bridge;



		if (bridge->support_hdcp)

			support_hdcp = true;
@@ -805,13 +771,13 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
	if (connector_type == DRM_MODE_CONNECTOR_Unknown)

		return ERR_PTR(-EINVAL);



	if (bridge_hdmi) {

	if (bridge_connector->bridge_hdmi) {

		if (!connector->ycbcr_420_allowed)

			supported_formats &= ~BIT(HDMI_COLORSPACE_YUV420);



		ret = drmm_connector_hdmi_init(drm, connector,

					       bridge_hdmi->vendor,

					       bridge_hdmi->product,

					       bridge_connector->bridge_hdmi->vendor,

					       bridge_connector->bridge_hdmi->product,

					       &drm_bridge_connector_funcs,

					       &drm_bridge_connector_hdmi_funcs,

					       connector_type, ddc,
@@ -827,14 +793,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
			return ERR_PTR(ret);

	}



	if (bridge_hdmi_audio || bridge_dp_audio) {

	if (bridge_connector->bridge_hdmi_audio ||

	    bridge_connector->bridge_dp_audio) {

		struct device *dev;

		struct drm_bridge *bridge;



		if (bridge_hdmi_audio)

			bridge = bridge_hdmi_audio;

		if (bridge_connector->bridge_hdmi_audio)

			bridge = bridge_connector->bridge_hdmi_audio;

		else

			bridge = bridge_dp_audio;

			bridge = bridge_connector->bridge_dp_audio;



		dev = bridge->hdmi_audio_dev;
@@ -848,9 +815,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
			return ERR_PTR(ret);

	}



	if (bridge_hdmi_cec &&

	    bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {

		struct drm_bridge *bridge = bridge_hdmi_cec;

	if (bridge_connector->bridge_hdmi_cec &&

	    bridge_connector->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {

		struct drm_bridge *bridge = bridge_connector->bridge_hdmi_cec;



		ret = drmm_connector_hdmi_cec_notifier_register(connector,

								NULL,
@@ -859,9 +826,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
			return ERR_PTR(ret);

	}



	if (bridge_hdmi_cec &&

	    bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {

		struct drm_bridge *bridge = bridge_hdmi_cec;

	if (bridge_connector->bridge_hdmi_cec &&

	    bridge_connector->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {

		struct drm_bridge *bridge = bridge_connector->bridge_hdmi_cec;



		ret = drmm_connector_hdmi_cec_register(connector,

						       &drm_bridge_connector_hdmi_cec_funcs,
@@ -874,9 +841,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 


	drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);



	if (bridge_hpd)

	if (bridge_connector->bridge_hpd)

		connector->polled = DRM_CONNECTOR_POLL_HPD;

	else if (bridge_detect)

	else if (bridge_connector->bridge_detect)

		connector->polled = DRM_CONNECTOR_POLL_CONNECT

				  | DRM_CONNECTOR_POLL_DISCONNECT;
@@ -887,15 +854,6 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, 
	    IS_ENABLED(CONFIG_DRM_DISPLAY_HDCP_HELPER))

		drm_connector_attach_content_protection_property(connector, true);



	bridge_connector->bridge_edid       = drm_bridge_get(bridge_edid);

	bridge_connector->bridge_hpd        = drm_bridge_get(bridge_hpd);

	bridge_connector->bridge_detect     = drm_bridge_get(bridge_detect);

	bridge_connector->bridge_modes      = drm_bridge_get(bridge_modes);

	bridge_connector->bridge_hdmi       = drm_bridge_get(bridge_hdmi);

	bridge_connector->bridge_hdmi_audio = drm_bridge_get(bridge_hdmi_audio);

	bridge_connector->bridge_dp_audio   = drm_bridge_get(bridge_dp_audio);

	bridge_connector->bridge_hdmi_cec   = drm_bridge_get(bridge_hdmi_cec);



	return connector;

}

EXPORT_SYMBOL_GPL(drm_bridge_connector_init);