* [PATCH 01/37] drm/connector: split drmm_connector_hdmi_init() in 3 parts
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 02/37] drm/connector: add drm_connector_hdmi_dynamic_init() Luca Ceresoli
` (35 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In preparation for adding a dynamic variant of drmm_connector_hdmi_init(),
split reusable parts of the code to subfunctions.
drmm_connector_hdmi_init() currently has 3 sections:
1. sanity checks
2. call drmm_connector_init()
3. initialize HDMI-specific fields not initialized at step 2
Split 1 and 3 to new functions, reusable independently.
No functional changes. Just moving code around.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/drm_connector.c | 105 +++++++++++++++++++++++++---------------
1 file changed, 65 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 3fa4d2082cd7..37ed73300a18 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -542,44 +542,13 @@ int drmm_connector_init(struct drm_device *dev,
}
EXPORT_SYMBOL(drmm_connector_init);
-/**
- * drmm_connector_hdmi_init - Init a preallocated HDMI connector
- * @dev: DRM device
- * @connector: A pointer to the HDMI connector to init
- * @vendor: HDMI Controller Vendor name
- * @product: HDMI Controller Product name
- * @funcs: callbacks for this connector
- * @hdmi_funcs: HDMI-related callbacks for this connector
- * @connector_type: user visible type of the connector
- * @ddc: optional pointer to the associated ddc adapter
- * @supported_formats: Bitmask of @drm_output_color_format listing supported output formats
- * @max_bpc: Maximum bits per char the HDMI connector supports
- *
- * Initialises a preallocated HDMI connector. Connectors can be
- * subclassed as part of driver connector objects.
- *
- * Cleanup is automatically handled with a call to
- * drm_connector_cleanup() in a DRM-managed action.
- *
- * The connector structure should be allocated with drmm_kzalloc().
- *
- * The @drm_connector_funcs.destroy hook must be NULL.
- *
- * Returns:
- * Zero on success, error code on failure.
- */
-int drmm_connector_hdmi_init(struct drm_device *dev,
- struct drm_connector *connector,
- const char *vendor, const char *product,
- const struct drm_connector_funcs *funcs,
- const struct drm_connector_hdmi_funcs *hdmi_funcs,
- int connector_type,
- struct i2c_adapter *ddc,
- unsigned long supported_formats,
- unsigned int max_bpc)
+static int drm_connector_hdmi_sanity_checks(struct drm_connector *connector,
+ const char *vendor, const char *product,
+ const struct drm_connector_hdmi_funcs *hdmi_funcs,
+ int connector_type,
+ unsigned long supported_formats,
+ unsigned int max_bpc)
{
- int ret;
-
if (!vendor || !product)
return -EINVAL;
@@ -606,10 +575,15 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
!hdmi_funcs->hdmi.write_infoframe)
return -EINVAL;
- ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
- if (ret)
- return ret;
+ return 0;
+}
+static void drm_connector_hdmi_init(struct drm_connector *connector,
+ const char *vendor, const char *product,
+ const struct drm_connector_hdmi_funcs *hdmi_funcs,
+ unsigned long supported_formats,
+ unsigned int max_bpc)
+{
connector->hdmi.supported_formats = supported_formats;
strtomem_pad(connector->hdmi.vendor, vendor, 0);
strtomem_pad(connector->hdmi.product, product, 0);
@@ -628,6 +602,57 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
drm_connector_attach_hdr_output_metadata_property(connector);
connector->hdmi.funcs = hdmi_funcs;
+}
+
+/**
+ * drmm_connector_hdmi_init - Init a preallocated HDMI connector
+ * @dev: DRM device
+ * @connector: A pointer to the HDMI connector to init
+ * @vendor: HDMI Controller Vendor name
+ * @product: HDMI Controller Product name
+ * @funcs: callbacks for this connector
+ * @hdmi_funcs: HDMI-related callbacks for this connector
+ * @connector_type: user visible type of the connector
+ * @ddc: optional pointer to the associated ddc adapter
+ * @supported_formats: Bitmask of @drm_output_color_format listing supported output formats
+ * @max_bpc: Maximum bits per char the HDMI connector supports
+ *
+ * Initialises a preallocated HDMI connector. Connectors can be
+ * subclassed as part of driver connector objects.
+ *
+ * Cleanup is automatically handled with a call to
+ * drm_connector_cleanup() in a DRM-managed action.
+ *
+ * The connector structure should be allocated with drmm_kzalloc().
+ *
+ * The @drm_connector_funcs.destroy hook must be NULL.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drmm_connector_hdmi_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const char *vendor, const char *product,
+ const struct drm_connector_funcs *funcs,
+ const struct drm_connector_hdmi_funcs *hdmi_funcs,
+ int connector_type,
+ struct i2c_adapter *ddc,
+ unsigned long supported_formats,
+ unsigned int max_bpc)
+{
+ int ret;
+
+ ret = drm_connector_hdmi_sanity_checks(connector, vendor, product, hdmi_funcs,
+ connector_type, supported_formats, max_bpc);
+ if (ret)
+ return ret;
+
+ ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
+ if (ret)
+ return ret;
+
+ drm_connector_hdmi_init(connector, vendor, product,
+ hdmi_funcs, supported_formats, max_bpc);
return 0;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 02/37] drm/connector: add drm_connector_hdmi_dynamic_init()
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
2026-05-19 10:37 ` [PATCH 01/37] drm/connector: split drmm_connector_hdmi_init() in 3 parts Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:04 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 03/37] drm/display: bridge-connector: rename variable for consistency Luca Ceresoli
` (34 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In preparation for adding hotpluggable bridges into the
drm_bridge_connector, we need connectors to be created dynamically, both
regular connectors and HDMI ones. drm_connector_init() already has a
dynamic variant, add one for HDMI connectors too.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/drm_connector.c | 50 +++++++++++++++++++++++++++++++++++++++++
include/drm/drm_connector.h | 9 ++++++++
2 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 37ed73300a18..5e7a3a438e5d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -658,6 +658,56 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
}
EXPORT_SYMBOL(drmm_connector_hdmi_init);
+/**
+ * drm_connector_hdmi_dynamic_init - Init a preallocated dynamic HDMI connector
+ * @dev: DRM device
+ * @connector: A pointer to the HDMI connector to init
+ * @vendor: HDMI Controller Vendor name
+ * @product: HDMI Controller Product name
+ * @funcs: callbacks for this connector
+ * @hdmi_funcs: HDMI-related callbacks for this connector
+ * @connector_type: user visible type of the connector
+ * @ddc: optional pointer to the associated ddc adapter
+ * @supported_formats: Bitmask of @hdmi_colorspace listing supported output formats
+ * @max_bpc: Maximum bits per char the HDMI connector supports
+ *
+ * Initialises a preallocated dynamic HDMI connector. Connectors can be
+ * subclassed as part of driver connector objects.
+ *
+ * See drm_connector_dynamic_init(), the same constraints apply here. This
+ * is just the HDMI version.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_connector_hdmi_dynamic_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const char *vendor, const char *product,
+ const struct drm_connector_funcs *funcs,
+ const struct drm_connector_hdmi_funcs *hdmi_funcs,
+ int connector_type,
+ struct i2c_adapter *ddc,
+ unsigned long supported_formats,
+ unsigned int max_bpc)
+{
+ int ret;
+
+ ret = drm_connector_hdmi_sanity_checks(connector, vendor, product, hdmi_funcs,
+ connector_type, supported_formats, max_bpc);
+ if (ret)
+ return ret;
+
+ ret = drm_connector_dynamic_init(dev, connector, funcs, connector_type, ddc);
+ if (ret)
+ return ret;
+
+ drm_connector_hdmi_init(connector, vendor, product,
+ hdmi_funcs, supported_formats, max_bpc);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_hdmi_dynamic_init);
+
/**
* drm_connector_attach_edid_property - attach edid property.
* @connector: the connector
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 5ad62c207d00..1f5dba947748 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2442,6 +2442,15 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
struct i2c_adapter *ddc,
unsigned long supported_formats,
unsigned int max_bpc);
+int drm_connector_hdmi_dynamic_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const char *vendor, const char *product,
+ const struct drm_connector_funcs *funcs,
+ const struct drm_connector_hdmi_funcs *hdmi_funcs,
+ int connector_type,
+ struct i2c_adapter *ddc,
+ unsigned long supported_formats,
+ unsigned int max_bpc);
void drm_connector_attach_edid_property(struct drm_connector *connector);
int drm_connector_register(struct drm_connector *connector);
int drm_connector_dynamic_register(struct drm_connector *connector);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 02/37] drm/connector: add drm_connector_hdmi_dynamic_init()
2026-05-19 10:37 ` [PATCH 02/37] drm/connector: add drm_connector_hdmi_dynamic_init() Luca Ceresoli
@ 2026-05-19 11:04 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:04 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional dereference of connector->state if the reset callback fails to allocate memory.
--
commit 7cc84b3b3fd13ae7b12a391a46d924249c29ae3d
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/connector: add drm_connector_hdmi_dynamic_init()
This commit introduces a dynamic initialization variant for HDMI connectors
to support hotpluggable bridges.
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 37ed73300a181..5e7a3a438e5db 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
[ ... ]
> @@ -658,6 +658,56 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
> +int drm_connector_hdmi_dynamic_init(struct drm_device *dev,
> + struct drm_connector *connector,
> + const char *vendor, const char *product,
> + const struct drm_connector_funcs *funcs,
> + const struct drm_connector_hdmi_funcs *hdmi_funcs,
> + int connector_type,
> + struct i2c_adapter *ddc,
> + unsigned long supported_formats,
> + unsigned int max_bpc)
> +{
> + int ret;
> +
> + ret = drm_connector_hdmi_sanity_checks(connector, vendor, product, hdmi_funcs,
> + connector_type, supported_formats, max_bpc);
> + if (ret)
> + return ret;
> +
> + ret = drm_connector_dynamic_init(dev, connector, funcs, connector_type, ddc);
> + if (ret)
> + return ret;
> +
> + drm_connector_hdmi_init(connector, vendor, product,
> + hdmi_funcs, supported_formats, max_bpc);
Could this lead to a NULL pointer dereference under memory pressure? I
realize this behavior pre-exists in drmm_connector_hdmi_init(), but the new
dynamic variant exposes the same pattern.
Looking at drm_connector_hdmi_init():
static void drm_connector_hdmi_init(...)
{
...
if (connector->funcs->reset)
connector->funcs->reset(connector);
drm_connector_attach_max_bpc_property(connector, 8, max_bpc);
}
For atomic drivers, the reset callback allocates connector->state, but since
it returns void, allocation failures are silent and connector->state remains
NULL.
The subsequent call to drm_connector_attach_max_bpc_property() then
unconditionally dereferences it:
connector->state->max_requested_bpc = max;
Would it be safer to verify connector->state is allocated after the reset
callback before continuing?
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_dynamic_init);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=2
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 03/37] drm/display: bridge-connector: rename variable for consistency
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
2026-05-19 10:37 ` [PATCH 01/37] drm/connector: split drmm_connector_hdmi_init() in 3 parts Luca Ceresoli
2026-05-19 10:37 ` [PATCH 02/37] drm/connector: add drm_connector_hdmi_dynamic_init() Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 04/37] drm/display: bridge-connector: store the drm_device pointer Luca Ceresoli
` (33 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
All struct drm_bridge_connector pointers are called bridge_connector except
this one. Rename it for consistency.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 649969fca141..bf9b8a6bca78 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -153,10 +153,10 @@ static void drm_bridge_connector_hpd_notify(struct drm_connector *connector,
}
}
-static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *drm_bridge_connector,
+static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *bridge_connector,
enum drm_connector_status status)
{
- struct drm_connector *connector = &drm_bridge_connector->base;
+ struct drm_connector *connector = &bridge_connector->base;
struct drm_device *dev = connector->dev;
mutex_lock(&dev->mode_config.mutex);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 04/37] drm/display: bridge-connector: store the drm_device pointer
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (2 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 03/37] drm/display: bridge-connector: rename variable for consistency Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 05/37] drm/display: bridge-connector: split code creating the connector to a subfunction Luca Ceresoli
` (32 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
Currently the struct drm_device pointer is only needed during the initial
drm_bridge_connector_init() and in drm_bridge_connector_handle_hpd() which
gets it from the struct drm_connector.
This will be insufficient when introducing bridge hotplugging, because:
* some of the actions in drm_bridge_connector_init() will have to be
performed later on, when a bridge is hot(un)plugged
* the connector will be removed and re-added based on hotplug events,
so the drm_connector might just not exist or its content be cleared
Store the drm_device pointer in struct drm_bridge_connector for any later
needs. Also convert drm_bridge_connector_handle_hpd() to use the newly
stored value.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index bf9b8a6bca78..62c4cb1e241f 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -60,6 +60,10 @@ struct drm_bridge_connector {
* @base: The base DRM connector
*/
struct drm_connector base;
+ /**
+ * @drm: The DRM device we belong to
+ */
+ struct drm_device *drm;
/**
* @encoder:
*
@@ -157,7 +161,7 @@ static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *bridge_
enum drm_connector_status status)
{
struct drm_connector *connector = &bridge_connector->base;
- struct drm_device *dev = connector->dev;
+ struct drm_device *dev = bridge_connector->drm;
mutex_lock(&dev->mode_config.mutex);
connector->status = status;
@@ -806,6 +810,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
if (ret)
return ERR_PTR(ret);
+ bridge_connector->drm = drm;
bridge_connector->encoder = encoder;
/*
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 05/37] drm/display: bridge-connector: split code creating the connector to a subfunction
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (3 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 04/37] drm/display: bridge-connector: store the drm_device pointer Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 06/37] drm/display: bridge-connector: use a drm_bridge_connector internally, not a drm_connector Luca Ceresoli
` (31 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In preparation to introduce bridge hotplug, split out from
drm_bridge_connector_init() the code adding the drm_connector into a
dedicated function. This will be needed to be able to add (and re-add) the
connector from different code paths.
No functional changes. Just moving code around.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 81 ++++++++++++++++----------
1 file changed, 50 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 62c4cb1e241f..6813c6008985 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -773,26 +773,19 @@ static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
}
/**
- * drm_bridge_connector_init - Initialise a connector for a chain of bridges
- * @drm: the DRM device
- * @encoder: the encoder where the bridge chain starts
- *
- * Create a new &drm_bridge_connector for the @drm device. The connector is
- * allocated, initialised, registered with the @drm device and attached to
- * @encoder.
+ * drm_bridge_connector_add_connector - add the drm_connector
+ * @bridge_connector: drm_bridge_connector to add the drm_connector to
*
- * The connector is associated with a chain of bridges that starts at
- * the @encoder. All bridges in the chain shall report bridge operation flags
- * (&drm_bridge->ops) and bridge output type (&drm_bridge->type), and none of
- * them may create a DRM connector directly.
+ * Preconditions: the drm_bridge_connector is initialized (the
+ * @bridge_connector->drm and @bridge_connector->encoder fields are
+ * assigned at least)
*
- * Returns a pointer to the new connector on success, or a negative error
- * pointer otherwise.
+ * Returns a pointer to the new drm_connector on success, or a negative
+ * error pointer otherwise.
*/
-struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
- struct drm_encoder *encoder)
+static struct drm_connector *
+drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector)
{
- 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;
@@ -802,17 +795,6 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
int connector_type;
int ret;
- bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector), GFP_KERNEL);
- 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->drm = drm;
- bridge_connector->encoder = encoder;
-
/*
* TODO: Handle doublescan_allowed and stereo_allowed.
*/
@@ -828,7 +810,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
* detection are available, we don't support hotplug detection at all.
*/
connector_type = DRM_MODE_CONNECTOR_Unknown;
- drm_for_each_bridge_in_chain_scoped(encoder, bridge) {
+ drm_for_each_bridge_in_chain_scoped(bridge_connector->encoder, bridge) {
if (!bridge->interlace_allowed)
connector->interlace_allowed = false;
if (!bridge->ycbcr_420_allowed)
@@ -984,7 +966,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
bridge_connector->hdmi_funcs.spd =
drm_bridge_connector_hdmi_spd_infoframe;
- ret = drmm_connector_hdmi_init(drm, connector,
+ ret = drmm_connector_hdmi_init(bridge_connector->drm, connector,
bridge_connector->bridge_hdmi->vendor,
bridge_connector->bridge_hdmi->product,
&drm_bridge_connector_funcs,
@@ -995,7 +977,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
if (ret)
return ERR_PTR(ret);
} else {
- ret = drmm_connector_init(drm, connector,
+ ret = drmm_connector_init(bridge_connector->drm, connector,
&drm_bridge_connector_funcs,
connector_type, ddc);
if (ret)
@@ -1063,10 +1045,47 @@ 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);
- ret = drm_connector_attach_encoder(connector, encoder);
+ ret = drm_connector_attach_encoder(connector, bridge_connector->encoder);
if (ret)
return ERR_PTR(ret);
return connector;
}
+
+/**
+ * drm_bridge_connector_init - Initialise a connector for a chain of bridges
+ * @drm: the DRM device
+ * @encoder: the encoder where the bridge chain starts
+ *
+ * Create a new &drm_bridge_connector for the @drm device. The connector is
+ * allocated, initialised, registered with the @drm device and attached to
+ * @encoder.
+ *
+ * The connector is associated with a chain of bridges that starts at
+ * the @encoder. All bridges in the chain shall report bridge operation flags
+ * (&drm_bridge->ops) and bridge output type (&drm_bridge->type), and none of
+ * them may create a DRM connector directly.
+ *
+ * Returns a pointer to the new connector on success, or a negative error
+ * pointer otherwise.
+ */
+struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
+ struct drm_encoder *encoder)
+{
+ struct drm_bridge_connector *bridge_connector;
+ int ret;
+
+ bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector), GFP_KERNEL);
+ 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->drm = drm;
+ bridge_connector->encoder = encoder;
+
+ return drm_bridge_connector_add_connector(bridge_connector);
+}
EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 06/37] drm/display: bridge-connector: use a drm_bridge_connector internally, not a drm_connector
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (4 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 05/37] drm/display: bridge-connector: split code creating the connector to a subfunction Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 07/37] drm/display: bridge-connector: extract drm_bridge_connector_get_bridges() Luca Ceresoli
` (30 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
Currently drm_bridge_connector_init() always returns the added connector or
errors out. When adding bridge hotplug the bridge-connector can be
successfully initialized without creating a connector, which can be added
later when the pipeline will be complete.
For this the internal function drm_bridge_connector_add_connector() must be
able to return a valid drm_bridge_connector even without any drm_connector.
In preparation to support bridge hotplug, change its return value to be the
same drm_bridge_connector pointer it gets as input, or a PTR_ERR.
No functional changes, just changing an internal API.
Note the return value could now become an int (0 or negative error) because
returning the same value received as input does not carry any added
value. However this would be change a lot of lines, so leave such change as
a future cleanup.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 6813c6008985..eb1912611cd6 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -780,10 +780,10 @@ static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
* @bridge_connector->drm and @bridge_connector->encoder fields are
* assigned at least)
*
- * Returns a pointer to the new drm_connector on success, or a negative
- * error pointer otherwise.
+ * Returns a pointer to @bridge_connector on success, or a negative error
+ * pointer otherwise.
*/
-static struct drm_connector *
+static struct drm_bridge_connector *
drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector)
{
struct drm_connector *connector;
@@ -1049,7 +1049,7 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
if (ret)
return ERR_PTR(ret);
- return connector;
+ return bridge_connector;
}
/**
@@ -1086,6 +1086,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
bridge_connector->drm = drm;
bridge_connector->encoder = encoder;
- return drm_bridge_connector_add_connector(bridge_connector);
+ bridge_connector = drm_bridge_connector_add_connector(bridge_connector);
+ if (IS_ERR(bridge_connector))
+ return ERR_CAST(bridge_connector);
+
+ return &bridge_connector->base;
}
EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 07/37] drm/display: bridge-connector: extract drm_bridge_connector_get_bridges()
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (5 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 06/37] drm/display: bridge-connector: use a drm_bridge_connector internally, not a drm_connector Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:01 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 08/37] drm/display: bridge-connector: return int from drm_bridge_connector_get_bridges() Luca Ceresoli
` (29 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
To support bridge hotplug we will need better control over the steps
involved in the crm_connector creation, especially in terms of error
management. This will be complicated by the length and amount of return
points in this function. In preparation, split the code to grab the various
bridge pointers out to a separate function.
No functional changes. Just moving code around.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 86 +++++++++++++++-----------
1 file changed, 50 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index eb1912611cd6..97d9a4c6d166 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -772,44 +772,24 @@ static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
drm_bridge_put(bridge_connector->bridge_hdmi_cec);
}
-/**
- * drm_bridge_connector_add_connector - add the drm_connector
- * @bridge_connector: drm_bridge_connector to add the drm_connector to
- *
- * Preconditions: the drm_bridge_connector is initialized (the
- * @bridge_connector->drm and @bridge_connector->encoder fields are
- * assigned at least)
- *
- * Returns a pointer to @bridge_connector on success, or a negative error
- * pointer otherwise.
- */
static struct drm_bridge_connector *
-drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector)
+drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
+ int *connector_type,
+ unsigned int *supported_formats,
+ unsigned int *max_bpc,
+ struct i2c_adapter **ddc,
+ struct drm_bridge **panel_bridge,
+ bool *support_hdcp)
{
- struct drm_connector *connector;
- struct i2c_adapter *ddc = NULL;
- struct drm_bridge *panel_bridge __free(drm_bridge_put) = NULL;
- unsigned int supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444);
- unsigned int max_bpc = 8;
- bool support_hdcp = false;
- int connector_type;
- int ret;
+ struct drm_connector *connector = &bridge_connector->base;
/*
* TODO: Handle doublescan_allowed and stereo_allowed.
*/
- connector = &bridge_connector->base;
connector->interlace_allowed = true;
connector->ycbcr_420_allowed = true;
+ *connector_type = DRM_MODE_CONNECTOR_Unknown;
- /*
- * Initialise connector status handling. First locate the furthest
- * bridges in the pipeline that support HPD and output detection. Then
- * initialise the connector polling mode, using HPD if available and
- * falling back to polling if supported. If neither HPD nor output
- * detection are available, we don't support hotplug detection at all.
- */
- connector_type = DRM_MODE_CONNECTOR_Unknown;
drm_for_each_bridge_in_chain_scoped(bridge_connector->encoder, bridge) {
if (!bridge->interlace_allowed)
connector->interlace_allowed = false;
@@ -865,9 +845,9 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
bridge_connector->bridge_hdmi = drm_bridge_get(bridge);
if (bridge->supported_formats)
- supported_formats = bridge->supported_formats;
+ *supported_formats = bridge->supported_formats;
if (bridge->max_bpc)
- max_bpc = bridge->max_bpc;
+ *max_bpc = bridge->max_bpc;
}
if (bridge->ops & DRM_BRIDGE_OP_HDMI_AUDIO) {
@@ -926,7 +906,7 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
}
if (drm_bridge_is_last(bridge))
- connector_type = bridge->type;
+ *connector_type = bridge->type;
#ifdef CONFIG_OF
if (drm_bridge_is_last(bridge) && bridge->of_node)
@@ -934,20 +914,54 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
#endif
if (bridge->ddc)
- ddc = bridge->ddc;
+ *ddc = bridge->ddc;
if (drm_bridge_is_panel(bridge)) {
- drm_bridge_put(panel_bridge);
- panel_bridge = drm_bridge_get(bridge);
+ drm_bridge_put(*panel_bridge);
+ *panel_bridge = drm_bridge_get(bridge);
}
if (bridge->support_hdcp)
- support_hdcp = true;
+ *support_hdcp = true;
}
if (connector_type == DRM_MODE_CONNECTOR_Unknown)
return ERR_PTR(-EINVAL);
+ return bridge_connector;
+}
+
+/**
+ * drm_bridge_connector_add_connector - add the drm_connector
+ * @bridge_connector: drm_bridge_connector to add the drm_connector to
+ *
+ * Preconditions: the drm_bridge_connector is initialized (the
+ * @bridge_connector->drm and @bridge_connector->encoder fields are
+ * assigned at least)
+ *
+ * Returns a pointer to @bridge_connector on success, or a negative error
+ * pointer otherwise.
+ */
+static struct drm_bridge_connector *
+drm_bridge_connector_add_connector(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;
+ unsigned int supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444);
+ unsigned int max_bpc = 8;
+ bool support_hdcp = false;
+ int connector_type;
+ int ret;
+
+ connector = &bridge_connector->base;
+
+ bridge_connector = drm_bridge_connector_get_bridges(bridge_connector, &connector_type,
+ &supported_formats, &max_bpc, &ddc,
+ &panel_bridge, &support_hdcp);
+ if (IS_ERR(bridge_connector))
+ return bridge_connector;
+
if (bridge_connector->bridge_hdmi) {
if (!connector->ycbcr_420_allowed)
supported_formats &= ~BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 07/37] drm/display: bridge-connector: extract drm_bridge_connector_get_bridges()
2026-05-19 10:37 ` [PATCH 07/37] drm/display: bridge-connector: extract drm_bridge_connector_get_bridges() Luca Ceresoli
@ 2026-05-19 11:01 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:01 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing pointer dereference when validating `connector_type`, causing the error check to be bypassed.
--
commit 5a4cd30beb768eaba2369b4630833475535f79bf
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/display: bridge-connector: extract drm_bridge_connector_get_bridges()
This commit splits out the code that gathers bridge pointers into a separate
function to prepare for better bridge hotplug support.
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
[ ... ]
> @@ -772,44 +772,24 @@ static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
> static struct drm_bridge_connector *
> -drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector)
> +drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
> + int *connector_type,
> + unsigned int *supported_formats,
> + unsigned int *max_bpc,
> + struct i2c_adapter **ddc,
> + struct drm_bridge **panel_bridge,
> + bool *support_hdcp)
> {
[ ... ]
> if (bridge->support_hdcp)
> - support_hdcp = true;
> + *support_hdcp = true;
> }
>
> if (connector_type == DRM_MODE_CONNECTOR_Unknown)
> return ERR_PTR(-EINVAL);
Is there a missing dereference on connector_type here?
Since connector_type is now an int pointer and DRM_MODE_CONNECTOR_Unknown
is 0, this evaluates as a NULL pointer check rather than checking the
value.
Because the address of a stack variable is passed to the function, the
pointer is never NULL, so this check will always fail and bypass the error
return.
Should this be checking *connector_type instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=7
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 08/37] drm/display: bridge-connector: return int from drm_bridge_connector_get_bridges()
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (6 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 07/37] drm/display: bridge-connector: extract drm_bridge_connector_get_bridges() Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:58 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 09/37] drm/display: bridge-connector: extract drm_bridge_connector_init_hdmi_audio_cec() Luca Ceresoli
` (28 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
This function returns a struct drm_bridge_connector * because this was the
case in the origin of its code. In current code it does not make sense
anymore: it can only return either the same pointer it received as input or
a negative ERR_PTR.
Simplify up error management by just returning an int (0 or negative
error).
No functional changes.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 61 +++++++++++++-------------
1 file changed, 30 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 97d9a4c6d166..d8a033ed8f41 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -772,14 +772,13 @@ static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
drm_bridge_put(bridge_connector->bridge_hdmi_cec);
}
-static struct drm_bridge_connector *
-drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
- int *connector_type,
- unsigned int *supported_formats,
- unsigned int *max_bpc,
- struct i2c_adapter **ddc,
- struct drm_bridge **panel_bridge,
- bool *support_hdcp)
+static int drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
+ int *connector_type,
+ unsigned int *supported_formats,
+ unsigned int *max_bpc,
+ struct i2c_adapter **ddc,
+ struct drm_bridge **panel_bridge,
+ bool *support_hdcp)
{
struct drm_connector *connector = &bridge_connector->base;
@@ -820,27 +819,27 @@ drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
}
if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
if (bridge_connector->bridge_hdmi)
- return ERR_PTR(-EBUSY);
+ return -EBUSY;
if (!bridge->funcs->hdmi_write_avi_infoframe ||
!bridge->funcs->hdmi_clear_avi_infoframe ||
!bridge->funcs->hdmi_write_hdmi_infoframe ||
!bridge->funcs->hdmi_clear_hdmi_infoframe)
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
if (bridge->ops & DRM_BRIDGE_OP_HDMI_AUDIO &&
(!bridge->funcs->hdmi_write_audio_infoframe ||
!bridge->funcs->hdmi_clear_audio_infoframe))
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
if (bridge->ops & DRM_BRIDGE_OP_HDMI_HDR_DRM_INFOFRAME &&
(!bridge->funcs->hdmi_write_hdr_drm_infoframe ||
!bridge->funcs->hdmi_clear_hdr_drm_infoframe))
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
if (bridge->ops & DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME &&
(!bridge->funcs->hdmi_write_spd_infoframe ||
!bridge->funcs->hdmi_clear_spd_infoframe))
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
bridge_connector->bridge_hdmi = drm_bridge_get(bridge);
@@ -852,57 +851,57 @@ drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
if (bridge->ops & DRM_BRIDGE_OP_HDMI_AUDIO) {
if (bridge_connector->bridge_hdmi_audio)
- return ERR_PTR(-EBUSY);
+ return -EBUSY;
if (bridge_connector->bridge_dp_audio)
- return ERR_PTR(-EBUSY);
+ return -EBUSY;
if (!bridge->hdmi_audio_max_i2s_playback_channels &&
!bridge->hdmi_audio_spdif_playback)
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
if (!bridge->funcs->hdmi_audio_prepare ||
!bridge->funcs->hdmi_audio_shutdown)
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
bridge_connector->bridge_hdmi_audio = drm_bridge_get(bridge);
}
if (bridge->ops & DRM_BRIDGE_OP_DP_AUDIO) {
if (bridge_connector->bridge_dp_audio)
- return ERR_PTR(-EBUSY);
+ return -EBUSY;
if (bridge_connector->bridge_hdmi_audio)
- return ERR_PTR(-EBUSY);
+ return -EBUSY;
if (!bridge->hdmi_audio_max_i2s_playback_channels &&
!bridge->hdmi_audio_spdif_playback)
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
if (!bridge->funcs->dp_audio_prepare ||
!bridge->funcs->dp_audio_shutdown)
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
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);
+ return -EBUSY;
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);
+ return -EBUSY;
bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge);
if (!bridge->funcs->hdmi_cec_enable ||
!bridge->funcs->hdmi_cec_log_addr ||
!bridge->funcs->hdmi_cec_transmit)
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
if (drm_bridge_is_last(bridge))
@@ -926,9 +925,9 @@ drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
}
if (connector_type == DRM_MODE_CONNECTOR_Unknown)
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
- return bridge_connector;
+ return 0;
}
/**
@@ -956,11 +955,11 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
connector = &bridge_connector->base;
- bridge_connector = drm_bridge_connector_get_bridges(bridge_connector, &connector_type,
- &supported_formats, &max_bpc, &ddc,
- &panel_bridge, &support_hdcp);
- if (IS_ERR(bridge_connector))
- return bridge_connector;
+ ret = drm_bridge_connector_get_bridges(bridge_connector, &connector_type,
+ &supported_formats, &max_bpc, &ddc,
+ &panel_bridge, &support_hdcp);
+ if (ret)
+ return ERR_PTR(ret);
if (bridge_connector->bridge_hdmi) {
if (!connector->ycbcr_420_allowed)
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 08/37] drm/display: bridge-connector: return int from drm_bridge_connector_get_bridges()
2026-05-19 10:37 ` [PATCH 08/37] drm/display: bridge-connector: return int from drm_bridge_connector_get_bridges() Luca Ceresoli
@ 2026-05-19 10:58 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 10:58 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The check `connector_type == DRM_MODE_CONNECTOR_Unknown` incorrectly compares a pointer to an integer constant (0) instead of dereferencing the pointer.
--
commit c54583277f834cc9c4092e1b01aee8c02b7a7b4b
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/display: bridge-connector: return int from drm_bridge_connector_get_bridges()
This commit simplifies the return value of drm_bridge_connector_get_bridges()
to return a standard integer (0 or negative error code) instead of returning
a struct drm_bridge_connector pointer or an error pointer.
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 97d9a4c6d1665..d8a033ed8f41e 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
[ ... ]
> @@ -926,9 +925,9 @@ drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
> }
>
> if (connector_type == DRM_MODE_CONNECTOR_Unknown)
Does this condition check the value or the pointer?
Since connector_type is passed as an int * and DRM_MODE_CONNECTOR_Unknown
evaluates to 0, this acts as a NULL check on the pointer itself rather than
checking the integer value.
Because the caller passes the address of a local stack variable
(&connector_type), this condition will always evaluate to false, meaning
the unknown connector type validation is dead code.
Should this be checking *connector_type instead?
While this logic error wasn't introduced by this specific patch, it was
likely exposed during the recent refactoring that extracted this function.
Since this patch modifies the return statement immediately below the condition,
it might be a good opportunity to fix the check.
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
>
> - return bridge_connector;
> + return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=8
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 09/37] drm/display: bridge-connector: extract drm_bridge_connector_init_hdmi_audio_cec()
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (7 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 08/37] drm/display: bridge-connector: return int from drm_bridge_connector_get_bridges() Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 10/37] drm/display: bridge-connector: return int from drm_bridge_connector_init_hdmi_audio_cec() Luca Ceresoli
` (27 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
To support bridge hotplug we will need better control over the steps
involved in the crm_connector creation, especially in terms of error
management. This will be complicated by the length and amount of return
points in this function. In preparation, split the code to initialize HDMI
audio and CEC fields out to a separate function.
No functional changes. Just moving code around.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 102 ++++++++++++++-----------
1 file changed, 57 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index d8a033ed8f41..e6306e98a167 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -930,6 +930,60 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_
return 0;
}
+static struct drm_bridge_connector *
+drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector *bridge_connector)
+{
+ int ret;
+
+ if (bridge_connector->bridge_hdmi_audio ||
+ bridge_connector->bridge_dp_audio) {
+ struct device *dev;
+ struct drm_bridge *bridge;
+
+ if (bridge_connector->bridge_hdmi_audio)
+ bridge = bridge_connector->bridge_hdmi_audio;
+ else
+ bridge = bridge_connector->bridge_dp_audio;
+
+ dev = bridge->hdmi_audio_dev;
+
+ ret = drm_connector_hdmi_audio_init(&bridge_connector->base, dev,
+ &drm_bridge_connector_hdmi_audio_funcs,
+ bridge->hdmi_audio_max_i2s_playback_channels,
+ bridge->hdmi_audio_i2s_formats,
+ bridge->hdmi_audio_spdif_playback,
+ bridge->hdmi_audio_dai_port);
+ if (ret)
+ return ERR_PTR(ret);
+ }
+
+ 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(&bridge_connector->base,
+ NULL,
+ bridge->hdmi_cec_dev);
+ if (ret)
+ return ERR_PTR(ret);
+ }
+
+ 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(&bridge_connector->base,
+ &drm_bridge_connector_hdmi_cec_funcs,
+ bridge->hdmi_cec_adapter_name,
+ bridge->hdmi_cec_available_las,
+ bridge->hdmi_cec_dev);
+ if (ret)
+ return ERR_PTR(ret);
+ }
+
+ return bridge_connector;
+}
+
/**
* drm_bridge_connector_add_connector - add the drm_connector
* @bridge_connector: drm_bridge_connector to add the drm_connector to
@@ -997,51 +1051,9 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
return ERR_PTR(ret);
}
- if (bridge_connector->bridge_hdmi_audio ||
- bridge_connector->bridge_dp_audio) {
- struct device *dev;
- struct drm_bridge *bridge;
-
- if (bridge_connector->bridge_hdmi_audio)
- bridge = bridge_connector->bridge_hdmi_audio;
- else
- bridge = bridge_connector->bridge_dp_audio;
-
- dev = bridge->hdmi_audio_dev;
-
- ret = drm_connector_hdmi_audio_init(connector, dev,
- &drm_bridge_connector_hdmi_audio_funcs,
- bridge->hdmi_audio_max_i2s_playback_channels,
- bridge->hdmi_audio_i2s_formats,
- bridge->hdmi_audio_spdif_playback,
- bridge->hdmi_audio_dai_port);
- if (ret)
- return ERR_PTR(ret);
- }
-
- 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,
- bridge->hdmi_cec_dev);
- if (ret)
- return ERR_PTR(ret);
- }
-
- 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,
- bridge->hdmi_cec_adapter_name,
- bridge->hdmi_cec_available_las,
- bridge->hdmi_cec_dev);
- if (ret)
- return ERR_PTR(ret);
- }
+ bridge_connector = drm_bridge_connector_init_hdmi_audio_cec(bridge_connector);
+ if (IS_ERR(bridge_connector))
+ return bridge_connector;
drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 10/37] drm/display: bridge-connector: return int from drm_bridge_connector_init_hdmi_audio_cec()
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (8 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 09/37] drm/display: bridge-connector: extract drm_bridge_connector_init_hdmi_audio_cec() Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 11/37] drm/display: bridge-connector: return int from drm_bridge_connector_add_connector() Luca Ceresoli
` (26 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
This function returns a struct drm_bridge_connector * because this was the
case in the origin of its code. In current code it does not make sense
anymore: it can only return either the same pointer it received as input or
a negative ERR_PTR.
Simplify up error management by just returning an int (0 or negative
error).
No functional changes.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index e6306e98a167..6066ca0c5624 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -930,8 +930,7 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_
return 0;
}
-static struct drm_bridge_connector *
-drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector *bridge_connector)
+static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector *bridge_connector)
{
int ret;
@@ -954,7 +953,7 @@ drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector *bridge_con
bridge->hdmi_audio_spdif_playback,
bridge->hdmi_audio_dai_port);
if (ret)
- return ERR_PTR(ret);
+ return ret;
}
if (bridge_connector->bridge_hdmi_cec &&
@@ -965,7 +964,7 @@ drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector *bridge_con
NULL,
bridge->hdmi_cec_dev);
if (ret)
- return ERR_PTR(ret);
+ return ret;
}
if (bridge_connector->bridge_hdmi_cec &&
@@ -978,10 +977,10 @@ drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector *bridge_con
bridge->hdmi_cec_available_las,
bridge->hdmi_cec_dev);
if (ret)
- return ERR_PTR(ret);
+ return ret;
}
- return bridge_connector;
+ return 0;
}
/**
@@ -1051,9 +1050,9 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
return ERR_PTR(ret);
}
- bridge_connector = drm_bridge_connector_init_hdmi_audio_cec(bridge_connector);
- if (IS_ERR(bridge_connector))
- return bridge_connector;
+ ret = drm_bridge_connector_init_hdmi_audio_cec(bridge_connector);
+ if (ret)
+ return ERR_PTR(ret);
drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 11/37] drm/display: bridge-connector: return int from drm_bridge_connector_add_connector()
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (9 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 10/37] drm/display: bridge-connector: return int from drm_bridge_connector_init_hdmi_audio_cec() Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 12/37] drm/display: bridge-connector: hoist error management to common code Luca Ceresoli
` (25 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
This function returns a struct drm_bridge_connector * because this was the
case in the origin of its code. In current code it does not make sense: it
can only return either the same pointer it received as input or a negative
ERR_PTR.
This forces users to an unnecessarily complex error management code.
Clean up error management by just returning an int (0 or negative error).
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 6066ca0c5624..982bc64d8361 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -991,11 +991,9 @@ static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector
* @bridge_connector->drm and @bridge_connector->encoder fields are
* assigned at least)
*
- * Returns a pointer to @bridge_connector on success, or a negative error
- * pointer otherwise.
+ * Returns 0 on success or a negative error otherwise.
*/
-static struct drm_bridge_connector *
-drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector)
+static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector)
{
struct drm_connector *connector;
struct i2c_adapter *ddc = NULL;
@@ -1012,7 +1010,7 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
&supported_formats, &max_bpc, &ddc,
&panel_bridge, &support_hdcp);
if (ret)
- return ERR_PTR(ret);
+ return ret;
if (bridge_connector->bridge_hdmi) {
if (!connector->ycbcr_420_allowed)
@@ -1041,18 +1039,18 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
supported_formats,
max_bpc);
if (ret)
- return ERR_PTR(ret);
+ return ret;
} else {
ret = drmm_connector_init(bridge_connector->drm, connector,
&drm_bridge_connector_funcs,
connector_type, ddc);
if (ret)
- return ERR_PTR(ret);
+ return ret;
}
ret = drm_bridge_connector_init_hdmi_audio_cec(bridge_connector);
if (ret)
- return ERR_PTR(ret);
+ return ret;
drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
@@ -1071,9 +1069,9 @@ drm_bridge_connector_add_connector(struct drm_bridge_connector *bridge_connector
ret = drm_connector_attach_encoder(connector, bridge_connector->encoder);
if (ret)
- return ERR_PTR(ret);
+ return ret;
- return bridge_connector;
+ return 0;
}
/**
@@ -1110,9 +1108,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
bridge_connector->drm = drm;
bridge_connector->encoder = encoder;
- bridge_connector = drm_bridge_connector_add_connector(bridge_connector);
- if (IS_ERR(bridge_connector))
- return ERR_CAST(bridge_connector);
+ ret = drm_bridge_connector_add_connector(bridge_connector);
+ if (ret)
+ return ERR_PTR(ret);
return &bridge_connector->base;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 12/37] drm/display: bridge-connector: hoist error management to common code
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (10 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 11/37] drm/display: bridge-connector: return int from drm_bridge_connector_add_connector() Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 13/37] drm/display: bridge-connector: move drm_bridge_connector_put_bridges() definition eariler Luca Ceresoli
` (24 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In prepataion to add more error management code common to the HDMI and
non-HDMI branches, move error management to be common to both cases.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 982bc64d8361..defe29e339b0 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -1038,15 +1038,13 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
connector_type, ddc,
supported_formats,
max_bpc);
- if (ret)
- return ret;
} else {
ret = drmm_connector_init(bridge_connector->drm, connector,
&drm_bridge_connector_funcs,
connector_type, ddc);
- if (ret)
- return ret;
}
+ if (ret)
+ return ret;
ret = drm_bridge_connector_init_hdmi_audio_cec(bridge_connector);
if (ret)
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 13/37] drm/display: bridge-connector: move drm_bridge_connector_put_bridges() definition eariler
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (11 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 12/37] drm/display: bridge-connector: hoist error management to common code Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 14/37] drm/display: bridge-connector: add non-drmm variant of drm_bridge_connector_put_bridges() Luca Ceresoli
` (23 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
drm_bridge_connector_put_bridges() will have to be called in other places
to support bridge hot-plugging.
In preparation for bridge hotplug support, move it near the top of the
file.
No functional changes. Just moving code around.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 28 +++++++++++++-------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index defe29e339b0..2e96af8efb55 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -140,6 +140,20 @@ struct drm_bridge_connector {
#define to_drm_bridge_connector(x) \
container_of(x, struct drm_bridge_connector, base)
+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);
+}
+
/* -----------------------------------------------------------------------------
* Bridge Connector Hot-Plug Handling
*/
@@ -758,20 +772,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);
-}
-
static int drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
int *connector_type,
unsigned int *supported_formats,
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 14/37] drm/display: bridge-connector: add non-drmm variant of drm_bridge_connector_put_bridges()
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (12 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 13/37] drm/display: bridge-connector: move drm_bridge_connector_put_bridges() definition eariler Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 15/37] drm/display: bridge-connector: allocate the connector dynamically Luca Ceresoli
` (22 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
drm_bridge_connector_put_bridges() is currently meant as a drmm cleanup
action. In preparation for bridge hotplug add a non-drmm action, so it can
be called more simply in non-drmm context.
To do this split drm_bridge_connector_put_bridges() in two variants, both
doing the same actions and only differing in the name and arguments:
* drm_bridge_connector_put_bridges() (same name as before) to be called
directly by the drm_bridge_connector code with a drm_bridge_connector
pointer as the only parameter
* drmm_bridge_connector_put_bridges() (same name + drmm prefix) providing
the same drmm calling convention as before
Update the only call site accordingly.
No functional changes.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 2e96af8efb55..1fd104f1647b 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -140,10 +140,8 @@ struct drm_bridge_connector {
#define to_drm_bridge_connector(x) \
container_of(x, struct drm_bridge_connector, base)
-static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
+static void drm_bridge_connector_put_bridges(struct drm_bridge_connector *bridge_connector)
{
- 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);
@@ -154,6 +152,13 @@ static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
drm_bridge_put(bridge_connector->bridge_hdmi_cec);
}
+static void drmm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
+{
+ struct drm_bridge_connector *bridge_connector = (struct drm_bridge_connector *)data;
+
+ drm_bridge_connector_put_bridges(bridge_connector);
+}
+
/* -----------------------------------------------------------------------------
* Bridge Connector Hot-Plug Handling
*/
@@ -1099,7 +1104,7 @@ 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);
+ ret = drmm_add_action(drm, drmm_bridge_connector_put_bridges, bridge_connector);
if (ret)
return ERR_PTR(ret);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 15/37] drm/display: bridge-connector: allocate the connector dynamically
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (13 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 14/37] drm/display: bridge-connector: add non-drmm variant of drm_bridge_connector_put_bridges() Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:15 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 16/37] drm/display: bridge-connector: move per-connector fields to the dynamic connector Luca Ceresoli
` (21 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
Currently the drm_bridge_connector has an embedded drm_connector, so their
allocation lifetimes are tied to each other. This is insufficient to
support DRM bridge hotplugging, which requires the connector to be added
and removed dynamically at runtime multiple times based on hotplug/unplug
events while the drm_bridge_connector is persistent.
Moreover the drm_connector is exposed to user space and thus an ongoing
operation (e.g. an ioctl) might last for an arbitrarily long time even
after the hardware gets removed. This means a new connector might have to
be added when the previous one is still referenced by user space.
In preparation to handle hotplug, allocate the drm-connector dynamically,
to allow:
* creating and destroying a connector multiple times during a single
drm_bridge_connector lifetime
* creating a new connector even though the previous one is still in use
and thus still refcounted and not yet freed
This commit does not introduce the actions in the two bullets (it will
happen in a later commit), it only moves to dynamic APIs for connector
allocation and init.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 156 ++++++++++++++++++-------
1 file changed, 114 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 1fd104f1647b..b4e2c8f1d32d 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0+
/*
* Copyright (C) 2019 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ * Copyright (C) 2026 GE HealthCare
+ * Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
*/
#include <linux/export.h>
@@ -53,13 +55,24 @@
*/
/**
- * struct drm_bridge_connector - A connector backed by a chain of bridges
+ * drm_bridge_connector_dynconn - wrapper struct for the dynamically
+ * created drm_connector
*/
-struct drm_bridge_connector {
+struct drm_bridge_connector_dynconn {
/**
- * @base: The base DRM connector
+ * @bridge_connector: the owning bridge connector
*/
- struct drm_connector base;
+ struct drm_bridge_connector *bridge_connector;
+ /**
+ * @connector: the drm_connector exposed to user space
+ */
+ struct drm_connector connector;
+};
+
+/**
+ * struct drm_bridge_connector - A connector backed by a chain of bridges
+ */
+struct drm_bridge_connector {
/**
* @drm: The DRM device we belong to
*/
@@ -70,6 +83,10 @@ struct drm_bridge_connector {
* The encoder at the start of the bridges chain.
*/
struct drm_encoder *encoder;
+ /**
+ * @dynconn: The DRM connector added by the drm_bridge_connector
+ */
+ struct drm_bridge_connector_dynconn *dynconn;
/**
* @bridge_edid:
*
@@ -137,8 +154,13 @@ struct drm_bridge_connector {
struct drm_connector_hdmi_funcs hdmi_funcs;
};
-#define to_drm_bridge_connector(x) \
- container_of(x, struct drm_bridge_connector, base)
+static struct drm_bridge_connector *to_drm_bridge_connector(const struct drm_connector *connector)
+{
+ const struct drm_bridge_connector_dynconn *dynconn =
+ container_of(connector, struct drm_bridge_connector_dynconn, connector);
+
+ return dynconn->bridge_connector;
+}
static void drm_bridge_connector_put_bridges(struct drm_bridge_connector *bridge_connector)
{
@@ -152,13 +174,6 @@ static void drm_bridge_connector_put_bridges(struct drm_bridge_connector *bridge
drm_bridge_put(bridge_connector->bridge_hdmi_cec);
}
-static void drmm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
-{
- struct drm_bridge_connector *bridge_connector = (struct drm_bridge_connector *)data;
-
- drm_bridge_connector_put_bridges(bridge_connector);
-}
-
/* -----------------------------------------------------------------------------
* Bridge Connector Hot-Plug Handling
*/
@@ -179,7 +194,7 @@ static void drm_bridge_connector_hpd_notify(struct drm_connector *connector,
static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *bridge_connector,
enum drm_connector_status status)
{
- struct drm_connector *connector = &bridge_connector->base;
+ struct drm_connector *connector = &bridge_connector->dynconn->connector;
struct drm_device *dev = bridge_connector->drm;
mutex_lock(&dev->mode_config.mutex);
@@ -299,6 +314,16 @@ static void drm_bridge_connector_reset(struct drm_connector *connector)
connector->state);
}
+static void drm_bridge_connector_dynconn_destroy(struct drm_connector *connector)
+{
+ struct drm_bridge_connector_dynconn *dynconn =
+ container_of(connector, struct drm_bridge_connector_dynconn, connector);
+
+ drm_connector_cleanup(connector);
+ drm_bridge_connector_put_bridges(dynconn->bridge_connector);
+ kfree(dynconn);
+}
+
static const struct drm_connector_funcs drm_bridge_connector_funcs = {
.reset = drm_bridge_connector_reset,
.detect = drm_bridge_connector_detect,
@@ -308,6 +333,7 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
.debugfs_init = drm_bridge_connector_debugfs_init,
.oob_hotplug_event = drm_bridge_connector_oob_hotplug_event,
+ .destroy = drm_bridge_connector_dynconn_destroy,
};
/* -----------------------------------------------------------------------------
@@ -777,7 +803,17 @@ static const struct drm_connector_hdmi_cec_funcs drm_bridge_connector_hdmi_cec_f
* Bridge Connector Initialisation
*/
-static int drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_connector,
+static void drm_bridge_connector_dynconn_release(struct drm_bridge_connector *bridge_connector)
+{
+ if (!bridge_connector->dynconn)
+ return;
+
+ drm_connector_unregister(&bridge_connector->dynconn->connector);
+ drm_connector_put(&bridge_connector->dynconn->connector);
+ bridge_connector->dynconn = NULL;
+}
+
+static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn *dynconn,
int *connector_type,
unsigned int *supported_formats,
unsigned int *max_bpc,
@@ -785,7 +821,8 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_
struct drm_bridge **panel_bridge,
bool *support_hdcp)
{
- struct drm_connector *connector = &bridge_connector->base;
+ struct drm_bridge_connector *bridge_connector = dynconn->bridge_connector;
+ struct drm_connector *connector = &dynconn->connector;
/*
* TODO: Handle doublescan_allowed and stereo_allowed.
@@ -937,6 +974,7 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector *bridge_
static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector *bridge_connector)
{
+ struct drm_connector *connector = &bridge_connector->dynconn->connector;
int ret;
if (bridge_connector->bridge_hdmi_audio ||
@@ -951,7 +989,7 @@ static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector
dev = bridge->hdmi_audio_dev;
- ret = drm_connector_hdmi_audio_init(&bridge_connector->base, dev,
+ ret = drm_connector_hdmi_audio_init(connector, dev,
&drm_bridge_connector_hdmi_audio_funcs,
bridge->hdmi_audio_max_i2s_playback_channels,
bridge->hdmi_audio_i2s_formats,
@@ -965,8 +1003,7 @@ static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector
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(&bridge_connector->base,
- NULL,
+ ret = drmm_connector_hdmi_cec_notifier_register(connector, NULL,
bridge->hdmi_cec_dev);
if (ret)
return ret;
@@ -976,7 +1013,7 @@ static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector
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(&bridge_connector->base,
+ ret = drmm_connector_hdmi_cec_register(connector,
&drm_bridge_connector_hdmi_cec_funcs,
bridge->hdmi_cec_adapter_name,
bridge->hdmi_cec_available_las,
@@ -1009,13 +1046,20 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
int connector_type;
int ret;
- connector = &bridge_connector->base;
+ struct drm_bridge_connector_dynconn *dynconn = kzalloc_obj(*dynconn);
+ if (!dynconn)
+ return -ENOMEM;
- ret = drm_bridge_connector_get_bridges(bridge_connector, &connector_type,
+ dynconn->bridge_connector = bridge_connector;
+ connector = &dynconn->connector;
+
+ ret = drm_bridge_connector_get_bridges(dynconn, &connector_type,
&supported_formats, &max_bpc, &ddc,
&panel_bridge, &support_hdcp);
- if (ret)
+ if (ret) {
+ kfree(dynconn);
return ret;
+ }
if (bridge_connector->bridge_hdmi) {
if (!connector->ycbcr_420_allowed)
@@ -1035,25 +1079,34 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
bridge_connector->hdmi_funcs.spd =
drm_bridge_connector_hdmi_spd_infoframe;
- ret = drmm_connector_hdmi_init(bridge_connector->drm, connector,
- bridge_connector->bridge_hdmi->vendor,
- bridge_connector->bridge_hdmi->product,
- &drm_bridge_connector_funcs,
- &bridge_connector->hdmi_funcs,
- connector_type, ddc,
- supported_formats,
- max_bpc);
+ ret = drm_connector_hdmi_dynamic_init(bridge_connector->drm, connector,
+ bridge_connector->bridge_hdmi->vendor,
+ bridge_connector->bridge_hdmi->product,
+ &drm_bridge_connector_funcs,
+ &bridge_connector->hdmi_funcs,
+ connector_type, ddc,
+ supported_formats,
+ max_bpc);
} else {
- ret = drmm_connector_init(bridge_connector->drm, connector,
- &drm_bridge_connector_funcs,
- connector_type, ddc);
+ ret = drm_connector_dynamic_init(bridge_connector->drm, connector,
+ &drm_bridge_connector_funcs,
+ connector_type, ddc);
}
- if (ret)
+ if (ret) {
+ drm_bridge_connector_put_bridges(dynconn->bridge_connector);
+ kfree(dynconn);
return ret;
+ }
+
+ /* From now on the connector is referenced and has to be put, not kfreed */
+
+ bridge_connector->dynconn = dynconn;
+
+ drm_bridge_connector_reset(connector);
ret = drm_bridge_connector_init_hdmi_audio_cec(bridge_connector);
if (ret)
- return ret;
+ goto err_put;
drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
@@ -1072,9 +1125,25 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
ret = drm_connector_attach_encoder(connector, bridge_connector->encoder);
if (ret)
- return ret;
+ goto err_put;
+
+ ret = drm_connector_dynamic_register(connector);
+ if (ret)
+ goto err_put;
return 0;
+
+err_put:
+ drm_connector_put(connector);
+ bridge_connector->dynconn = NULL;
+ return ret;
+}
+
+static void drm_bridge_connector_fini(struct drm_device *dev, void *res)
+{
+ struct drm_bridge_connector *bridge_connector = (struct drm_bridge_connector *)res;
+
+ drm_bridge_connector_dynconn_release(bridge_connector);
}
/**
@@ -1104,10 +1173,6 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
if (!bridge_connector)
return ERR_PTR(-ENOMEM);
- ret = drmm_add_action(drm, drmm_bridge_connector_put_bridges, bridge_connector);
- if (ret)
- return ERR_PTR(ret);
-
bridge_connector->drm = drm;
bridge_connector->encoder = encoder;
@@ -1115,6 +1180,13 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
if (ret)
return ERR_PTR(ret);
- return &bridge_connector->base;
+ ret = drmm_add_action_or_reset(drm, drm_bridge_connector_fini, bridge_connector);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (!bridge_connector->dynconn)
+ return ERR_PTR(-EINVAL);
+
+ return &bridge_connector->dynconn->connector;
}
EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 15/37] drm/display: bridge-connector: allocate the connector dynamically
2026-05-19 10:37 ` [PATCH 15/37] drm/display: bridge-connector: allocate the connector dynamically Luca Ceresoli
@ 2026-05-19 11:15 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:15 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Dynamic connector is passed to `drmm_` CEC cleanup actions, leading to a Use-After-Free if initialization fails.
- [High] Unconditional dereference of `dynconn` in the HPD handler causes a potential NULL pointer dereference.
- [Medium] Bridge references acquired during connector initialization are leaked on error paths.
--
commit b1940f18716e1942290e7aa7cf6a8cdaa0380840
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/display: bridge-connector: allocate the connector dynamically
This commit changes drm_bridge_connector to allocate its drm_connector
dynamically instead of embedding it. This allows recreating the connector
during hotplug events while the bridge connector object remains persistent.
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 1fd104f1647b8..b4e2c8f1d32d6 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
[ ... ]
> @@ -179,7 +194,7 @@ static void drm_bridge_connector_hpd_notify(struct drm_connector *connector,
> static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *bridge_connector,
> enum drm_connector_status status)
> {
> - struct drm_connector *connector = &bridge_connector->base;
> + struct drm_connector *connector = &bridge_connector->dynconn->connector;
Since the connector is now allocated dynamically, is it possible for a
hotplug event to trigger when bridge_connector->dynconn is NULL? If so,
will dereferencing dynconn here cause a NULL pointer dereference?
[ ... ]
> @@ -1009,13 +1046,20 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
[ ... ]
> - ret = drm_bridge_connector_get_bridges(bridge_connector, &connector_type,
> + ret = drm_bridge_connector_get_bridges(dynconn, &connector_type,
> &supported_formats, &max_bpc, &ddc,
> &panel_bridge, &support_hdcp);
> - if (ret)
> + if (ret) {
> + kfree(dynconn);
> return ret;
> + }
If drm_bridge_connector_get_bridges() encounters an error and returns early,
it may have already acquired some bridge references. Are those references
leaked here since drm_bridge_connector_put_bridges() isn't called before
returning?
[ ... ]
> @@ -1072,9 +1125,25 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
>
> ret = drm_connector_attach_encoder(connector, bridge_connector->encoder);
> if (ret)
> - return ret;
> + goto err_put;
> +
> + ret = drm_connector_dynamic_register(connector);
> + if (ret)
> + goto err_put;
>
> return 0;
> +
> +err_put:
> + drm_connector_put(connector);
> + bridge_connector->dynconn = NULL;
> + return ret;
> +}
If drm_connector_dynamic_register() fails, the code goes to err_put and
calls drm_connector_put(connector), which will drop the refcount and free
dynconn.
However, drm_bridge_connector_init_hdmi_audio_cec() registers cleanup
actions tied to the drm_device using drmm_connector_hdmi_cec_register() and
drmm_connector_hdmi_cec_notifier_register().
Will those drmm actions cause a use-after-free when the drm_device is
eventually released, since they will attempt to dereference the already
freed connector?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=15
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 16/37] drm/display: bridge-connector: move per-connector fields to the dynamic connector
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (14 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 15/37] drm/display: bridge-connector: allocate the connector dynamically Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:17 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 17/37] drm/display: bridge-connector: protect dynconn creation and destruction with a mutex Luca Ceresoli
` (20 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
The various bridge pointers and the hdmi_funcs pointer are per-connector
specific. In other words, when introducing bridge hotplug and the
dynamically-created connector, they will have to be set per each created
connector, possibly with different values from any previously created
connector.
Those pointers are currently in struct drm_bridge_connector, which will
persist across multiple connector creations and destructions. This means
when a dynamic connector is created before the previous one is destroyed,
these fields would be overwritten by the new connector but the old one will
still reference them. This would lead to possible put of incorrect briges,
resulting in use-after-free and leaks.
In preparation for bridge hotplug, move all those fields from struct
drm_bridge_connector to struct drm_bridge_connector_dynconn, so they have
the same allocation and lifetime of the connector they refer to.
Update the many locations where these fields are used.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 370 +++++++++++++------------
1 file changed, 187 insertions(+), 183 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index b4e2c8f1d32d..bd421b6beb7c 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -67,26 +67,6 @@ struct drm_bridge_connector_dynconn {
* @connector: the drm_connector exposed to user space
*/
struct drm_connector connector;
-};
-
-/**
- * struct drm_bridge_connector - A connector backed by a chain of bridges
- */
-struct drm_bridge_connector {
- /**
- * @drm: The DRM device we belong to
- */
- struct drm_device *drm;
- /**
- * @encoder:
- *
- * The encoder at the start of the bridges chain.
- */
- struct drm_encoder *encoder;
- /**
- * @dynconn: The DRM connector added by the drm_bridge_connector
- */
- struct drm_bridge_connector_dynconn *dynconn;
/**
* @bridge_edid:
*
@@ -154,24 +134,47 @@ struct drm_bridge_connector {
struct drm_connector_hdmi_funcs hdmi_funcs;
};
-static struct drm_bridge_connector *to_drm_bridge_connector(const struct drm_connector *connector)
+/**
+ * struct drm_bridge_connector - A connector backed by a chain of bridges
+ */
+struct drm_bridge_connector {
+ /**
+ * @drm: The DRM device we belong to
+ */
+ struct drm_device *drm;
+ /**
+ * @encoder:
+ *
+ * The encoder at the start of the bridges chain.
+ */
+ struct drm_encoder *encoder;
+ /**
+ * @dynconn: The DRM connector added by the drm_bridge_connector
+ */
+ struct drm_bridge_connector_dynconn *dynconn;
+};
+
+static struct drm_bridge_connector_dynconn *
+to_drm_bridge_connector_dynconn(const struct drm_connector *connector)
{
- const struct drm_bridge_connector_dynconn *dynconn =
- container_of(connector, struct drm_bridge_connector_dynconn, connector);
+ return container_of(connector, struct drm_bridge_connector_dynconn, connector);
+}
- return dynconn->bridge_connector;
+static struct drm_bridge_connector *to_drm_bridge_connector(const struct drm_connector *connector)
+{
+ return to_drm_bridge_connector_dynconn(connector)->bridge_connector;
}
-static void drm_bridge_connector_put_bridges(struct drm_bridge_connector *bridge_connector)
+static void drm_bridge_connector_put_bridges(struct drm_bridge_connector_dynconn *dynconn)
{
- 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_put(dynconn->bridge_edid);
+ drm_bridge_put(dynconn->bridge_hpd);
+ drm_bridge_put(dynconn->bridge_detect);
+ drm_bridge_put(dynconn->bridge_modes);
+ drm_bridge_put(dynconn->bridge_hdmi);
+ drm_bridge_put(dynconn->bridge_hdmi_audio);
+ drm_bridge_put(dynconn->bridge_dp_audio);
+ drm_bridge_put(dynconn->bridge_hdmi_cec);
}
/* -----------------------------------------------------------------------------
@@ -225,7 +228,9 @@ static void drm_bridge_connector_enable_hpd(struct drm_connector *connector)
{
struct drm_bridge_connector *bridge_connector =
to_drm_bridge_connector(connector);
- struct drm_bridge *hpd = bridge_connector->bridge_hpd;
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
+ struct drm_bridge *hpd = dynconn->bridge_hpd;
if (hpd)
drm_bridge_hpd_enable(hpd, drm_bridge_connector_hpd_cb,
@@ -234,9 +239,9 @@ static void drm_bridge_connector_enable_hpd(struct drm_connector *connector)
static void drm_bridge_connector_disable_hpd(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
- struct drm_bridge *hpd = bridge_connector->bridge_hpd;
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
+ struct drm_bridge *hpd = dynconn->bridge_hpd;
if (hpd)
drm_bridge_hpd_disable(hpd);
@@ -249,10 +254,10 @@ static void drm_bridge_connector_disable_hpd(struct drm_connector *connector)
static enum drm_connector_status
drm_bridge_connector_detect(struct drm_connector *connector, bool force)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
- struct drm_bridge *detect = bridge_connector->bridge_detect;
- struct drm_bridge *hdmi = bridge_connector->bridge_hdmi;
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
+ struct drm_bridge *detect = dynconn->bridge_detect;
+ struct drm_bridge *hdmi = dynconn->bridge_hdmi;
enum drm_connector_status status;
if (detect) {
@@ -281,9 +286,9 @@ drm_bridge_connector_detect(struct drm_connector *connector, bool force)
static void drm_bridge_connector_force(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
- struct drm_bridge *hdmi = bridge_connector->bridge_hdmi;
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
+ struct drm_bridge *hdmi = dynconn->bridge_hdmi;
if (hdmi)
drm_atomic_helper_connector_hdmi_force(connector);
@@ -305,11 +310,11 @@ static void drm_bridge_connector_debugfs_init(struct drm_connector *connector,
static void drm_bridge_connector_reset(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
drm_atomic_helper_connector_reset(connector);
- if (bridge_connector->bridge_hdmi)
+ if (dynconn->bridge_hdmi)
__drm_atomic_helper_connector_hdmi_reset(connector,
connector->state);
}
@@ -320,7 +325,7 @@ static void drm_bridge_connector_dynconn_destroy(struct drm_connector *connector
container_of(connector, struct drm_bridge_connector_dynconn, connector);
drm_connector_cleanup(connector);
- drm_bridge_connector_put_bridges(dynconn->bridge_connector);
+ drm_bridge_connector_put_bridges(dynconn);
kfree(dynconn);
}
@@ -370,15 +375,15 @@ static int drm_bridge_connector_get_modes_edid(struct drm_connector *connector,
static int drm_bridge_connector_get_modes(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
/*
* If there is a HDMI bridge, EDID has been updated as a part of
* the .detect(). Just update the modes here.
*/
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (bridge)
return drm_edid_connector_add_modes(connector);
@@ -386,7 +391,7 @@ static int drm_bridge_connector_get_modes(struct drm_connector *connector)
* If display exposes EDID, then we parse that in the normal way to
* build table of supported modes.
*/
- bridge = bridge_connector->bridge_edid;
+ bridge = dynconn->bridge_edid;
if (bridge)
return drm_bridge_connector_get_modes_edid(connector, bridge);
@@ -394,7 +399,7 @@ static int drm_bridge_connector_get_modes(struct drm_connector *connector)
* Otherwise if the display pipeline reports modes (e.g. with a fixed
* resolution panel or an analog TV output), query it.
*/
- bridge = bridge_connector->bridge_modes;
+ bridge = dynconn->bridge_modes;
if (bridge)
return bridge->funcs->get_modes(bridge, connector);
@@ -410,10 +415,10 @@ static enum drm_mode_status
drm_bridge_connector_mode_valid(struct drm_connector *connector,
const struct drm_display_mode *mode)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
- if (bridge_connector->bridge_hdmi)
+ if (dynconn->bridge_hdmi)
return drm_hdmi_connector_mode_valid(connector, mode);
return MODE_OK;
@@ -422,10 +427,10 @@ drm_bridge_connector_mode_valid(struct drm_connector *connector,
static int drm_bridge_connector_atomic_check(struct drm_connector *connector,
struct drm_atomic_commit *state)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
- if (bridge_connector->bridge_hdmi)
+ if (dynconn->bridge_hdmi)
return drm_atomic_helper_connector_hdmi_check(connector, state);
return 0;
@@ -444,11 +449,11 @@ drm_bridge_connector_tmds_char_rate_valid(const struct drm_connector *connector,
const struct drm_display_mode *mode,
unsigned long long tmds_rate)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return MODE_ERROR;
@@ -460,11 +465,11 @@ drm_bridge_connector_tmds_char_rate_valid(const struct drm_connector *connector,
static int drm_bridge_connector_clear_avi_infoframe(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -474,11 +479,11 @@ static int drm_bridge_connector_clear_avi_infoframe(struct drm_connector *connec
static int drm_bridge_connector_write_avi_infoframe(struct drm_connector *connector,
const u8 *buffer, size_t len)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -487,11 +492,11 @@ static int drm_bridge_connector_write_avi_infoframe(struct drm_connector *connec
static int drm_bridge_connector_clear_hdmi_infoframe(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -501,11 +506,11 @@ static int drm_bridge_connector_clear_hdmi_infoframe(struct drm_connector *conne
static int drm_bridge_connector_write_hdmi_infoframe(struct drm_connector *connector,
const u8 *buffer, size_t len)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -514,11 +519,11 @@ static int drm_bridge_connector_write_hdmi_infoframe(struct drm_connector *conne
static int drm_bridge_connector_clear_audio_infoframe(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -528,11 +533,11 @@ static int drm_bridge_connector_clear_audio_infoframe(struct drm_connector *conn
static int drm_bridge_connector_write_audio_infoframe(struct drm_connector *connector,
const u8 *buffer, size_t len)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -541,11 +546,11 @@ static int drm_bridge_connector_write_audio_infoframe(struct drm_connector *conn
static int drm_bridge_connector_clear_hdr_drm_infoframe(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -555,11 +560,11 @@ static int drm_bridge_connector_clear_hdr_drm_infoframe(struct drm_connector *co
static int drm_bridge_connector_write_hdr_drm_infoframe(struct drm_connector *connector,
const u8 *buffer, size_t len)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -568,11 +573,11 @@ static int drm_bridge_connector_write_hdr_drm_infoframe(struct drm_connector *co
static int drm_bridge_connector_clear_spd_infoframe(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -582,11 +587,11 @@ static int drm_bridge_connector_clear_spd_infoframe(struct drm_connector *connec
static int drm_bridge_connector_write_spd_infoframe(struct drm_connector *connector,
const u8 *buffer, size_t len)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi;
+ bridge = dynconn->bridge_hdmi;
if (!bridge)
return -EINVAL;
@@ -596,11 +601,11 @@ static int drm_bridge_connector_write_spd_infoframe(struct drm_connector *connec
static const struct drm_edid *
drm_bridge_connector_read_edid(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_edid;
+ bridge = dynconn->bridge_edid;
if (!bridge)
return NULL;
@@ -638,12 +643,12 @@ static const struct drm_connector_infoframe_funcs drm_bridge_connector_hdmi_spd_
static int drm_bridge_connector_audio_startup(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- if (bridge_connector->bridge_hdmi_audio) {
- bridge = bridge_connector->bridge_hdmi_audio;
+ if (dynconn->bridge_hdmi_audio) {
+ bridge = dynconn->bridge_hdmi_audio;
if (!bridge->funcs->hdmi_audio_startup)
return 0;
@@ -651,8 +656,8 @@ static int drm_bridge_connector_audio_startup(struct drm_connector *connector)
return bridge->funcs->hdmi_audio_startup(bridge, connector);
}
- if (bridge_connector->bridge_dp_audio) {
- bridge = bridge_connector->bridge_dp_audio;
+ if (dynconn->bridge_dp_audio) {
+ bridge = dynconn->bridge_dp_audio;
if (!bridge->funcs->dp_audio_startup)
return 0;
@@ -667,18 +672,18 @@ static int drm_bridge_connector_audio_prepare(struct drm_connector *connector,
struct hdmi_codec_daifmt *fmt,
struct hdmi_codec_params *hparms)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- if (bridge_connector->bridge_hdmi_audio) {
- bridge = bridge_connector->bridge_hdmi_audio;
+ if (dynconn->bridge_hdmi_audio) {
+ bridge = dynconn->bridge_hdmi_audio;
return bridge->funcs->hdmi_audio_prepare(bridge, connector, fmt, hparms);
}
- if (bridge_connector->bridge_dp_audio) {
- bridge = bridge_connector->bridge_dp_audio;
+ if (dynconn->bridge_dp_audio) {
+ bridge = dynconn->bridge_dp_audio;
return bridge->funcs->dp_audio_prepare(bridge, connector, fmt, hparms);
}
@@ -688,17 +693,17 @@ static int drm_bridge_connector_audio_prepare(struct drm_connector *connector,
static void drm_bridge_connector_audio_shutdown(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- if (bridge_connector->bridge_hdmi_audio) {
- bridge = bridge_connector->bridge_hdmi_audio;
+ if (dynconn->bridge_hdmi_audio) {
+ bridge = dynconn->bridge_hdmi_audio;
bridge->funcs->hdmi_audio_shutdown(bridge, connector);
}
- if (bridge_connector->bridge_dp_audio) {
- bridge = bridge_connector->bridge_dp_audio;
+ if (dynconn->bridge_dp_audio) {
+ bridge = dynconn->bridge_dp_audio;
bridge->funcs->dp_audio_shutdown(bridge, connector);
}
}
@@ -706,12 +711,12 @@ static void drm_bridge_connector_audio_shutdown(struct drm_connector *connector)
static int drm_bridge_connector_audio_mute_stream(struct drm_connector *connector,
bool enable, int direction)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- if (bridge_connector->bridge_hdmi_audio) {
- bridge = bridge_connector->bridge_hdmi_audio;
+ if (dynconn->bridge_hdmi_audio) {
+ bridge = dynconn->bridge_hdmi_audio;
if (!bridge->funcs->hdmi_audio_mute_stream)
return -ENOTSUPP;
@@ -720,8 +725,8 @@ static int drm_bridge_connector_audio_mute_stream(struct drm_connector *connecto
enable, direction);
}
- if (bridge_connector->bridge_dp_audio) {
- bridge = bridge_connector->bridge_dp_audio;
+ if (dynconn->bridge_dp_audio) {
+ bridge = dynconn->bridge_dp_audio;
if (!bridge->funcs->dp_audio_mute_stream)
return -ENOTSUPP;
@@ -742,22 +747,22 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud
static int drm_bridge_connector_hdmi_cec_enable(struct drm_connector *connector, bool enable)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi_cec;
+ bridge = dynconn->bridge_hdmi_cec;
return bridge->funcs->hdmi_cec_enable(bridge, enable);
}
static int drm_bridge_connector_hdmi_cec_log_addr(struct drm_connector *connector, u8 logical_addr)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi_cec;
+ bridge = dynconn->bridge_hdmi_cec;
return bridge->funcs->hdmi_cec_log_addr(bridge, logical_addr);
}
@@ -767,11 +772,11 @@ static int drm_bridge_connector_hdmi_cec_transmit(struct drm_connector *connecto
u32 signal_free_time,
struct cec_msg *msg)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi_cec;
+ bridge = dynconn->bridge_hdmi_cec;
return bridge->funcs->hdmi_cec_transmit(bridge, attempts,
signal_free_time,
@@ -780,11 +785,11 @@ static int drm_bridge_connector_hdmi_cec_transmit(struct drm_connector *connecto
static int drm_bridge_connector_hdmi_cec_init(struct drm_connector *connector)
{
- struct drm_bridge_connector *bridge_connector =
- to_drm_bridge_connector(connector);
+ struct drm_bridge_connector_dynconn *dynconn =
+ to_drm_bridge_connector_dynconn(connector);
struct drm_bridge *bridge;
- bridge = bridge_connector->bridge_hdmi_cec;
+ bridge = dynconn->bridge_hdmi_cec;
if (!bridge->funcs->hdmi_cec_init)
return 0;
@@ -821,7 +826,6 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
struct drm_bridge **panel_bridge,
bool *support_hdcp)
{
- struct drm_bridge_connector *bridge_connector = dynconn->bridge_connector;
struct drm_connector *connector = &dynconn->connector;
/*
@@ -831,7 +835,7 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
connector->ycbcr_420_allowed = true;
*connector_type = DRM_MODE_CONNECTOR_Unknown;
- drm_for_each_bridge_in_chain_scoped(bridge_connector->encoder, bridge) {
+ drm_for_each_bridge_in_chain_scoped(dynconn->bridge_connector->encoder, bridge) {
if (!bridge->interlace_allowed)
connector->interlace_allowed = false;
if (!bridge->ycbcr_420_allowed)
@@ -841,26 +845,26 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
* Ensure the last bridge declares OP_EDID or OP_MODES or both.
*/
if (bridge->ops & DRM_BRIDGE_OP_EDID || bridge->ops & DRM_BRIDGE_OP_MODES) {
- drm_bridge_put(bridge_connector->bridge_edid);
- bridge_connector->bridge_edid = NULL;
- drm_bridge_put(bridge_connector->bridge_modes);
- bridge_connector->bridge_modes = NULL;
+ drm_bridge_put(dynconn->bridge_edid);
+ dynconn->bridge_edid = NULL;
+ drm_bridge_put(dynconn->bridge_modes);
+ dynconn->bridge_modes = NULL;
if (bridge->ops & DRM_BRIDGE_OP_EDID)
- bridge_connector->bridge_edid = drm_bridge_get(bridge);
+ dynconn->bridge_edid = drm_bridge_get(bridge);
if (bridge->ops & DRM_BRIDGE_OP_MODES)
- bridge_connector->bridge_modes = drm_bridge_get(bridge);
+ dynconn->bridge_modes = 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);
+ drm_bridge_put(dynconn->bridge_hpd);
+ dynconn->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);
+ drm_bridge_put(dynconn->bridge_detect);
+ dynconn->bridge_detect = drm_bridge_get(bridge);
}
if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
- if (bridge_connector->bridge_hdmi)
+ if (dynconn->bridge_hdmi)
return -EBUSY;
if (!bridge->funcs->hdmi_write_avi_infoframe ||
!bridge->funcs->hdmi_clear_avi_infoframe ||
@@ -883,7 +887,7 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
!bridge->funcs->hdmi_clear_spd_infoframe))
return -EINVAL;
- bridge_connector->bridge_hdmi = drm_bridge_get(bridge);
+ dynconn->bridge_hdmi = drm_bridge_get(bridge);
if (bridge->supported_formats)
*supported_formats = bridge->supported_formats;
@@ -892,10 +896,10 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
}
if (bridge->ops & DRM_BRIDGE_OP_HDMI_AUDIO) {
- if (bridge_connector->bridge_hdmi_audio)
+ if (dynconn->bridge_hdmi_audio)
return -EBUSY;
- if (bridge_connector->bridge_dp_audio)
+ if (dynconn->bridge_dp_audio)
return -EBUSY;
if (!bridge->hdmi_audio_max_i2s_playback_channels &&
@@ -906,14 +910,14 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
!bridge->funcs->hdmi_audio_shutdown)
return -EINVAL;
- bridge_connector->bridge_hdmi_audio = drm_bridge_get(bridge);
+ dynconn->bridge_hdmi_audio = drm_bridge_get(bridge);
}
if (bridge->ops & DRM_BRIDGE_OP_DP_AUDIO) {
- if (bridge_connector->bridge_dp_audio)
+ if (dynconn->bridge_dp_audio)
return -EBUSY;
- if (bridge_connector->bridge_hdmi_audio)
+ if (dynconn->bridge_hdmi_audio)
return -EBUSY;
if (!bridge->hdmi_audio_max_i2s_playback_channels &&
@@ -924,21 +928,21 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
!bridge->funcs->dp_audio_shutdown)
return -EINVAL;
- bridge_connector->bridge_dp_audio = drm_bridge_get(bridge);
+ dynconn->bridge_dp_audio = drm_bridge_get(bridge);
}
if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
- if (bridge_connector->bridge_hdmi_cec)
+ if (dynconn->bridge_hdmi_cec)
return -EBUSY;
- bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge);
+ dynconn->bridge_hdmi_cec = drm_bridge_get(bridge);
}
if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
- if (bridge_connector->bridge_hdmi_cec)
+ if (dynconn->bridge_hdmi_cec)
return -EBUSY;
- bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge);
+ dynconn->bridge_hdmi_cec = drm_bridge_get(bridge);
if (!bridge->funcs->hdmi_cec_enable ||
!bridge->funcs->hdmi_cec_log_addr ||
@@ -972,20 +976,20 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
return 0;
}
-static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector *bridge_connector)
+static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector_dynconn *dynconn)
{
- struct drm_connector *connector = &bridge_connector->dynconn->connector;
+ struct drm_connector *connector = &dynconn->connector;
int ret;
- if (bridge_connector->bridge_hdmi_audio ||
- bridge_connector->bridge_dp_audio) {
+ if (dynconn->bridge_hdmi_audio ||
+ dynconn->bridge_dp_audio) {
struct device *dev;
struct drm_bridge *bridge;
- if (bridge_connector->bridge_hdmi_audio)
- bridge = bridge_connector->bridge_hdmi_audio;
+ if (dynconn->bridge_hdmi_audio)
+ bridge = dynconn->bridge_hdmi_audio;
else
- bridge = bridge_connector->bridge_dp_audio;
+ bridge = dynconn->bridge_dp_audio;
dev = bridge->hdmi_audio_dev;
@@ -999,9 +1003,9 @@ static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector
return ret;
}
- 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;
+ if (dynconn->bridge_hdmi_cec &&
+ dynconn->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
+ struct drm_bridge *bridge = dynconn->bridge_hdmi_cec;
ret = drmm_connector_hdmi_cec_notifier_register(connector, NULL,
bridge->hdmi_cec_dev);
@@ -1009,9 +1013,9 @@ static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector
return ret;
}
- 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;
+ if (dynconn->bridge_hdmi_cec &&
+ dynconn->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
+ struct drm_bridge *bridge = dynconn->bridge_hdmi_cec;
ret = drmm_connector_hdmi_cec_register(connector,
&drm_bridge_connector_hdmi_cec_funcs,
@@ -1061,29 +1065,29 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
return ret;
}
- if (bridge_connector->bridge_hdmi) {
+ if (dynconn->bridge_hdmi) {
if (!connector->ycbcr_420_allowed)
supported_formats &= ~BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);
- bridge_connector->hdmi_funcs = drm_bridge_connector_hdmi_funcs;
+ dynconn->hdmi_funcs = drm_bridge_connector_hdmi_funcs;
- if (bridge_connector->bridge_hdmi->ops & DRM_BRIDGE_OP_HDMI_AUDIO)
- bridge_connector->hdmi_funcs.audio =
+ if (dynconn->bridge_hdmi->ops & DRM_BRIDGE_OP_HDMI_AUDIO)
+ dynconn->hdmi_funcs.audio =
drm_bridge_connector_hdmi_audio_infoframe;
- if (bridge_connector->bridge_hdmi->ops & DRM_BRIDGE_OP_HDMI_HDR_DRM_INFOFRAME)
- bridge_connector->hdmi_funcs.hdr_drm =
+ if (dynconn->bridge_hdmi->ops & DRM_BRIDGE_OP_HDMI_HDR_DRM_INFOFRAME)
+ dynconn->hdmi_funcs.hdr_drm =
drm_bridge_connector_hdmi_hdr_drm_infoframe;
- if (bridge_connector->bridge_hdmi->ops & DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME)
- bridge_connector->hdmi_funcs.spd =
+ if (dynconn->bridge_hdmi->ops & DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME)
+ dynconn->hdmi_funcs.spd =
drm_bridge_connector_hdmi_spd_infoframe;
ret = drm_connector_hdmi_dynamic_init(bridge_connector->drm, connector,
- bridge_connector->bridge_hdmi->vendor,
- bridge_connector->bridge_hdmi->product,
+ dynconn->bridge_hdmi->vendor,
+ dynconn->bridge_hdmi->product,
&drm_bridge_connector_funcs,
- &bridge_connector->hdmi_funcs,
+ &dynconn->hdmi_funcs,
connector_type, ddc,
supported_formats,
max_bpc);
@@ -1093,7 +1097,7 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
connector_type, ddc);
}
if (ret) {
- drm_bridge_connector_put_bridges(dynconn->bridge_connector);
+ drm_bridge_connector_put_bridges(dynconn);
kfree(dynconn);
return ret;
}
@@ -1104,15 +1108,15 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
drm_bridge_connector_reset(connector);
- ret = drm_bridge_connector_init_hdmi_audio_cec(bridge_connector);
+ ret = drm_bridge_connector_init_hdmi_audio_cec(dynconn);
if (ret)
goto err_put;
drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
- if (bridge_connector->bridge_hpd)
+ if (dynconn->bridge_hpd)
connector->polled = DRM_CONNECTOR_POLL_HPD;
- else if (bridge_connector->bridge_detect)
+ else if (dynconn->bridge_detect)
connector->polled = DRM_CONNECTOR_POLL_CONNECT
| DRM_CONNECTOR_POLL_DISCONNECT;
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 16/37] drm/display: bridge-connector: move per-connector fields to the dynamic connector
2026-05-19 10:37 ` [PATCH 16/37] drm/display: bridge-connector: move per-connector fields to the dynamic connector Luca Ceresoli
@ 2026-05-19 11:17 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:17 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] HPD and OOB hotplug events are misrouted to the newest dynamic connector and can cause a NULL pointer dereference.
- [High] Use-After-Free due to drmm_ CEC registration for dynamically freed connectors.
- [Medium] Memory leak of bridge references on error path in drm_bridge_connector_add_connector().
--
commit 2da47a06a9f55cba5481389b325a75953c4b3912
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/display: bridge-connector: move per-connector fields to the dynamic connector
This commit moves various bridge pointers and the hdmi_funcs pointer from
struct drm_bridge_connector to struct drm_bridge_connector_dynconn. This
ensures these fields share the same allocation and lifetime as the
dynamically-created connector they reference, in preparation for bridge
hotplugging support.
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -225,7 +228,9 @@ static void drm_bridge_connector_enable_hpd(struct drm_connector *connector)
> {
> struct drm_bridge_connector *bridge_connector =
> to_drm_bridge_connector(connector);
> - struct drm_bridge *hpd = bridge_connector->bridge_hpd;
> + struct drm_bridge_connector_dynconn *dynconn =
> + to_drm_bridge_connector_dynconn(connector);
> + struct drm_bridge *hpd = dynconn->bridge_hpd;
>
> if (hpd)
> drm_bridge_hpd_enable(hpd, drm_bridge_connector_hpd_cb,
> bridge_connector);
> }
Does registering bridge_connector as the callback data here cause hotplug
detection events to be misrouted?
When an event fires, the handler resolves the target via
&bridge_connector->dynconn->connector. If multiple dynamic connectors are
created concurrently, bridge_connector->dynconn will point to the most
recently created one, potentially corrupting the state of a newer connector.
Additionally, if bridge_connector->dynconn has been set to NULL (such as
after a hot-unplug), could this path trigger a NULL pointer dereference when
the callback accesses connector->status?
[ ... ]
> @@ -320,7 +325,7 @@ static void drm_bridge_connector_dynconn_destroy(struct drm_connector *connector
> container_of(connector, struct drm_bridge_connector_dynconn, connector);
>
> drm_connector_cleanup(connector);
> - drm_bridge_connector_put_bridges(dynconn->bridge_connector);
> + drm_bridge_connector_put_bridges(dynconn);
> kfree(dynconn);
> }
Since drm_bridge_connector_init_hdmi_audio_cec() registers components using
drmm_connector_hdmi_cec_notifier_register() and
drmm_connector_hdmi_cec_register(), the cleanup actions are deferred and
bound to the global drm_device (connector->dev).
Because the connector is explicitly freed here via kfree(dynconn), will
the delayed drmm_ actions dereference the freed connector pointer when the
drm_device is eventually torn down, leading to a use-after-free regression?
[ ... ]
> @@ -841,26 +845,26 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
> * Ensure the last bridge declares OP_EDID or OP_MODES or both.
> */
> if (bridge->ops & DRM_BRIDGE_OP_EDID || bridge->ops & DRM_BRIDGE_OP_MODES) {
> - drm_bridge_put(bridge_connector->bridge_edid);
> - bridge_connector->bridge_edid = NULL;
> - drm_bridge_put(bridge_connector->bridge_modes);
> - bridge_connector->bridge_modes = NULL;
> + drm_bridge_put(dynconn->bridge_edid);
> + dynconn->bridge_edid = NULL;
> + drm_bridge_put(dynconn->bridge_modes);
> + dynconn->bridge_modes = NULL;
>
> if (bridge->ops & DRM_BRIDGE_OP_EDID)
> - bridge_connector->bridge_edid = drm_bridge_get(bridge);
> + dynconn->bridge_edid = drm_bridge_get(bridge);
When this function populates these bridge references in dynconn, if it later
encounters an error (like returning -EINVAL for missing callbacks), the
caller drm_bridge_connector_add_connector() will free dynconn without calling
drm_bridge_connector_put_bridges().
Could this regression result in leaking these newly acquired bridge references
on the error path?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=16
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 17/37] drm/display: bridge-connector: protect dynconn creation and destruction with a mutex
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (15 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 16/37] drm/display: bridge-connector: move per-connector fields to the dynamic connector Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:18 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 18/37] drm/bridge: samsung-dsim: remove the panel_bridge on host_detach Luca Ceresoli
` (19 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
Bridge hotplug will make the connector dynamically created and destroyed
based on hotplug events.
In preparation for that, add a mutex to mutually exclude connector creation
and destruction.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index bd421b6beb7c..b83da529cc7a 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -8,6 +8,7 @@
#include <linux/export.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/property.h>
#include <linux/slab.h>
@@ -152,6 +153,11 @@ struct drm_bridge_connector {
* @dynconn: The DRM connector added by the drm_bridge_connector
*/
struct drm_bridge_connector_dynconn *dynconn;
+ /**
+ * @dynconn_mutex: Protect @dynconn from concurrent creation and
+ * destruction
+ */
+ struct mutex dynconn_mutex;
};
static struct drm_bridge_connector_dynconn *
@@ -810,6 +816,8 @@ static const struct drm_connector_hdmi_cec_funcs drm_bridge_connector_hdmi_cec_f
static void drm_bridge_connector_dynconn_release(struct drm_bridge_connector *bridge_connector)
{
+ guard(mutex)(&bridge_connector->dynconn_mutex);
+
if (!bridge_connector->dynconn)
return;
@@ -1050,6 +1058,8 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
int connector_type;
int ret;
+ guard(mutex)(&bridge_connector->dynconn_mutex);
+
struct drm_bridge_connector_dynconn *dynconn = kzalloc_obj(*dynconn);
if (!dynconn)
return -ENOMEM;
@@ -1177,6 +1187,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
if (!bridge_connector)
return ERR_PTR(-ENOMEM);
+ mutex_init(&bridge_connector->dynconn_mutex);
bridge_connector->drm = drm;
bridge_connector->encoder = encoder;
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 17/37] drm/display: bridge-connector: protect dynconn creation and destruction with a mutex
2026-05-19 10:37 ` [PATCH 17/37] drm/display: bridge-connector: protect dynconn creation and destruction with a mutex Luca Ceresoli
@ 2026-05-19 11:18 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:18 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing state check inside the locked section of drm_bridge_connector_add_connector() allows overwriting an existing connector
- [High] Unprotected access to `dynconn` in drm_bridge_connector_handle_hpd() leads to NULL pointer dereference
- [High] Use-After-Free caused by mixing `drmm` cleanup actions with dynamically freed connectors
- [Medium] Mixing goto-based error handling with scope-based guard() cleanup in drm_bridge_connector_add_connector()
- [Medium] Missing mutex_destroy() or drmm_mutex_init() for `dynconn_mutex`
--
commit 83c9dccacbc0bf010c7c667632a3090398d3e6ed
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/display: bridge-connector: protect dynconn creation and destruction with a mutex
This commit introduces a mutex to mutually exclude connector creation and
destruction based on bridge hotplug events. It adds dynconn_mutex to
the drm_bridge_connector structure and uses guard() to protect the
relevant sections.
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index bd421b6beb7c4..b83da529cc7af 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -152,6 +153,11 @@ struct drm_bridge_connector {
> * @dynconn: The DRM connector added by the drm_bridge_connector
> */
> struct drm_bridge_connector_dynconn *dynconn;
> + /**
> + * @dynconn_mutex: Protect @dynconn from concurrent creation and
> + * destruction
> + */
> + struct mutex dynconn_mutex;
> };
Does drm_bridge_connector_handle_hpd() need to use this new mutex?
Since the connector lifecycle is now dynamic, if an asynchronous HPD event
fires concurrently with a bridge detachment (which calls
drm_bridge_connector_dynconn_release() and sets dynconn to NULL under the
mutex), could drm_bridge_connector_handle_hpd() fetch the connector without
the lock and trigger a NULL pointer dereference or use-after-free?
Could the dynamic lifecycle of the connector also cause a use-after-free
with drmm cleanup actions?
drm_bridge_connector_add_connector() calls
drm_bridge_connector_init_hdmi_audio_cec(), which uses
drmm_connector_hdmi_cec_register() to register a cleanup action tied to the
drm_device's lifetime.
Because the connector is now dynamically managed and freed via kfree when a
bridge is detached, won't the drmm cleanup action hold a stale pointer and
execute on an already-freed connector when the DRM device is unregistered?
[ ... ]
> @@ -1050,6 +1058,8 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
> int connector_type;
> int ret;
>
> + guard(mutex)(&bridge_connector->dynconn_mutex);
> +
Does introducing guard() here violate the cleanup subsystem guidelines?
The kernel cleanup subsystem guidelines forbid mixing goto-based error
handling with scope-based cleanups like guard() in the same function because
it creates confusing ownership semantics.
Later in this function we see:
if (ret)
goto err_put;
return 0;
err_put:
drm_connector_put(connector);
bridge_connector->dynconn = NULL;
return ret;
Could this mix of guard() and goto cause issues?
> struct drm_bridge_connector_dynconn *dynconn = kzalloc_obj(*dynconn);
Could concurrent hotplug events overwrite an existing connector here?
While later patches in the series introduce
drm_bridge_connector_try_complete() which checks
if (bridge_connector->dynconn), that lockless check happens outside of
dynconn_mutex.
If two threads pass that check simultaneously, they will sequentially
acquire dynconn_mutex here. Would the second thread then unconditionally
overwrite bridge_connector->dynconn with a new allocation, leaking the
first connector's memory and leaving a dangling registered connector?
[ ... ]
> @@ -1177,6 +1187,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> if (!bridge_connector)
> return ERR_PTR(-ENOMEM);
>
> + mutex_init(&bridge_connector->dynconn_mutex);
Should this be drmm_mutex_init() instead?
bridge_connector is allocated with drmm_kzalloc(), meaning its memory is
tied to the DRM device's lifecycle. Since the registered cleanup action
drm_bridge_connector_fini() does not call mutex_destroy(), could freeing
the structure leave an active mutex and trigger kernel debug warnings when
CONFIG_DEBUG_MUTEXES is enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=17
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 18/37] drm/bridge: samsung-dsim: remove the panel_bridge on host_detach
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (16 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 17/37] drm/display: bridge-connector: protect dynconn creation and destruction with a mutex Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 19/37] drm/bridge: samsung-dsim: move drm_bridge_add() call to probe Luca Ceresoli
` (18 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In preparation for DRM bridge hot-plugging, we need to handle the dynamic
lifetime of the following bridge in case the samsung-dsim is always present
and the following bridge (next_bridge) is hot-unplugged.
Based on the 'if (!IS_ERR(panel))' check in samsung_dsim_host_attach(), the
next_bridge could be A) a panel bridge created by this driver via
devm_drm_panel_bridge_add() or B) a pre-existing bridge obtained via
of_drm_find_and_get_bridge().
For case B) we need to put that reference when the next_bridge is removed,
which is already handled by calling drm_bridge_clear_and_put() in
samsung_dsim_host_detach() and in the samsung_dsim_host_attach() error
management code.
In case A) we additionally have to remove the panel bridge. Currently it is
created by devm_drm_panel_bridge_add(), which adds two devm actions with
the refcounted panel bridge:
- drm_bridge_put_void() via devm_drm_bridge_alloc() on panel->dev
- devm_drm_panel_bridge_release() via devm_drm_panel_bridge_add_typed()
on the consumer device (samsung-dsim)
The first action is OK: being tied to panel->dev it will happen when the
panel is unplugged.
The second action is bound to the consumer device, so the devm semantics is
not useful here when introducing hotplug. Indeed we need to drop the
next_bridge in samsung_dsim_host_detach() anyway, before the driver .remove
function, in order to support {add, {attach, detach} x N, remove} hotplug
event sequences.
Thus move to the non-devm drm_panel_bridge_add() along with the matching
drm_panel_bridge_remove(), so the lifetime of the panel-bridge is tied to
the host_attach/host_detach cycle and not the whole samsung-dsim device
lifetime.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
In a previous discussion with Maxime he mentioned a plan to make every
drm_panel always have a wrapping bridge. With that done, all the code
handling the panel and adding the panel_bridge would become useless here
(and in many other places) and could be entirely removed. This patch is a
temporary solution until that happens. The best pointer I could find to
that discussion is [0], but there might be more recent material I could not
find at the moment.
[0] https://lore.kernel.org/lkml/20250218-faithful-white-magpie-da9ac9@houat/
---
drivers/gpu/drm/bridge/samsung-dsim.c | 17 ++++++++++++++---
include/drm/bridge/samsung-dsim.h | 2 ++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 5b799619e07e..2af287221e22 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1951,14 +1951,16 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
if (!remote)
return -ENODEV;
+ dsi->panel_bridge_added = false;
panel = of_drm_find_panel(remote);
if (!IS_ERR(panel)) {
- next_bridge = devm_drm_panel_bridge_add(dev, panel);
+ next_bridge = drm_panel_bridge_add(panel);
if (IS_ERR(next_bridge)) {
ret = PTR_ERR(next_bridge);
next_bridge = NULL; // Inhibit the cleanup action on an ERR_PTR
} else {
drm_bridge_get(next_bridge);
+ dsi->panel_bridge_added = true;
}
} else {
next_bridge = of_drm_find_and_get_bridge(remote);
@@ -1989,7 +1991,7 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
ret = samsung_dsim_register_te_irq(dsi, &device->dev);
if (ret)
- goto err_remove_bridge;
+ goto err_remove_panel_bridge;
}
// The next bridge can be used by host_ops->attach
@@ -2011,8 +2013,12 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
drm_bridge_clear_and_put(&dsi->bridge.next_bridge);
if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO))
samsung_dsim_unregister_te_irq(dsi);
-err_remove_bridge:
+err_remove_panel_bridge:
drm_bridge_remove(&dsi->bridge);
+ if (dsi->panel_bridge_added) {
+ drm_panel_bridge_remove(next_bridge);
+ dsi->panel_bridge_added = false;
+ }
return ret;
}
@@ -2022,6 +2028,11 @@ static int samsung_dsim_host_detach(struct mipi_dsi_host *host,
struct samsung_dsim *dsi = host_to_dsi(host);
const struct samsung_dsim_plat_data *pdata = dsi->plat_data;
+ if (dsi->panel_bridge_added) {
+ drm_panel_bridge_remove(dsi->bridge.next_bridge);
+ dsi->panel_bridge_added = false;
+ }
+
if (pdata->host_ops && pdata->host_ops->detach)
pdata->host_ops->detach(dsi, device);
diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
index 03005e474704..3620f7aa9394 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -131,6 +131,8 @@ struct samsung_dsim {
const struct samsung_dsim_plat_data *plat_data;
void *priv;
+
+ bool panel_bridge_added; /* true iff next_bridge is a panel_bridge we created */
};
extern int samsung_dsim_probe(struct platform_device *pdev);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 19/37] drm/bridge: samsung-dsim: move drm_bridge_add() call to probe
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (17 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 18/37] drm/bridge: samsung-dsim: remove the panel_bridge on host_detach Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:16 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 20/37] drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available Luca Ceresoli
` (17 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
This bridge driver calls drm_bridge_add() in the DSI host .attach callback
instead of in the probe function.
This works for current use cases but is problematic for supporting hotplug
of DRM bridges. The problematic case is when this DSI host is always
present while its DSI device is hot-pluggable. In such case with the
current code the DRM card will not be populated until after the DSI device
attaches to the host, which could happen a very long time after booting, or
even not happen at all.
The reason is that the previous pipeline component (the encoder in this
case) when probing cannot find the samsung-dsim bridge. What happens is:
[1 and 2 can happen in any order, same result]
1) samsung-dsim probes (does not drm_bridge_add() itself)
2) The lcdif starts probing multiple times, but
lcdif_probe
-> lcdif_load
-> lcdif_attach_bridge
-> devm_drm_of_get_bridge() returns -EPROBE_DEFER because
the samsung-dsim is not in the global bridge_list
(deferred probe pending: imx-lcdif: Cannot connect bridge)
The samsung-dsim will not drm_bridge_add() itself until a DSI device will
try to mipi_dsi_attach() to the DSI Host, which can happen arbitratily late
on hot-pluggable hardware.
As a preliminary step to supporting hotplug move drm_bridge_add() at probe
time, so that the samsung-dsim DSI host bridge is available during boot,
even without a connected DSI device. This results in:
1) samsung-dsim probes (and adds to drm_bridge_add() itself)
2) The lcdif starts probing multiple times, but
lcdif_probe
-> lcdif_load
-> lcdif_attach_bridge
-> devm_drm_of_get_bridge() --> OK, returns samsung-dsim ptr
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 2af287221e22..cefb20ec68ad 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1980,8 +1980,6 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
mipi_dsi_pixel_format_to_bpp(device->format),
device->mode_flags);
- drm_bridge_add(&dsi->bridge);
-
/*
* This is a temporary solution and should be made by more generic way.
*
@@ -2014,7 +2012,6 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO))
samsung_dsim_unregister_te_irq(dsi);
err_remove_panel_bridge:
- drm_bridge_remove(&dsi->bridge);
if (dsi->panel_bridge_added) {
drm_panel_bridge_remove(next_bridge);
dsi->panel_bridge_added = false;
@@ -2040,8 +2037,6 @@ static int samsung_dsim_host_detach(struct mipi_dsi_host *host,
samsung_dsim_unregister_te_irq(dsi);
- drm_bridge_remove(&dsi->bridge);
-
return 0;
}
@@ -2242,6 +2237,8 @@ int samsung_dsim_probe(struct platform_device *pdev)
goto err_disable_runtime;
}
+ drm_bridge_add(&dsi->bridge);
+
return 0;
err_disable_runtime:
@@ -2255,6 +2252,8 @@ void samsung_dsim_remove(struct platform_device *pdev)
{
struct samsung_dsim *dsi = platform_get_drvdata(pdev);
+ drm_bridge_remove(&dsi->bridge);
+
pm_runtime_disable(&pdev->dev);
if (dsi->plat_data->host_ops && dsi->plat_data->host_ops->unregister_host)
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 19/37] drm/bridge: samsung-dsim: move drm_bridge_add() call to probe
2026-05-19 10:37 ` [PATCH 19/37] drm/bridge: samsung-dsim: move drm_bridge_add() call to probe Luca Ceresoli
@ 2026-05-19 11:16 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:16 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Moving drm_bridge_add() to probe() breaks probe deferral, causing a permanent -EINVAL failure.
- [Medium] Lockless data race on bridge state and next_bridge pointer when the panel attaches asynchronously.
--
commit 8b9a88707049c09bf2d0cddc2aeab67e8c2ea0b4
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/bridge: samsung-dsim: move drm_bridge_add() call to probe
This commit moves the drm_bridge_add() call from the DSI host attach
callback to the probe function to allow the DRM card to populate even
without a connected DSI device.
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
[ ... ]
> @@ -2222,6 +2217,8 @@ int samsung_dsim_probe(struct platform_device *pdev)
> goto err_disable_runtime;
> }
>
> + drm_bridge_add(&dsi->bridge);
> +
> return 0;
Does moving drm_bridge_add() to the probe function break probe deferral?
By registering the DRM bridge here, the bridge becomes immediately available
to the DRM master (like imx-lcdif) before the downstream DSI panel has probed.
When the DRM master probes, it finds the bridge and calls drm_bridge_attach().
This routes to samsung_dsim_attach(), which unconditionally attempts to attach
dsi->bridge.next_bridge.
Because the panel hasn't attached yet, next_bridge is NULL. Passing a NULL
bridge to drm_bridge_attach() returns -EINVAL. This seems to cause the
display controller's probe to fail permanently instead of properly deferring
via -EPROBE_DEFER.
I noticed this is resolved later in the series by commit b9977e0ebaa4d
(drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet
available), but it introduces a regression at this point in the tree.
Also, does this introduce a data race on the bridge state when the panel
attaches asynchronously?
Since the bridge is now exposed to the global bridge_list early, the DRM core
or master could access it while the downstream panel is still probing. The
dsi->bridge.next_bridge pointer and configurations like lanes, format, and
mode_flags are updated later in samsung_dsim_host_attach() without acquiring
any locks.
Is it possible for atomic callbacks or the DRM master's probe to read a
partially-updated next_bridge pointer or uninitialized configuration state
during this window?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=19
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 20/37] drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (18 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 19/37] drm/bridge: samsung-dsim: move drm_bridge_add() call to probe Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:13 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 21/37] drm/bridge: initialize chain_node list head on allocation Luca Ceresoli
` (16 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
When samsung_dsim_attach() is called but the next_bridge is not (yet)
known, it calls drm_bridge_attach() unconditionally. In turn
drm_bridge_attach() will return -EINVAL because the bridge pointer in
NULL. -EINVAL is propagated transparently to the caller, resulting in the
whole attach operation to fail.
This is fine for current use cases, but not when introducing for bridge
hotplug, which implies the next bridge could be hot-plugged later on.
In preparation to support bridge hotplug, consider the absence of a
next_bridge an -EPROBE_DEFER, not a hard error.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index cefb20ec68ad..3c70a45c5dce 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1828,6 +1828,9 @@ static int samsung_dsim_attach(struct drm_bridge *bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+ if (!dsi->bridge.next_bridge)
+ return -EPROBE_DEFER;
+
return drm_bridge_attach(encoder, dsi->bridge.next_bridge, bridge,
flags);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 20/37] drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available
2026-05-19 10:37 ` [PATCH 20/37] drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available Luca Ceresoli
@ 2026-05-19 11:13 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:13 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double fetch of `dsi->bridge.next_bridge` in `samsung_dsim_attach()` causes a race condition leading to unintended `-EINVAL` errors or Use-After-Free.
--
commit b9977e0ebaa4dd776acdd10e8085cc7df8ae862c
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available
When samsung_dsim_attach() is called without a known next_bridge, it previously
returned -EINVAL. This commit changes it to return -EPROBE_DEFER instead, in
preparation for bridge hotplug support.
> drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available
This isn't a bug, but there is a typo in the subject line ("is" instead of
"if").
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 555e343b386d0..83c74d57d0d44 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1828,6 +1828,9 @@ static int samsung_dsim_attach(struct drm_bridge *bridge,
> {
> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>
> + if (!dsi->bridge.next_bridge)
> + return -EPROBE_DEFER;
> +
> return drm_bridge_attach(encoder, dsi->bridge.next_bridge, bridge,
> flags);
Can this double fetch of dsi->bridge.next_bridge cause a race condition
leading to unintended -EINVAL errors or a use-after-free?
The commit message states this change is in preparation for bridge hotplug,
meaning hot-unplug events could occur concurrently.
If a detach executes concurrently during an unplug event, it could clear
dsi->bridge.next_bridge between the if check and the call to
drm_bridge_attach(). This would cause the first fetch to see a valid pointer,
but the second fetch to read NULL, resulting in drm_bridge_attach() returning
-EINVAL instead of -EPROBE_DEFER.
Furthermore, even if the value is fetched into a local variable to prevent the
double fetch, could the bridge be freed by the concurrent detach before
drm_bridge_attach() takes its own reference?
Does this require proper synchronization, such as holding a lock while
checking and obtaining a reference to the bridge?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=20
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 21/37] drm/bridge: initialize chain_node list head on allocation
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (19 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 20/37] drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 22/37] drm/bridge: initialize chain_node list head on detach and attach errors Luca Ceresoli
` (15 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In preparation to add a check to detect whether a bridge is not yet
attached, ensure the chain_node list_head is always empty [as in
list_empty()] since it is allocated, until it is attached.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/drm_bridge.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 687b36eea0c7..657f6609d7b1 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -378,6 +378,7 @@ void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
return ERR_PTR(-ENOMEM);
bridge = container + offset;
+ INIT_LIST_HEAD(&bridge->chain_node);
INIT_LIST_HEAD(&bridge->list);
bridge->container = container;
bridge->funcs = funcs;
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 22/37] drm/bridge: initialize chain_node list head on detach and attach errors
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (20 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 21/37] drm/bridge: initialize chain_node list head on allocation Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:17 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 23/37] drm/encoder: add drm_encoder_cleanup_from() Luca Ceresoli
` (14 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
When a bridge is detached it is removed from the encoder bridge_chain list,
but the bridge::chain_node list head is not cleared. This is going to be
problematic with the upcoming hotplug bridge support because if a bridge is
detached from the encoder chain but not yet removed, when later detaching
it the encoder code may think it is still attached, thus trying to detach
it twice.
Avoid this by clearing the list head on detach, so there's a clear and
simple way to know when a bridge is not attached anymore.
Do the same in the error management code in drm_bridge_attach(), so that
chain_node is always empty [as in list_empty()] when it is not
(yet|anymore) in the bridge chain.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/drm_bridge.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 657f6609d7b1..7f2d1a81d730 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -598,7 +598,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
bridge->dev = NULL;
bridge->encoder = NULL;
mutex_lock(&encoder->bridge_chain_mutex);
- list_del(&bridge->chain_node);
+ list_del_init(&bridge->chain_node);
mutex_unlock(&encoder->bridge_chain_mutex);
if (ret != -EPROBE_DEFER)
@@ -629,7 +629,7 @@ void drm_bridge_detach(struct drm_bridge *bridge)
if (bridge->funcs->detach)
bridge->funcs->detach(bridge);
- list_del(&bridge->chain_node);
+ list_del_init(&bridge->chain_node);
bridge->dev = NULL;
drm_bridge_put(bridge);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 22/37] drm/bridge: initialize chain_node list head on detach and attach errors
2026-05-19 10:37 ` [PATCH 22/37] drm/bridge: initialize chain_node list head on detach and attach errors Luca Ceresoli
@ 2026-05-19 11:17 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:17 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Data race and NULL pointer dereference in drm_bridge_attach() error path during dynamic hotplug
--
commit b83693320d718d054d9327b487d4044213901c1e
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/bridge: initialize chain_node list head on detach and attach errors
This commit updates drm_bridge_detach() and the error path of
drm_bridge_attach() to use list_del_init() instead of list_del(). This
ensures the bridge chain_node list head is clearly empty when detached,
preventing double-detach issues during hotplug operations.
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index f179e00745b7c..5b89168411aa0 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -598,7 +598,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> bridge->dev = NULL;
> bridge->encoder = NULL;
> mutex_lock(&encoder->bridge_chain_mutex);
> - list_del(&bridge->chain_node);
> + list_del_init(&bridge->chain_node);
> mutex_unlock(&encoder->bridge_chain_mutex);
This is an existing issue not introduced by this patch, but since dynamic
bridge hotplugging support is being introduced, can this sequence cause a NULL
pointer dereference?
In this error path, bridge->dev and bridge->encoder are cleared to NULL
outside the encoder->bridge_chain_mutex critical section, while the bridge
is still linked into the encoder->bridge_chain.
If drm_bridge_attach() is executed at runtime and fails, a concurrent
userspace atomic modeset iterating over encoder->bridge_chain could observe
the bridge with bridge->encoder == NULL. Drivers typically use this to
fetch the connector:
connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
If bridge->encoder is NULL, this returns a NULL connector, which could lead
to a crash when calling drm_connector_index(connector).
Should the field assignments be moved inside the mutex-protected block, or
after the bridge is removed from the list?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=22
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 23/37] drm/encoder: add drm_encoder_cleanup_from()
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (21 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 22/37] drm/bridge: initialize chain_node list head on detach and attach errors Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:14 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 24/37] drm/atomic: move drm_atomic_helper_disable_all() and drm_atomic_helper_shutdown() from drm_atomic_helper to drm_atomic Luca Ceresoli
` (13 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
Supporting hardware whose final part of the DRM pipeline can be physically
removed requires the ability to detach all bridges from a given point to
the end of the pipeline.
Introduce a variant of drm_encoder_cleanup() for this.
Take care to not try detaching non-attached bridges. This is needed because
when two or more bridges are removed not in the backwards order,
drm_encoder_cleanup_from() is called more than once for bridges closer to
the panel.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Note: in theory drm_encoder_cleanup() is now a superset of
drm_encoder_cleanup_from() and may be simplified to just call
drm_encoder_cleanup_from() and then do the extra actions. However the
common code is subtly different in terms of locking and checks, so this
would complicate the code in this patch and has thus been kept separate for
the time being to make reviewing sompler. Reimplementing
drm_encoder_cleanup() by using drm_encoder_cleanup_from() cvacn be done
later on.
A much simpler and now obsolete version of this patch (missing locking and
checks) previously appeared in
https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-13-9d6f2c9c3058@bootlin.com/
---
drivers/gpu/drm/drm_encoder.c | 38 ++++++++++++++++++++++++++++++++++++++
include/drm/drm_encoder.h | 1 +
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 0d5dbed06db4..40ece477b302 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -179,6 +179,44 @@ int drm_encoder_init(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_encoder_init);
+/**
+ * drm_encoder_cleanup_from - remove a given bridge and all the following
+ * @encoder: encoder whole list of bridges shall be pruned
+ * @bridge: first bridge to remove
+ *
+ * Removes from an encoder all the bridges starting with a given bridge
+ * and until the end of the chain.
+ *
+ * Does nothing if the bridge is not attached to an encoder chain.
+ *
+ * This should not be used in "normal" DRM pipelines. It is only useful for
+ * devices whose final part of the DRM chain can be physically removed and
+ * later reconnected (possibly with different hardware).
+ */
+void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge *bridge)
+{
+ struct drm_bridge *next;
+ LIST_HEAD(tmplist);
+
+ /*
+ * We need the bridge_chain_mutex to modify the chain, but
+ * drm_bridge_detach() will call DRM_MODESET_LOCK_ALL_BEGIN() (in
+ * drm_modeset_lock_fini()), resulting in a possible ABBA circular
+ * deadlock. Avoid it by first moving all the bridges to a
+ * temporary list holding the lock, and then calling
+ * drm_bridge_detach() without the lock.
+ */
+ mutex_lock(&encoder->bridge_chain_mutex);
+ if (!list_empty(&bridge->chain_node))
+ list_for_each_entry_safe_from(bridge, next, &encoder->bridge_chain, chain_node)
+ list_move_tail(&bridge->chain_node, &tmplist);
+ mutex_unlock(&encoder->bridge_chain_mutex);
+
+ while (!list_empty(&tmplist))
+ drm_bridge_detach(list_first_entry(&tmplist, struct drm_bridge, chain_node));
+}
+EXPORT_SYMBOL(drm_encoder_cleanup_from);
+
/**
* drm_encoder_cleanup - cleans up an initialised encoder
* @encoder: encoder to cleanup
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index eded7c34481a..d2a59f95692f 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -324,6 +324,7 @@ static inline struct drm_encoder *drm_encoder_find(struct drm_device *dev,
}
void drm_encoder_cleanup(struct drm_encoder *encoder);
+void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge *bridge);
/**
* drm_for_each_encoder_mask - iterate over encoders specified by bitmask
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 23/37] drm/encoder: add drm_encoder_cleanup_from()
2026-05-19 10:37 ` [PATCH 23/37] drm/encoder: add drm_encoder_cleanup_from() Luca Ceresoli
@ 2026-05-19 11:14 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:14 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Infinite loop (kernel lockup) if `drm_bridge_detach()` returns early.
- [High] Unsafe list membership check leads to out-of-bounds traversal and memory corruption.
- [High] Moving bridges to a stack list breaks bridge chain iterators during detach callbacks.
--
commit 36261e8ecb547e8946c95a0aed4ce37428b2d56e
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/encoder: add drm_encoder_cleanup_from()
Introduce a variant of drm_encoder_cleanup() to detach all bridges from
a given point to the end of the pipeline.
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
[ ... ]
> +void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge *bridge)
> +{
> + struct drm_bridge *next;
> + LIST_HEAD(tmplist);
> +
> + /*
> + * We need the bridge_chain_mutex to modify the chain, but
[ ... ]
> + */
> + mutex_lock(&encoder->bridge_chain_mutex);
> + if (!list_empty(&bridge->chain_node))
> + list_for_each_entry_safe_from(bridge, next, &encoder->bridge_chain, chain_node)
> + list_move_tail(&bridge->chain_node, &tmplist);
> + mutex_unlock(&encoder->bridge_chain_mutex);
Could this list_empty() check lead to an out-of-bounds traversal?
The check confirms the bridge is on a list, but not necessarily the
provided encoder's chain. If a caller mistakenly passes a bridge from a
different encoder, or if a concurrent caller processes a bridge already moved
to another thread's tmplist, list_for_each_entry_safe_from() will iterate
over that unrelated list.
Since the loop termination checks against &encoder->bridge_chain, it will
never find the list head, running past the end of the foreign list and
corrupting memory.
> +
> + while (!list_empty(&tmplist))
> + drm_bridge_detach(list_first_entry(&tmplist, struct drm_bridge, chain_node));
Can this loop become infinite if drm_bridge_detach() returns early?
Looking at drm_bridge_detach() in drm_bridge.c:
void drm_bridge_detach(struct drm_bridge *bridge)
{
if (WARN_ON(!bridge))
return;
if (WARN_ON(!bridge->dev))
return;
...
list_del_init(&bridge->chain_node);
...
If an early return triggers before list_del_init(), the bridge remains in
tmplist. The list never becomes empty, and the loop will process the exact
same failing bridge indefinitely. Should the bridge be unlinked from the
temporary list before calling drm_bridge_detach()?
Also, does moving bridges to a stack-allocated tmplist break bridge chain
iterators during the detach callbacks?
drm_bridge_detach() executes bridge->funcs->detach(bridge). If a driver
calls a chain traversal API like drm_bridge_chain_get_next_bridge() during
this detach callback, the macro checks:
list_is_last(&bridge->chain_node, &bridge->encoder->bridge_chain)
Because the bridge is currently on tmplist (a stack variable), this boundary
check fails. This causes the iterator to read past the list end, returning
the stack tmplist head cast as a struct drm_bridge *, leading to memory
corruption.
> +}
> +EXPORT_SYMBOL(drm_encoder_cleanup_from);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=23
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 24/37] drm/atomic: move drm_atomic_helper_disable_all() and drm_atomic_helper_shutdown() from drm_atomic_helper to drm_atomic
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (22 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 23/37] drm/encoder: add drm_encoder_cleanup_from() Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:57 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 25/37] drm/bridge: shutdown and cleanup on bridge unplug Luca Ceresoli
` (12 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
To support the upcoming DRM bridge hotplug, the drm_bridge.c code will need
to call drm_atomic_helper_shutdown() to disable the pipeline when the tail
bridges of the pipeline are removed without tearing down the card.
However this would create a module dependency loop between the drm module
(where drm_encoder is) and the drm_kms_helper module where
drm_atomic_helper_shutdown() function currently is and which already
depends on the drm module.
Solve by moving drm_atomic_helper_shutdown(), along with its callee
drm_atomic_helper_disable_all(), to drm_atomic which is in the drm
module. Use identical names except for dropping the "_helper" infix, and
make the original functions a deprecated wrapper to the new ones.
No changes to the functions body.
No functional changes except for moving the code to a different module.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/drm_atomic.c | 115 ++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_atomic_helper.c | 76 ++----------------------
include/drm/drm_atomic.h | 3 +
3 files changed, 124 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 9b2009262c97..7d12c062c16d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1993,6 +1993,121 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
}
EXPORT_SYMBOL(__drm_atomic_helper_set_config);
+/**
+ * drm_atomic_disable_all - disable all currently active outputs
+ * @dev: DRM device
+ * @ctx: lock acquisition context
+ *
+ * Loops through all connectors, finding those that aren't turned off and then
+ * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
+ * that they are connected to.
+ *
+ * This is used for example in suspend/resume to disable all currently active
+ * functions when suspending. If you just want to shut down everything at e.g.
+ * driver unload, look at drm_atomic_helper_shutdown().
+ *
+ * Note that if callers haven't already acquired all modeset locks this might
+ * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ *
+ * See also:
+ * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and
+ * drm_atomic_helper_shutdown().
+ */
+int drm_atomic_disable_all(struct drm_device *dev,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_atomic_commit *state;
+ struct drm_connector_state *conn_state;
+ struct drm_connector *conn;
+ struct drm_plane_state *plane_state;
+ struct drm_plane *plane;
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ int ret, i;
+
+ state = drm_atomic_commit_alloc(dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = ctx;
+
+ drm_for_each_crtc(crtc, dev) {
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto free;
+ }
+
+ crtc_state->active = false;
+
+ ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL);
+ if (ret < 0)
+ goto free;
+
+ ret = drm_atomic_add_affected_planes(state, crtc);
+ if (ret < 0)
+ goto free;
+
+ ret = drm_atomic_add_affected_connectors(state, crtc);
+ if (ret < 0)
+ goto free;
+ }
+
+ for_each_new_connector_in_state(state, conn, conn_state, i) {
+ ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
+ if (ret < 0)
+ goto free;
+ }
+
+ for_each_new_plane_in_state(state, plane, plane_state, i) {
+ ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
+ if (ret < 0)
+ goto free;
+
+ drm_atomic_set_fb_for_plane(plane_state, NULL);
+ }
+
+ ret = drm_atomic_commit(state);
+free:
+ drm_atomic_commit_put(state);
+ return ret;
+}
+EXPORT_SYMBOL(drm_atomic_disable_all);
+
+/**
+ * drm_atomic_shutdown - shutdown all CRTC
+ * @dev: DRM device
+ *
+ * This shuts down all CRTC, which is useful for driver unloading. Shutdown on
+ * suspend should instead be handled with drm_atomic_helper_suspend(), since
+ * that also takes a snapshot of the modeset state to be restored on resume.
+ *
+ * This is just a convenience wrapper around drm_atomic_helper_disable_all(),
+ * and it is the atomic version of drm_helper_force_disable_all().
+ */
+void drm_atomic_shutdown(struct drm_device *dev)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
+
+ if (dev == NULL)
+ return;
+
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+
+ ret = drm_atomic_disable_all(dev, &ctx);
+ if (ret)
+ drm_err(dev,
+ "Disabling all crtc's during unload failed with %i\n",
+ ret);
+
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+}
+EXPORT_SYMBOL(drm_atomic_shutdown);
+
static void drm_atomic_private_obj_print_state(struct drm_printer *p,
const struct drm_private_state *state)
{
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 51f39edc31ed..afedfab9b568 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3533,6 +3533,8 @@ EXPORT_SYMBOL(drm_atomic_helper_set_config);
* @dev: DRM device
* @ctx: lock acquisition context
*
+ * Deprecated wrapper to drm_atomic_disable_all().
+ *
* Loops through all connectors, finding those that aren't turned off and then
* turns them off by setting their DPMS mode to OFF and deactivating the CRTC
* that they are connected to.
@@ -3554,61 +3556,7 @@ EXPORT_SYMBOL(drm_atomic_helper_set_config);
int drm_atomic_helper_disable_all(struct drm_device *dev,
struct drm_modeset_acquire_ctx *ctx)
{
- struct drm_atomic_commit *state;
- struct drm_connector_state *conn_state;
- struct drm_connector *conn;
- struct drm_plane_state *plane_state;
- struct drm_plane *plane;
- struct drm_crtc_state *crtc_state;
- struct drm_crtc *crtc;
- int ret, i;
-
- state = drm_atomic_commit_alloc(dev);
- if (!state)
- return -ENOMEM;
-
- state->acquire_ctx = ctx;
-
- drm_for_each_crtc(crtc, dev) {
- crtc_state = drm_atomic_get_crtc_state(state, crtc);
- if (IS_ERR(crtc_state)) {
- ret = PTR_ERR(crtc_state);
- goto free;
- }
-
- crtc_state->active = false;
-
- ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL);
- if (ret < 0)
- goto free;
-
- ret = drm_atomic_add_affected_planes(state, crtc);
- if (ret < 0)
- goto free;
-
- ret = drm_atomic_add_affected_connectors(state, crtc);
- if (ret < 0)
- goto free;
- }
-
- for_each_new_connector_in_state(state, conn, conn_state, i) {
- ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
- if (ret < 0)
- goto free;
- }
-
- for_each_new_plane_in_state(state, plane, plane_state, i) {
- ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
- if (ret < 0)
- goto free;
-
- drm_atomic_set_fb_for_plane(plane_state, NULL);
- }
-
- ret = drm_atomic_commit(state);
-free:
- drm_atomic_commit_put(state);
- return ret;
+ return drm_atomic_disable_all(dev, ctx);
}
EXPORT_SYMBOL(drm_atomic_helper_disable_all);
@@ -3664,6 +3612,8 @@ EXPORT_SYMBOL(drm_atomic_helper_reset_crtc);
* drm_atomic_helper_shutdown - shutdown all CRTC
* @dev: DRM device
*
+ * Deprecated wrapper to drm_atomic_shutdown().
+ *
* This shuts down all CRTC, which is useful for driver unloading. Shutdown on
* suspend should instead be handled with drm_atomic_helper_suspend(), since
* that also takes a snapshot of the modeset state to be restored on resume.
@@ -3673,21 +3623,7 @@ EXPORT_SYMBOL(drm_atomic_helper_reset_crtc);
*/
void drm_atomic_helper_shutdown(struct drm_device *dev)
{
- struct drm_modeset_acquire_ctx ctx;
- int ret;
-
- if (dev == NULL)
- return;
-
- DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
-
- ret = drm_atomic_helper_disable_all(dev, &ctx);
- if (ret)
- drm_err(dev,
- "Disabling all crtc's during unload failed with %i\n",
- ret);
-
- DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+ return drm_atomic_shutdown(dev);
}
EXPORT_SYMBOL(drm_atomic_helper_shutdown);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 1a80a8cdf269..9eb3d1bfa084 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -1373,5 +1373,8 @@ drm_atomic_get_old_bridge_state(const struct drm_atomic_commit *state,
struct drm_bridge_state *
drm_atomic_get_new_bridge_state(const struct drm_atomic_commit *state,
struct drm_bridge *bridge);
+int drm_atomic_disable_all(struct drm_device *dev,
+ struct drm_modeset_acquire_ctx *ctx);
+void drm_atomic_shutdown(struct drm_device *dev);
#endif /* DRM_ATOMIC_H_ */
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 24/37] drm/atomic: move drm_atomic_helper_disable_all() and drm_atomic_helper_shutdown() from drm_atomic_helper to drm_atomic
2026-05-19 10:37 ` [PATCH 24/37] drm/atomic: move drm_atomic_helper_disable_all() and drm_atomic_helper_shutdown() from drm_atomic_helper to drm_atomic Luca Ceresoli
@ 2026-05-19 10:57 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 10:57 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Outdated function references in kernel-doc blocks after rename.
- [Low] Use of return with an expression in a void function.
--
commit 8c33c13c9d809fde9c0dc4ffcc59c09b05d47d10
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/atomic: move drm_atomic_helper_disable_all() and drm_atomic_helper_shutdown() from drm_atomic_helper to drm_atomic
This commit moves drm_atomic_helper_shutdown() and
drm_atomic_helper_disable_all() to the drm_atomic module, renaming them to
drop the "_helper" infix. The move resolves a module dependency loop to
support upcoming DRM bridge hotplug features, replacing the original
functions with deprecated wrappers.
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 170de30c28ae4..36fc57e147cb5 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1987,6 +1987,121 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
[ ... ]
> + * This is used for example in suspend/resume to disable all currently active
> + * functions when suspending. If you just want to shut down everything at e.g.
> + * driver unload, look at drm_atomic_helper_shutdown().
This isn't a bug, but since the functions were renamed, should this
kernel-doc comment be updated to point to the new drm_atomic_shutdown()
function instead of the deprecated drm_atomic_helper_shutdown() wrapper?
[ ... ]
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and
> + * drm_atomic_helper_shutdown().
> + */
> +int drm_atomic_disable_all(struct drm_device *dev,
Similar to the above, should this refer to drm_atomic_shutdown()?
[ ... ]
> +/**
> + * drm_atomic_shutdown - shutdown all CRTC
> + * @dev: DRM device
> + *
> + * This shuts down all CRTC, which is useful for driver unloading. Shutdown on
> + * suspend should instead be handled with drm_atomic_helper_suspend(), since
> + * that also takes a snapshot of the modeset state to be restored on resume.
> + *
> + * This is just a convenience wrapper around drm_atomic_helper_disable_all(),
> + * and it is the atomic version of drm_helper_force_disable_all().
> + */
> +void drm_atomic_shutdown(struct drm_device *dev)
Should this documentation state that it is a wrapper around the new
drm_atomic_disable_all() rather than the deprecated
drm_atomic_helper_disable_all()?
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 51f39edc31edc..afedfab9b5682 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
[ ... ]
> @@ -3673,21 +3623,7 @@ EXPORT_SYMBOL(drm_atomic_helper_reset_crtc);
> */
> void drm_atomic_helper_shutdown(struct drm_device *dev)
> {
> - struct drm_modeset_acquire_ctx ctx;
> - int ret;
> -
> - if (dev == NULL)
> - return;
> -
> - DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> -
> - ret = drm_atomic_helper_disable_all(dev, &ctx);
> - if (ret)
> - drm_err(dev,
> - "Disabling all crtc's during unload failed with %i\n",
> - ret);
> -
> - DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> + return drm_atomic_shutdown(dev);
> }
Since both drm_atomic_helper_shutdown() and drm_atomic_shutdown() are declared
as returning void, does returning an expression here violate ISO C standards?
It might be better to just call drm_atomic_shutdown(dev) without the return
keyword to avoid compiler warnings from stricter configurations like sparse or
Clang.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=24
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 25/37] drm/bridge: shutdown and cleanup on bridge unplug
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (23 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 24/37] drm/atomic: move drm_atomic_helper_disable_all() and drm_atomic_helper_shutdown() from drm_atomic_helper to drm_atomic Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:09 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 26/37] drm: event-notifier: add mechanism to notify about hotplug events Luca Ceresoli
` (11 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
With the upcoming support for DRM bridge hot(un)plugging, bridges can be
removed at any time withotu tearing down the entire card. When this
happens, shutdown the pipeline and detach from the encoder chain the bridge
being removed along with all the following ones.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/drm_bridge.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 7f2d1a81d730..d45fb74ec8c2 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -462,9 +462,17 @@ EXPORT_SYMBOL(devm_drm_bridge_add);
* it won't be found by users via of_drm_find_and_get_bridge(), and add it
* to the lingering bridge list, to keep track of it until its allocated
* memory is eventually freed.
+ *
+ * If the bridge is attached, also disable the active output and detach
+ * this bridge and the following ones.
*/
void drm_bridge_remove(struct drm_bridge *bridge)
{
+ if (bridge->encoder) {
+ drm_atomic_shutdown(bridge->dev);
+ drm_encoder_cleanup_from(bridge->encoder, bridge);
+ }
+
mutex_lock(&bridge_lock);
list_move_tail(&bridge->list, &bridge_lingering_list);
mutex_unlock(&bridge_lock);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 25/37] drm/bridge: shutdown and cleanup on bridge unplug
2026-05-19 10:37 ` [PATCH 25/37] drm/bridge: shutdown and cleanup on bridge unplug Luca Ceresoli
@ 2026-05-19 11:09 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:09 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Use-After-Free (UAF) due to dangling `bridge->encoder` pointer
- [Critical] Stack corruption via cross-thread list iteration in `drm_encoder_cleanup_from()`
- [High] Unintended system-wide display shutdown (Logic Error)
--
commit c68fe5f8aeb721d87ce6a0a639bb852359b46771
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/bridge: shutdown and cleanup on bridge unplug
This commit aims to handle DRM bridge hot-unplugging by shutting down the
pipeline and detaching the removed bridge and subsequent ones, rather than
tearing down the entire card.
> With the upcoming support for DRM bridge hot(un)plugging, bridges can be
> removed at any time withotu tearing down the entire card.
The commit message states the patch avoids tearing down the entire card, but
looking at the code, could this implementation still cause a system-wide
display shutdown?
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 5b89168411aa0..e7e17bf4fdf9e 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
[ ... ]
> @@ -462,9 +462,17 @@ EXPORT_SYMBOL(devm_drm_bridge_add);
> void drm_bridge_remove(struct drm_bridge *bridge)
> {
> + if (bridge->encoder) {
> + drm_atomic_shutdown(bridge->dev);
Looking at drm_atomic_shutdown(), it unconditionally calls
drm_atomic_disable_all(dev, &ctx), which disables all CRTCs on the device.
If a single bridge is hot-unplugged, won't this unexpectedly shut down all
other independent, active displays on the same system?
> + drm_encoder_cleanup_from(bridge->encoder, bridge);
Is there a possibility of a use-after-free here if bridge->encoder is left
dangling?
When a DRM device is torn down, drm_encoder_cleanup() iterates over the
chain and calls drm_bridge_detach(). That function clears bridge->dev but
does not clear bridge->encoder, and the encoder memory is subsequently freed.
Later, when the bridge driver is unbound, drm_bridge_remove() is executed.
Since bridge->encoder is never cleared, it will still evaluate to true,
passing a dangling pointer to drm_encoder_cleanup_from().
drm_encoder_cleanup_from() would then attempt to acquire the mutex from the
freed encoder memory:
void drm_encoder_cleanup_from(...) {
...
mutex_lock(&encoder->bridge_chain_mutex);
...
}
Additionally, could calling this function concurrently for bridges on the
same chain cause stack list corruption?
Looking at drm_encoder_cleanup_from(), it uses a stack-allocated tmplist:
void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge *bridge)
{
...
LIST_HEAD(tmplist);
mutex_lock(&encoder->bridge_chain_mutex);
if (!list_empty(&bridge->chain_node))
list_for_each_entry_safe_from(bridge, next, &encoder->bridge_chain, chain_node)
list_move_tail(&bridge->chain_node, &tmplist);
mutex_unlock(&encoder->bridge_chain_mutex);
...
}
If two threads concurrently remove two bridges in the same chain (for example,
Thread 1 removes Bridge A, and Thread 2 removes downstream Bridge B):
Thread 1 moves both bridges to its tmplist and unlocks the mutex.
Thread 2 then locks the mutex and sees !list_empty(&bridge_B->chain_node) is
true because Bridge B is linked within Thread 1's tmplist.
Thread 2 then executes list_for_each_entry_safe_from() looking for the
termination condition &encoder->bridge_chain. Because Bridge B is inside
Thread 1's stack list, it would iterate locklessly into Thread 1's stack
memory.
Would this cause Thread 2 to interpret Thread 1's stack LIST_HEAD as a
drm_bridge and attempt to move it, corrupting both threads' lists and stack
frames?
> + }
> +
> mutex_lock(&bridge_lock);
> list_move_tail(&bridge->list, &bridge_lingering_list);
> mutex_unlock(&bridge_lock);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=25
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 26/37] drm: event-notifier: add mechanism to notify about hotplug events
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (24 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 25/37] drm/bridge: shutdown and cleanup on bridge unplug Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:06 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 27/37] drm/bridge: notify about detached bridges Luca Ceresoli
` (10 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In preparation for supporting DRM bridge hotplug, add an event notifier to
allow interested parties to be notified about events they need to react to.
For the initial implementation of bridge hotplug, two events are needed:
bridge detach (happening in drm_bridge.c) and MIPI device attach to MIPI
host (happening in drm_mipi_dsi.c).
For this reason implement the event notifier in a new common file that
event producers can easily use to send events.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
A different approach I have considered is keeping the event notifier in
drm_bridge.c (as in [0]) instead of a new centralized file. But then
another notifier would be needed in drm_mipi_dsi.c for the DSI attach
event. That would be particularly awkward because the designated component
to implement hotplug is the drm_bridge_connector, which would then need to
depend on DRM_MIPI_DSI even though it does nothing MIPI specific.
[0] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-12-9d6f2c9c3058@bootlin.com/
---
drivers/gpu/drm/Kconfig | 3 ++
drivers/gpu/drm/Makefile | 2 ++
drivers/gpu/drm/drm_event_notifier.c | 58 ++++++++++++++++++++++++++++++++++++
include/drm/drm_event_notifier.h | 36 ++++++++++++++++++++++
4 files changed, 99 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 8f5a8d3012e4..18eb33e0e5a0 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -33,6 +33,9 @@ endmenu
if DRM
+config DRM_EVENT_NOTIFIER
+ tristate
+
config DRM_MIPI_DBI
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd783..18c9cceacdaa 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -96,6 +96,8 @@ drm-$(CONFIG_DRM_PANIC_SCREEN_QR_CODE) += drm_panic_qr.o
drm-$(CONFIG_DRM_RAS) += drm_ras.o drm_ras_nl.o drm_ras_genl_family.o
obj-$(CONFIG_DRM) += drm.o
+obj-$(CONFIG_DRM_EVENT_NOTIFIER) += drm_event_notifier.o
+
obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
obj-$(CONFIG_DRM_PANEL_BACKLIGHT_QUIRKS) += drm_panel_backlight_quirks.o
diff --git a/drivers/gpu/drm/drm_event_notifier.c b/drivers/gpu/drm/drm_event_notifier.c
new file mode 100644
index 000000000000..76af4dd4cdb0
--- /dev/null
+++ b/drivers/gpu/drm/drm_event_notifier.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Internal event notifier for DRM drivers
+ *
+ * Copyright (C) 2026 GE HealthCare
+ * Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
+ */
+
+#include <linux/module.h>
+#include <linux/notifier.h>
+
+#include <drm/drm_event_notifier.h>
+
+static BLOCKING_NOTIFIER_HEAD(drm_event_notifier);
+
+/**
+ * drm_event_notifier_register - Register to be notified of DRM events
+ * @nb: the notifier block to be registered
+ *
+ * @nb will be notified of events defined in &drm_event_notifier_event
+ *
+ * Returns 0 on success, %-EEXIST on error.
+ */
+int drm_event_notifier_register(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&drm_event_notifier, nb);
+}
+EXPORT_SYMBOL(drm_event_notifier_register);
+
+/**
+ * drm_event_notifier_unregister - Unregister from be notified of DRM events
+ * @nb: the notifier block to be unregistered
+ *
+ * @nb will stop being notified of events defined in &drm_event_notifier_event
+ *
+ * Returns zero on success or %-ENOENT on failure.
+ */
+int drm_event_notifier_unregister(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&drm_event_notifier, nb);
+}
+EXPORT_SYMBOL(drm_event_notifier_unregister);
+
+/**
+ * drm_event_notifier_notify - Emit an event to be notified to registered
+ * entities
+ * @event: event ID as defined in &drm_event_notifier_event
+ * @data: metadata associated to the event
+ */
+void drm_event_notifier_notify(unsigned long event, void *data)
+{
+ blocking_notifier_call_chain(&drm_event_notifier, event, data);
+}
+EXPORT_SYMBOL(drm_event_notifier_notify);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Luca Ceresoli <luca.ceresoli@bootlin.com>");
+MODULE_DESCRIPTION("Notifier for DRM components addition/removal and attach/detach");
diff --git a/include/drm/drm_event_notifier.h b/include/drm/drm_event_notifier.h
new file mode 100644
index 000000000000..78afc014ec9a
--- /dev/null
+++ b/include/drm/drm_event_notifier.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Internal event notifier for DRM drivers
+ *
+ * Copyright (C) 2026 GE HealthCare
+ * Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
+ */
+
+#ifndef _DRM_EVENT_NOTIFIER_H_
+#define _DRM_EVENT_NOTIFIER_H_
+
+/**
+ * enum drm_event_notifier_event - DRM bridge events
+ */
+enum drm_event_notifier_event {
+ /**
+ * @DRM_MIPI_DSI_ATTACHED: A MIPI DSI device has just been attached
+ * to its MIPI DSI host. @data is a pointer to the &struct
+ * mipi_dsi_device that has just attached.
+ */
+ DRM_MIPI_DSI_ATTACHED,
+ /**
+ * @DRM_BRIDGE_NOTIFY_DETACHED: A bridge has just been detached
+ * from the encoder bridge chain. Emitted at the end of
+ * drm_bridge_detach(), after removing the bridge from the encoder
+ * chain. @data is a pointer to the &struct drm_bridge that has
+ * just been detached.
+ */
+ DRM_BRIDGE_DETACHED,
+};
+
+int drm_event_notifier_register(struct notifier_block *nb);
+int drm_event_notifier_unregister(struct notifier_block *nb);
+void drm_event_notifier_notify(unsigned long event, void *data);
+
+#endif /* _DRM_EVENT_NOTIFIER_H_ */
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 26/37] drm: event-notifier: add mechanism to notify about hotplug events
2026-05-19 10:37 ` [PATCH 26/37] drm: event-notifier: add mechanism to notify about hotplug events Luca Ceresoli
@ 2026-05-19 11:06 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:06 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Global blocking notifier chain couples independent DRM devices.
- [Low] Missing forward declaration for `struct notifier_block` in header.
- [Low] Kernel-doc mismatch for DRM_BRIDGE_DETACHED enum value.
--
commit 233daf5d4ef6810bfafb5fb5ba8829a7db6987d3
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm: event-notifier: add mechanism to notify about hotplug events
This patch introduces a DRM event notifier to allow interested parties
to be notified about bridge hotplug events. It adds a new common file
implementing the notifier and defines initial events for bridge detach
and MIPI DSI attach.
> diff --git a/drivers/gpu/drm/drm_event_notifier.c b/drivers/gpu/drm/drm_event_notifier.c
> new file mode 100644
> index 0000000000000..76af4dd4cdb02
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_event_notifier.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Internal event notifier for DRM drivers
> + *
> + * Copyright (C) 2026 GE HealthCare
> + * Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +
> +#include <drm/drm_event_notifier.h>
> +
> +static BLOCKING_NOTIFIER_HEAD(drm_event_notifier);
Does using a single, global blocking notifier for all DRM hotplug events risk
coupling independent DRM devices?
When an event occurs on one device, it appears this will trigger callbacks for
every registered listener across the entire system. Could this lead to
cross-device lock contention, or cause listeners on unrelated hardware to
unexpectedly react to the event?
[ ... ]
> diff --git a/include/drm/drm_event_notifier.h b/include/drm/drm_event_notifier.h
> new file mode 100644
> index 0000000000000..78afc014ec9a6
> --- /dev/null
> +++ b/include/drm/drm_event_notifier.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Internal event notifier for DRM drivers
> + *
> + * Copyright (C) 2026 GE HealthCare
> + * Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
> + */
> +
> +#ifndef _DRM_EVENT_NOTIFIER_H_
> +#define _DRM_EVENT_NOTIFIER_H_
> +
> +/**
> + * enum drm_event_notifier_event - DRM bridge events
> + */
> +enum drm_event_notifier_event {
> + /**
> + * @DRM_MIPI_DSI_ATTACHED: A MIPI DSI device has just been attached
> + * to its MIPI DSI host. @data is a pointer to the &struct
> + * mipi_dsi_device that has just attached.
> + */
> + DRM_MIPI_DSI_ATTACHED,
> + /**
> + * @DRM_BRIDGE_NOTIFY_DETACHED: A bridge has just been detached
This isn't a bug, but the kernel-doc references @DRM_BRIDGE_NOTIFY_DETACHED
while the actual enum value is DRM_BRIDGE_DETACHED. Will this cause warnings
during documentation builds?
> + * from the encoder bridge chain. Emitted at the end of
> + * drm_bridge_detach(), after removing the bridge from the encoder
> + * chain. @data is a pointer to the &struct drm_bridge that has
> + * just been detached.
> + */
> + DRM_BRIDGE_DETACHED,
> +};
> +
> +int drm_event_notifier_register(struct notifier_block *nb);
Should there be a forward declaration for struct notifier_block here, or an
include for <linux/notifier.h>?
Without it, files including this header might get compiler warnings about the
struct being declared inside a parameter list, since kernel headers are
typically expected to be self-contained.
> +int drm_event_notifier_unregister(struct notifier_block *nb);
> +void drm_event_notifier_notify(unsigned long event, void *data);
> +
> +#endif /* _DRM_EVENT_NOTIFIER_H_ */
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=26
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 27/37] drm/bridge: notify about detached bridges
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (25 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 26/37] drm: event-notifier: add mechanism to notify about hotplug events Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:32 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 28/37] drm/mipi-dsi: turn DRM_MIPI_DSI into a tristate Luca Ceresoli
` (9 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In preparation to support DRM bridge hotplug, let the drm_bridge code emit
an event when a bridge is detached, so that this event can trigger the
actions needed to deconfigure the pipeline and unregister the connector as
appropriate.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/drm_bridge.c | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 18eb33e0e5a0..081660f3a0f2 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -17,6 +17,7 @@ menuconfig DRM
# device and dmabuf fd. Let's make sure that is available for our userspace.
select KCMP
select VIDEO
+ select DRM_EVENT_NOTIFIER
help
Kernel-level support for the Direct Rendering Infrastructure (DRI)
introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index d45fb74ec8c2..c62d17e84d4f 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -34,6 +34,7 @@
#include <drm/drm_debugfs.h>
#include <drm/drm_edid.h>
#include <drm/drm_encoder.h>
+#include <drm/drm_event_notifier.h>
#include <drm/drm_file.h>
#include <drm/drm_of.h>
#include <drm/drm_print.h>
@@ -638,6 +639,9 @@ void drm_bridge_detach(struct drm_bridge *bridge)
bridge->funcs->detach(bridge);
list_del_init(&bridge->chain_node);
+
+ drm_event_notifier_notify(DRM_BRIDGE_DETACHED, bridge);
+
bridge->dev = NULL;
drm_bridge_put(bridge);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 27/37] drm/bridge: notify about detached bridges
2026-05-19 10:37 ` [PATCH 27/37] drm/bridge: notify about detached bridges Luca Ceresoli
@ 2026-05-19 11:32 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:32 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dangling pointer: `bridge->encoder` is not cleared on detach, leading to potential Use-After-Free.
- [High] Premature teardown of atomic state, list linkages, and bridge resources before pipeline deconfiguration leads to UAF and list corruption.
--
commit 0a150cb9f7d5a0eebc9c0a446f33672c0a09a80b
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/bridge: notify about detached bridges
In preparation to support DRM bridge hotplug, let the drm_bridge code emit
an event when a bridge is detached, so that this event can trigger the
actions needed to deconfigure the pipeline and unregister the connector as
appropriate.
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index e7e17bf4fdf9e..62413fff35fcb 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
[ ... ]
> @@ -638,6 +639,9 @@ void drm_bridge_detach(struct drm_bridge *bridge)
> bridge->funcs->detach(bridge);
>
> list_del_init(&bridge->chain_node);
> +
> + drm_event_notifier_notify(DRM_BRIDGE_DETACHED, bridge);
The commit message mentions this event is meant to trigger actions to
deconfigure the pipeline. Since this event is emitted after tearing down the
bridge's atomic state, list linkages, and bridge resources, could this cause
issues when the listener attempts to safely disable the pipeline?
Specifically, earlier in this function:
if (drm_bridge_is_atomic(bridge))
drm_atomic_private_obj_fini(&bridge->base);
This destroys the bridge's modeset lock and frees its active atomic state. Any
concurrent atomic commit (like a userspace page flip) or a teardown commit
triggered by the listener might access freed memory or destroyed locks.
Additionally:
list_del_init(&bridge->chain_node);
This locklessly modifies the encoder's bridge chain, which might corrupt the
list if a concurrent atomic commit is iterating the chain.
Finally, bridge->funcs->detach() has already executed, meaning the bridge has
released its internal resources, potentially making subsequent atomic disable
operations unsafe. Should the notification be emitted before the bridge state
and linkages are forcefully torn down?
> +
> bridge->dev = NULL;
Does this code also need to clear bridge->encoder?
Since bridges are refcounted, and listeners or other users might hold their
own references via drm_bridge_get(), the bridge can outlive the encoder.
If the encoder is destroyed (e.g., during driver unbind), could any access
to bridge->encoder by asynchronous event listeners or delayed cleanup
workers result in a use-after-free?
> drm_bridge_put(bridge);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=27
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 28/37] drm/mipi-dsi: turn DRM_MIPI_DSI into a tristate
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (26 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 27/37] drm/bridge: notify about detached bridges Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:07 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 29/37] drm/mipi-dsi: notify about DSI attach Luca Ceresoli
` (8 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
DRM_MIPI_DSI is currently a bool, but there's no reason to not be allowed
to build it as a loadable module.
Moreover being a bool prevents DRM_MIPI_DSI to depend on a tristate module
that is configured as 'm'.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 081660f3a0f2..3093d5f30724 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -43,7 +43,7 @@ config DRM_MIPI_DBI
select DRM_KMS_HELPER
config DRM_MIPI_DSI
- bool
+ tristate
depends on DRM
config DRM_KMS_HELPER
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 28/37] drm/mipi-dsi: turn DRM_MIPI_DSI into a tristate
2026-05-19 10:37 ` [PATCH 28/37] drm/mipi-dsi: turn DRM_MIPI_DSI into a tristate Luca Ceresoli
@ 2026-05-19 11:07 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:07 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing module_exit() routine in drm_mipi_dsi.c
- [Low] Linker error due to IS_ENABLED(CONFIG_DRM_MIPI_DSI) evaluating to true when built as a module
--
commit 980deb1ce9940525b50bf77a81377ee0a4c56aad
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/mipi-dsi: turn DRM_MIPI_DSI into a tristate
This commit converts the DRM_MIPI_DSI configuration option from a boolean
to a tristate, allowing it to be built as a loadable module. This also
enables other tristate modules to depend on it when it is built as a module.
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index e730e04447418..6a14bb03607a7 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -43,7 +43,7 @@ config DRM_MIPI_DBI
> select DRM_KMS_HELPER
>
> config DRM_MIPI_DSI
> - bool
> + tristate
Since DRM_MIPI_DSI can now be built as a module, does drm_mipi_dsi.c
need a module_exit() routine?
Currently, drm_mipi_dsi.c uses postcore_initcall(mipi_dsi_bus_init) but does
not define a corresponding module_exit() function to unregister the bus.
Without this, the module will be permanently pinned in memory after loading
and cannot be unloaded.
Could this change also cause a linker regression if CONFIG_DRM=y and
CONFIG_DRM_MIPI_DSI=m?
Core DRM code like drm_of.c uses IS_ENABLED(CONFIG_DRM_MIPI_DSI) to
conditionally compile drm_of_get_dsi_bus(). IS_ENABLED() evaluates to true
for both y and m.
If DRM is built-in and MIPI_DSI is a module, drm_of.c will compile
drm_of_get_dsi_bus(), which calls of_find_mipi_dsi_host_by_node(). This
function is exported by the drm_mipi_dsi module, so it won't be present in
vmlinux, resulting in an undefined reference.
Does the Kconfig dependency need to be adjusted, or should
IS_REACHABLE(CONFIG_DRM_MIPI_DSI) be used instead to avoid this?
> depends on DRM
>
> config DRM_KMS_HELPER
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=28
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 29/37] drm/mipi-dsi: notify about DSI attach
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (27 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 28/37] drm/mipi-dsi: turn DRM_MIPI_DSI into a tristate Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:13 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 30/37] drm/bridge: add drm_bridge_is_tail() to know whether a bridge completes the pipeline Luca Ceresoli
` (7 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
In preparation to support DRM bridge hotplug, let the drm_mipi_dsi code
emit an event when a DSI device is attached to the corresponding DSI host,
so that this event can trigger the actions needed to deconfigure the
pipeline and unregister the connector as appropriate.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/drm_mipi_dsi.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 3093d5f30724..288845b6621b 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -45,6 +45,7 @@ config DRM_MIPI_DBI
config DRM_MIPI_DSI
tristate
depends on DRM
+ select DRM_EVENT_NOTIFIER
config DRM_KMS_HELPER
tristate
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 3ac1dd5ad640..eaa474da4a51 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -34,6 +34,7 @@
#include <linux/slab.h>
#include <drm/display/drm_dsc.h>
+#include <drm/drm_event_notifier.h>
#include <drm/drm_mipi_dsi.h>
#include <drm/drm_print.h>
@@ -386,6 +387,8 @@ int mipi_dsi_attach(struct mipi_dsi_device *dsi)
dsi->attached = true;
+ drm_event_notifier_notify(DRM_MIPI_DSI_ATTACHED, dsi);
+
return 0;
}
EXPORT_SYMBOL(mipi_dsi_attach);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 29/37] drm/mipi-dsi: notify about DSI attach
2026-05-19 10:37 ` [PATCH 29/37] drm/mipi-dsi: notify about DSI attach Luca Ceresoli
@ 2026-05-19 11:13 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:13 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message incorrectly claims that the attach event will trigger deconfiguration and unregistration.
--
commit 24c698fb4d9fc9ee14ca2fe29864b46f2adbe654
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/mipi-dsi: notify about DSI attach
This commit introduces a notification event when a DSI device is attached
to its host. This change is made in preparation for supporting DRM bridge
hotplug.
> In preparation to support DRM bridge hotplug, let the drm_mipi_dsi code
> emit an event when a DSI device is attached to the corresponding DSI host,
> so that this event can trigger the actions needed to deconfigure the
> pipeline and unregister the connector as appropriate.
This isn't a bug in the code, but does the commit message describe the wrong
actions for an attach event? Attaching a device typically triggers pipeline
configuration and connector registration, whereas deconfiguration and
unregistration are normally associated with a detach event.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=29
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 30/37] drm/bridge: add drm_bridge_is_tail() to know whether a bridge completes the pipeline
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (28 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 29/37] drm/mipi-dsi: notify about DSI attach Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:59 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 31/37] drm/bridge: panel: implement .is_tail Luca Ceresoli
` (6 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
For bridge hotplug we need to successfully probe a card with an incomplete
bridge chain, i.e. a chain whose last bridge currently in bridge_chain
needs a next_bridge to work. Such a card would have no connector, and be
able to add one as soon as the followong bridges are added up to the tail
bridge (e.g. a panel_bridge or a connector_bridge).
Currently common DRM code is unable to tell whether the last bridge
currently in the chain is the tail bridge (= the pipeline is complete) or
not.
Add drm_bridge_is_tail(), and a .is_tail func for it to rely on, so common
code can know whether a bridge is a tail bridge or needs a next_bridge.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/drm_bridge.c | 28 ++++++++++++++++++++++++++++
include/drm/drm_bridge.h | 19 +++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index c62d17e84d4f..2b539c9749a6 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -646,6 +646,34 @@ void drm_bridge_detach(struct drm_bridge *bridge)
drm_bridge_put(bridge);
}
+/**
+ * drm_bridge_is_tail - check whether the bridge is the last required to
+ * have a full pipeline
+ * @bridge: the bridge to check
+ *
+ * Tell whether this is a tail bridge, i.e. a bridge that does not need a
+ * next bridge to work. E.g. a panel_bridge is final, a DSI-to-LVDS bridge
+ * is not.
+ *
+ * In case of hotplug the last bridge currently in the chain (as in
+ * drm_bridge_chain_get_last_bridge()) might not be final, and be waiting
+ * for a pipeline tail to be connected.
+ *
+ * Return: true if this bridge does not need a next bridge to work, false
+ * otherwise
+ */
+bool drm_bridge_is_tail(struct drm_bridge *bridge)
+{
+ if (!(bridge->ops & DRM_BRIDGE_OP_IS_TAIL)) {
+ drm_warn_once(bridge->dev, "is_tail func not implemented by bridge %ps!",
+ bridge->funcs);
+ return false;
+ }
+
+ return bridge->funcs->is_tail(bridge);
+}
+EXPORT_SYMBOL(drm_bridge_is_tail);
+
/**
* DOC: bridge operations
*
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4ba3a5deef9a..d783a6fb93a0 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -78,6 +78,19 @@ struct drm_bridge_funcs {
int (*attach)(struct drm_bridge *bridge, struct drm_encoder *encoder,
enum drm_bridge_attach_flags flags);
+ /**
+ * @is_tail:
+ *
+ * Returns true if this is a tail bridge, i.e. it does not need a
+ * next bridge to work. E.g. a panel_bridge is a tail bridge, a
+ * DSI-to-LVDS bridge is not a tail bridge (no matter whether the
+ * next bridge is present or not).
+ *
+ * The @is_tail callback is optional but it is required if the
+ * bridge is part of a pipeline with hot-pluggable components.
+ */
+ bool (*is_tail)(struct drm_bridge *bridge);
+
/**
* @destroy:
*
@@ -1092,6 +1105,11 @@ enum drm_bridge_ops {
* &drm_bridge_funcs->hdmi_clear_spd_infoframe callbacks.
*/
DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME = BIT(10),
+ /**
+ * @DRM_BRIDGE_OP_IS_TAIL: The bridge implements the
+ * &drm_bridge_funcs->is_tail callback.
+ */
+ DRM_BRIDGE_OP_IS_TAIL = BIT(11),
};
/**
@@ -1323,6 +1341,7 @@ void drm_bridge_remove(struct drm_bridge *bridge);
int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
struct drm_bridge *previous,
enum drm_bridge_attach_flags flags);
+bool drm_bridge_is_tail(struct drm_bridge *bridge);
#ifdef CONFIG_OF
struct drm_bridge *of_drm_find_and_get_bridge(struct device_node *np);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 30/37] drm/bridge: add drm_bridge_is_tail() to know whether a bridge completes the pipeline
2026-05-19 10:37 ` [PATCH 30/37] drm/bridge: add drm_bridge_is_tail() to know whether a bridge completes the pipeline Luca Ceresoli
@ 2026-05-19 10:59 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 10:59 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm_bridge_is_tail() emits a kernel warning and defaults to false for bridges omitting the optional .is_tail callback.
--
commit 285e9eabd414f7b37079e41dd2875f7b3e588911
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/bridge: add drm_bridge_is_tail() to know whether a bridge completes the pipeline
This commit introduces drm_bridge_is_tail() and an .is_tail callback to
determine if a bridge completes the display pipeline. This is useful for
handling bridge hotplug scenarios where a pipeline might initially be
incomplete.
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 62413fff35fcb..10af107823902 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
[ ... ]
> @@ -646,6 +646,34 @@ void drm_bridge_detach(struct drm_bridge *bridge)
[ ... ]
> +bool drm_bridge_is_tail(struct drm_bridge *bridge)
> +{
> + if (!(bridge->ops & DRM_BRIDGE_OP_IS_TAIL)) {
> + drm_warn_once(bridge->dev, "is_tail func not implemented by bridge %ps!",
> + bridge->funcs);
> + return false;
> + }
> +
> + return bridge->funcs->is_tail(bridge);
> +}
> +EXPORT_SYMBOL(drm_bridge_is_tail);
Will developers writing non-hot-pluggable bridges trigger spurious kernel
warnings at runtime here if they follow the documentation and omit this
callback?
Additionally, returning false when the callback is omitted means that legacy
static tail bridges that haven't been updated will be seen as needing a next
bridge. Could this result in display initialization failures for existing
drivers across the kernel?
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index fdac9a526f387..0547d5010b080 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
[ ... ]
> @@ -78,6 +78,19 @@ struct drm_bridge_funcs {
[ ... ]
> + /**
> + * @is_tail:
> + *
> + * Returns true if this is a tail bridge, i.e. it does not need a
> + * next bridge to work. E.g. a panel_bridge is a tail bridge, a
> + * DSI-to-LVDS bridge is not a tail bridge (no matter whether the
> + * next bridge is present or not).
> + *
> + * The @is_tail callback is optional but it is required if the
> + * bridge is part of a pipeline with hot-pluggable components.
> + */
> + bool (*is_tail)(struct drm_bridge *bridge);
Since this documentation explicitly states the callback is optional for bridges
that aren't part of a hot-pluggable pipeline, is it safe for
drm_bridge_is_tail() to default to false and warn when it is omitted?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=30
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 31/37] drm/bridge: panel: implement .is_tail
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (29 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 30/37] drm/bridge: add drm_bridge_is_tail() to know whether a bridge completes the pipeline Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 15:12 ` Neil Armstrong
2026-05-19 10:37 ` [PATCH 32/37] drm/bridge: display-connector: " Luca Ceresoli
` (5 subsequent siblings)
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
This bridge is always a tail bridge, i.e. it never needs a following bridge
to complete the pipeline. Add a is_tail func to expose this.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/bridge/panel.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 4978ec98a082..102f987b1235 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -58,6 +58,11 @@ static const struct drm_connector_funcs panel_bridge_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
+static bool panel_bridge_is_tail(struct drm_bridge *bridge)
+{
+ return true;
+}
+
static int panel_bridge_attach(struct drm_bridge *bridge,
struct drm_encoder *encoder,
enum drm_bridge_attach_flags flags)
@@ -206,6 +211,7 @@ static void panel_bridge_debugfs_init(struct drm_bridge *bridge,
}
static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
+ .is_tail = panel_bridge_is_tail,
.attach = panel_bridge_attach,
.detach = panel_bridge_detach,
.atomic_pre_enable = panel_bridge_atomic_pre_enable,
@@ -297,7 +303,7 @@ struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,
panel_bridge->panel = panel;
panel_bridge->bridge.of_node = panel->dev->of_node;
- panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES;
+ panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES | DRM_BRIDGE_OP_IS_TAIL;
panel_bridge->bridge.type = connector_type;
panel_bridge->bridge.pre_enable_prev_first = panel->prepare_prev_first;
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 31/37] drm/bridge: panel: implement .is_tail
2026-05-19 10:37 ` [PATCH 31/37] drm/bridge: panel: implement .is_tail Luca Ceresoli
@ 2026-05-19 15:12 ` Neil Armstrong
0 siblings, 0 replies; 59+ messages in thread
From: Neil Armstrong @ 2026-05-19 15:12 UTC (permalink / raw)
To: Luca Ceresoli, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Andrzej Hajda,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Inki Dae, Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner,
Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel
On 5/19/26 12:37, Luca Ceresoli wrote:
> This bridge is always a tail bridge, i.e. it never needs a following bridge
> to complete the pipeline. Add a is_tail func to expose this.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
> drivers/gpu/drm/bridge/panel.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index 4978ec98a082..102f987b1235 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -58,6 +58,11 @@ static const struct drm_connector_funcs panel_bridge_connector_funcs = {
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> +static bool panel_bridge_is_tail(struct drm_bridge *bridge)
> +{
> + return true;
> +}
> +
> static int panel_bridge_attach(struct drm_bridge *bridge,
> struct drm_encoder *encoder,
> enum drm_bridge_attach_flags flags)
> @@ -206,6 +211,7 @@ static void panel_bridge_debugfs_init(struct drm_bridge *bridge,
> }
>
> static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
> + .is_tail = panel_bridge_is_tail,
> .attach = panel_bridge_attach,
> .detach = panel_bridge_detach,
> .atomic_pre_enable = panel_bridge_atomic_pre_enable,
> @@ -297,7 +303,7 @@ struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,
> panel_bridge->panel = panel;
>
> panel_bridge->bridge.of_node = panel->dev->of_node;
> - panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES;
> + panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES | DRM_BRIDGE_OP_IS_TAIL;
> panel_bridge->bridge.type = connector_type;
> panel_bridge->bridge.pre_enable_prev_first = panel->prepare_prev_first;
>
>
This looks reasonable
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Thanks,
Neil
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 32/37] drm/bridge: display-connector: implement .is_tail
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (30 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 31/37] drm/bridge: panel: implement .is_tail Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 33/37] drm/bridge: samsung-dsim: " Luca Ceresoli
` (4 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
This bridge is always a tail bridge, i.e. it never needs a following bridge
to complete the pipeline. Add a is_tail func to expose this.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/bridge/display-connector.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index 16c0631adeb1..1c8111bdc89e 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -33,6 +33,11 @@ to_display_connector(struct drm_bridge *bridge)
return container_of(bridge, struct display_connector, bridge);
}
+static bool display_connector_is_tail(struct drm_bridge *bridge)
+{
+ return true;
+}
+
static int display_connector_attach(struct drm_bridge *bridge,
struct drm_encoder *encoder,
enum drm_bridge_attach_flags flags)
@@ -176,6 +181,7 @@ static u32 *display_connector_get_input_bus_fmts(struct drm_bridge *bridge,
}
static const struct drm_bridge_funcs display_connector_bridge_funcs = {
+ .is_tail = display_connector_is_tail,
.attach = display_connector_attach,
.detect = display_connector_bridge_detect,
.edid_read = display_connector_edid_read,
@@ -370,6 +376,7 @@ static int display_connector_probe(struct platform_device *pdev)
conn->bridge.of_node = pdev->dev.of_node;
+ conn->bridge.ops = DRM_BRIDGE_OP_IS_TAIL;
if (conn->bridge.ddc)
conn->bridge.ops |= DRM_BRIDGE_OP_EDID
| DRM_BRIDGE_OP_DETECT;
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 33/37] drm/bridge: samsung-dsim: implement .is_tail
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (31 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 32/37] drm/bridge: display-connector: " Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 34/37] drm/bridge: ti-sn65dsi83: " Luca Ceresoli
` (3 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
This bridge is never a tail bridge, i.e. it always needs a following bridge
to complete the pipeline. Add a is_tail func to expose this.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 3c70a45c5dce..55f4b0f16ade 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1716,6 +1716,11 @@ static const u32 samsung_dsim_pixel_output_fmts[] = {
MEDIA_BUS_FMT_RGB888_1X24,
};
+static bool samsung_dsim_is_tail(struct drm_bridge *bridge)
+{
+ return false;
+}
+
static bool samsung_dsim_pixel_output_fmt_supported(u32 fmt)
{
int i;
@@ -1836,6 +1841,7 @@ static int samsung_dsim_attach(struct drm_bridge *bridge,
}
static const struct drm_bridge_funcs samsung_dsim_bridge_funcs = {
+ .is_tail = samsung_dsim_is_tail,
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.atomic_reset = drm_atomic_helper_bridge_reset,
@@ -2225,6 +2231,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
pm_runtime_enable(dev);
+ dsi->bridge.ops = DRM_BRIDGE_OP_IS_TAIL;
dsi->bridge.of_node = dev->of_node;
dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 34/37] drm/bridge: ti-sn65dsi83: implement .is_tail
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (32 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 33/37] drm/bridge: samsung-dsim: " Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 35/37] drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER Luca Ceresoli
` (2 subsequent siblings)
36 siblings, 0 replies; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
This bridge is never a tail bridge, i.e. it always needs a following bridge
to complete the pipeline. Add a is_tail func to expose this.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index a4f2a86a09be..5613c8812318 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -293,6 +293,11 @@ static struct sn65dsi83 *bridge_to_sn65dsi83(struct drm_bridge *bridge)
return container_of(bridge, struct sn65dsi83, bridge);
}
+static bool sn65dsi83_is_tail(struct drm_bridge *bridge)
+{
+ return false;
+}
+
static int sn65dsi83_attach(struct drm_bridge *bridge,
struct drm_encoder *encoder,
enum drm_bridge_attach_flags flags)
@@ -792,6 +797,7 @@ sn65dsi83_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
}
static const struct drm_bridge_funcs sn65dsi83_funcs = {
+ .is_tail = sn65dsi83_is_tail,
.attach = sn65dsi83_attach,
.detach = sn65dsi83_detach,
.atomic_enable = sn65dsi83_atomic_enable,
@@ -1066,6 +1072,7 @@ static int sn65dsi83_probe(struct i2c_client *client)
dev_set_drvdata(dev, ctx);
i2c_set_clientdata(client, ctx);
+ ctx->bridge.ops = DRM_BRIDGE_OP_IS_TAIL;
ctx->bridge.of_node = dev->of_node;
ctx->bridge.pre_enable_prev_first = true;
ctx->bridge.type = DRM_MODE_CONNECTOR_LVDS;
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH 35/37] drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (33 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 34/37] drm/bridge: ti-sn65dsi83: " Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:21 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 36/37] drm/display: bridge-connector: handle bridge hotplug Luca Ceresoli
2026-05-19 10:37 ` [PATCH 37/37] drm/mxsfb/lcdif: enable " Luca Ceresoli
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
With bridge hotplug support, when a bridge attach func returns
-EPROBE_DEFER it means the following bridge is not yet connected, but it
could be later on. In this case don't fail the entire attach operation, so
the card can probe (without a connector) and be ready to handle a future
hotplug that completes the pipeline.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/drm_bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 2b539c9749a6..05702ece77c9 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -593,7 +593,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
if (bridge->funcs->attach) {
ret = bridge->funcs->attach(bridge, encoder, flags);
- if (ret < 0)
+ if (ret < 0 && ret != -EPROBE_DEFER)
goto err_reset_bridge;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 35/37] drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER
2026-05-19 10:37 ` [PATCH 35/37] drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER Luca Ceresoli
@ 2026-05-19 11:21 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:21 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Masking `-EPROBE_DEFER` leaves failed, uninitialized bridges in the active bridge chain, leading to UAF and crashes.
- [High] Unconditionally swallowing `-EPROBE_DEFER` breaks the generic probe deferral mechanism for all bridge resources.
- [Low] The probe deferral diagnostic block in `err_reset_bridge` is now unreachable dead code.
--
commit 17390f0ac2160f3f951784d20085063db510a04e
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER
This commit modifies drm_bridge_attach() to bypass the error cleanup path
when a bridge's attach callback returns -EPROBE_DEFER. This is intended to
allow the display pipeline to probe even if a downstream bridge is not yet
connected.
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 10af107823902..594cb1468266e 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -593,7 +593,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>
> if (bridge->funcs->attach) {
> ret = bridge->funcs->attach(bridge, encoder, flags);
> - if (ret < 0)
> + if (ret < 0 && ret != -EPROBE_DEFER)
> goto err_reset_bridge;
> }
If the bridge's attach callback returns -EPROBE_DEFER, skipping the
err_reset_bridge path will cause the function to return 0 (success) instead
of propagating the error. Does this break the generic probe deferral
mechanism? -EPROBE_DEFER could be returned because any required resource
(such as clocks or regulators) is missing, and returning 0 means the upstream
driver will finish probing without the driver core ever retrying.
Additionally, because the bridge was added to encoder->bridge_chain prior to
calling the attach callback:
mutex_lock(&encoder->bridge_chain_mutex);
if (previous)
list_add(&bridge->chain_node, &previous->chain_node);
else
list_add(&bridge->chain_node, &encoder->bridge_chain);
mutex_unlock(&encoder->bridge_chain_mutex);
If we don't go to err_reset_bridge to remove it from the list, the bridge is
left permanently linked in the active chain even though its attach operation
failed and was rolled back. Will this cause use-after-free issues or crashes
when the DRM core later traverses the chain to invoke pipeline callbacks?
Finally, lower down in this function at the err_reset_bridge label, there is
a check for -EPROBE_DEFER:
err_reset_bridge:
...
if (ret != -EPROBE_DEFER)
DRM_ERROR(...);
else
dev_err_probe(encoder->dev->dev, -EPROBE_DEFER, ...);
Since the err_reset_bridge label is now only reachable when ret is strictly
not equal to -EPROBE_DEFER, is the else branch containing dev_err_probe() now
unreachable dead code?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=35
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 36/37] drm/display: bridge-connector: handle bridge hotplug
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (34 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 35/37] drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:15 ` sashiko-bot
2026-05-19 10:37 ` [PATCH 37/37] drm/mxsfb/lcdif: enable " Luca Ceresoli
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
The drm_bridge_connector is nowadays the recommended way to implement DRM
connectors when a chain of bridges is used, and as such it is the ideal
component to implement DRM bridge hotplug. Thus, let the
drm_bridge_connector be created once for the whole drm_encoder lifetime (as
it is now), but make it able of creating and destroying the DRM connector
reacting on bridge hot(un)plug events.
Unfortunately the current drm_bridge_connector_init() API is unable to
support DRM bridge hotplug because it returns a drm_connector, and with
hotplug the connector might start existing only later, and also be
destroyed and recreated multiple times.
So add a new API which is similar but returns a pointer to the
drm_bridge_connector, which is persistent, and supports bridge hotplug by
creating and destroying the drm_connector based on hotplug events.
Because drm_bridge_connector_init() is really a drmm function (it does only
drmm allocations), but without a drmm_ prefix, take this as an opportunity
to call the new function with the same name but a drmm_ prefix:
drmm_bridge_connector_init(). The old API can be replaced over time when
needed to support hotplug, and eventually be removed if unused.
The new drmm_bridge_connector_init() API differs from the old one in a few
aspects in order to support bridge hotplug:
* if the bridge chain is not (yet) complete, returns successfully without
adding a drm_connector (in this case the old API returns an error)
* registers an event notifier block to be able of reacting to events
related to bridge hotplug and create/destroy the drm_connector
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
I'm tempted to let drmm_bridge_connector_init() return int (0 or negative
error) instead of a struct drm_bridge_connector *, but I'm not sure whether
the drm_bridge_connector pointer may be useful in the future to invoke
actions on the bridge_connector after its creation. Opnions about this
would be welcome.
---
drivers/gpu/drm/display/drm_bridge_connector.c | 157 +++++++++++++++++++++++++
include/drm/drm_bridge_connector.h | 2 +
2 files changed, 159 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index b83da529cc7a..2ce6c04e2fc7 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -20,6 +20,7 @@
#include <drm/drm_device.h>
#include <drm/drm_edid.h>
#include <drm/drm_managed.h>
+#include <drm/drm_event_notifier.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -158,6 +159,10 @@ struct drm_bridge_connector {
* destruction
*/
struct mutex dynconn_mutex;
+ /**
+ * @drm_event_nb: notifier to receive DRM hotplug-related events
+ */
+ struct notifier_block drm_event_nb;
};
static struct drm_bridge_connector_dynconn *
@@ -1037,6 +1042,19 @@ static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector_
return 0;
}
+static bool drm_bridge_connector_pipeline_is_complete(struct drm_bridge_connector *bridge_connector)
+{
+ struct drm_bridge *last_bridge __free(drm_bridge_put) =
+ drm_bridge_chain_get_last_bridge(bridge_connector->encoder);
+
+ if (!last_bridge || !drm_bridge_is_tail(last_bridge)) {
+ drm_dbg_driver(bridge_connector->drm, "pipeline not (yet) fully connected");
+ return false;
+ }
+
+ return true;
+}
+
/**
* drm_bridge_connector_add_connector - add the drm_connector
* @bridge_connector: drm_bridge_connector to add the drm_connector to
@@ -1153,6 +1171,79 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
return ret;
}
+/*
+ * Propagate the attach chain and possibly add a drm_connector after a new
+ * drm_bridge is hot-plugged.
+ *
+ * The connector is added only if the pipeline is now complete. This could
+ * not be the case for various reasons:
+ *
+ * - the new bridge is just unrelated to our encoder
+ * - the new bridge is not be the next one in the pipeline
+ * - the new bridge is the next in the pipeline but the pipeline is not yet
+ * complete
+ *
+ * All these cases are normal, not an error.
+ */
+static void drm_bridge_connector_try_complete(struct drm_bridge_connector *bridge_connector)
+{
+ int err;
+
+ /*
+ * drm_connector already present, the new bridge must be for
+ * another card
+ */
+ if (bridge_connector->dynconn)
+ return;
+
+ /* Propagate the attach call chain to newly hotplugged bridge(s) */
+ struct drm_bridge *last_bridge __free(drm_bridge_put) =
+ drm_bridge_chain_get_last_bridge(bridge_connector->encoder);
+
+ /* Encoder chain empty? */
+ if (!last_bridge)
+ return;
+
+ err = last_bridge->funcs->attach(last_bridge, bridge_connector->encoder,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+ if (err)
+ return;
+
+ /* Add the connector if the pipeline is now complete */
+ if (drm_bridge_connector_pipeline_is_complete(bridge_connector))
+ drm_bridge_connector_add_connector(bridge_connector);
+}
+
+static int drm_bridge_connector_handle_event(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct drm_bridge_connector *bridge_connector =
+ container_of(nb, struct drm_bridge_connector, drm_event_nb);
+
+ switch (event) {
+ case DRM_MIPI_DSI_ATTACHED:
+ /* One or more bridges hot-plugged, try adding the drm_connector */
+ drm_bridge_connector_try_complete(bridge_connector);
+ break;
+ case DRM_BRIDGE_DETACHED:
+ {
+ /*
+ * A bridge was unplugged, remove the drm_connector if it's
+ * part of the same pipeline
+ */
+ struct drm_bridge *bridge = (struct drm_bridge *)data;
+
+ if (bridge_connector->dynconn &&
+ bridge->encoder == bridge_connector->encoder)
+ drm_bridge_connector_dynconn_release(bridge_connector);
+ break;
+ }
+ default:
+ }
+
+ return NOTIFY_DONE;
+}
+
static void drm_bridge_connector_fini(struct drm_device *dev, void *res)
{
struct drm_bridge_connector *bridge_connector = (struct drm_bridge_connector *)res;
@@ -1205,3 +1296,69 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
return &bridge_connector->dynconn->connector;
}
EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
+
+static void drm_bridge_connector_notifier_unregister(struct drm_device *dev, void *res)
+{
+ struct notifier_block *nb = (struct notifier_block *)res;
+
+ drm_event_notifier_unregister(nb);
+}
+
+/**
+ * drmm_bridge_connector_init - Initialise the bridge_connector for a chain
+ * of bridges, with bridge hotplug support
+ * @drm: the DRM device
+ * @encoder: the encoder where the bridge chain starts
+ *
+ * Allocate, initialise and register a &drm_bridge_connector with the @drm
+ * device. The connector is associated with a chain of bridges that starts at
+ * the @encoder. All bridges in the chain shall report bridge operation flags
+ * (&drm_bridge->ops) and bridge output type (&drm_bridge->type), and none of
+ * them may create a DRM connector directly.
+ *
+ * Also adds a drm_connector if the bridge chain is complete, otherwise the
+ * &drm_bridge_connector will still be successfully created but without
+ * adding a drm_connector. In both cases the &drm_bridge_connector will
+ * receive events notifying bridge hot-plug and hot-unplug and add or
+ * destroy the drm_connector accordingly.
+ *
+ * Returns a pointer to the new &drm_bridge_connector on success, or a
+ * negative error pointer otherwise.
+ */
+struct drm_bridge_connector *drmm_bridge_connector_init(struct drm_device *drm,
+ struct drm_encoder *encoder)
+{
+ struct drm_bridge_connector *bridge_connector;
+ int ret;
+
+ bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector), GFP_KERNEL);
+ if (!bridge_connector)
+ return ERR_PTR(-ENOMEM);
+
+ mutex_init(&bridge_connector->dynconn_mutex);
+ bridge_connector->drm = drm;
+ bridge_connector->encoder = encoder;
+ bridge_connector->drm_event_nb.notifier_call = drm_bridge_connector_handle_event;
+
+ if (drm_bridge_connector_pipeline_is_complete(bridge_connector)) {
+ ret = drm_bridge_connector_add_connector(bridge_connector);
+ if (ret)
+ return ERR_PTR(ret);
+ }
+
+ ret = drmm_add_action_or_reset(drm, drm_bridge_connector_fini, bridge_connector);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ret = drm_event_notifier_register(&bridge_connector->drm_event_nb);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ret = drmm_add_action_or_reset(drm, drm_bridge_connector_notifier_unregister,
+ &bridge_connector->drm_event_nb);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(drmm_bridge_connector_init);
diff --git a/include/drm/drm_bridge_connector.h b/include/drm/drm_bridge_connector.h
index 69630815fb09..1c78221c0857 100644
--- a/include/drm/drm_bridge_connector.h
+++ b/include/drm/drm_bridge_connector.h
@@ -10,6 +10,8 @@ struct drm_connector;
struct drm_device;
struct drm_encoder;
+struct drm_bridge_connector *drmm_bridge_connector_init(struct drm_device *drm,
+ struct drm_encoder *encoder);
struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
struct drm_encoder *encoder);
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 36/37] drm/display: bridge-connector: handle bridge hotplug
2026-05-19 10:37 ` [PATCH 36/37] drm/display: bridge-connector: handle bridge hotplug Luca Ceresoli
@ 2026-05-19 11:15 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:15 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] `drmm_bridge_connector_init()` returns NULL instead of the allocated pointer on success.
- [High] Unchecked execution of the optional `attach` callback causes NULL pointer dereference.
- [High] TOCTOU race condition in `dynconn` creation leads to memory leaks and duplicate connector registrations.
- [High] Dynamic connector destruction on hot-unplug leaks `drmm` actions, leading to a Use-After-Free.
- [High] Lockless traversal of `encoder->bridge_chain` risks list corruption and Use-After-Free.
--
commit 31a28cb0ac7316fddfb3b8148c8e28f4da0bce4f
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/display: bridge-connector: handle bridge hotplug
This commit updates the DRM bridge connector infrastructure to handle bridge
hot-plugging by creating and destroying the DRM connector dynamically. It
introduces the drmm_bridge_connector_init() API, which registers a notifier
to react to attach/detach events and manage the connector lifecycle.
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index b83da529cc7af..2ce6c04e2fc7a 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
[ ... ]
> @@ -1037,6 +1042,19 @@ static int drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector_
> return 0;
> }
>
> +static bool drm_bridge_connector_pipeline_is_complete(struct drm_bridge_connector *bridge_connector)
> +{
> + struct drm_bridge *last_bridge __free(drm_bridge_put) =
> + drm_bridge_chain_get_last_bridge(bridge_connector->encoder);
Is it safe to call drm_bridge_chain_get_last_bridge() here without holding
bridge_connector->encoder->bridge_chain_mutex? It seems this can be called
asynchronously from hotplug event notifiers, potentially traversing the
list while it is being modified.
> +
> + if (!last_bridge || !drm_bridge_is_tail(last_bridge)) {
> + drm_dbg_driver(bridge_connector->drm, "pipeline not (yet) fully connected");
> + return false;
> + }
> +
> + return true;
> +}
[ ... ]
> @@ -1153,6 +1171,79 @@ static int drm_bridge_connector_add_connector(struct drm_bridge_connector *bridg
> return ret;
> }
>
> +/*
> + * Propagate the attach chain and possibly add a drm_connector after a new
> + * drm_bridge is hot-plugged.
> + *
> + * The connector is added only if the pipeline is now complete. This could
> + * not be the case for various reasons:
> + *
> + * - the new bridge is just unrelated to our encoder
> + * - the new bridge is not be the next one in the pipeline
> + * - the new bridge is the next in the pipeline but the pipeline is not yet
> + * complete
> + *
> + * All these cases are normal, not an error.
> + */
> +static void drm_bridge_connector_try_complete(struct drm_bridge_connector *bridge_connector)
> +{
> + int err;
> +
> + /*
> + * drm_connector already present, the new bridge must be for
> + * another card
> + */
> + if (bridge_connector->dynconn)
Could there be a race condition here? If multiple hotplug events are processed
concurrently, two threads could observe dynconn as NULL here outside the
lock, and both proceed to allocate and assign it in
drm_bridge_connector_add_connector().
Would it be better to re-verify bridge_connector->dynconn after acquiring
dynconn_mutex inside drm_bridge_connector_add_connector()?
> + return;
> +
> + /* Propagate the attach call chain to newly hotplugged bridge(s) */
> + struct drm_bridge *last_bridge __free(drm_bridge_put) =
> + drm_bridge_chain_get_last_bridge(bridge_connector->encoder);
As mentioned above, should this list traversal be protected by
bridge_connector->encoder->bridge_chain_mutex?
> +
> + /* Encoder chain empty? */
> + if (!last_bridge)
> + return;
> +
> + err = last_bridge->funcs->attach(last_bridge, bridge_connector->encoder,
> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
What happens if the last bridge does not implement the attach callback?
Since attach is optional in struct drm_bridge_funcs, should there be a check
for last_bridge->funcs->attach before calling it to avoid a NULL pointer
dereference?
> + if (err)
> + return;
> +
> + /* Add the connector if the pipeline is now complete */
> + if (drm_bridge_connector_pipeline_is_complete(bridge_connector))
> + drm_bridge_connector_add_connector(bridge_connector);
> +}
> +
> +static int drm_bridge_connector_handle_event(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct drm_bridge_connector *bridge_connector =
> + container_of(nb, struct drm_bridge_connector, drm_event_nb);
> +
> + switch (event) {
> + case DRM_MIPI_DSI_ATTACHED:
> + /* One or more bridges hot-plugged, try adding the drm_connector */
> + drm_bridge_connector_try_complete(bridge_connector);
> + break;
> + case DRM_BRIDGE_DETACHED:
> + {
> + /*
> + * A bridge was unplugged, remove the drm_connector if it's
> + * part of the same pipeline
> + */
> + struct drm_bridge *bridge = (struct drm_bridge *)data;
> +
> + if (bridge_connector->dynconn &&
> + bridge->encoder == bridge_connector->encoder)
> + drm_bridge_connector_dynconn_release(bridge_connector);
Does drm_bridge_connector_dynconn_release() safely clean up the drmm actions
registered in drm_bridge_connector_init_hdmi_audio_cec()?
Since drmm_connector_hdmi_cec_register() and similar tie cleanup actions to
the drm_device, releasing the connector here and freeing the dynconn
structure might leave those actions active, potentially causing a
use-after-free when the device is eventually destroyed.
> + break;
> + }
> + default:
> + }
> +
> + return NOTIFY_DONE;
> +}
[ ... ]
> +struct drm_bridge_connector *drmm_bridge_connector_init(struct drm_device *drm,
> + struct drm_encoder *encoder)
> +{
> + struct drm_bridge_connector *bridge_connector;
> + int ret;
> +
> + bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector), GFP_KERNEL);
> + if (!bridge_connector)
> + return ERR_PTR(-ENOMEM);
> +
> + mutex_init(&bridge_connector->dynconn_mutex);
> + bridge_connector->drm = drm;
> + bridge_connector->encoder = encoder;
> + bridge_connector->drm_event_nb.notifier_call = drm_bridge_connector_handle_event;
> +
> + if (drm_bridge_connector_pipeline_is_complete(bridge_connector)) {
> + ret = drm_bridge_connector_add_connector(bridge_connector);
> + if (ret)
> + return ERR_PTR(ret);
> + }
> +
> + ret = drmm_add_action_or_reset(drm, drm_bridge_connector_fini, bridge_connector);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ret = drm_event_notifier_register(&bridge_connector->drm_event_nb);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ret = drmm_add_action_or_reset(drm, drm_bridge_connector_notifier_unregister,
> + &bridge_connector->drm_event_nb);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return 0;
Should this return the bridge_connector pointer instead of 0? Callers using
IS_ERR() will treat 0 (NULL) as success and potentially dereference it.
> +}
> +EXPORT_SYMBOL_GPL(drmm_bridge_connector_init);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=36
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 37/37] drm/mxsfb/lcdif: enable bridge hotplug
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
` (35 preceding siblings ...)
2026-05-19 10:37 ` [PATCH 36/37] drm/display: bridge-connector: handle bridge hotplug Luca Ceresoli
@ 2026-05-19 10:37 ` Luca Ceresoli
2026-05-19 11:33 ` sashiko-bot
36 siblings, 1 reply; 59+ messages in thread
From: Luca Ceresoli @ 2026-05-19 10:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Inki Dae,
Jagan Teki, Marek Szyprowski, Marek Vasut, Stefan Agner, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Hui Pu, Ian Ray, Thomas Petazzoni, dri-devel, linux-kernel, imx,
linux-arm-kernel, Luca Ceresoli
Switch to the new drmm_bridge_connector_init() so the drm_bridge_connector
will handle bridge hotplugging.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/gpu/drm/mxsfb/lcdif_drv.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index e2173c4d6fc2..0a7c197dbc1e 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -58,7 +58,7 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
struct of_endpoint of_ep;
struct drm_bridge *bridge;
struct drm_encoder *encoder;
- struct drm_connector *connector;
+ struct drm_bridge_connector *bridge_connector;
int ret;
if (!of_device_is_available(remote))
@@ -94,9 +94,9 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
"Failed to attach bridge for endpoint%u\n",
of_ep.id);
- connector = drm_bridge_connector_init(lcdif->drm, encoder);
- if (IS_ERR(connector))
- return dev_err_probe(dev, PTR_ERR(connector),
+ bridge_connector = drmm_bridge_connector_init(lcdif->drm, encoder);
+ if (IS_ERR(bridge_connector))
+ return dev_err_probe(dev, PTR_ERR(bridge_connector),
"Failed to init bridge_connector for endpoint%u\n",
of_ep.id);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH 37/37] drm/mxsfb/lcdif: enable bridge hotplug
2026-05-19 10:37 ` [PATCH 37/37] drm/mxsfb/lcdif: enable " Luca Ceresoli
@ 2026-05-19 11:33 ` sashiko-bot
0 siblings, 0 replies; 59+ messages in thread
From: sashiko-bot @ 2026-05-19 11:33 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use-After-Free via global DRM event notifier due to lifetime mismatch.
- [High] The newly introduced API `drmm_bridge_connector_init()` returns NULL instead of the allocated pointer on success.
--
commit cc8f05ac9a777f8eb1ce4636df74812c2db5a92c
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
drm/mxsfb/lcdif: enable bridge hotplug
This commit switches to the new drmm_bridge_connector_init() function
to allow the drm_bridge_connector to handle bridge hotplugging.
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
[ ... ]
> @@ -94,9 +94,9 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
> "Failed to attach bridge for endpoint%u\n",
> of_ep.id);
>
> - connector = drm_bridge_connector_init(lcdif->drm, encoder);
> - if (IS_ERR(connector))
> - return dev_err_probe(dev, PTR_ERR(connector),
> + bridge_connector = drmm_bridge_connector_init(lcdif->drm, encoder);
Could this regression lead to a use-after-free if the physical device
is unbound?
The encoder passed to drmm_bridge_connector_init() is allocated earlier in
this function using devm_kzalloc(), which ties its lifetime to the physical
device.
However, drmm_bridge_connector_init() ties the new bridge connector to the DRM
device's lifetime and registers a global hotplug notifier that references this
encoder.
If the physical device is unbound while a userspace file descriptor is still
open, the DRM device and its notifier remain active after devres has freed the
encoder.
A subsequent bridge hotplug event (such as DRM_MIPI_DSI_ATTACHED) could then
trigger the notifier, causing it to dereference the freed encoder when locking
encoder->bridge_chain_mutex.
Would it be safer to allocate the encoder using drmm_encoder_alloc() instead
of devm_kzalloc() so its lifetime matches the DRM device?
> + if (IS_ERR(bridge_connector))
> + return dev_err_probe(dev, PTR_ERR(bridge_connector),
Does the new drmm_bridge_connector_init() API return a NULL pointer instead
of the allocated object on success?
Looking at the core implementation of drmm_bridge_connector_init(), it appears
to correctly allocate and initialize the bridge connector, but incorrectly ends
with return 0 instead of returning the allocated object.
Since IS_ERR() evaluates to false for NULL, the check here passes, but this
regression means any code trying to use the returned bridge_connector pointer
later will encounter a NULL pointer dereference.
Does the implementation in drivers/gpu/drm/display/drm_bridge_connector.c
need to be updated to return the allocated object?
> "Failed to init bridge_connector for endpoint%u\n",
> of_ep.id);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=37
^ permalink raw reply [flat|nested] 59+ messages in thread