[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20251017-drm-bridge-alloc-getput-bridge-connector-fix-hdmi_cec-v2-2-667abf6d47c0@bootlin.com>
Date: Fri, 17 Oct 2025 18:15:05 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Marek Szyprowski <m.szyprowski@...sung.com>,
Naresh Kamboju <naresh.kamboju@...aro.org>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Hui Pu <Hui.Pu@...ealthcare.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
Luca Ceresoli <luca.ceresoli@...tlin.com>
Subject: [PATCH v2 2/3] drm/display: bridge_connector: get/put the stored
bridges
drm_bridge_connector_init() takes eight pointers to various bridges, some
of which can be identical, and stores them in pointers inside struct
drm_bridge_connector. Get a reference to each of the taken bridges and put
it on cleanup.
Achieve this by adding a drmm cleanup callback whic puts all the non-NULL
bridges. Using drmm ensures the cleanup happens on drm_device teardown,
whichever is the return value of this function.
Four of these pointers (edid, hpd, detect and modes) can be written
multiple times (up to once per loop iterations), in order to eventually
store the last matching bridge. So when one of those pointers is
overwritten, we need to put the reference that we got during the previous
assignment. Add a drm_bridge_put() before writing them to handle this.
Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
---
This patch was added in v2:
- it is a rewrite of the original (buggy) patch, but simpler after
realizing thanks to drmm we don't need the temporary pointers as the
stored pointers will be put just before deallocating
- compared to the original patch, moved the panel_bridge changes to a
separate patch for clarity
---
drivers/gpu/drm/display/drm_bridge_connector.c | 52 +++++++++++++++++++-------
1 file changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index a5bdd6c1064399ece6b19560f145b877c9e0680e..95ccf86527129edaa6fcc75c6202985e73c46da8 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -618,6 +618,20 @@ 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
@@ -649,6 +663,10 @@ 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;
/*
@@ -672,14 +690,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)
- 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_EDID) {
+ drm_bridge_put(bridge_connector->bridge_edid);
+ bridge_connector->bridge_edid = drm_bridge_get(bridge);
+ }
+ if (bridge->ops & DRM_BRIDGE_OP_HPD) {
+ drm_bridge_put(bridge_connector->bridge_hpd);
+ bridge_connector->bridge_hpd = drm_bridge_get(bridge);
+ }
+ if (bridge->ops & DRM_BRIDGE_OP_DETECT) {
+ drm_bridge_put(bridge_connector->bridge_detect);
+ bridge_connector->bridge_detect = drm_bridge_get(bridge);
+ }
+ if (bridge->ops & DRM_BRIDGE_OP_MODES) {
+ drm_bridge_put(bridge_connector->bridge_modes);
+ bridge_connector->bridge_modes = drm_bridge_get(bridge);
+ }
if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
if (bridge_connector->bridge_hdmi)
return ERR_PTR(-EBUSY);
@@ -687,7 +713,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
!bridge->funcs->hdmi_clear_infoframe)
return ERR_PTR(-EINVAL);
- bridge_connector->bridge_hdmi = bridge;
+ bridge_connector->bridge_hdmi = drm_bridge_get(bridge);
if (bridge->supported_formats)
supported_formats = bridge->supported_formats;
@@ -710,7 +736,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
!bridge->funcs->hdmi_audio_shutdown)
return ERR_PTR(-EINVAL);
- bridge_connector->bridge_hdmi_audio = bridge;
+ bridge_connector->bridge_hdmi_audio = drm_bridge_get(bridge);
}
if (bridge->ops & DRM_BRIDGE_OP_DP_AUDIO) {
@@ -728,21 +754,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
!bridge->funcs->dp_audio_shutdown)
return ERR_PTR(-EINVAL);
- bridge_connector->bridge_dp_audio = bridge;
+ bridge_connector->bridge_dp_audio = drm_bridge_get(bridge);
}
if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
if (bridge_connector->bridge_hdmi_cec)
return ERR_PTR(-EBUSY);
- bridge_connector->bridge_hdmi_cec = bridge;
+ bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge);
}
if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
if (bridge_connector->bridge_hdmi_cec)
return ERR_PTR(-EBUSY);
- bridge_connector->bridge_hdmi_cec = bridge;
+ bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge);
if (!bridge->funcs->hdmi_cec_enable ||
!bridge->funcs->hdmi_cec_log_addr ||
--
2.51.0
Powered by blists - more mailing lists