[PATCH v2 00/37] drm/bridge: Convert all bridges to atomic

[PATCH v2 00/37] drm/bridge: Convert all bridges to atomic

[Maxime Ripard]

Hi,

Over the years, most of the bridges have been converted to atomic
modesetting and hooks, but not all of them. This forces us to maintain
two different code path in quite a few places, which is pretty
bothersome. The switch to atomic modesetting for legacy bridges though
is pretty trivial, and we don't have a lot of drivers still using the
legacy path.

This series converts all bridges to atomic modesetting and drops the
legacy codepaths where relevant.

Let me know what you think,
Maxime

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Changes in v2:
- Rebase on top of current drm-misc-next
- Improve commit messages
- Fix the references in the doc
- Link to v1: https://lore.kernel.org/r/20260617-drm-all-atomic-bridges-v1-0-b63e6316166b@kernel.org

---
Maxime Ripard (37):
      drm/adp: mipi: Add atomic bridge state handlers
      drm/bridge: analogix-anx6345: Switch to atomic bridge callbacks
      drm/bridge: analogix-anx78xx: Switch to atomic bridge callbacks
      drm/bridge: aux-bridge: Add atomic bridge state handlers
      drm/bridge: aux-hpd-bridge: Add atomic bridge state handlers
      drm/bridge: chrontel-ch7033: Switch to atomic bridge callbacks
      drm/bridge: cros-ec-anx7688: Add atomic bridge state handlers
      drm/bridge: lontium-lt8713sx: Add atomic bridge state handlers
      drm/bridge: lontium-lt8912b: Switch to atomic bridge callbacks
      drm/bridge: lontium-lt9611uxc: Add atomic bridge state handlers
      drm/bridge: lvds-codec: Switch to atomic bridge callbacks
      drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: Add atomic bridge state handlers
      drm/bridge: microchip-lvds: Add atomic bridge state handlers
      drm/bridge: nxp-ptn3460: Switch to atomic bridge callbacks
      drm/bridge: of-display-mode-bridge: Add atomic bridge state handlers
      drm/bridge: parade-ps8622: Switch to atomic bridge callbacks
      drm/bridge: sii9234: Add atomic bridge state handlers
      drm/bridge: sil-sii8620: Add atomic bridge state handlers
      drm/bridge: simple-bridge: Switch to atomic bridge callbacks
      drm/bridge: tc358764: Switch to atomic bridge callbacks
      drm/bridge: tda998x: Switch to atomic bridge callbacks
      drm/bridge: ti-tfp410: Switch to atomic bridge callbacks
      drm/bridge: ti-tpd12s015: Add atomic bridge state handlers
      drm/bridge: thc63lvd1024: Switch to atomic bridge callbacks
      drm/bridge: waveshare-dsi: Switch to atomic bridge callbacks
      drm/exynos: mic: Switch to atomic bridge callbacks
      drm/mcde: dsi: Add atomic bridge state handlers
      drm/mediatek: dpi: Switch to atomic bridge callbacks
      drm/msm: dsi: Switch to atomic bridge callbacks
      drm/omap: dpi: Switch to atomic bridge callbacks
      drm/omap: dsi: Switch to atomic bridge callbacks
      drm/omap: sdi: Switch to atomic bridge callbacks
      drm/omap: venc: Switch to atomic bridge callbacks
      drm/sti: dvo: Switch to atomic bridge callbacks
      drm/sti: hda: Switch to atomic bridge callbacks
      drm/sti: hdmi: Switch to atomic bridge callbacks
      drm/bridge: Remove legacy bridge callback support

 drivers/gpu/drm/adp/adp-mipi.c                     |   4 +
 drivers/gpu/drm/bridge/analogix/analogix-anx6345.c |  13 ++-
 drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c |  13 ++-
 drivers/gpu/drm/bridge/aux-bridge.c                |   4 +
 drivers/gpu/drm/bridge/aux-hpd-bridge.c            |   4 +
 drivers/gpu/drm/bridge/chrontel-ch7033.c           |  13 ++-
 drivers/gpu/drm/bridge/cros-ec-anx7688.c           |   4 +
 drivers/gpu/drm/bridge/lontium-lt8713sx.c          |   4 +
 drivers/gpu/drm/bridge/lontium-lt8912b.c           |   8 +-
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c         |   3 +
 drivers/gpu/drm/bridge/lvds-codec.c                |  10 +-
 .../drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c   |   3 +
 drivers/gpu/drm/bridge/microchip-lvds.c            |   3 +
 drivers/gpu/drm/bridge/nxp-ptn3460.c               |  17 ++--
 drivers/gpu/drm/bridge/of-display-mode-bridge.c    |   4 +
 drivers/gpu/drm/bridge/parade-ps8622.c             |  18 ++--
 drivers/gpu/drm/bridge/sii9234.c                   |   4 +
 drivers/gpu/drm/bridge/sil-sii8620.c               |   4 +
 drivers/gpu/drm/bridge/simple-bridge.c             |  13 ++-
 drivers/gpu/drm/bridge/tc358764.c                  |  13 ++-
 drivers/gpu/drm/bridge/tda998x_drv.c               |  13 ++-
 drivers/gpu/drm/bridge/thc63lvd1024.c              |  14 ++-
 drivers/gpu/drm/bridge/ti-tfp410.c                 |  10 +-
 drivers/gpu/drm/bridge/ti-tpd12s015.c              |   4 +
 drivers/gpu/drm/bridge/waveshare-dsi.c             |  14 ++-
 drivers/gpu/drm/drm_bridge.c                       |  61 ++++--------
 drivers/gpu/drm/exynos/exynos_drm_mic.c            |  14 ++-
 drivers/gpu/drm/mcde/mcde_dsi.c                    |   3 +
 drivers/gpu/drm/mediatek/mtk_dpi.c                 |  10 +-
 drivers/gpu/drm/msm/dsi/dsi_manager.c              |  13 ++-
 drivers/gpu/drm/omapdrm/dss/dpi.c                  |  14 ++-
 drivers/gpu/drm/omapdrm/dss/dsi.c                  |  14 ++-
 drivers/gpu/drm/omapdrm/dss/sdi.c                  |  14 ++-
 drivers/gpu/drm/omapdrm/dss/venc.c                 |  14 ++-
 drivers/gpu/drm/sti/sti_dvo.c                      |  20 ++--
 drivers/gpu/drm/sti/sti_hda.c                      |  20 ++--
 drivers/gpu/drm/sti/sti_hdmi.c                     |  20 ++--
 drivers/gpu/drm/tests/drm_bridge_test.c            | 104 ---------------------
 include/drm/drm_bridge.h                           | 103 --------------------
 39 files changed, 287 insertions(+), 351 deletions(-)
---
base-commit: 928e01b05e0a147db8df1152f95acbb801630308
change-id: 20260615-drm-all-atomic-bridges-4da4fe7df58f

Best regards,
-- 
Maxime Ripard <mripard@kernel.org>

[PATCH v2 01/37] drm/adp: mipi: Add atomic bridge state handlers

[Maxime Ripard]

The mipi bridge doesn't implement any enable or disable callbacks, but
is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Reviewed-by: Joshua Peisach <jpeisach@ubuntu.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
To: Sasha Finkelstein <k@chaosmail.tech>
Cc: Janne Grunau <j@jannau.net>
Cc: dri-devel@lists.freedesktop.org
Cc: asahi@lists.linux.dev
---
 drivers/gpu/drm/adp/adp-mipi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/adp/adp-mipi.c b/drivers/gpu/drm/adp/adp-mipi.c
index cba7d32150a9..232bb9539423 100644
--- a/drivers/gpu/drm/adp/adp-mipi.c
+++ b/drivers/gpu/drm/adp/adp-mipi.c
@@ -3,10 +3,11 @@
 #include <linux/component.h>
 #include <linux/iopoll.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_mipi_dsi.h>
 
 #define DSI_GEN_HDR 0x6c
 #define DSI_GEN_PLD_DATA 0x70
@@ -220,10 +221,13 @@ static int adp_dsi_bridge_attach(struct drm_bridge *bridge,
 
 	return drm_bridge_attach(encoder, adp->next_bridge, bridge, flags);
 }
 
 static const struct drm_bridge_funcs adp_dsi_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach	= adp_dsi_bridge_attach,
 };
 
 static int adp_mipi_probe(struct platform_device *pdev)
 {

-- 
2.54.0

Re: [PATCH v2 01/37] drm/adp: mipi: Add atomic bridge state handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Repeatedly binding and unbinding the DSI panel driver causes a `devres` memory leak and leaves a stale, UAF-prone bridge in the global bridge list.
--

--- Patch [1]: [PATCH v2 01/37] drm/adp: mipi: Add atomic bridge state handlers ---
commit b7611e75cc83200ed90d21e397cf46cc1f10033a
Author: Maxime Ripard <mripard@kernel.org>

drm/adp: mipi: Add atomic bridge state handlers

The mipi bridge doesn't implement any enable or disable callbacks, but
is still missing the atomic state handlers. In order to remove the
legacy bridge callback support from the DRM bridge core, add them.

> diff --git a/drivers/gpu/drm/adp/adp-mipi.c b/drivers/gpu/drm/adp/adp-mipi.c
> index cba7d32150a98..232bb95394237 100644
> --- a/drivers/gpu/drm/adp/adp-mipi.c
> +++ b/drivers/gpu/drm/adp/adp-mipi.c
[ ... ]
> @@ -222,6 +223,9 @@ static int adp_dsi_bridge_attach(struct drm_bridge *bridge,
>  }
>  
>  static const struct drm_bridge_funcs adp_dsi_bridge_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.attach	= adp_dsi_bridge_attach,
>  };
>  

[Severity: High]
This is a pre-existing issue, but while reviewing these bridge changes, I
noticed a resource management issue in adp_dsi_host_detach() in the same
file.

When a user unbinds the DSI panel driver, adp_dsi_host_detach() executes:

drivers/gpu/drm/adp/adp-mipi.c:adp_dsi_host_detach() {
    ...
    component_del(host->dev, &adp_dsi_component_ops);
    drm_bridge_remove(&adp->bridge);
    return 0;
}

However, during panel probe, adp_dsi_host_attach() allocates a panel bridge
using devm_drm_of_get_bridge(), tying the allocation to the host device
(adp->dsi.dev) rather than the panel device:

drivers/gpu/drm/adp/adp-mipi.c:adp_dsi_host_attach() {
    ...
    next = devm_drm_of_get_bridge(adp->dsi.dev, adp->dsi.dev->of_node, 1, 0);
    ...
}

Because the devres action is tied to the host device, unbinding the panel
does not free the bridge allocation. The drm_panel_bridge remains in the
global bridge list but points to the freed drm_panel.

If the panel is repeatedly unbound and bound, does this cause a devres memory
leak of bridge allocations?

Furthermore, if the DRM driver probes the bridge list later, could it
encounter this stale bridge and dereference the freed panel, leading to a
use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=1

[PATCH v2 02/37] drm/bridge: analogix-anx6345: Switch to atomic bridge callbacks

[Maxime Ripard]

The analogix-anx6345 bridge uses the legacy non-atomic enable and
disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
index f3fe47b12edc..eba5c6dcb5ad 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
@@ -564,11 +564,12 @@ anx6345_bridge_mode_valid(struct drm_bridge *bridge,
 		return MODE_CLOCK_HIGH;
 
 	return MODE_OK;
 }
 
-static void anx6345_bridge_disable(struct drm_bridge *bridge)
+static void anx6345_bridge_disable(struct drm_bridge *bridge,
+				   struct drm_atomic_commit *commit)
 {
 	struct anx6345 *anx6345 = bridge_to_anx6345(bridge);
 
 	/* Power off all modules except configuration registers access */
 	anx6345_set_bits(anx6345->map[I2C_IDX_TXCOM], SP_POWERDOWN_CTRL_REG,
@@ -578,11 +579,12 @@ static void anx6345_bridge_disable(struct drm_bridge *bridge)
 
 	if (anx6345->powered)
 		anx6345_poweroff(anx6345);
 }
 
-static void anx6345_bridge_enable(struct drm_bridge *bridge)
+static void anx6345_bridge_enable(struct drm_bridge *bridge,
+				  struct drm_atomic_commit *commit)
 {
 	struct anx6345 *anx6345 = bridge_to_anx6345(bridge);
 	int err;
 
 	if (anx6345->panel)
@@ -598,15 +600,18 @@ static void anx6345_bridge_enable(struct drm_bridge *bridge)
 	if (err)
 		DRM_ERROR("Failed to enable DP output: %d\n", err);
 }
 
 static const struct drm_bridge_funcs anx6345_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = anx6345_bridge_attach,
 	.detach = anx6345_bridge_detach,
 	.mode_valid = anx6345_bridge_mode_valid,
-	.disable = anx6345_bridge_disable,
-	.enable = anx6345_bridge_enable,
+	.atomic_disable = anx6345_bridge_disable,
+	.atomic_enable = anx6345_bridge_enable,
 };
 
 static void unregister_i2c_dummy_clients(struct anx6345 *anx6345)
 {
 	unsigned int i;

-- 
2.54.0

Re: [PATCH v2 02/37] drm/bridge: analogix-anx6345: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The panel is enabled before it is prepared (powered on) in the bridge enable sequence, violating the DRM panel API contracts and causing potential hardware issues.
- [High] Bridge `atomic_enable` and `atomic_disable` callbacks invoke sleeping functions, violating atomic context constraints.
--

--- Patch [2]: [PATCH v2 02/37] drm/bridge: analogix-anx6345: Switch to atomic bridge callbacks ---
commit c3398824f07e9152c87164f9494fed0231949fb3
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: analogix-anx6345: Switch to atomic bridge callbacks
    
The analogix-anx6345 bridge uses the legacy non-atomic enable and
disable bridge callbacks.
    
In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> index f3fe47b12edca..eba5c6dcb5ada 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
[ ... ]
> @@ -580,7 +581,8 @@ static void anx6345_bridge_disable(struct drm_bridge *bridge)
>  		anx6345_poweroff(anx6345);
>  }
>  
> -static void anx6345_bridge_enable(struct drm_bridge *bridge)
> +static void anx6345_bridge_enable(struct drm_bridge *bridge,
> +				  struct drm_atomic_commit *commit)
>  {
>  	struct anx6345 *anx6345 = bridge_to_anx6345(bridge);
>  	int err;
> @@ -600,11 +602,14 @@ static void anx6345_bridge_enable(struct drm_bridge *bridge)

[Severity: High]
This is a pre-existing issue, but does this function enable the panel before
it is prepared?

In anx6345_bridge_enable(), we have:

	if (anx6345->panel)
		drm_panel_enable(anx6345->panel);

	err = anx6345_start(anx6345);

And anx6345_start() eventually calls anx6345_poweron() which prepares the
panel:

	if (anx6345->panel)
		drm_panel_prepare(anx6345->panel);

On a secondary modeset (after the device has been disabled),
anx6345->powered is false. anx6345_bridge_enable() calls drm_panel_enable()
which turns on the backlight. It then calls anx6345_start() which invokes
anx6345_poweron() and drm_panel_prepare() to power on the LCD logic.

Could this inverted enable -> prepare sequence turn on the backlight while
the panel power is off, potentially causing latch-up and physically damaging
the LCD logic on some displays?

>  }
>  
>  static const struct drm_bridge_funcs anx6345_bridge_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.attach = anx6345_bridge_attach,
>  	.detach = anx6345_bridge_detach,
>  	.mode_valid = anx6345_bridge_mode_valid,
> -	.disable = anx6345_bridge_disable,
> -	.enable = anx6345_bridge_enable,
> +	.atomic_disable = anx6345_bridge_disable,
> +	.atomic_enable = anx6345_bridge_enable,

[Severity: High]
This is a pre-existing issue, but does mapping these callbacks to atomic
contexts conflict with the sleeping functions they invoke?

The bridge atomic_enable and atomic_disable callbacks execute in the context
of atomic commits (e.g., drm_atomic_helper_commit_tail), where sleeping is
forbidden.

However, anx6345_bridge_enable() and anx6345_bridge_disable() invoke
hardware sequencing functions like anx6345_start(), anx6345_poweron(), and
anx6345_poweroff().

For example, anx6345_start() contains:

	usleep_range(10000, 15000);

And anx6345_poweron() contains:

	gpiod_set_value_cansleep(anx6345->gpiod_reset, 1);
	usleep_range(1000, 2000);

Could calling these sleeping functions from the atomic context cause kernel
panics or BUG warnings (scheduling while atomic)?

>  };
>  
>  static void unregister_i2c_dummy_clients(struct anx6345 *anx6345)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=2

[PATCH v2 03/37] drm/bridge: analogix-anx78xx: Switch to atomic bridge callbacks

[Maxime Ripard]

The analogix-anx78xx bridge uses the legacy non-atomic enable and
disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
index ba0fc149a9e7..32186efa6f50 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
@@ -935,11 +935,12 @@ anx78xx_bridge_mode_valid(struct drm_bridge *bridge,
 		return MODE_CLOCK_HIGH;
 
 	return MODE_OK;
 }
 
-static void anx78xx_bridge_disable(struct drm_bridge *bridge)
+static void anx78xx_bridge_disable(struct drm_bridge *bridge,
+				   struct drm_atomic_commit *commit)
 {
 	struct anx78xx *anx78xx = bridge_to_anx78xx(bridge);
 
 	/* Power off all modules except configuration registers access */
 	anx78xx_set_bits(anx78xx->map[I2C_IDX_TX_P2], SP_POWERDOWN_CTRL_REG,
@@ -973,11 +974,12 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
 
 unlock:
 	mutex_unlock(&anx78xx->lock);
 }
 
-static void anx78xx_bridge_enable(struct drm_bridge *bridge)
+static void anx78xx_bridge_enable(struct drm_bridge *bridge,
+				  struct drm_atomic_commit *commit)
 {
 	struct anx78xx *anx78xx = bridge_to_anx78xx(bridge);
 	int err;
 
 	err = anx78xx_start(anx78xx);
@@ -990,16 +992,19 @@ static void anx78xx_bridge_enable(struct drm_bridge *bridge)
 	if (err)
 		DRM_ERROR("Failed to set HPD: %d\n", err);
 }
 
 static const struct drm_bridge_funcs anx78xx_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = anx78xx_bridge_attach,
 	.detach = anx78xx_bridge_detach,
 	.mode_valid = anx78xx_bridge_mode_valid,
-	.disable = anx78xx_bridge_disable,
+	.atomic_disable = anx78xx_bridge_disable,
 	.mode_set = anx78xx_bridge_mode_set,
-	.enable = anx78xx_bridge_enable,
+	.atomic_enable = anx78xx_bridge_enable,
 };
 
 static irqreturn_t anx78xx_hpd_threaded_handler(int irq, void *data)
 {
 	struct anx78xx *anx78xx = data;

-- 
2.54.0

[PATCH v2 04/37] drm/bridge: aux-bridge: Add atomic bridge state handlers

[Maxime Ripard]

The aux-bridge bridge doesn't implement any enable or disable callbacks,
but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/aux-bridge.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c
index 1ed21a8713bf..169939d23310 100644
--- a/drivers/gpu/drm/bridge/aux-bridge.c
+++ b/drivers/gpu/drm/bridge/aux-bridge.c
@@ -7,10 +7,11 @@
 #include <linux/auxiliary_bus.h>
 #include <linux/export.h>
 #include <linux/module.h>
 #include <linux/of.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/bridge/aux-bridge.h>
 
 static DEFINE_IDA(drm_aux_bridge_ida);
 
@@ -102,10 +103,13 @@ static int drm_aux_bridge_attach(struct drm_bridge *bridge,
 	return drm_bridge_attach(encoder, data->next_bridge, bridge,
 				 DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 }
 
 static const struct drm_bridge_funcs drm_aux_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach	= drm_aux_bridge_attach,
 };
 
 static int drm_aux_bridge_probe(struct auxiliary_device *auxdev,
 				const struct auxiliary_device_id *id)

-- 
2.54.0

[PATCH v2 05/37] drm/bridge: aux-hpd-bridge: Add atomic bridge state handlers

[Maxime Ripard]

The aux-hpd-bridge bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/aux-hpd-bridge.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
index f02a38a2638a..a2e525aa5788 100644
--- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
+++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
@@ -7,10 +7,11 @@
 #include <linux/auxiliary_bus.h>
 #include <linux/export.h>
 #include <linux/module.h>
 #include <linux/of.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/bridge/aux-bridge.h>
 
 static DEFINE_IDA(drm_aux_hpd_bridge_ida);
 
@@ -163,10 +164,13 @@ static int drm_aux_hpd_bridge_attach(struct drm_bridge *bridge,
 {
 	return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
 }
 
 static const struct drm_bridge_funcs drm_aux_hpd_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach	= drm_aux_hpd_bridge_attach,
 };
 
 static int drm_aux_hpd_bridge_probe(struct auxiliary_device *auxdev,
 				    const struct auxiliary_device_id *id)

-- 
2.54.0

[PATCH v2 06/37] drm/bridge: chrontel-ch7033: Switch to atomic bridge callbacks

[Maxime Ripard]

The chrontel-ch7033 bridge uses the legacy non-atomic enable and disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/chrontel-ch7033.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c b/drivers/gpu/drm/bridge/chrontel-ch7033.c
index a237c65ebd69..92d464727e41 100644
--- a/drivers/gpu/drm/bridge/chrontel-ch7033.c
+++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
@@ -328,19 +328,21 @@ static enum drm_mode_status ch7033_bridge_mode_valid(struct drm_bridge *bridge,
 	if (mode->vdisplay >= 1080)
 		return MODE_BAD_VVALUE;
 	return MODE_OK;
 }
 
-static void ch7033_bridge_disable(struct drm_bridge *bridge)
+static void ch7033_bridge_disable(struct drm_bridge *bridge,
+				  struct drm_atomic_commit *commit)
 {
 	struct ch7033_priv *priv = bridge_to_ch7033_priv(bridge);
 
 	regmap_write(priv->regmap, 0x03, 0x04);
 	regmap_update_bits(priv->regmap, 0x52, RESETDB, 0x00);
 }
 
-static void ch7033_bridge_enable(struct drm_bridge *bridge)
+static void ch7033_bridge_enable(struct drm_bridge *bridge,
+				 struct drm_atomic_commit *commit)
 {
 	struct ch7033_priv *priv = bridge_to_ch7033_priv(bridge);
 
 	regmap_write(priv->regmap, 0x03, 0x04);
 	regmap_update_bits(priv->regmap, 0x52, RESETDB, RESETDB);
@@ -512,15 +514,18 @@ static void ch7033_bridge_mode_set(struct drm_bridge *bridge,
 	regmap_write(priv->regmap, 0x11, mode->clock >> 8);
 	regmap_write(priv->regmap, 0x12, mode->clock);
 }
 
 static const struct drm_bridge_funcs ch7033_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = ch7033_bridge_attach,
 	.detach = ch7033_bridge_detach,
 	.mode_valid = ch7033_bridge_mode_valid,
-	.disable = ch7033_bridge_disable,
-	.enable = ch7033_bridge_enable,
+	.atomic_disable = ch7033_bridge_disable,
+	.atomic_enable = ch7033_bridge_enable,
 	.mode_set = ch7033_bridge_mode_set,
 };
 
 static const struct regmap_config ch7033_regmap_config = {
 	.reg_bits = 8,

-- 
2.54.0

[PATCH v2 07/37] drm/bridge: cros-ec-anx7688: Add atomic bridge state handlers

[Maxime Ripard]

The cros-ec-anx7688 bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Benson Leung <bleung@chromium.org>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: chrome-platform@lists.linux.dev
---
 drivers/gpu/drm/bridge/cros-ec-anx7688.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/cros-ec-anx7688.c b/drivers/gpu/drm/bridge/cros-ec-anx7688.c
index a35dae9b56e2..da16c793e3ba 100644
--- a/drivers/gpu/drm/bridge/cros-ec-anx7688.c
+++ b/drivers/gpu/drm/bridge/cros-ec-anx7688.c
@@ -3,10 +3,11 @@
  * CrOS EC ANX7688 HDMI->DP bridge driver
  *
  * Copyright 2020 Google LLC
  */
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_print.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
@@ -90,10 +91,13 @@ static bool cros_ec_anx7688_bridge_mode_fixup(struct drm_bridge *bridge,
 
 	return totalbw >= requiredbw;
 }
 
 static const struct drm_bridge_funcs cros_ec_anx7688_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.mode_fixup = cros_ec_anx7688_bridge_mode_fixup,
 };
 
 static int cros_ec_anx7688_bridge_probe(struct i2c_client *client)
 {

-- 
2.54.0

Re: [PATCH v2 07/37] drm/bridge: cros-ec-anx7688: Add atomic bridge state handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] NULL pointer dereference in `drm_state_dump` if the initial atomic bridge state allocation fails.
--

--- Patch [7]: [PATCH v2 07/37] drm/bridge: cros-ec-anx7688: Add atomic bridge state handlers ---
commit 45bb4ccee956e78effd5cb908d42d26a9cefaebb
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: cros-ec-anx7688: Add atomic bridge state handlers

This commit adds atomic state handlers to the cros-ec-anx7688 bridge. This is
done to remove the legacy bridge callback support from the DRM bridge core.

> diff --git a/drivers/gpu/drm/bridge/cros-ec-anx7688.c b/drivers/gpu/drm/bridge/cros-ec-anx7688.c
> --- a/drivers/gpu/drm/bridge/cros-ec-anx7688.c
> +++ b/drivers/gpu/drm/bridge/cros-ec-anx7688.c

[ ... ]

> @@ -92,6 +93,9 @@ static bool cros_ec_anx7688_bridge_mode_fixup(struct drm_bridge *bridge,
>  }
>  
>  static const struct drm_bridge_funcs cros_ec_anx7688_bridge_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,

[Severity: High]
This isn't a bug introduced by this patch, but adding atomic state handlers
causes drm_bridge_is_atomic() to return true, which exposes a pre-existing
NULL pointer dereference in the DRM core.

When drm_bridge_attach() calls drm_atomic_private_obj_init(), the object
is added to privobj_list before the state is successfully allocated:

drivers/gpu/drm/drm_atomic.c:drm_atomic_private_obj_init() {
	list_add_tail(&obj->head, &dev->mode_config.privobj_list);

	state = obj->funcs->atomic_create_state(obj);
	if (IS_ERR(state))
		return PTR_ERR(state);

	obj->state = state;
}

If atomic_create_state() fails, drm_bridge_attach() ignores the error:

drivers/gpu/drm/drm_bridge.c:drm_bridge_attach() {
	if (drm_bridge_is_atomic(bridge))
		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
					    &drm_bridge_priv_state_funcs);

	return 0;
}

This leaves the object on privobj_list with a NULL obj->state.

If drm_state_dump() is later triggered, it will crash when it unconditionally
dereferences the NULL state:

drivers/gpu/drm/drm_atomic.c:drm_atomic_private_obj_print_state() {
	struct drm_private_obj *obj = state->obj;
}

Could drm_bridge_attach() be updated to check the return value of
drm_atomic_private_obj_init(), or should drm_atomic_private_obj_init()
remove the object from the list on failure?

> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.mode_fixup = cros_ec_anx7688_bridge_mode_fixup,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=7

[PATCH v2 08/37] drm/bridge: lontium-lt8713sx: Add atomic bridge state handlers

[Maxime Ripard]

The lontium-lt8713sx bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/lontium-lt8713sx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8713sx.c b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
index cee485adf5e5..4a6420b876da 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8713sx.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
@@ -16,10 +16,11 @@
 #include <linux/regulator/consumer.h>
 #include <linux/sizes.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_of.h>
 
 #define FW_FILE "lt8713sx_fw.bin"
 
@@ -506,10 +507,13 @@ static const struct attribute_group *lt8713sx_attr_groups[] = {
 	&lt8713sx_attr_group,
 	NULL,
 };
 
 static const struct drm_bridge_funcs lt8713sx_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = lt8713sx_bridge_attach,
 };
 
 static int lt8713sx_probe(struct i2c_client *client)
 {

-- 
2.54.0

Re: [PATCH v2 08/37] drm/bridge: lontium-lt8713sx: Add atomic bridge state handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Double-free of the firmware object in the error paths of firmware update.
- [High] Missing minimum firmware size check causes massive buffer overflow/underflow.
- [High] drm_atomic_private_obj_init() leaves the object in privobj_list on error, and drm_bridge_attach() ignores the error, leading to crashes.
--

--- Patch [8]: [PATCH v2 08/37] drm/bridge: lontium-lt8713sx: Add atomic bridge state handlers ---
commit cc389f313911703ff4573192b0ab28ddca13b386
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: lontium-lt8713sx: Add atomic bridge state handlers

The lontium-lt8713sx bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers. In order to
remove the legacy bridge callback support from the DRM bridge core, add them.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt8713sx.c b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
> index cee485adf5e56..4a6420b876da6 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8713sx.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8713sx.c

[ ... ]

> @@ -508,6 +509,9 @@ static const struct attribute_group *lt8713sx_attr_groups[] = {
>  };
>  
>  static const struct drm_bridge_funcs lt8713sx_bridge_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.attach = lt8713sx_bridge_attach,
>  };

[Severity: High]
This isn't a bug introduced by this patch, but by adding .atomic_create_state
you are enabling the atomic path in drm_bridge_attach() which will now call
drm_atomic_private_obj_init().

If drm_atomic_private_obj_init() fails, it returns an error but leaves the
object in dev->mode_config.privobj_list with a NULL obj->state:

drm_atomic.c:drm_atomic_private_obj_init()
	list_add_tail(&obj->head, &dev->mode_config.privobj_list);

	state = obj->funcs->atomic_create_state(obj);
	if (IS_ERR(state))
		return PTR_ERR(state);

	obj->state = state;

Then drm_bridge_attach() ignores this return value:

drm_bridge.c:drm_bridge_attach()
	if (drm_bridge_is_atomic(bridge))
		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
					    &drm_bridge_priv_state_funcs);

	return 0;

If a state dump is triggered later, drm_state_dump() iterates over
privobj_list and calls drm_atomic_private_obj_print_state(p, obj->state).
Since obj->state is NULL, this will cause a NULL pointer dereference.

Should the return value of drm_atomic_private_obj_init() be checked in
drm_bridge_attach(), and should drm_atomic_private_obj_init() remove the
object from the list on failure?

[Severity: High]
This is a pre-existing issue, but I noticed a potential double-free in the
firmware update error paths of this driver.

In lt8713sx_prepare_firmware_data():

	release_firmware(lt8713sx->fw);
	return -EINVAL;

This frees the firmware object but leaves lt8713sx->fw pointing to the freed
memory. Then the caller lt8713sx_firmware_update() handles the error:

error:
	if (lt8713sx->fw) {
		release_firmware(lt8713sx->fw);
		lt8713sx->fw = NULL;
	}

Could this result in a double-free of the firmware object?

[Severity: High]
This is a pre-existing issue, but I also noticed a potential buffer overflow
in lt8713sx_prepare_firmware_data():

	memcpy(lt8713sx->fw_buffer, lt8713sx->fw->data, SZ_64K - 1);
	...
	memcpy(lt8713sx->fw_buffer + SZ_64K,
	       lt8713sx->fw->data + SZ_64K,
	       lt8713sx->fw->size - SZ_64K);

If a user provides a firmware file smaller than 64KB, would the first memcpy
read out of bounds, and would lt8713sx->fw->size - SZ_64K underflow resulting
in a massive out-of-bounds copy?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=8

[PATCH v2 09/37] drm/bridge: lontium-lt8912b: Switch to atomic bridge callbacks

[Maxime Ripard]

The lontium-lt8912b bridge uses the legacy non-atomic enable bridge
callback.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Acked-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # Verdin AM62
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
To: Francesco Dolcini <francesco@dolcini.it>
---
 drivers/gpu/drm/bridge/lontium-lt8912b.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index 729b12b67470..df5d07e7644c 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -455,11 +455,12 @@ static void lt8912_bridge_mode_set(struct drm_bridge *bridge,
 	struct lt8912 *lt = bridge_to_lt8912(bridge);
 
 	drm_display_mode_to_videomode(adj, &lt->mode);
 }
 
-static void lt8912_bridge_enable(struct drm_bridge *bridge)
+static void lt8912_bridge_enable(struct drm_bridge *bridge,
+				 struct drm_atomic_commit *commit)
 {
 	struct lt8912 *lt = bridge_to_lt8912(bridge);
 
 	lt8912_video_on(lt);
 }
@@ -632,15 +633,18 @@ static const struct drm_edid *lt8912_bridge_edid_read(struct drm_bridge *bridge,
 	dev_warn(lt->dev, "The connected bridge does not supports DRM_BRIDGE_OP_EDID\n");
 	return NULL;
 }
 
 static const struct drm_bridge_funcs lt8912_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = lt8912_bridge_attach,
 	.detach = lt8912_bridge_detach,
 	.mode_valid = lt8912_bridge_mode_valid,
 	.mode_set = lt8912_bridge_mode_set,
-	.enable = lt8912_bridge_enable,
+	.atomic_enable = lt8912_bridge_enable,
 	.detect = lt8912_bridge_detect,
 	.edid_read = lt8912_bridge_edid_read,
 };
 
 static int lt8912_bridge_resume(struct device *dev)

-- 
2.54.0

[PATCH v2 10/37] drm/bridge: lontium-lt9611uxc: Add atomic bridge state handlers

[Maxime Ripard]

The lontium-lt9611uxc bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index 9427cc2358ae..6625ba23761c 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -463,10 +463,13 @@ static void lt9611uxc_hdmi_audio_shutdown(struct drm_bridge *bridge,
 					  struct drm_connector *connector)
 {
 }
 
 static const struct drm_bridge_funcs lt9611uxc_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = lt9611uxc_bridge_attach,
 	.mode_valid = lt9611uxc_bridge_mode_valid,
 	.mode_set = lt9611uxc_bridge_mode_set,
 	.detect = lt9611uxc_bridge_detect,
 	.edid_read = lt9611uxc_bridge_edid_read,

-- 
2.54.0

[PATCH v2 11/37] drm/bridge: lvds-codec: Switch to atomic bridge callbacks

[Maxime Ripard]

The lvds-codec bridge uses the legacy non-atomic enable and disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/lvds-codec.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c
index d1135dc3b99f..a82ea0c944eb 100644
--- a/drivers/gpu/drm/bridge/lvds-codec.c
+++ b/drivers/gpu/drm/bridge/lvds-codec.c
@@ -41,11 +41,12 @@ static int lvds_codec_attach(struct drm_bridge *bridge,
 
 	return drm_bridge_attach(encoder, lvds_codec->panel_bridge,
 				 bridge, flags);
 }
 
-static void lvds_codec_enable(struct drm_bridge *bridge)
+static void lvds_codec_enable(struct drm_bridge *bridge,
+			      struct drm_atomic_commit *commit)
 {
 	struct lvds_codec *lvds_codec = to_lvds_codec(bridge);
 	int ret;
 
 	ret = regulator_enable(lvds_codec->vcc);
@@ -57,11 +58,12 @@ static void lvds_codec_enable(struct drm_bridge *bridge)
 
 	if (lvds_codec->powerdown_gpio)
 		gpiod_set_value_cansleep(lvds_codec->powerdown_gpio, 0);
 }
 
-static void lvds_codec_disable(struct drm_bridge *bridge)
+static void lvds_codec_disable(struct drm_bridge *bridge,
+			       struct drm_atomic_commit *commit)
 {
 	struct lvds_codec *lvds_codec = to_lvds_codec(bridge);
 	int ret;
 
 	if (lvds_codec->powerdown_gpio)
@@ -98,12 +100,12 @@ lvds_codec_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
 	return input_fmts;
 }
 
 static const struct drm_bridge_funcs funcs = {
 	.attach = lvds_codec_attach,
-	.enable = lvds_codec_enable,
-	.disable = lvds_codec_disable,
+	.atomic_enable = lvds_codec_enable,
+	.atomic_disable = lvds_codec_disable,
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_create_state = drm_atomic_helper_bridge_create_state,
 	.atomic_get_input_bus_fmts = lvds_codec_atomic_get_input_bus_fmts,
 };

-- 
2.54.0

[PATCH v2 12/37] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: Add atomic bridge state handlers

[Maxime Ripard]

The megachips-stdpxxxx-ge-b850v3-fw bridge doesn't implement any enable
or disable callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Reviewed-by: Ian Ray <ian.ray@gehealthcare.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Peter Senna Tschudin <peter.senna@gmail.com>
To: Ian Ray <ian.ray@ge.com>
To: Martyn Welch <martyn.welch@collabora.co.uk>
---
 drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
index 2d02cc69f237..25ff0a0acc93 100644
--- a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
+++ b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
@@ -212,10 +212,13 @@ static int ge_b850v3_lvds_attach(struct drm_bridge *bridge,
 
 	return ge_b850v3_lvds_create_connector(bridge);
 }
 
 static const struct drm_bridge_funcs ge_b850v3_lvds_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = ge_b850v3_lvds_attach,
 	.detect = ge_b850v3_lvds_bridge_detect,
 	.edid_read = ge_b850v3_lvds_edid_read,
 };
 

-- 
2.54.0

Re: [PATCH v2 12/37] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: Add atomic bridge state handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The IRQ handler dereferences a global pointer that is set to NULL on partial device unbind, leading to a race condition and a guaranteed kernel crash if an interrupt fires.
- [High] Unbinding and rebinding either of the I2C drivers permanently breaks the bridge.
--

--- Patch [12]: [PATCH v2 12/37] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: Add atomic bridge state handlers ---
commit a14f762105b409d757000a124c825edac22d569b
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: Add atomic bridge state handlers

The megachips-stdpxxxx-ge-b850v3-fw bridge doesn't implement any enable
or disable callbacks, but is still missing the atomic state handlers.
In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

> diff --git a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> index 2d02cc69f2374..25ff0a0acc934 100644
> --- a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> +++ b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> @@ -214,6 +214,9 @@ static int ge_b850v3_lvds_attach(struct drm_bridge *bridge,
>  }
>  
>  static const struct drm_bridge_funcs ge_b850v3_lvds_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.attach = ge_b850v3_lvds_attach,
>  	.detect = ge_b850v3_lvds_bridge_detect,
>  	.edid_read = ge_b850v3_lvds_edid_read,
>  };

[Severity: High]
This is a pre-existing issue, but unbinding and rebinding either of the I2C
drivers appears to permanently break the bridge.

When either device unbinds, ge_b850v3_lvds_remove() clears the global
ge_b850v3_lvds_ptr:

drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c:ge_b850v3_lvds_remove() {
    ...
    ge_b850v3_lvds_ptr = NULL;
    ...
}

If the unbound device is rebound, ge_b850v3_lvds_init() allocates a new struct:

drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c:ge_b850v3_lvds_init() {
    ...
    ge_b850v3_lvds_ptr = devm_drm_bridge_alloc(dev, struct ge_b850v3_lvds,
                                               bridge, &ge_b850v3_lvds_funcs);
    ...
}

Because the other device's probe function does not run again, the new struct 
never receives the i2c_client pointer for the other device. Does this cause the 
initialization check in stdp2690_ge_b850v3_fw_probe() to fail and prevent 
the bridge from registering?

drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c:stdp2690_ge_b850v3_fw_probe() {
    ...
    /* Only register after both bridges are probed */
    if (!ge_b850v3_lvds_ptr->stdp4028_i2c)
        return 0;
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but could this logic also introduce
a race condition leading to a NULL pointer dereference?

Because the IRQ is managed via devm_request_threaded_irq() on stdp4028, it 
remains active until devres cleanup runs. If the interrupt fires after 
ge_b850v3_lvds_remove() sets ge_b850v3_lvds_ptr to NULL, the IRQ handler 
unconditionally dereferences the NULL pointer:

drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c:ge_b850v3_lvds_irq_handler() {
    struct i2c_client *stdp4028_i2c = ge_b850v3_lvds_ptr->stdp4028_i2c;
    ...
}

Could this lead to a kernel panic if an interrupt occurs during or after an 
unbind?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=12

[PATCH v2 13/37] drm/bridge: microchip-lvds: Add atomic bridge state handlers

[Maxime Ripard]

The microchip-lvds bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Reviewed-by: Manikandan Muralidharan <manikandan.m@microchip.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
To: Manikandan Muralidharan <manikandan.m@microchip.com>
To: Dharma Balasubiramani <dharma.b@microchip.com>
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index 5fb8633f43c5..dd4ffc9f4df3 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -165,10 +165,13 @@ static void mchp_lvds_atomic_disable(struct drm_bridge *bridge,
 	pm_runtime_put(lvds->dev);
 	clk_disable_unprepare(lvds->pclk);
 }
 
 static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = mchp_lvds_attach,
 	.atomic_enable = mchp_lvds_atomic_enable,
 	.atomic_disable = mchp_lvds_atomic_disable,
 };
 

-- 
2.54.0

Re: [PATCH v2 13/37] drm/bridge: microchip-lvds: Add atomic bridge state handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing `.remove` hook for `drm_bridge_remove` causes a use-after-free upon module unload or device unbind.
- [High] Sleeping function usleep_range() is called from an atomic commit path.
- [High] Incorrect masking of the WPKEY password truncates the unlock key, preventing hardware configuration.
--

--- Patch [13]: [PATCH v2 13/37] drm/bridge: microchip-lvds: Add atomic bridge state handlers ---
commit 489328dd1eddcba7ecc56ad3f142d3a6ed0da2f5
Author: Maxime Ripard <mripard@kernel.org>
drm/bridge: microchip-lvds: Add atomic bridge state handlers

The microchip-lvds bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers. In order to
remove the legacy bridge callback support from the DRM bridge core, add
them.

> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
> index 5fb8633f43c5c..dd4ffc9f4df3f 100644
> --- a/drivers/gpu/drm/bridge/microchip-lvds.c
> +++ b/drivers/gpu/drm/bridge/microchip-lvds.c
> @@ -167,6 +167,9 @@ static void mchp_lvds_atomic_disable(struct drm_bridge *bridge,
>  }
>  
>  static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.attach = mchp_lvds_attach,
>  	.atomic_enable = mchp_lvds_atomic_enable,

[Severity: High]
This isn't a bug introduced by this patch, but does the atomic enable path
safely handle sleeping functions?

The subsystem guide states that bridge atomic enable callbacks run in
atomic context. However, mchp_lvds_atomic_enable() calls
lvds_serialiser_on(), which has a hardware polling loop using usleep_range():

drivers/gpu/drm/bridge/microchip-lvds.c:lvds_serialiser_on() {
	...
	usleep_range(1000, 2000);
	...
}

Can this cause kernel warnings or potential deadlocks during the atomic
commit path?

>  	.atomic_disable = mchp_lvds_atomic_disable,
>  };

[Severity: High]
This is a pre-existing issue, but is the WPKEY password being truncated
during initialization?

In lvds_serialiser_on(), the password is masked directly without shifting:

drivers/gpu/drm/bridge/microchip-lvds.c:lvds_serialiser_on() {
	...
	lvds_writel(lvds, LVDSC_WPMR, (LVDSC_WPMR_WPKEY_PSSWD &
				       LVDSC_WPMR_WPKEY_MASK));
	...
}

If LVDSC_WPMR_WPKEY_PSSWD is 0x4C5644 and LVDSC_WPMR_WPKEY_MASK is
GENMASK(31, 8) (0xFFFFFF00), the lower byte 'D' (0x44) gets truncated to
0x00. 

Does this prevent the write protection from being unlocked, causing
subsequent LVDS configuration writes to be ignored by the hardware?

[Severity: Critical]
This is also a pre-existing issue, but does the driver leave the bridge
registered on unload?

The bridge is registered globally in mchp_lvds_probe() using
drm_bridge_add(), but the platform driver lacks a .remove callback:

drivers/gpu/drm/bridge/microchip-lvds.c:mchp_lvds_driver {
	.probe = mchp_lvds_probe,
	.driver = {
		.name = "microchip-lvds",
		.of_match_table = mchp_lvds_dt_ids,
	},
};

Since drm_bridge_remove() is never called to unlink the bridge, and the
mchp_lvds structure is freed automatically by devres, could this lead to a
use-after-free the next time the DRM core traverses the global bridge list?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=13

[PATCH v2 14/37] drm/bridge: nxp-ptn3460: Switch to atomic bridge callbacks

[Maxime Ripard]

The nxp-ptn3460 bridge uses the legacy non-atomic pre_enable and disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/nxp-ptn3460.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index 7acb11f16dc1..a9b4ac2defc9 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -109,11 +109,12 @@ static int ptn3460_select_edid(struct ptn3460_bridge *ptn_bridge)
 	}
 
 	return 0;
 }
 
-static void ptn3460_pre_enable(struct drm_bridge *bridge)
+static void ptn3460_pre_enable(struct drm_bridge *bridge,
+			       struct drm_atomic_commit *commit)
 {
 	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
 	int ret;
 
 	if (ptn_bridge->enabled)
@@ -137,11 +138,12 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
 		DRM_ERROR("Select EDID failed ret=%d\n", ret);
 
 	ptn_bridge->enabled = true;
 }
 
-static void ptn3460_disable(struct drm_bridge *bridge)
+static void ptn3460_disable(struct drm_bridge *bridge,
+			    struct drm_atomic_commit *commit)
 {
 	struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
 
 	if (!ptn_bridge->enabled)
 		return;
@@ -161,11 +163,11 @@ static const struct drm_edid *ptn3460_edid_read(struct drm_bridge *bridge,
 	bool power_off;
 	u8 *edid;
 	int ret;
 
 	power_off = !ptn_bridge->enabled;
-	ptn3460_pre_enable(&ptn_bridge->bridge);
+	ptn3460_pre_enable(&ptn_bridge->bridge, NULL);
 
 	edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
 	if (!edid) {
 		DRM_ERROR("Failed to allocate EDID\n");
 		goto out;
@@ -180,11 +182,11 @@ static const struct drm_edid *ptn3460_edid_read(struct drm_bridge *bridge,
 
 	drm_edid = drm_edid_alloc(edid, EDID_LENGTH);
 
 out:
 	if (power_off)
-		ptn3460_disable(&ptn_bridge->bridge);
+		ptn3460_disable(&ptn_bridge->bridge, NULL);
 
 	return drm_edid;
 }
 
 static int ptn3460_connector_get_modes(struct drm_connector *connector)
@@ -246,12 +248,15 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
 
 	return ret;
 }
 
 static const struct drm_bridge_funcs ptn3460_bridge_funcs = {
-	.pre_enable = ptn3460_pre_enable,
-	.disable = ptn3460_disable,
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_pre_enable = ptn3460_pre_enable,
+	.atomic_disable = ptn3460_disable,
 	.attach = ptn3460_bridge_attach,
 	.edid_read = ptn3460_edid_read,
 };
 
 static int ptn3460_probe(struct i2c_client *client)

-- 
2.54.0

Re: [PATCH v2 14/37] drm/bridge: nxp-ptn3460: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Memory leak of the dynamically allocated edid buffer in ptn3460_edid_read on the success and drm_edid_alloc failure paths.
- [High] Lockless concurrent access to the bridge's power state (ptn_bridge->enabled) causes a TOCTOU race and hardware state mismatch.
--

--- Patch [14]: [PATCH v2 14/37] drm/bridge: nxp-ptn3460: Switch to atomic bridge callbacks ---
commit 3ac8d8062ac1622a61e5a518f71eee0beb4385a3
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: nxp-ptn3460: Switch to atomic bridge callbacks

The nxp-ptn3460 bridge uses the legacy non-atomic pre_enable and disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

> diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
> index 7acb11f16dc19..a9b4ac2defc99 100644
> --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
> +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
[ ... ]
> @@ -163,7 +165,7 @@ static const struct drm_edid *ptn3460_edid_read(struct drm_bridge *bridge,
>  	int ret;
>  
>  	power_off = !ptn_bridge->enabled;
> -	ptn3460_pre_enable(&ptn_bridge->bridge);
> +	ptn3460_pre_enable(&ptn_bridge->bridge, NULL);

[Severity: High]
This is a pre-existing issue, but does lockless access to ptn_bridge->enabled
create a race condition with asynchronous atomic commits?

If ptn3460_edid_read() starts while the display is off, it saves
power_off = true. If an asynchronous DRM atomic commit worker simultaneously
invokes ptn3460_pre_enable(), it sets ptn_bridge->enabled = true and
powers up the hardware.

>  
>  	edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
>  	if (!edid) {
> @@ -182,7 +184,7 @@ static const struct drm_edid *ptn3460_edid_read(struct drm_bridge *bridge,
>  
>  out:
>  	if (power_off)
> -		ptn3460_disable(&ptn_bridge->bridge);
> +		ptn3460_disable(&ptn_bridge->bridge, NULL);

[Severity: High]
This isn't a bug introduced by this patch, but does this leak the dynamically
allocated edid buffer?

drm_edid_alloc() duplicates the data into a new container but does not take
ownership of the original pointer. When ptn3460_edid_read() reaches this out
block and returns, the edid buffer allocated via kmalloc() earlier is not
freed. Since this path can be triggered repeatedly via the
DRM_IOCTL_MODE_GETCONNECTOR ioctl, could this eventually lead to memory
exhaustion?

Additionally, following up on the race condition mentioned above, if power_off
was evaluated as true before a concurrent atomic commit, this path will call
ptn3460_disable(). That function locklessly modifies ptn_bridge->enabled = false
and powers down the hardware, causing the display to go blank against the atomic
commit's expectations.

>  
>  	return drm_edid;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=14

[PATCH v2 15/37] drm/bridge: of-display-mode-bridge: Add atomic bridge state handlers

[Maxime Ripard]

The of-display-mode-bridge bridge doesn't implement any enable or
disable callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/of-display-mode-bridge.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/of-display-mode-bridge.c b/drivers/gpu/drm/bridge/of-display-mode-bridge.c
index e66dae168fd0..af3e727e91c9 100644
--- a/drivers/gpu/drm/bridge/of-display-mode-bridge.c
+++ b/drivers/gpu/drm/bridge/of-display-mode-bridge.c
@@ -7,10 +7,11 @@
  * Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
  */
 
 #include <linux/export.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_modes.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/bridge/of-display-mode-bridge.h>
 
@@ -50,10 +51,13 @@ static int of_display_mode_bridge_get_modes(struct drm_bridge *bridge,
 
 	return 0;
 }
 
 static const struct drm_bridge_funcs of_display_mode_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = of_display_mode_bridge_attach,
 	.get_modes = of_display_mode_bridge_get_modes,
 };
 
 struct drm_bridge *devm_drm_of_display_mode_bridge(struct device *dev,

-- 
2.54.0

[PATCH v2 16/37] drm/bridge: parade-ps8622: Switch to atomic bridge callbacks

[Maxime Ripard]

The parade-ps8622 bridge uses the legacy non-atomic pre_enable, disable
and post_disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/parade-ps8622.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c b/drivers/gpu/drm/bridge/parade-ps8622.c
index f879a1df077d..33844db753aa 100644
--- a/drivers/gpu/drm/bridge/parade-ps8622.c
+++ b/drivers/gpu/drm/bridge/parade-ps8622.c
@@ -334,11 +334,12 @@ static int ps8622_backlight_update(struct backlight_device *bl)
 
 static const struct backlight_ops ps8622_backlight_ops = {
 	.update_status	= ps8622_backlight_update,
 };
 
-static void ps8622_pre_enable(struct drm_bridge *bridge)
+static void ps8622_pre_enable(struct drm_bridge *bridge,
+			      struct drm_atomic_commit *commit)
 {
 	struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
 	int ret;
 
 	if (ps8622->enabled)
@@ -379,17 +380,19 @@ static void ps8622_pre_enable(struct drm_bridge *bridge)
 	}
 
 	ps8622->enabled = true;
 }
 
-static void ps8622_disable(struct drm_bridge *bridge)
+static void ps8622_disable(struct drm_bridge *bridge,
+			   struct drm_atomic_commit *commit)
 {
 	/* Delay after panel is disabled */
 	msleep(PS8622_PWMO_END_T12_MS);
 }
 
-static void ps8622_post_disable(struct drm_bridge *bridge)
+static void ps8622_post_disable(struct drm_bridge *bridge,
+				struct drm_atomic_commit *commit)
 {
 	struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
 
 	if (!ps8622->enabled)
 		return;
@@ -426,13 +429,16 @@ static int ps8622_attach(struct drm_bridge *bridge,
 	return drm_bridge_attach(ps8622->bridge.encoder, ps8622->panel_bridge,
 				 &ps8622->bridge, flags);
 }
 
 static const struct drm_bridge_funcs ps8622_bridge_funcs = {
-	.pre_enable = ps8622_pre_enable,
-	.disable = ps8622_disable,
-	.post_disable = ps8622_post_disable,
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_pre_enable = ps8622_pre_enable,
+	.atomic_disable = ps8622_disable,
+	.atomic_post_disable = ps8622_post_disable,
 	.attach = ps8622_attach,
 };
 
 static const struct of_device_id ps8622_devices[] = {
 	{.compatible = "parade,ps8622",},

-- 
2.54.0

Re: [PATCH v2 16/37] drm/bridge: parade-ps8622: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Bridge atomic callbacks use sleeping functions (`msleep`, `usleep_range`) while running in an atomic context path.
--

--- Patch [16]: [PATCH v2 16/37] drm/bridge: parade-ps8622: Switch to atomic bridge callbacks ---
commit 6103d7b41fd50ff49b7002eef1519ecf77486219
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: parade-ps8622: Switch to atomic bridge callbacks

This commit updates the parade-ps8622 bridge to use atomic bridge callbacks
instead of the legacy ones. The functions have been updated to accept the
drm_atomic_commit state handlers.

> diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c b/drivers/gpu/drm/bridge/parade-ps8622.c
> index f879a1df077d2..33844db753aa5 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8622.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8622.c
> @@ -336,7 +336,8 @@ static const struct backlight_ops ps8622_backlight_ops = {
>  	.update_status	= ps8622_backlight_update,
>  };
>  
> -static void ps8622_pre_enable(struct drm_bridge *bridge)
> +static void ps8622_pre_enable(struct drm_bridge *bridge,
> +			      struct drm_atomic_commit *commit)
>  {
>  	struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);
>  	int ret;

[Severity: High]
This is a pre-existing issue, but this function calls a sleeping function
further down in the body:

	usleep_range(PS8622_RST_HIGH_T2_MIN_US + PS8622_POWER_RISE_T1_MAX_US,
		     PS8622_RST_HIGH_T2_MAX_US + PS8622_POWER_RISE_T1_MIN_US);

Could this cause kernel warnings or deadlocks? The drm_atomic_helper_commit_tail()
path directly invokes these bridge atomic callbacks, and sleeping is forbidden
in this atomic context.

[ ... ]

> @@ -381,13 +382,15 @@ static void ps8622_pre_enable(struct drm_bridge *bridge)
>  	ps8622->enabled = true;
>  }
>  
> -static void ps8622_disable(struct drm_bridge *bridge)
> +static void ps8622_disable(struct drm_bridge *bridge,
> +			   struct drm_atomic_commit *commit)
>  {
>  	/* Delay after panel is disabled */
>  	msleep(PS8622_PWMO_END_T12_MS);

[Severity: High]
This isn't a bug introduced by this patch, but does calling msleep() here
violate the atomic context requirements?

This callback can be executed from drm_atomic_helper_commit_tail() where
sleeping is forbidden.

>  }
>  
> -static void ps8622_post_disable(struct drm_bridge *bridge)
> +static void ps8622_post_disable(struct drm_bridge *bridge,
> +				struct drm_atomic_commit *commit)
>  {
>  	struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge);

[Severity: High]
This is a pre-existing issue, but similar to the other callbacks, this path
also contains calls to sleeping functions later in the body:

	usleep_range(PS8622_POWER_FALL_T16_MAX_US,
		     2 * PS8622_POWER_FALL_T16_MAX_US);
	...
	msleep(PS8622_POWER_OFF_T17_MS);

Is it safe to use sleeping functions here since this bridge atomic
callback also runs in an atomic context path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=16

[PATCH v2 17/37] drm/bridge: sii9234: Add atomic bridge state handlers

[Maxime Ripard]

The sii9234 bridge doesn't implement any enable or disable callbacks,
but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/sii9234.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index bb1bed03eb5b..a2cbbf031b4a 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -11,10 +11,11 @@
  *    Erik Gilling <konkers@android.com>
  *    Shankar Bandal <shankar.b@samsung.com>
  *    Dharam Kumar <dharam.kr@samsung.com>
  */
 #include <drm/bridge/mhl.h>
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
 
 #include <linux/delay.h>
@@ -876,10 +877,13 @@ static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
 
 	return MODE_OK;
 }
 
 static const struct drm_bridge_funcs sii9234_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.mode_valid = sii9234_mode_valid,
 };
 
 static int sii9234_probe(struct i2c_client *client)
 {

-- 
2.54.0

[PATCH v2 18/37] drm/bridge: sil-sii8620: Add atomic bridge state handlers

[Maxime Ripard]

The sil-sii8620 bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/sil-sii8620.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 982306eb4f0a..22c81b38b41e 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -7,10 +7,11 @@
  */
 
 #include <linux/unaligned.h>
 
 #include <drm/bridge/mhl.h>
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_encoder.h>
 
@@ -2278,10 +2279,13 @@ static bool sii8620_mode_fixup(struct drm_bridge *bridge,
 
 	return true;
 }
 
 static const struct drm_bridge_funcs sii8620_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = sii8620_attach,
 	.detach = sii8620_detach,
 	.mode_fixup = sii8620_mode_fixup,
 	.mode_valid = sii8620_mode_valid,
 };

-- 
2.54.0

Re: [PATCH v2 18/37] drm/bridge: sil-sii8620: Add atomic bridge state handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The `sii8620_mode_fixup` callback mutates persistent driver state (`ctx->use_packed_pixel`), violating the atomic check phase contract.
- [High] `drm_bridge_attach` ignores the return value of `drm_atomic_private_obj_init`, and `drm_atomic_private_obj_init` fails to roll back on error, leading to a corrupted private object list and a subsequent NULL pointer dereference.
--

--- Patch [18]: [PATCH v2 18/37] drm/bridge: sil-sii8620: Add atomic bridge state handlers ---
commit b25b0389a6c4219ff91245a5da61562f89c23dda
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: sil-sii8620: Add atomic bridge state handlers

The sil-sii8620 bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers.
In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 982306eb4f0a7..22c81b38b41e8 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c

[ ... ]

> @@ -2280,6 +2281,9 @@ static bool sii8620_mode_fixup(struct drm_bridge *bridge,
>  }
>  
>  static const struct drm_bridge_funcs sii8620_bridge_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.attach = sii8620_attach,
>  	.detach = sii8620_detach,
>  	.mode_fixup = sii8620_mode_fixup,

[Severity: High]
This isn't a bug introduced by this patch, but doesn't sii8620_mode_fixup()
mutate persistent driver state by assigning ctx->use_packed_pixel?

If userspace submits a DRM_MODE_ATOMIC_TEST_ONLY commit with a mode requiring
a different pixel packing, drm_atomic_helper_check_modeset() will call
drm_atomic_bridge_chain_check(), invoking this callback:

drivers/gpu/drm/bridge/sil-sii8620.c:sii8620_mode_fixup() {
	...
	ctx->use_packed_pixel = sii8620_is_packing_required(ctx, adjusted_mode);
	...
}

Will this cause the driver's global state to be modified during a test-only
commit, leading to a subsequent modeset or asynchronous interrupt programming
the hardware using the test-only pixel packing value?

[Severity: High]
This is also a pre-existing issue, but by adding atomic bridge state handlers,
does this bridge now expose an error handling bug in the DRM core?

Looking at drm_bridge_attach() in drivers/gpu/drm/drm_bridge.c, it ignores
the return value of drm_atomic_private_obj_init():

drivers/gpu/drm/drm_bridge.c:drm_bridge_attach() {
	...
	if (drm_bridge_is_atomic(bridge))
		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
					    &drm_bridge_priv_state_funcs);

	return 0;
}

If memory allocation fails in atomic_create_state,
drm_atomic_private_obj_init() returns an error but fails to clean up:

drivers/gpu/drm/drm_atomic.c:drm_atomic_private_obj_init() {
	...
	list_add_tail(&obj->head, &dev->mode_config.privobj_list);

	state = obj->funcs->atomic_create_state(obj);
	if (IS_ERR(state))
		return PTR_ERR(state);
	...
}

Will this leave obj->head in dev->mode_config.privobj_list with a NULL
obj->state?

If so, could this cause a kernel oops later if drm_state_dump() (e.g. via
debugfs) iterates privobj_list and calls
drm_atomic_private_obj_print_state() which dereferences the NULL state
via state->obj?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=18

[PATCH v2 19/37] drm/bridge: simple-bridge: Switch to atomic bridge callbacks

[Maxime Ripard]

The simple-bridge bridge uses the legacy non-atomic enable and disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/simple-bridge.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
index cc13c98f9be6..3fba7002322e 100644
--- a/drivers/gpu/drm/bridge/simple-bridge.c
+++ b/drivers/gpu/drm/bridge/simple-bridge.c
@@ -130,11 +130,12 @@ static int simple_bridge_attach(struct drm_bridge *bridge,
 	drm_connector_attach_encoder(&sbridge->connector, encoder);
 
 	return 0;
 }
 
-static void simple_bridge_enable(struct drm_bridge *bridge)
+static void simple_bridge_enable(struct drm_bridge *bridge,
+				 struct drm_atomic_commit *commit)
 {
 	struct simple_bridge *sbridge = drm_bridge_to_simple_bridge(bridge);
 	int ret;
 
 	if (sbridge->vdd) {
@@ -144,24 +145,28 @@ static void simple_bridge_enable(struct drm_bridge *bridge)
 	}
 
 	gpiod_set_value_cansleep(sbridge->enable, 1);
 }
 
-static void simple_bridge_disable(struct drm_bridge *bridge)
+static void simple_bridge_disable(struct drm_bridge *bridge,
+				  struct drm_atomic_commit *commit)
 {
 	struct simple_bridge *sbridge = drm_bridge_to_simple_bridge(bridge);
 
 	gpiod_set_value_cansleep(sbridge->enable, 0);
 
 	if (sbridge->vdd)
 		regulator_disable(sbridge->vdd);
 }
 
 static const struct drm_bridge_funcs simple_bridge_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach		= simple_bridge_attach,
-	.enable		= simple_bridge_enable,
-	.disable	= simple_bridge_disable,
+	.atomic_enable = simple_bridge_enable,
+	.atomic_disable = simple_bridge_disable,
 };
 
 static int simple_bridge_probe(struct platform_device *pdev)
 {
 	struct simple_bridge *sbridge;

-- 
2.54.0

[PATCH v2 20/37] drm/bridge: tc358764: Switch to atomic bridge callbacks

[Maxime Ripard]

The tc358764 bridge uses the legacy non-atomic pre_enable and
post_disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/tc358764.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
index 084e9d898e22..343f336a9911 100644
--- a/drivers/gpu/drm/bridge/tc358764.c
+++ b/drivers/gpu/drm/bridge/tc358764.c
@@ -265,11 +265,12 @@ static void tc358764_reset(struct tc358764 *ctx)
 	usleep_range(1000, 2000);
 	gpiod_set_value(ctx->gpio_reset, 0);
 	usleep_range(1000, 2000);
 }
 
-static void tc358764_post_disable(struct drm_bridge *bridge)
+static void tc358764_post_disable(struct drm_bridge *bridge,
+				  struct drm_atomic_commit *commit)
 {
 	struct tc358764 *ctx = bridge_to_tc358764(bridge);
 	int ret;
 
 	tc358764_reset(ctx);
@@ -277,11 +278,12 @@ static void tc358764_post_disable(struct drm_bridge *bridge)
 	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
 	if (ret < 0)
 		dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
 }
 
-static void tc358764_pre_enable(struct drm_bridge *bridge)
+static void tc358764_pre_enable(struct drm_bridge *bridge,
+				struct drm_atomic_commit *commit)
 {
 	struct tc358764 *ctx = bridge_to_tc358764(bridge);
 	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
@@ -302,12 +304,15 @@ static int tc358764_attach(struct drm_bridge *bridge,
 
 	return drm_bridge_attach(encoder, ctx->next_bridge, bridge, flags);
 }
 
 static const struct drm_bridge_funcs tc358764_bridge_funcs = {
-	.post_disable = tc358764_post_disable,
-	.pre_enable = tc358764_pre_enable,
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_post_disable = tc358764_post_disable,
+	.atomic_pre_enable = tc358764_pre_enable,
 	.attach = tc358764_attach,
 };
 
 static int tc358764_parse_dt(struct tc358764 *ctx)
 {

-- 
2.54.0

[PATCH v2 21/37] drm/bridge: tda998x: Switch to atomic bridge callbacks

[Maxime Ripard]

The tda998x bridge uses the legacy non-atomic enable and disable bridge
callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Russell King <linux@armlinux.org.uk>
---
 drivers/gpu/drm/bridge/tda998x_drv.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tda998x_drv.c b/drivers/gpu/drm/bridge/tda998x_drv.c
index 6c427bc75896..16c745591175 100644
--- a/drivers/gpu/drm/bridge/tda998x_drv.c
+++ b/drivers/gpu/drm/bridge/tda998x_drv.c
@@ -1408,11 +1408,12 @@ static enum drm_mode_status tda998x_bridge_mode_valid(struct drm_bridge *bridge,
 	if (mode->vtotal >= BIT(11))
 		return MODE_BAD_VVALUE;
 	return MODE_OK;
 }
 
-static void tda998x_bridge_enable(struct drm_bridge *bridge)
+static void tda998x_bridge_enable(struct drm_bridge *bridge,
+				  struct drm_atomic_commit *commit)
 {
 	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
 
 	if (!priv->is_on) {
 		/* enable video ports, audio will be enabled later */
@@ -1426,11 +1427,12 @@ static void tda998x_bridge_enable(struct drm_bridge *bridge)
 
 		priv->is_on = true;
 	}
 }
 
-static void tda998x_bridge_disable(struct drm_bridge *bridge)
+static void tda998x_bridge_disable(struct drm_bridge *bridge,
+				   struct drm_atomic_commit *commit)
 {
 	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
 
 	if (priv->is_on) {
 		/* disable video ports */
@@ -1743,16 +1745,19 @@ static void tda998x_bridge_hpd_disable(struct drm_bridge *bridge)
 
 	cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
 }
 
 static const struct drm_bridge_funcs tda998x_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = tda998x_bridge_attach,
 	.detach = tda998x_bridge_detach,
 	.mode_valid = tda998x_bridge_mode_valid,
-	.disable = tda998x_bridge_disable,
+	.atomic_disable = tda998x_bridge_disable,
 	.mode_set = tda998x_bridge_mode_set,
-	.enable = tda998x_bridge_enable,
+	.atomic_enable = tda998x_bridge_enable,
 	.edid_read = tda998x_bridge_edid_read,
 	.detect = tda998x_bridge_detect,
 	.hpd_enable = tda998x_bridge_hpd_enable,
 	.hpd_disable = tda998x_bridge_hpd_disable,
 };

-- 
2.54.0

Re: [PATCH v2 21/37] drm/bridge: tda998x: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The atomic_enable and atomic_disable bridge callbacks call sleeping functions but execute in an atomic context.
--

--- Patch [21]: [PATCH v2 21/37] drm/bridge: tda998x: Switch to atomic bridge callbacks ---
commit 3c3197b45d3a1fb57e72b15de2a23cbccfa9ab6d
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: tda998x: Switch to atomic bridge callbacks

This patch switches the tda998x bridge driver from legacy non-atomic enable
and disable callbacks to their atomic counterparts. It updates the callback
signatures and the drm_bridge_funcs structure accordingly.

> diff --git a/drivers/gpu/drm/bridge/tda998x_drv.c b/drivers/gpu/drm/bridge/tda998x_drv.c
> index 6c427bc75896b..16c7455911758 100644
> --- a/drivers/gpu/drm/bridge/tda998x_drv.c
> +++ b/drivers/gpu/drm/bridge/tda998x_drv.c
[ ... ]
> @@ -1410,7 +1410,8 @@ static enum drm_mode_status tda998x_bridge_mode_valid(struct drm_bridge *bridge,
>  	return MODE_OK;
>  }
>  
> -static void tda998x_bridge_enable(struct drm_bridge *bridge)
> +static void tda998x_bridge_enable(struct drm_bridge *bridge,
> +				  struct drm_atomic_commit *commit)
>  {

[Severity: High]
This isn't a bug introduced by this patch, but will this cause sleeping in
atomic context warnings or deadlocks?

The tda998x_bridge_enable() callback executes in an atomic context under
drm_atomic_helper_commit_tail(), where sleeping is forbidden. However, it
calls reg_write():

static void tda998x_bridge_enable(...)
{
	...
	if (!priv->is_on) {
		reg_write(priv, REG_ENA_VP_0, 0xff);

which takes a mutex (mutex_lock(&priv->mutex)) and initiates an I2C
transfer, both of which can sleep. Migrating to the atomic_enable hook
retains this behavior in the atomic commit path.

>  	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
>  
[ ... ]
> @@ -1428,7 +1429,8 @@ static void tda998x_bridge_enable(struct drm_bridge *bridge)
>  	}
>  }
>  
> -static void tda998x_bridge_disable(struct drm_bridge *bridge)
> +static void tda998x_bridge_disable(struct drm_bridge *bridge,
> +				   struct drm_atomic_commit *commit)
>  {

[Severity: High]
This is a pre-existing issue, but does this face the same atomic context
problem as tda998x_bridge_enable()?

Similar to the enable path, tda998x_bridge_disable() runs in an atomic
context but invokes reg_write() which can sleep:

static void tda998x_bridge_disable(...)
{
	...
	if (priv->is_on) {
		reg_write(priv, REG_ENA_VP_0, 0x00);

>  	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=21

[PATCH v2 22/37] drm/bridge: ti-tfp410: Switch to atomic bridge callbacks

[Maxime Ripard]

The ti-tfp410 bridge uses the legacy non-atomic enable and disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/ti-tfp410.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index 6f06f7b4e992..f0a14f52aaac 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -172,18 +172,20 @@ static void tfp410_detach(struct drm_bridge *bridge)
 		drm_bridge_hpd_disable(dvi->bridge.next_bridge);
 		cancel_delayed_work_sync(&dvi->hpd_work);
 	}
 }
 
-static void tfp410_enable(struct drm_bridge *bridge)
+static void tfp410_enable(struct drm_bridge *bridge,
+			  struct drm_atomic_commit *commit)
 {
 	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
 
 	gpiod_set_value_cansleep(dvi->powerdown, 0);
 }
 
-static void tfp410_disable(struct drm_bridge *bridge)
+static void tfp410_disable(struct drm_bridge *bridge,
+			   struct drm_atomic_commit *commit)
 {
 	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
 
 	gpiod_set_value_cansleep(dvi->powerdown, 1);
 }
@@ -240,12 +242,12 @@ static int tfp410_atomic_check(struct drm_bridge *bridge,
 }
 
 static const struct drm_bridge_funcs tfp410_bridge_funcs = {
 	.attach		= tfp410_attach,
 	.detach		= tfp410_detach,
-	.enable		= tfp410_enable,
-	.disable	= tfp410_disable,
+	.atomic_enable = tfp410_enable,
+	.atomic_disable = tfp410_disable,
 	.mode_valid	= tfp410_mode_valid,
 	.atomic_create_state = drm_atomic_helper_bridge_create_state,
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts,

-- 
2.54.0

Re: [PATCH v2 22/37] drm/bridge: ti-tfp410: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is a use-after-free vulnerability and missing cleanup of the HPD registration on the `tfp410_attach` error path.
--

--- Patch [22]: [PATCH v2 22/37] drm/bridge: ti-tfp410: Switch to atomic bridge callbacks ---
commit fb525e8e18293a40cab99e1a43c62c7f5ee18083
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: ti-tfp410: Switch to atomic bridge callbacks

This commit switches the ti-tfp410 bridge from legacy non-atomic enable and
disable callbacks to their atomic counterparts. It also adds the necessary
atomic state handlers to help remove legacy support from the DRM bridge core.

> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> index 6f06f7b4e9926..f0a14f52aaace 100644
> --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
[ ... ]
> @@ -242,8 +244,8 @@ static int tfp410_atomic_check(struct drm_bridge *bridge,
>  static const struct drm_bridge_funcs tfp410_bridge_funcs = {
>  	.attach		= tfp410_attach,
>  	.detach		= tfp410_detach,
> -	.enable		= tfp410_enable,
> -	.disable	= tfp410_disable,

[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a
use-after-free risk and missing cleanup of the HPD registration on the error
path in tfp410_attach().

When tfp410_attach() successfully registers the HPD callback on the next bridge,
but drm_connector_init_with_ddc() subsequently fails, it returns an error
without disabling the HPD callback:

drivers/gpu/drm/bridge/ti-tfp410.c:tfp410_attach() {
    ...
	ret = drm_connector_init_with_ddc(bridge->dev, &dvi->connector,
					  &tfp410_con_funcs,
					  dvi->bridge.next_bridge->type,
					  dvi->bridge.next_bridge->ddc);
	if (ret) {
		dev_err(dvi->dev, "drm_connector_init_with_ddc() failed: %d\n",
			ret);
		return ret;
	}
    ...
}

If the attachment fails, .detach() isn't called, and the next bridge might still
retain the HPD callback pointing to the freed dvi structure. A hardware
hotplug event could then trigger the callback and dereference the freed pointer.

Should this error path deregister the HPD callback before returning?

> +	.atomic_enable = tfp410_enable,
> +	.atomic_disable = tfp410_disable,
>  	.mode_valid	= tfp410_mode_valid,
>  	.atomic_create_state = drm_atomic_helper_bridge_create_state,
>  	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=22

[PATCH v2 23/37] drm/bridge: ti-tpd12s015: Add atomic bridge state handlers

[Maxime Ripard]

The ti-tpd12s015 bridge doesn't implement any enable or disable
callbacks, but is still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/ti-tpd12s015.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c
index 136e47ad1a10..e7b172ab7f55 100644
--- a/drivers/gpu/drm/bridge/ti-tpd12s015.c
+++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c
@@ -17,10 +17,11 @@
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 
 struct tpd12s015_device {
 	struct drm_bridge bridge;
 
@@ -94,10 +95,13 @@ static void tpd12s015_hpd_disable(struct drm_bridge *bridge)
 
 	gpiod_set_value_cansleep(tpd->ct_cp_hpd_gpio, 0);
 }
 
 static const struct drm_bridge_funcs tpd12s015_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach			= tpd12s015_attach,
 	.detach			= tpd12s015_detach,
 	.detect			= tpd12s015_bridge_detect,
 	.hpd_enable		= tpd12s015_hpd_enable,
 	.hpd_disable		= tpd12s015_hpd_disable,

-- 
2.54.0

Re: [PATCH v2 23/37] drm/bridge: ti-tpd12s015: Add atomic bridge state handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Release of devm-managed GPIOs if the bridge is accessed after the platform device unbinds.
- [High] Race condition where the HPD IRQ is active before its associated mutexes are initialized, and remains active after they are destroyed.
--

--- Patch [23]: [PATCH v2 23/37] drm/bridge: ti-tpd12s015: Add atomic bridge state handlers ---
commit 8627a6c33db1265257320b25744b5861c235ec83
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: ti-tpd12s015: Add atomic bridge state handlers

The ti-tpd12s015 bridge doesn't implement any enable or disable callbacks,
but is still missing the atomic state handlers. In order to remove the legacy
bridge callback support from the DRM bridge core, add them.

> diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c
> index 136e47ad1a10d..e7b172ab7f552 100644
> --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c
> +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c
[ ... ]
> @@ -96,6 +97,9 @@ static void tpd12s015_hpd_disable(struct drm_bridge *bridge)
>  }
>  
>  static const struct drm_bridge_funcs tpd12s015_bridge_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.attach			= tpd12s015_attach,
>  	.detach			= tpd12s015_detach,
>  	.detect			= tpd12s015_bridge_detect,

[Severity: High]
This isn't a bug introduced by this patch, but could this detect callback
race with driver unbinding and cause a use-after-free?

The DRM core holds a reference to the bridge, allowing it to outlive the
platform device's devres lifecycle. However, the GPIO descriptors are
allocated using devm_gpiod_get_index() in tpd12s015_probe():

gpio = devm_gpiod_get_index(&pdev->dev, NULL, 2, GPIOD_IN);

If the platform device unbinds, devres frees the GPIOs. If the still-attached
bridge's detect callback (tpd12s015_detect) is then invoked by the DRM core,
it accesses the freed GPIO:

if (gpiod_get_value_cansleep(tpd->hpd_gpio))

Will this cause memory corruption or a kernel oops if the bridge is accessed
after the device unbinds?


[Severity: High]
This is another pre-existing issue, but is there a potential race condition
with the HPD IRQ and bridge mutex initialization?

In tpd12s015_probe(), the HPD IRQ is requested via devm before
drm_bridge_add() initializes bridge->hpd_mutex:

ret = devm_request_threaded_irq(&pdev->dev, tpd->hpd_irq, NULL,
                                tpd12s015_hpd_isr, ...);

If an interrupt fires immediately, tpd12s015_hpd_isr() calls
drm_bridge_hpd_notify(), which attempts to lock the uninitialized hpd_mutex.

Similarly, in tpd12s015_remove():

drm_bridge_remove(&tpd->bridge);

drm_bridge_remove() is called to destroy the mutexes while the devm-managed
IRQ is still active. Can this lead to locking an uninitialized or destroyed
mutex if the HPD pin toggles during probe or removal?

>  	.hpd_enable		= tpd12s015_hpd_enable,
>  	.hpd_disable		= tpd12s015_hpd_disable,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=23

[PATCH v2 24/37] drm/bridge: thc63lvd1024: Switch to atomic bridge callbacks

[Maxime Ripard]

The thc63lvd1024 bridge uses the legacy non-atomic enable and disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/thc63lvd1024.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
index c804222846c3..679b424ba0c3 100644
--- a/drivers/gpu/drm/bridge/thc63lvd1024.c
+++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
@@ -11,10 +11,11 @@
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_panel.h>
 
 enum thc63_ports {
 	THC63_LVDS_IN0,
@@ -79,11 +80,12 @@ static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
 		return MODE_CLOCK_HIGH;
 
 	return MODE_OK;
 }
 
-static void thc63_enable(struct drm_bridge *bridge)
+static void thc63_enable(struct drm_bridge *bridge,
+			 struct drm_atomic_commit *commit)
 {
 	struct thc63_dev *thc63 = to_thc63(bridge);
 	int ret;
 
 	ret = regulator_enable(thc63->vcc);
@@ -95,11 +97,12 @@ static void thc63_enable(struct drm_bridge *bridge)
 
 	gpiod_set_value(thc63->pdwn, 0);
 	gpiod_set_value(thc63->oe, 1);
 }
 
-static void thc63_disable(struct drm_bridge *bridge)
+static void thc63_disable(struct drm_bridge *bridge,
+			  struct drm_atomic_commit *commit)
 {
 	struct thc63_dev *thc63 = to_thc63(bridge);
 	int ret;
 
 	gpiod_set_value(thc63->oe, 0);
@@ -110,14 +113,17 @@ static void thc63_disable(struct drm_bridge *bridge)
 		dev_err(thc63->dev,
 			"Failed to disable regulator \"vcc\": %d\n", ret);
 }
 
 static const struct drm_bridge_funcs thc63_bridge_func = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach	= thc63_attach,
 	.mode_valid = thc63_mode_valid,
-	.enable = thc63_enable,
-	.disable = thc63_disable,
+	.atomic_enable = thc63_enable,
+	.atomic_disable = thc63_disable,
 };
 
 static int thc63_parse_dt(struct thc63_dev *thc63)
 {
 	struct device_node *endpoint;

-- 
2.54.0

[PATCH v2 25/37] drm/bridge: waveshare-dsi: Switch to atomic bridge callbacks

[Maxime Ripard]

The waveshare-dsi bridge uses the legacy non-atomic enable and disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/bridge/waveshare-dsi.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/waveshare-dsi.c b/drivers/gpu/drm/bridge/waveshare-dsi.c
index ded57f298d64..9c9825c9b7c5 100644
--- a/drivers/gpu/drm/bridge/waveshare-dsi.c
+++ b/drivers/gpu/drm/bridge/waveshare-dsi.c
@@ -10,10 +10,11 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_graph.h>
 #include <linux/regmap.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 
@@ -90,29 +91,34 @@ static int ws_bridge_bridge_attach(struct drm_bridge *bridge,
 
 	return drm_bridge_attach(encoder, ws->next_bridge,
 				 &ws->bridge, flags);
 }
 
-static void ws_bridge_bridge_enable(struct drm_bridge *bridge)
+static void ws_bridge_bridge_enable(struct drm_bridge *bridge,
+				    struct drm_atomic_commit *commit)
 {
 	struct ws_bridge *ws = bridge_to_ws_bridge(bridge);
 
 	regmap_write(ws->reg_map, 0xad, 0x01);
 	backlight_enable(ws->backlight);
 }
 
-static void ws_bridge_bridge_disable(struct drm_bridge *bridge)
+static void ws_bridge_bridge_disable(struct drm_bridge *bridge,
+				     struct drm_atomic_commit *commit)
 {
 	struct ws_bridge *ws = bridge_to_ws_bridge(bridge);
 
 	backlight_disable(ws->backlight);
 	regmap_write(ws->reg_map, 0xad, 0x00);
 }
 
 static const struct drm_bridge_funcs ws_bridge_bridge_funcs = {
-	.enable = ws_bridge_bridge_enable,
-	.disable = ws_bridge_bridge_disable,
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_enable = ws_bridge_bridge_enable,
+	.atomic_disable = ws_bridge_bridge_disable,
 	.attach = ws_bridge_bridge_attach,
 };
 
 static int ws_bridge_bl_update_status(struct backlight_device *bl)
 {

-- 
2.54.0

Re: [PATCH v2 25/37] drm/bridge: waveshare-dsi: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Freed `regmap` and `backlight` resources are accessed during `atomic_enable` and `atomic_disable` if the driver is unbound while the bridge is still attached.
- [High] The bridge's atomic_enable and atomic_disable callbacks execute sleeping functions, violating atomic context requirements.
--

--- Patch [25]: [PATCH v2 25/37] drm/bridge: waveshare-dsi: Switch to atomic bridge callbacks ---
commit 5ef73541c5abfd37236a332fd681aeaf873d8239
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: waveshare-dsi: Switch to atomic bridge callbacks
    
The waveshare-dsi bridge uses the legacy non-atomic enable and disable
bridge callbacks. In order to remove the legacy bridge callback support
from the DRM bridge core, switch to their atomic counterparts and add the
bridge atomic state handlers.

> diff --git a/drivers/gpu/drm/bridge/waveshare-dsi.c b/drivers/gpu/drm/bridge/waveshare-dsi.c
> index ded57f298d640..9c9825c9b7c59 100644
> --- a/drivers/gpu/drm/bridge/waveshare-dsi.c
> +++ b/drivers/gpu/drm/bridge/waveshare-dsi.c

[ ... ]

> @@ -92,7 +93,8 @@ static int ws_bridge_bridge_attach(struct drm_bridge *bridge,
>  				 &ws->bridge, flags);
>  }
>  
> -static void ws_bridge_bridge_enable(struct drm_bridge *bridge)
> +static void ws_bridge_bridge_enable(struct drm_bridge *bridge,
> +				    struct drm_atomic_commit *commit)
>  {
>  	struct ws_bridge *ws = bridge_to_ws_bridge(bridge);
>  
>  	regmap_write(ws->reg_map, 0xad, 0x01);
>  	backlight_enable(ws->backlight);

[Severity: High]
This isn't a bug introduced by this patch, but does accessing ws->reg_map and
ws->backlight here risk a use-after-free if the driver is unbound?

The bridge object is managed by DRM and stays alive as long as it's attached
to the encoder. However, the regmap and backlight resources are device-managed
(devm) and are freed immediately upon driver unbind (e.g., manual unbind via
sysfs or hardware hot-unplug).

If a modeset or DPMS change occurs after unbind, the DRM core calls the
bridge's atomic_enable callback, which dereferences the freed ws->reg_map and
ws->backlight.

[Severity: High]
This is a pre-existing issue, but does calling sleeping functions from this
atomic callback violate context requirements?

According to the DRM subsystem guidelines, drm_atomic_helper_commit_tail()
(which calls these bridge callbacks) executes in atomic context where sleeping
is forbidden.

However, ws_bridge_bridge_enable() calls regmap_write() (which uses an I2C
transfer and acquires a mutex) and backlight_enable() (which acquires a mutex),
both of which can sleep.

>  }
>  
> -static void ws_bridge_bridge_disable(struct drm_bridge *bridge)
> +static void ws_bridge_bridge_disable(struct drm_bridge *bridge,
> +				     struct drm_atomic_commit *commit)
>  {
>  	struct ws_bridge *ws = bridge_to_ws_bridge(bridge);
>  
>  	backlight_disable(ws->backlight);
>  	regmap_write(ws->reg_map, 0xad, 0x00);

[Severity: High]
This is a pre-existing issue, but similar to the enable path, could these
calls result in a use-after-free or violate atomic context requirements?

If a modeset or DPMS change occurs after the driver is unbound, atomic_disable
will dereference the freed ws->reg_map and ws->backlight. Additionally,
calling backlight_disable() and regmap_write() executes sleeping functions in
atomic context.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=25

[PATCH v2 26/37] drm/exynos: mic: Switch to atomic bridge callbacks

[Maxime Ripard]

The mic bridge uses the legacy non-atomic pre_enable and post_disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Inki Dae <inki.dae@samsung.com>
To: Seung-Woo Kim <sw0312.kim@samsung.com>
To: Kyungmin Park <kyungmin.park@samsung.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index e68c954ec3e6..3069f958137f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -19,10 +19,11 @@
 #include <linux/regmap.h>
 
 #include <video/of_videomode.h>
 #include <video/videomode.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_print.h>
 
 #include "exynos_drm_drv.h"
@@ -226,11 +227,12 @@ static void mic_set_reg_on(struct exynos_mic *mic, bool enable)
 
 	reg |= MIC_UPD_REG;
 	writel(reg, mic->reg + MIC_OP);
 }
 
-static void mic_post_disable(struct drm_bridge *bridge)
+static void mic_post_disable(struct drm_bridge *bridge,
+			     struct drm_atomic_commit *commit)
 {
 	struct exynos_mic *mic = bridge->driver_private;
 
 	mutex_lock(&mic_mutex);
 	if (!mic->enabled)
@@ -255,11 +257,12 @@ static void mic_mode_set(struct drm_bridge *bridge,
 	drm_display_mode_to_videomode(mode, &mic->vm);
 	mic->i80_mode = to_exynos_crtc(bridge->encoder->crtc)->i80_mode;
 	mutex_unlock(&mic_mutex);
 }
 
-static void mic_pre_enable(struct drm_bridge *bridge)
+static void mic_pre_enable(struct drm_bridge *bridge,
+			   struct drm_atomic_commit *commit)
 {
 	struct exynos_mic *mic = bridge->driver_private;
 	int ret;
 
 	mutex_lock(&mic_mutex);
@@ -293,13 +296,16 @@ static void mic_pre_enable(struct drm_bridge *bridge)
 unlock:
 	mutex_unlock(&mic_mutex);
 }
 
 static const struct drm_bridge_funcs mic_bridge_funcs = {
-	.post_disable = mic_post_disable,
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_post_disable = mic_post_disable,
 	.mode_set = mic_mode_set,
-	.pre_enable = mic_pre_enable,
+	.atomic_pre_enable = mic_pre_enable,
 };
 
 static int exynos_mic_bind(struct device *dev, struct device *master,
 			   void *data)
 {

-- 
2.54.0

Re: [PATCH v2 26/37] drm/exynos: mic: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The result of `exynos_drm_crtc_get_by_type()` is dereferenced directly without an error pointer check, leading to a kernel panic.
--

--- Patch [26]: [PATCH v2 26/37] drm/exynos: mic: Switch to atomic bridge callbacks ---
commit 1ac627a57ce38903ca33c5eb6829642986ddaad3
Author: Maxime Ripard <mripard@kernel.org>

drm/exynos: mic: Switch to atomic bridge callbacks

This commit switches the mic bridge driver to use atomic pre_enable and
post_disable callbacks instead of the legacy non-atomic ones. It also adds
the necessary atomic state handlers to support the transition in the DRM
bridge core.

> diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> index e68c954ec3e61..3069f958137f5 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c

[ ... ]

> @@ -295,9 +298,12 @@ static void mic_pre_enable(struct drm_bridge *bridge)
>  }
>  
>  static const struct drm_bridge_funcs mic_bridge_funcs = {
> -	.post_disable = mic_post_disable,
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_post_disable = mic_post_disable,
>  	.mode_set = mic_mode_set,
> -	.pre_enable = mic_pre_enable,
> +	.atomic_pre_enable = mic_pre_enable,
>  };
>  
>  static int exynos_mic_bind(struct device *dev, struct device *master,
>  			   void *data)

[Severity: High]
This is a pre-existing issue, but can this function trigger a kernel
panic on invalid or incomplete devicetree configurations?

Looking at exynos_mic_bind(), exynos_drm_crtc_get_by_type() can return
an error pointer like ERR_PTR(-ENODEV) if the LCD CRTC component is
not found:

	struct exynos_drm_crtc *crtc = exynos_drm_crtc_get_by_type(drm_dev,
						       EXYNOS_DISPLAY_TYPE_LCD);
	struct drm_encoder *e, *encoder = NULL;

	drm_for_each_encoder(e, drm_dev)
		if (e->possible_crtcs == drm_crtc_mask(&crtc->base))
			encoder = e;

Since the code passes &crtc->base to drm_crtc_mask() without checking
if crtc is an error pointer first, and drm_crtc_mask() dereferences it to
access crtc->index, won't this cause a panic during probe if the component
is missing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=26

[PATCH v2 27/37] drm/mcde: dsi: Add atomic bridge state handlers

[Maxime Ripard]

The dsi bridge doesn't implement any enable or disable callbacks, but is
still missing the atomic state handlers.

In order to remove the legacy bridge callback support from the DRM
bridge core, add them.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Acked-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
To: Linus Walleij <linusw@kernel.org>
---
 drivers/gpu/drm/mcde/mcde_dsi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
index 47d45897ed06..5cf44ccb02cf 100644
--- a/drivers/gpu/drm/mcde/mcde_dsi.c
+++ b/drivers/gpu/drm/mcde/mcde_dsi.c
@@ -1061,10 +1061,13 @@ static int mcde_dsi_bridge_attach(struct drm_bridge *bridge,
 	/* Attach the DSI bridge to the output (panel etc) bridge */
 	return drm_bridge_attach(encoder, d->bridge.next_bridge, bridge, flags);
 }
 
 static const struct drm_bridge_funcs mcde_dsi_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = mcde_dsi_bridge_attach,
 	.mode_set = mcde_dsi_bridge_mode_set,
 };
 
 static int mcde_dsi_bind(struct device *dev, struct device *master,

-- 
2.54.0

Re: [PATCH v2 27/37] drm/mcde: dsi: Add atomic bridge state handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `drm_bridge_remove()` in driver unbind leads to memory leak and Use-After-Free via dangling global list pointer.
--

--- Patch [27]: [PATCH v2 27/37] drm/mcde: dsi: Add atomic bridge state handlers ---
commit 6ab8c58e40b6724f223de55f6cba1eb9cd4b73eb
Author: Maxime Ripard <mripard@kernel.org>

    drm/mcde: dsi: Add atomic bridge state handlers

This commit adds atomic bridge state handlers (atomic_create_state,
atomic_destroy_state, atomic_duplicate_state) to the mcde_dsi bridge. This
helps facilitate the removal of legacy bridge callback support from the DRM
bridge core.

> diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
> index 47d45897ed069..5cf44ccb02cf9 100644
> --- a/drivers/gpu/drm/mcde/mcde_dsi.c
> +++ b/drivers/gpu/drm/mcde/mcde_dsi.c
> @@ -1063,6 +1063,9 @@ static int mcde_dsi_bridge_attach(struct drm_bridge *bridge,
>  }
>  
>  static const struct drm_bridge_funcs mcde_dsi_bridge_funcs = {
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.attach = mcde_dsi_bridge_attach,
>  	.mode_set = mcde_dsi_bridge_mode_set,
>  };

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the bridge
operations I noticed a pre-existing issue with how the bridge is removed.

In mcde_dsi_bind(), the bridge is registered and added to the global bridge
list:

drivers/gpu/drm/mcde/mcde_dsi.c:mcde_dsi_bind() {
	...
	/* Create a bridge for this DSI channel */
	d->bridge.of_node = dev->of_node;
	drm_bridge_add(&d->bridge);
	...
}

However, in mcde_dsi_unbind(), there is no corresponding call to
drm_bridge_remove():

drivers/gpu/drm/mcde/mcde_dsi.c:mcde_dsi_unbind() {
	struct mcde_dsi *d = dev_get_drvdata(dev);

	if (d->panel)
		drm_panel_bridge_remove(d->bridge.next_bridge);
	regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET,
			   PRCM_DSI_SW_RESET_DSI0_SW_RESETN, 0);
}

Does this leave the bridge in the global bridge_list, leading to a memory
leak when unbinding the driver?

Furthermore, if another driver traverses the global DRM bridge list after this
driver is unloaded, could it dereference the dangling pointers (like
bridge->dev and bridge->funcs) and cause a Use-After-Free kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=27

[PATCH v2 28/37] drm/mediatek: dpi: Switch to atomic bridge callbacks

[Maxime Ripard]

The dpi bridge uses the legacy non-atomic enable and disable bridge
callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on is_bridge @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Chun-Kuang Hu <chunkuang.hu@kernel.org>
To: Philipp Zabel <p.zabel@pengutronix.de>
To: Matthias Brugger <matthias.bgg@gmail.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: linux-mediatek@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/gpu/drm/mediatek/mtk_dpi.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 959c994eef24..0e4f430de983 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -848,21 +848,23 @@ static void mtk_dpi_bridge_mode_set(struct drm_bridge *bridge,
 	struct mtk_dpi *dpi = bridge_to_dpi(bridge);
 
 	drm_mode_copy(&dpi->mode, adjusted_mode);
 }
 
-static void mtk_dpi_bridge_disable(struct drm_bridge *bridge)
+static void mtk_dpi_bridge_disable(struct drm_bridge *bridge,
+				   struct drm_atomic_commit *commit)
 {
 	struct mtk_dpi *dpi = bridge_to_dpi(bridge);
 
 	mtk_dpi_power_off(dpi);
 
 	if (dpi->pinctrl && dpi->pins_gpio)
 		pinctrl_select_state(dpi->pinctrl, dpi->pins_gpio);
 }
 
-static void mtk_dpi_bridge_enable(struct drm_bridge *bridge)
+static void mtk_dpi_bridge_enable(struct drm_bridge *bridge,
+				  struct drm_atomic_commit *commit)
 {
 	struct mtk_dpi *dpi = bridge_to_dpi(bridge);
 
 	if (dpi->pinctrl && dpi->pins_dpi)
 		pinctrl_select_state(dpi->pinctrl, dpi->pins_dpi);
@@ -980,12 +982,12 @@ static void mtk_dpi_debugfs_init(struct drm_bridge *bridge, struct dentry *root)
 
 static const struct drm_bridge_funcs mtk_dpi_bridge_funcs = {
 	.attach = mtk_dpi_bridge_attach,
 	.mode_set = mtk_dpi_bridge_mode_set,
 	.mode_valid = mtk_dpi_bridge_mode_valid,
-	.disable = mtk_dpi_bridge_disable,
-	.enable = mtk_dpi_bridge_enable,
+	.atomic_disable = mtk_dpi_bridge_disable,
+	.atomic_enable = mtk_dpi_bridge_enable,
 	.atomic_check = mtk_dpi_bridge_atomic_check,
 	.atomic_get_output_bus_fmts = mtk_dpi_bridge_atomic_get_output_bus_fmts,
 	.atomic_get_input_bus_fmts = mtk_dpi_bridge_atomic_get_input_bus_fmts,
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,

-- 
2.54.0

Re: [PATCH v2 28/37] drm/mediatek: dpi: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Modifying persistent driver state in `atomic_check` instead of atomic bridge state.
- [High] The bridge enable callback ignores the return value of mtk_dpi_power_on(), proceeding to access hardware registers even if clock enablement fails.
--

--- Patch [28]: [PATCH v2 28/37] drm/mediatek: dpi: Switch to atomic bridge callbacks ---
Note: The format of this review has been altered to use a free-form summary
instead of inline diff quoting due to text recitation restrictions.

commit 396f1ff8f70eedabf798b2a8fbc7bac8f10dfaad
Author: Maxime Ripard <mripard@kernel.org>
drm/mediatek: dpi: Switch to atomic bridge callbacks

This commit switches the dpi bridge to use atomic bridge callbacks instead
of the legacy non-atomic ones, updating the function signatures and adding
atomic state handlers via a Coccinelle script.

[Severity: High]
This is a pre-existing issue, but I noticed something while reviewing the
atomic conversions in mtk_dpi_bridge_atomic_check().

The check callback appears to mutate the global dpi structure directly:

mtk_dpi_bridge_atomic_check()
    dpi->output_fmt = out_bus_format;
    dpi->bit_num = mtk_dpi_bus_fmt_bit_num(out_bus_format);
    ...

Rather than modifying the local bridge_state, the persistent driver state
is altered. Could this cause state corruption if a userspace application
performs a test-only atomic commit (DRM_MODE_ATOMIC_TEST_ONLY)?

It looks like a test-only commit might permanently alter the active
configuration used by mtk_dpi_set_display_mode(), potentially breaking
subsequent display operations.

[Severity: High]
This is also a pre-existing issue, but looking at the newly renamed
mtk_dpi_bridge_enable() callback, it calls mtk_dpi_power_on(dpi) without
checking its return value:

mtk_dpi_bridge_enable()
    ...
    mtk_dpi_power_on(dpi);
    mtk_dpi_set_display_mode(dpi, &dpi->mode);
    mtk_dpi_enable(dpi);

If any of the required clocks (like engine_clk, tvd_clk, or pixel_clk)
fail to enable, mtk_dpi_power_on() will return an error, but the caller
proceeds to access hardware registers anyway.

Will accessing this unclocked register space via mtk_dpi_set_display_mode()
and mtk_dpi_enable() trigger a bus fault and result in an immediate system
panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=28

[PATCH v2 29/37] drm/msm: dsi: Switch to atomic bridge callbacks

[Maxime Ripard]

The dsi bridge uses the legacy non-atomic pre_enable and post_disable
bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Rob Clark <robin.clark@oss.qualcomm.com>
To: Dmitry Baryshkov <lumag@kernel.org>
Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
Cc: Jessica Zhang <jesszhan0024@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 46faeaae9d94..959daa88b1d0 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -273,11 +273,12 @@ static void dsi_mgr_bridge_power_off(struct drm_bridge *bridge)
 	}
 	msm_dsi_host_power_off(host);
 	dsi_mgr_phy_disable(id);
 }
 
-static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge)
+static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge,
+				      struct drm_atomic_commit *commit)
 {
 	int id = dsi_mgr_bridge_get_id(bridge);
 	struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
 	struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1);
 	struct mipi_dsi_host *host = msm_dsi->host;
@@ -329,11 +330,12 @@ void msm_dsi_manager_tpg_enable(void)
 		if (IS_BONDED_DSI() && s_dsi)
 			msm_dsi_host_test_pattern_en(s_dsi->host);
 	}
 }
 
-static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge)
+static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge,
+					struct drm_atomic_commit *commit)
 {
 	int id = dsi_mgr_bridge_get_id(bridge);
 	struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
 	struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1);
 	struct mipi_dsi_host *host = msm_dsi->host;
@@ -443,13 +445,16 @@ static int dsi_mgr_bridge_attach(struct drm_bridge *bridge,
 	return drm_bridge_attach(encoder, msm_dsi->next_bridge,
 				 bridge, flags);
 }
 
 static const struct drm_bridge_funcs dsi_mgr_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = dsi_mgr_bridge_attach,
-	.pre_enable = dsi_mgr_bridge_pre_enable,
-	.post_disable = dsi_mgr_bridge_post_disable,
+	.atomic_pre_enable = dsi_mgr_bridge_pre_enable,
+	.atomic_post_disable = dsi_mgr_bridge_post_disable,
 	.mode_set = dsi_mgr_bridge_mode_set,
 	.mode_valid = dsi_mgr_bridge_mode_valid,
 };
 
 /* initialize bridge */

-- 
2.54.0

[PATCH v2 30/37] drm/omap: dpi: Switch to atomic bridge callbacks

[Maxime Ripard]

The dpi bridge uses the legacy non-atomic enable and disable bridge
callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/dss/dpi.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
index 9f86db774c39..419915e33816 100644
--- a/drivers/gpu/drm/omapdrm/dss/dpi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
@@ -20,10 +20,11 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/string.h>
 #include <linux/sys_soc.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 
 #include "dss.h"
 #include "omapdss.h"
 
@@ -480,11 +481,12 @@ static void dpi_bridge_mode_set(struct drm_bridge *bridge,
 	struct dpi_data *dpi = drm_bridge_to_dpi(bridge);
 
 	dpi->pixelclock = adjusted_mode->clock * 1000;
 }
 
-static void dpi_bridge_enable(struct drm_bridge *bridge)
+static void dpi_bridge_enable(struct drm_bridge *bridge,
+			      struct drm_atomic_commit *commit)
 {
 	struct dpi_data *dpi = drm_bridge_to_dpi(bridge);
 	int r;
 
 	if (dpi->vdds_dsi_reg) {
@@ -531,11 +533,12 @@ static void dpi_bridge_enable(struct drm_bridge *bridge)
 err_get_dispc:
 	if (dpi->vdds_dsi_reg)
 		regulator_disable(dpi->vdds_dsi_reg);
 }
 
-static void dpi_bridge_disable(struct drm_bridge *bridge)
+static void dpi_bridge_disable(struct drm_bridge *bridge,
+			       struct drm_atomic_commit *commit)
 {
 	struct dpi_data *dpi = drm_bridge_to_dpi(bridge);
 
 	dss_mgr_disable(&dpi->output);
 
@@ -550,16 +553,19 @@ static void dpi_bridge_disable(struct drm_bridge *bridge)
 	if (dpi->vdds_dsi_reg)
 		regulator_disable(dpi->vdds_dsi_reg);
 }
 
 static const struct drm_bridge_funcs dpi_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = dpi_bridge_attach,
 	.mode_valid = dpi_bridge_mode_valid,
 	.mode_fixup = dpi_bridge_mode_fixup,
 	.mode_set = dpi_bridge_mode_set,
-	.enable = dpi_bridge_enable,
-	.disable = dpi_bridge_disable,
+	.atomic_enable = dpi_bridge_enable,
+	.atomic_disable = dpi_bridge_disable,
 };
 
 static void dpi_bridge_init(struct dpi_data *dpi)
 {
 	dpi->bridge.of_node = dpi->pdev->dev.of_node;

-- 
2.54.0

Re: [PATCH v2 30/37] drm/omap: dpi: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbalanced resource teardown on bridge enable error paths leads to PM runtime underflow and kernel panic.
--

--- Patch [30]: [PATCH v2 30/37] drm/omap: dpi: Switch to atomic bridge callbacks ---
commit 6fda2bd3a893061ddf4b74080bd409f4c3cc6741
Author: Maxime Ripard <mripard@kernel.org>

drm/omap: dpi: Switch to atomic bridge callbacks

The dpi bridge uses the legacy non-atomic enable and disable bridge
callbacks.  In order to remove the legacy bridge callback support from the
DRM bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
> index 9f86db774c395..419915e338163 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c

[ ... ]

> @@ -482,7 +483,8 @@ static void dpi_bridge_mode_set(struct drm_bridge *bridge,
>  	dpi->pixelclock = adjusted_mode->clock * 1000;
>  }
>  
> -static void dpi_bridge_enable(struct drm_bridge *bridge)
> +static void dpi_bridge_enable(struct drm_bridge *bridge,
> +			      struct drm_atomic_commit *commit)
>  {
>  	struct dpi_data *dpi = drm_bridge_to_dpi(bridge);
>  	int r;

[Severity: High]
This is a pre-existing issue, but does this early return on failure lead to
unbalanced resource teardown and a potential kernel panic?

Looking at dpi_bridge_enable() in drivers/gpu/drm/omapdrm/dss/dpi.c, if
initialization fails, the function cleans up and returns early:

	if (dpi->vdds_dsi_reg) {
		r = regulator_enable(dpi->vdds_dsi_reg);
		if (r)
			return;
	}

Because the callback returns void, the DRM core considers the bridge enabled.
During pipeline teardown, dpi_bridge_disable() executes and unconditionally
drops a PM runtime reference:

	dispc_runtime_put(dpi->dss->dispc);

Since the failed dpi_bridge_enable() never acquired (or already cleaned up)
this PM runtime reference, does this extra put operation incorrectly drop the
reference held by the CRTC (omap_crtc_atomic_enable())?

If this drops the DISPC PM runtime usage count to 0 and immediately powers
down the dispc hardware, could the later call to drm_crtc_vblank_off() from
omap_crtc_atomic_disable() access powered-down DISPC registers and trigger
an L3 bus error or imprecise external abort?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=30

[PATCH v2 31/37] drm/omap: dsi: Switch to atomic bridge callbacks

[Maxime Ripard]

The dsi bridge uses the legacy non-atomic enable and disable bridge
callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/dss/dsi.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 27fe7bca9e2c..7aaee94f86e4 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -33,10 +33,11 @@
 #include <linux/of_graph.h>
 #include <linux/of_platform.h>
 #include <linux/component.h>
 #include <linux/sys_soc.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_panel.h>
 #include <video/mipi_display.h>
 
@@ -4652,11 +4653,12 @@ static void dsi_bridge_mode_set(struct drm_bridge *bridge,
 	struct dsi_data *dsi = drm_bridge_to_dsi(bridge);
 
 	dsi_set_config(&dsi->output, adjusted_mode);
 }
 
-static void dsi_bridge_enable(struct drm_bridge *bridge)
+static void dsi_bridge_enable(struct drm_bridge *bridge,
+			      struct drm_atomic_commit *commit)
 {
 	struct dsi_data *dsi = drm_bridge_to_dsi(bridge);
 	struct omap_dss_device *dssdev = &dsi->output;
 
 	cancel_delayed_work_sync(&dsi->dsi_disable_work);
@@ -4671,11 +4673,12 @@ static void dsi_bridge_enable(struct drm_bridge *bridge)
 	dsi->video_enabled = true;
 
 	dsi_bus_unlock(dsi);
 }
 
-static void dsi_bridge_disable(struct drm_bridge *bridge)
+static void dsi_bridge_disable(struct drm_bridge *bridge,
+			       struct drm_atomic_commit *commit)
 {
 	struct dsi_data *dsi = drm_bridge_to_dsi(bridge);
 	struct omap_dss_device *dssdev = &dsi->output;
 
 	cancel_delayed_work_sync(&dsi->dsi_disable_work);
@@ -4690,15 +4693,18 @@ static void dsi_bridge_disable(struct drm_bridge *bridge)
 
 	dsi_bus_unlock(dsi);
 }
 
 static const struct drm_bridge_funcs dsi_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = dsi_bridge_attach,
 	.mode_valid = dsi_bridge_mode_valid,
 	.mode_set = dsi_bridge_mode_set,
-	.enable = dsi_bridge_enable,
-	.disable = dsi_bridge_disable,
+	.atomic_enable = dsi_bridge_enable,
+	.atomic_disable = dsi_bridge_disable,
 };
 
 static void dsi_bridge_init(struct dsi_data *dsi)
 {
 	dsi->bridge.of_node = dsi->host.dev->of_node;

-- 
2.54.0

Re: [PATCH v2 31/37] drm/omap: dsi: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `te_timeout_work` is cancelled asynchronously during driver detach, causing a possible Use-After-Free.
--

--- Patch [31]: [PATCH v2 31/37] drm/omap: dsi: Switch to atomic bridge callbacks ---
commit 7b874c0db789f5326cf81422dc10c73534a0a3cf
Author: Maxime Ripard <mripard@kernel.org>

drm/omap: dsi: Switch to atomic bridge callbacks

This commit switches the legacy non-atomic enable and disable bridge
callbacks in the OMAP DSI driver to their atomic counterparts. It also adds
the necessary atomic state handlers to support removing legacy bridge
callback support from the DRM bridge core.

> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 27fe7bca9e2cf..7aaee94f86e41 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but can this asynchronous cancellation lead
to a use-after-free during driver detach?

Looking at driver detach paths, omap_dsi_host_detach() calls
omap_dsi_unregister_te_irq(), which uses the asynchronous cancellation:

drivers/gpu/drm/omapdrm/dss/dsi.c:omap_dsi_unregister_te_irq() {
        ...
        cancel_delayed_work(&dsi->te_timeout_work);
        ...
}

Because it does not wait for a currently executing work callback to finish,
could dsi_remove() and devres cleanup complete while
omap_dsi_te_timeout_work_callback() is still running?

If the devres cleanup frees the dsi struct (allocated via
devm_drm_bridge_alloc()), would the concurrent callback then dereference
the freed dsi pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=31

[PATCH v2 32/37] drm/omap: sdi: Switch to atomic bridge callbacks

[Maxime Ripard]

The sdi bridge uses the legacy non-atomic enable and disable bridge
callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/dss/sdi.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
index df4cbc683e2c..d4a3c0ecaaf2 100644
--- a/drivers/gpu/drm/omapdrm/dss/sdi.c
+++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
@@ -14,10 +14,11 @@
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/string.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 
 #include "dss.h"
 #include "omapdss.h"
 
@@ -195,11 +196,12 @@ static void sdi_bridge_mode_set(struct drm_bridge *bridge,
 	struct sdi_device *sdi = drm_bridge_to_sdi(bridge);
 
 	sdi->pixelclock = adjusted_mode->clock * 1000;
 }
 
-static void sdi_bridge_enable(struct drm_bridge *bridge)
+static void sdi_bridge_enable(struct drm_bridge *bridge,
+			      struct drm_atomic_commit *commit)
 {
 	struct sdi_device *sdi = drm_bridge_to_sdi(bridge);
 	struct dispc_clock_info dispc_cinfo;
 	unsigned long fck;
 	int r;
@@ -258,11 +260,12 @@ static void sdi_bridge_enable(struct drm_bridge *bridge)
 	dispc_runtime_put(sdi->dss->dispc);
 err_get_dispc:
 	regulator_disable(sdi->vdds_sdi_reg);
 }
 
-static void sdi_bridge_disable(struct drm_bridge *bridge)
+static void sdi_bridge_disable(struct drm_bridge *bridge,
+			       struct drm_atomic_commit *commit)
 {
 	struct sdi_device *sdi = drm_bridge_to_sdi(bridge);
 
 	dss_mgr_disable(&sdi->output);
 
@@ -272,16 +275,19 @@ static void sdi_bridge_disable(struct drm_bridge *bridge)
 
 	regulator_disable(sdi->vdds_sdi_reg);
 }
 
 static const struct drm_bridge_funcs sdi_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = sdi_bridge_attach,
 	.mode_valid = sdi_bridge_mode_valid,
 	.mode_fixup = sdi_bridge_mode_fixup,
 	.mode_set = sdi_bridge_mode_set,
-	.enable = sdi_bridge_enable,
-	.disable = sdi_bridge_disable,
+	.atomic_enable = sdi_bridge_enable,
+	.atomic_disable = sdi_bridge_disable,
 };
 
 static void sdi_bridge_init(struct sdi_device *sdi)
 {
 	sdi->bridge.of_node = sdi->pdev->dev.of_node;

-- 
2.54.0

Re: [PATCH v2 32/37] drm/omap: sdi: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `sdi_bridge_enable` and `sdi_bridge_disable` call sleeping functions in atomic context.
--

--- Patch [32]: [PATCH v2 32/37] drm/omap: sdi: Switch to atomic bridge callbacks ---
commit fe567fd05157eb0a22dd0fa9bede0ea86a947f4e
Author: Maxime Ripard <mripard@kernel.org>

drm/omap: sdi: Switch to atomic bridge callbacks

This commit updates the omap sdi bridge to use atomic bridge callbacks instead
of legacy non-atomic ones. It modifies the enable and disable bridge callbacks
to their atomic counterparts and adds atomic state handlers.

> diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
> --- a/drivers/gpu/drm/omapdrm/dss/sdi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
[ ... ]
> -static void sdi_bridge_enable(struct drm_bridge *bridge)
> +static void sdi_bridge_enable(struct drm_bridge *bridge,
> +			      struct drm_atomic_commit *commit)
>  {
>  	struct sdi_device *sdi = drm_bridge_to_sdi(bridge);
>  	struct dispc_clock_info dispc_cinfo;

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to system
deadlocks by calling sleeping functions from an atomic context?

By wiring this to the atomic_enable callback, it is now executed in the
atomic commit tail path where sleeping is forbidden. Looking at the contents
of sdi_bridge_enable(), it calls:

	r = regulator_enable(sdi->vdds_sdi_reg);
	if (r)
		return;

	r = dispc_runtime_get(sdi->dss->dispc);

Both regulator_enable() (which acquires a mutex) and dispc_runtime_get()
(which calls pm_runtime_get_sync()) can sleep.

[ ... ]
> -static void sdi_bridge_disable(struct drm_bridge *bridge)
> +static void sdi_bridge_disable(struct drm_bridge *bridge,
> +			       struct drm_atomic_commit *commit)
>  {
>  	struct sdi_device *sdi = drm_bridge_to_sdi(bridge);

[Severity: High]
This is a pre-existing issue, but does switching this function to be an
atomic callback introduce sleeping in atomic context warnings here as well?

Similar to the enable path above, sdi_bridge_disable() calls:

	dispc_runtime_put(sdi->dss->dispc);

	regulator_disable(sdi->vdds_sdi_reg);

Could these sleeping functions trigger kernel warnings when called during
display atomic modeset operations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=32

[PATCH v2 33/37] drm/omap: venc: Switch to atomic bridge callbacks

[Maxime Ripard]

The venc bridge uses the legacy non-atomic enable and disable bridge
callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/dss/venc.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c b/drivers/gpu/drm/omapdrm/dss/venc.c
index 9b5d53dc361e..5416c668a205 100644
--- a/drivers/gpu/drm/omapdrm/dss/venc.c
+++ b/drivers/gpu/drm/omapdrm/dss/venc.c
@@ -23,10 +23,11 @@
 #include <linux/of.h>
 #include <linux/of_graph.h>
 #include <linux/component.h>
 #include <linux/sys_soc.h>
 
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 
 #include "omapdss.h"
 #include "dss.h"
 
@@ -612,18 +613,20 @@ static void venc_bridge_mode_set(struct drm_bridge *bridge,
 	}
 
 	dispc_set_tv_pclk(venc->dss->dispc, 13500000);
 }
 
-static void venc_bridge_enable(struct drm_bridge *bridge)
+static void venc_bridge_enable(struct drm_bridge *bridge,
+			       struct drm_atomic_commit *commit)
 {
 	struct venc_device *venc = drm_bridge_to_venc(bridge);
 
 	venc_power_on(venc);
 }
 
-static void venc_bridge_disable(struct drm_bridge *bridge)
+static void venc_bridge_disable(struct drm_bridge *bridge,
+				struct drm_atomic_commit *commit)
 {
 	struct venc_device *venc = drm_bridge_to_venc(bridge);
 
 	venc_power_off(venc);
 }
@@ -651,16 +654,19 @@ static int venc_bridge_get_modes(struct drm_bridge *bridge,
 
 	return ARRAY_SIZE(modes);
 }
 
 static const struct drm_bridge_funcs venc_bridge_funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.attach = venc_bridge_attach,
 	.mode_valid = venc_bridge_mode_valid,
 	.mode_fixup = venc_bridge_mode_fixup,
 	.mode_set = venc_bridge_mode_set,
-	.enable = venc_bridge_enable,
-	.disable = venc_bridge_disable,
+	.atomic_enable = venc_bridge_enable,
+	.atomic_disable = venc_bridge_disable,
 	.get_modes = venc_bridge_get_modes,
 };
 
 static void venc_bridge_init(struct venc_device *venc)
 {

-- 
2.54.0

Re: [PATCH v2 33/37] drm/omap: venc: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The atomic_enable and atomic_disable bridge callbacks invoke functions that sleep, violating the DRM atomic context rules.
--

--- Patch [33]: [PATCH v2 33/37] drm/omap: venc: Switch to atomic bridge callbacks ---
commit d92bb384d028f7e7472ce6ad66e7c5996b3db815
Author: Maxime Ripard <mripard@kernel.org>

drm/omap: venc: Switch to atomic bridge callbacks

The venc bridge uses the legacy non-atomic enable and disable bridge
callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

> diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c b/drivers/gpu/drm/omapdrm/dss/venc.c
> index 9b5d53dc361e6..5416c668a2055 100644
> --- a/drivers/gpu/drm/omapdrm/dss/venc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/venc.c
[ ... ]
> @@ -614,14 +615,16 @@ static void venc_bridge_mode_set(struct drm_bridge *bridge,
>  	dispc_set_tv_pclk(venc->dss->dispc, 13500000);
>  }
>  
> -static void venc_bridge_enable(struct drm_bridge *bridge)
> +static void venc_bridge_enable(struct drm_bridge *bridge,
> +			       struct drm_atomic_commit *commit)
>  {
>  	struct venc_device *venc = drm_bridge_to_venc(bridge);
>  
>  	venc_power_on(venc);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does calling venc_power_on()
here violate DRM atomic context rules?

The atomic_enable callback can be reached from drm_atomic_helper_commit_tail()
during non-blocking atomic commits, which can be initiated by untrusted
userspace via DRM/KMS operations. This path constitutes an atomic context
where sleeping is strictly forbidden.

However, venc_power_on() internally calls sleeping functions like
regulator_enable() and pm_runtime_get_sync() (via venc_runtime_get()):

drivers/gpu/drm/omapdrm/dss/venc.c:venc_power_on() {
	...
	r = venc_runtime_get(venc);
	...
	r = regulator_enable(venc->vdda_dac_reg);
	...
}

Calling sleeping functions in this context could cause kernel warnings like
BUG: scheduling while atomic, system instability, or potential deadlocks.

>  
> -static void venc_bridge_disable(struct drm_bridge *bridge)
> +static void venc_bridge_disable(struct drm_bridge *bridge,
> +				struct drm_atomic_commit *commit)
>  {
>  	struct venc_device *venc = drm_bridge_to_venc(bridge);
>  
>  	venc_power_off(venc);
>  }

[Severity: High]
This is a pre-existing issue, but does venc_power_off() also call sleeping
functions from an atomic context?

Similar to venc_bridge_enable() above, this atomic_disable callback is executed
from the atomic commit path. If venc_power_off() sleeps, this could also cause
scheduling while atomic warnings and instability.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=33

[PATCH v2 34/37] drm/sti: dvo: Switch to atomic bridge callbacks

[Maxime Ripard]

The dvo bridge uses the legacy non-atomic pre_enable, enable, disable
and post_disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Alain Volmat <alain.volmat@foss.st.com>
To: Raphael Gallais-Pou <rgallaispou@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/sti/sti_dvo.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
index 7484d3c3f4ed..b48099468eb9 100644
--- a/drivers/gpu/drm/sti/sti_dvo.c
+++ b/drivers/gpu/drm/sti/sti_dvo.c
@@ -207,11 +207,12 @@ static void dvo_debugfs_init(struct sti_dvo *dvo, struct drm_minor *minor)
 	drm_debugfs_create_files(dvo_debugfs_files,
 				 ARRAY_SIZE(dvo_debugfs_files),
 				 minor->debugfs_root, minor);
 }
 
-static void sti_dvo_disable(struct drm_bridge *bridge)
+static void sti_dvo_disable(struct drm_bridge *bridge,
+			    struct drm_atomic_commit *commit)
 {
 	struct sti_dvo *dvo = bridge->driver_private;
 
 	if (!dvo->enabled)
 		return;
@@ -230,11 +231,12 @@ static void sti_dvo_disable(struct drm_bridge *bridge)
 	clk_disable_unprepare(dvo->clk);
 
 	dvo->enabled = false;
 }
 
-static void sti_dvo_pre_enable(struct drm_bridge *bridge)
+static void sti_dvo_pre_enable(struct drm_bridge *bridge,
+			       struct drm_atomic_commit *commit)
 {
 	struct sti_dvo *dvo = bridge->driver_private;
 	struct dvo_config *config = dvo->config;
 	u32 val;
 
@@ -318,20 +320,24 @@ static void sti_dvo_set_mode(struct drm_bridge *bridge,
 
 	/* For now, we only support 24bit data enable (DE) synchro format */
 	dvo->config = &rgb_24bit_de_cfg;
 }
 
-static void sti_dvo_bridge_nope(struct drm_bridge *bridge)
+static void sti_dvo_bridge_nope(struct drm_bridge *bridge,
+				struct drm_atomic_commit *commit)
 {
 	/* do nothing */
 }
 
 static const struct drm_bridge_funcs sti_dvo_bridge_funcs = {
-	.pre_enable = sti_dvo_pre_enable,
-	.enable = sti_dvo_bridge_nope,
-	.disable = sti_dvo_disable,
-	.post_disable = sti_dvo_bridge_nope,
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_pre_enable = sti_dvo_pre_enable,
+	.atomic_enable = sti_dvo_bridge_nope,
+	.atomic_disable = sti_dvo_disable,
+	.atomic_post_disable = sti_dvo_bridge_nope,
 	.mode_set = sti_dvo_set_mode,
 };
 
 static int sti_dvo_connector_get_modes(struct drm_connector *connector)
 {

-- 
2.54.0

Re: [PATCH v2 34/37] drm/sti: dvo: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] NULL pointer dereference when `clk_set_rate()` fails in `sti_dvo_set_mode()`.
--

--- Patch [34]: [PATCH v2 34/37] drm/sti: dvo: Switch to atomic bridge callbacks ---
commit 377cffc6cdd80910f39bda909df8ad2af0ea4cf2
Author: Maxime Ripard <mripard@kernel.org>

drm/sti: dvo: Switch to atomic bridge callbacks

This commit switches the sti dvo bridge from using legacy non-atomic bridge
callbacks to their atomic counterparts. It also adds atomic state handlers
as part of the effort to remove legacy support from the DRM bridge core.

> diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> index 7484d3c3f4ed5..b48099468eb97 100644
> --- a/drivers/gpu/drm/sti/sti_dvo.c
> +++ b/drivers/gpu/drm/sti/sti_dvo.c

[ ... ]

> @@ -232,7 +233,8 @@ static void sti_dvo_disable(struct drm_bridge *bridge)
>  	dvo->enabled = false;
>  }
>  
> -static void sti_dvo_pre_enable(struct drm_bridge *bridge)
> +static void sti_dvo_pre_enable(struct drm_bridge *bridge,
> +			       struct drm_atomic_commit *commit)
>  {
>  	struct sti_dvo *dvo = bridge->driver_private;
>  	struct dvo_config *config = dvo->config;

[Severity: High]
This isn't a bug introduced by this patch, but can this lead to a NULL
pointer dereference?

If clk_set_rate() fails during the modeset in sti_dvo_set_mode(), it returns
early and skips setting dvo->config:

drivers/gpu/drm/sti/sti_dvo.c:sti_dvo_set_mode() {
    ...
	ret = clk_set_rate(dvo->clk_pix, rate);
	if (ret < 0) {
		DRM_ERROR("Cannot set rate (%dHz) for dvo_pix clk\n", rate);
		return;
	}
    ...
	dvo->config = &rgb_24bit_de_cfg;
}

When sti_dvo_pre_enable() is called later on the atomic commit path, config
is assigned dvo->config (which would be NULL if it was the first modeset),
and then it gets dereferenced unconditionally shortly after:

drivers/gpu/drm/sti/sti_dvo.c:sti_dvo_pre_enable() {
    ...
	if (config->awg_fwgen_fct) {
    ...
}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=34

[PATCH v2 35/37] drm/sti: hda: Switch to atomic bridge callbacks

[Maxime Ripard]

The hda bridge uses the legacy non-atomic pre_enable, enable, disable
and post_disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Alain Volmat <alain.volmat@foss.st.com>
To: Raphael Gallais-Pou <rgallaispou@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/sti/sti_hda.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
index 360a88ca8f0c..3806dd2c9be3 100644
--- a/drivers/gpu/drm/sti/sti_hda.c
+++ b/drivers/gpu/drm/sti/sti_hda.c
@@ -403,11 +403,12 @@ static void sti_hda_configure_awg(struct sti_hda *hda, u32 *awg_instr, int nb)
 		hda_write(hda, awg_instr[i], HDA_SYNC_AWGI + i * 4);
 	for (i = nb; i < AWG_MAX_INST; i++)
 		hda_write(hda, 0, HDA_SYNC_AWGI + i * 4);
 }
 
-static void sti_hda_disable(struct drm_bridge *bridge)
+static void sti_hda_disable(struct drm_bridge *bridge,
+			    struct drm_atomic_commit *commit)
 {
 	struct sti_hda *hda = drm_bridge_to_sti_hda(bridge);
 	u32 val;
 
 	if (!hda->enabled)
@@ -428,11 +429,12 @@ static void sti_hda_disable(struct drm_bridge *bridge)
 	clk_disable_unprepare(hda->clk_pix);
 
 	hda->enabled = false;
 }
 
-static void sti_hda_pre_enable(struct drm_bridge *bridge)
+static void sti_hda_pre_enable(struct drm_bridge *bridge,
+			       struct drm_atomic_commit *commit)
 {
 	struct sti_hda *hda = drm_bridge_to_sti_hda(bridge);
 	u32 val, i, mode_idx;
 	u32 src_filter_y, src_filter_c;
 	u32 *coef_y, *coef_c;
@@ -562,20 +564,24 @@ static void sti_hda_set_mode(struct drm_bridge *bridge,
 	if (ret < 0)
 		DRM_ERROR("Cannot set rate (%dHz) for hda_pix clk\n",
 			  mode->clock * 1000);
 }
 
-static void sti_hda_bridge_nope(struct drm_bridge *bridge)
+static void sti_hda_bridge_nope(struct drm_bridge *bridge,
+				struct drm_atomic_commit *commit)
 {
 	/* do nothing */
 }
 
 static const struct drm_bridge_funcs sti_hda_bridge_funcs = {
-	.pre_enable = sti_hda_pre_enable,
-	.enable = sti_hda_bridge_nope,
-	.disable = sti_hda_disable,
-	.post_disable = sti_hda_bridge_nope,
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_pre_enable = sti_hda_pre_enable,
+	.atomic_enable = sti_hda_bridge_nope,
+	.atomic_disable = sti_hda_disable,
+	.atomic_post_disable = sti_hda_bridge_nope,
 	.mode_set = sti_hda_set_mode,
 };
 
 static int sti_hda_connector_get_modes(struct drm_connector *connector)
 {

-- 
2.54.0

[PATCH v2 36/37] drm/sti: hdmi: Switch to atomic bridge callbacks

[Maxime Ripard]

The hdmi bridge uses the legacy non-atomic pre_enable, enable, disable
and post_disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

Generated by the following Coccinelle script:

@ is_bridge @
identifier funcs;
@@

 struct drm_bridge_funcs funcs = {
	...,
 };

@ has_create_state depends on is_bridge @
identifier funcs, f;
@@

struct drm_bridge_funcs funcs = {
  ...,
  .atomic_create_state = f,
  ...,
};

@ update_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	...,
 };

@ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.pre_enable = f,
+	.atomic_pre_enable = f,
 	...,
 };

@ update_pre_enable_impl depends on update_pre_enable_struct @
identifier update_pre_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_enable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.enable = f,
+	.atomic_enable = f,
 	...,
 };

@ update_enable_impl depends on update_enable_struct @
identifier update_enable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.disable = f,
+	.atomic_disable = f,
 	...,
 };

@ update_disable_impl depends on update_disable_struct @
identifier update_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

@ update_post_disable_struct depends on (is_bridge && !has_create_state) @
identifier is_bridge.funcs;
identifier f;
@@

 struct drm_bridge_funcs funcs = {
 	...,
-	.post_disable = f,
+	.atomic_post_disable = f,
 	...,
 };

@ update_post_disable_impl depends on update_post_disable_struct @
identifier update_post_disable_struct.f;
identifier b;
@@

-void f(struct drm_bridge *b)
+void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
 {
	...
 }

Signed-off-by: Maxime Ripard <mripard@kernel.org>

---
To: Alain Volmat <alain.volmat@foss.st.com>
To: Raphael Gallais-Pou <rgallaispou@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/sti/sti_hdmi.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f8222e60b1e0..7ba9e0b7ba6c 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -750,11 +750,12 @@ static void hdmi_debugfs_init(struct sti_hdmi *hdmi, struct drm_minor *minor)
 	drm_debugfs_create_files(hdmi_debugfs_files,
 				 ARRAY_SIZE(hdmi_debugfs_files),
 				 minor->debugfs_root, minor);
 }
 
-static void sti_hdmi_disable(struct drm_bridge *bridge)
+static void sti_hdmi_disable(struct drm_bridge *bridge,
+			     struct drm_atomic_commit *commit)
 {
 	struct sti_hdmi *hdmi = drm_bridge_to_sti_hdmi(bridge);
 
 	u32 val = hdmi_read(hdmi, HDMI_CFG);
 
@@ -882,11 +883,12 @@ static int hdmi_audio_configure(struct sti_hdmi *hdmi)
 	hdmi_write(hdmi, audio_cfg, HDMI_AUDIO_CFG);
 
 	return hdmi_audio_infoframe_config(hdmi);
 }
 
-static void sti_hdmi_pre_enable(struct drm_bridge *bridge)
+static void sti_hdmi_pre_enable(struct drm_bridge *bridge,
+				struct drm_atomic_commit *commit)
 {
 	struct sti_hdmi *hdmi = drm_bridge_to_sti_hdmi(bridge);
 
 	DRM_DEBUG_DRIVER("\n");
 
@@ -962,20 +964,24 @@ static void sti_hdmi_set_mode(struct drm_bridge *bridge,
 			  mode->clock * 1000);
 		return;
 	}
 }
 
-static void sti_hdmi_bridge_nope(struct drm_bridge *bridge)
+static void sti_hdmi_bridge_nope(struct drm_bridge *bridge,
+				 struct drm_atomic_commit *commit)
 {
 	/* do nothing */
 }
 
 static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {
-	.pre_enable = sti_hdmi_pre_enable,
-	.enable = sti_hdmi_bridge_nope,
-	.disable = sti_hdmi_disable,
-	.post_disable = sti_hdmi_bridge_nope,
+	.atomic_create_state = drm_atomic_helper_bridge_create_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_pre_enable = sti_hdmi_pre_enable,
+	.atomic_enable = sti_hdmi_bridge_nope,
+	.atomic_disable = sti_hdmi_disable,
+	.atomic_post_disable = sti_hdmi_bridge_nope,
 	.mode_set = sti_hdmi_set_mode,
 };
 
 static int sti_hdmi_connector_get_modes(struct drm_connector *connector)
 {

-- 
2.54.0

Re: [PATCH v2 36/37] drm/sti: hdmi: Switch to atomic bridge callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Memory leak and global list corruption (dangling pointer) if component_add() fails in sti_hdmi_probe().
- [High] Resource leak of the audio platform device and potential NULL pointer dereference if sti_hdmi_bind() fails.
--

--- Patch [36]: [PATCH v2 36/37] drm/sti: hdmi: Switch to atomic bridge callbacks ---
commit fe4cd2364e238c4d447b17cda3c258ebb7c65628
Author: Maxime Ripard <mripard@kernel.org>

drm/sti: hdmi: Switch to atomic bridge callbacks

The hdmi bridge uses the legacy non-atomic pre_enable, enable, disable
and post_disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index f8222e60b1e01..7ba9e0b7ba6c0 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c

[ ... ]

> @@ -964,16 +966,20 @@ static void sti_hdmi_set_mode(struct drm_bridge *bridge,
>  	}
>  }
>  
> -static void sti_hdmi_bridge_nope(struct drm_bridge *bridge)
> +static void sti_hdmi_bridge_nope(struct drm_bridge *bridge,
> +				 struct drm_atomic_commit *commit)
>  {
>  	/* do nothing */
>  }
>  
>  static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {
> -	.pre_enable = sti_hdmi_pre_enable,
> -	.enable = sti_hdmi_bridge_nope,
> -	.disable = sti_hdmi_disable,
> -	.post_disable = sti_hdmi_bridge_nope,
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_pre_enable = sti_hdmi_pre_enable,
> +	.atomic_enable = sti_hdmi_bridge_nope,
> +	.atomic_disable = sti_hdmi_disable,
> +	.atomic_post_disable = sti_hdmi_bridge_nope,
>  	.mode_set = sti_hdmi_set_mode,
>  };

[Severity: High]
This is a pre-existing issue, but does sti_hdmi_probe() leak memory and
leave a dangling pointer if component_add() fails?

drivers/gpu/drm/sti/sti_hdmi.c:sti_hdmi_probe() {
	...
	drm_bridge_add(&hdmi->bridge);
	return component_add(&pdev->dev, &sti_hdmi_ops);
}

If component_add() fails, the function returns an error without calling
drm_bridge_remove().

Because the bridge is never removed, the reference count does not drop to
zero during devres cleanup. Could another driver try to access the corrupted
bridge in the global bridge_list, leading to a use-after-free panic when it
accesses the freed devres-managed resources?

[Severity: High]
This isn't a bug introduced by this patch, but does the error handling in
sti_hdmi_bind() leave the audio platform device registered while setting the
connector to NULL?

drivers/gpu/drm/sti/sti_hdmi.c:sti_hdmi_bind() {
	...
err_sysfs:
	hdmi->drm_connector = NULL;
	return -EINVAL;
}

If sti_hdmi_register_audio_driver() succeeds but a subsequent initialization
step fails, we jump to the err_sysfs label, which clears hdmi->drm_connector
without unregistering the active audio device.

Since the ALSA framework may still invoke operations on the registered codec,
can this trigger a NULL pointer dereference when hdmi_audio_get_eld() is
called?

drivers/gpu/drm/sti/sti_hdmi.c:hdmi_audio_get_eld() {
	...
	struct drm_connector *connector = hdmi->drm_connector;

	DRM_DEBUG_DRIVER("\n");
	mutex_lock(&connector->eld_mutex);
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=36

[PATCH v2 37/37] drm/bridge: Remove legacy bridge callback support

[Maxime Ripard]

All bridge drivers have been converted to the atomic variants of the
enable, disable, pre_enable, and post_disable callbacks.

Remove the deprecated legacy hooks from drm_bridge_funcs, the
drm_bridge_is_atomic() helper that was only needed to distinguish
between atomic and non-atomic bridges, and the legacy bridge test
cases.

Since all bridges are now atomic, unconditionally initialize the
private object state at attach time.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_bridge.c            |  61 ++++++-------------
 drivers/gpu/drm/tests/drm_bridge_test.c | 104 --------------------------------
 include/drm/drm_bridge.h                | 103 -------------------------------
 3 files changed, 20 insertions(+), 248 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 6ead9b75ae3d..6fb443984dff 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -553,15 +553,10 @@ static const struct drm_private_state_funcs drm_bridge_priv_state_funcs = {
 	.atomic_create_state = drm_bridge_atomic_create_priv_state,
 	.atomic_duplicate_state = drm_bridge_atomic_duplicate_priv_state,
 	.atomic_destroy_state = drm_bridge_atomic_destroy_priv_state,
 };
 
-static bool drm_bridge_is_atomic(struct drm_bridge *bridge)
-{
-	return bridge->funcs->atomic_create_state != NULL;
-}
-
 /**
  * drm_bridge_attach - attach the bridge to an encoder's chain
  *
  * @encoder: DRM encoder
  * @bridge: bridge to attach
@@ -627,13 +622,12 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
 		ret = bridge->funcs->attach(bridge, encoder, flags);
 		if (ret < 0)
 			goto err_reset_bridge;
 	}
 
-	if (drm_bridge_is_atomic(bridge))
-		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
-					    &drm_bridge_priv_state_funcs);
+	drm_atomic_private_obj_init(bridge->dev, &bridge->base,
+				    &drm_bridge_priv_state_funcs);
 
 	return 0;
 
 err_reset_bridge:
 	bridge->dev = NULL;
@@ -662,12 +656,11 @@ void drm_bridge_detach(struct drm_bridge *bridge)
 		return;
 
 	if (WARN_ON(!bridge->dev))
 		return;
 
-	if (drm_bridge_is_atomic(bridge))
-		drm_atomic_private_obj_fini(&bridge->base);
+	drm_atomic_private_obj_fini(&bridge->base);
 
 	if (bridge->funcs->detach)
 		bridge->funcs->detach(bridge);
 
 	list_del(&bridge->chain_node);
@@ -688,13 +681,13 @@ void drm_bridge_detach(struct drm_bridge *bridge)
  *   operations. They are used by the legacy CRTC and the atomic modeset
  *   helpers to perform mode validation, fixup and setting, and enable and
  *   disable the bridge automatically.
  *
  *   The enable and disable operations are split in
- *   &drm_bridge_funcs.pre_enable, &drm_bridge_funcs.enable,
- *   &drm_bridge_funcs.disable and &drm_bridge_funcs.post_disable to provide
- *   finer-grained control.
+ *   &drm_bridge_funcs.atomic_pre_enable, &drm_bridge_funcs.atomic_enable,
+ *   &drm_bridge_funcs.atomic_disable and &drm_bridge_funcs.atomic_post_disable
+ *   to provide finer-grained control.
  *
  *   Bridge drivers may implement the legacy version of those operations, or
  *   the atomic version (prefixed with atomic\_), in which case they shall also
  *   implement the atomic state bookkeeping operations
  *   (&drm_bridge_funcs.atomic_duplicate_state,
@@ -831,14 +824,13 @@ EXPORT_SYMBOL(drm_bridge_chain_mode_set);
 /**
  * drm_atomic_bridge_chain_disable - disables all bridges in the encoder chain
  * @bridge: bridge control structure
  * @state: atomic state being committed
  *
- * Calls &drm_bridge_funcs.atomic_disable (falls back on
- * &drm_bridge_funcs.disable) op for all the bridges in the encoder chain,
- * starting from the last bridge to the first. These are called before calling
- * &drm_encoder_helper_funcs.atomic_disable
+ * Calls &drm_bridge_funcs.atomic_disable op for all the bridges in the encoder
+ * chain, starting from the last bridge to the first. These are called before
+ * calling &drm_encoder_helper_funcs.atomic_disable
  *
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge,
 				     struct drm_atomic_commit *state)
@@ -850,15 +842,12 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge,
 		return;
 
 	encoder = bridge->encoder;
 	mutex_lock(&encoder->bridge_chain_mutex);
 	list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
-		if (iter->funcs->atomic_disable) {
+		if (iter->funcs->atomic_disable)
 			iter->funcs->atomic_disable(iter, state);
-		} else if (iter->funcs->disable) {
-			iter->funcs->disable(iter);
-		}
 
 		if (iter == bridge)
 			break;
 	}
 	mutex_unlock(&encoder->bridge_chain_mutex);
@@ -868,24 +857,21 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
 static void drm_atomic_bridge_call_post_disable(struct drm_bridge *bridge,
 						struct drm_atomic_commit *state)
 {
 	if (state && bridge->funcs->atomic_post_disable)
 		bridge->funcs->atomic_post_disable(bridge, state);
-	else if (bridge->funcs->post_disable)
-		bridge->funcs->post_disable(bridge);
 }
 
 /**
  * drm_atomic_bridge_chain_post_disable - cleans up after disabling all bridges
  *					  in the encoder chain
  * @bridge: bridge control structure
  * @state: atomic state being committed
  *
- * Calls &drm_bridge_funcs.atomic_post_disable (falls back on
- * &drm_bridge_funcs.post_disable) op for all the bridges in the encoder chain,
- * starting from the first bridge to the last. These are called after completing
- * &drm_encoder_helper_funcs.atomic_disable
+ * Calls &drm_bridge_funcs.atomic_post_disable op for all the bridges in the
+ * encoder chain, starting from the first bridge to the last. These are called
+ * after completing &drm_encoder_helper_funcs.atomic_disable
  *
  * If a bridge sets @pre_enable_prev_first, then the @post_disable for that
  * bridge will be called before the previous one to reverse the @pre_enable
  * calling direction.
  *
@@ -965,24 +951,21 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
 static void drm_atomic_bridge_call_pre_enable(struct drm_bridge *bridge,
 					      struct drm_atomic_commit *state)
 {
 	if (state && bridge->funcs->atomic_pre_enable)
 		bridge->funcs->atomic_pre_enable(bridge, state);
-	else if (bridge->funcs->pre_enable)
-		bridge->funcs->pre_enable(bridge);
 }
 
 /**
  * drm_atomic_bridge_chain_pre_enable - prepares for enabling all bridges in
  *					the encoder chain
  * @bridge: bridge control structure
  * @state: atomic state being committed
  *
- * Calls &drm_bridge_funcs.atomic_pre_enable (falls back on
- * &drm_bridge_funcs.pre_enable) op for all the bridges in the encoder chain,
- * starting from the last bridge to the first. These are called before calling
- * &drm_encoder_helper_funcs.atomic_enable
+ * Calls &drm_bridge_funcs.atomic_pre_enable op for all the bridges in the
+ * encoder chain, starting from the last bridge to the first. These are called
+ * before calling &drm_encoder_helper_funcs.atomic_enable
  *
  * If a bridge sets @pre_enable_prev_first, then the pre_enable for the
  * prev bridge will be called before pre_enable of this bridge.
  *
  * Example:
@@ -1055,29 +1038,25 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
 /**
  * drm_atomic_bridge_chain_enable - enables all bridges in the encoder chain
  * @first_bridge: bridge control structure
  * @state: atomic state being committed
  *
- * Calls &drm_bridge_funcs.atomic_enable (falls back on
- * &drm_bridge_funcs.enable) op for all the bridges in the encoder chain,
- * starting from the first bridge to the last. These are called after completing
- * &drm_encoder_helper_funcs.atomic_enable
+ * Calls &drm_bridge_funcs.atomic_enable op for all the bridges in the encoder
+ * chain, starting from the first bridge to the last. These are called after
+ * completing &drm_encoder_helper_funcs.atomic_enable
  *
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_atomic_bridge_chain_enable(struct drm_bridge *first_bridge,
 				    struct drm_atomic_commit *state)
 {
 	if (!first_bridge)
 		return;
 
 	drm_for_each_bridge_in_chain_from(first_bridge, bridge)
-		if (bridge->funcs->atomic_enable) {
+		if (bridge->funcs->atomic_enable)
 			bridge->funcs->atomic_enable(bridge, state);
-		} else if (bridge->funcs->enable) {
-			bridge->funcs->enable(bridge);
-		}
 }
 EXPORT_SYMBOL(drm_atomic_bridge_chain_enable);
 
 static int drm_atomic_bridge_check(struct drm_bridge *bridge,
 				   struct drm_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index 10a32620e962..bb4cf8bc6ea5 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -72,30 +72,10 @@ static void drm_test_bridge_priv_destroy(struct drm_bridge *bridge)
 	struct drm_bridge_init_priv *priv = (struct drm_bridge_init_priv *)bridge_priv->data;
 
 	priv->destroyed = true;
 }
 
-static void drm_test_bridge_enable(struct drm_bridge *bridge)
-{
-	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
-
-	priv->enable_count++;
-}
-
-static void drm_test_bridge_disable(struct drm_bridge *bridge)
-{
-	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
-
-	priv->disable_count++;
-}
-
-static const struct drm_bridge_funcs drm_test_bridge_legacy_funcs = {
-	.destroy		= drm_test_bridge_priv_destroy,
-	.enable			= drm_test_bridge_enable,
-	.disable		= drm_test_bridge_disable,
-};
-
 static void drm_test_bridge_atomic_enable(struct drm_bridge *bridge,
 					  struct drm_atomic_commit *state)
 {
 	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
 
@@ -693,39 +673,12 @@ static void drm_test_drm_bridge_get_current_state_atomic(struct kunit *test)
 
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 }
 
-/*
- * Test that drm_bridge_get_current_state() returns NULL for a
- * non-atomic bridge.
- */
-static void drm_test_drm_bridge_get_current_state_legacy(struct kunit *test)
-{
-	struct drm_bridge_init_priv *priv;
-	struct drm_bridge *bridge;
-
-	priv = drm_test_bridge_init(test, &drm_test_bridge_legacy_funcs);
-	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
-
-	/*
-	 * NOTE: Strictly speaking, we should take the bridge->base.lock
-	 * before calling that function. However, bridge->base is only
-	 * initialized if the bridge is atomic, while we explicitly
-	 * initialize one that isn't there.
-	 *
-	 * In order to avoid unnecessary warnings, let's skip the
-	 * locking. The function would return NULL in all cases anyway,
-	 * so we don't really have any concurrency to worry about.
-	 */
-	bridge = &priv->test_bridge->bridge;
-	KUNIT_EXPECT_NULL(test, drm_bridge_get_current_state(bridge));
-}
-
 static struct kunit_case drm_bridge_get_current_state_tests[] = {
 	KUNIT_CASE(drm_test_drm_bridge_get_current_state_atomic),
-	KUNIT_CASE(drm_test_drm_bridge_get_current_state_legacy),
 	{ }
 };
 
 
 static struct kunit_suite drm_bridge_get_current_state_test_suite = {
@@ -826,66 +779,10 @@ static void drm_test_drm_bridge_helper_reset_crtc_atomic_disabled(struct kunit *
 
 	KUNIT_EXPECT_EQ(test, bridge_priv->enable_count, 0);
 	KUNIT_EXPECT_EQ(test, bridge_priv->disable_count, 0);
 }
 
-/*
- * Test that a non-atomic bridge is properly power-cycled when calling
- * drm_bridge_helper_reset_crtc().
- */
-static void drm_test_drm_bridge_helper_reset_crtc_legacy(struct kunit *test)
-{
-	struct drm_modeset_acquire_ctx ctx;
-	struct drm_bridge_init_priv *priv;
-	struct drm_display_mode *mode;
-	struct drm_bridge_priv *bridge_priv;
-	int ret;
-
-	priv = drm_test_bridge_init(test, &drm_test_bridge_legacy_funcs);
-	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
-
-	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
-	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
-
-	drm_modeset_acquire_init(&ctx, 0);
-
-retry_commit:
-	ret = drm_kunit_helper_enable_crtc_connector(test,
-						     &priv->drm, priv->crtc,
-						     priv->connector,
-						     mode,
-						     &ctx);
-	if (ret == -EDEADLK) {
-		drm_modeset_backoff(&ctx);
-		goto retry_commit;
-	}
-	KUNIT_ASSERT_EQ(test, ret, 0);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-
-	bridge_priv = priv->test_bridge;
-	KUNIT_ASSERT_EQ(test, bridge_priv->enable_count, 1);
-	KUNIT_ASSERT_EQ(test, bridge_priv->disable_count, 0);
-
-	drm_modeset_acquire_init(&ctx, 0);
-
-retry_reset:
-	ret = drm_bridge_helper_reset_crtc(&bridge_priv->bridge, &ctx);
-	if (ret == -EDEADLK) {
-		drm_modeset_backoff(&ctx);
-		goto retry_reset;
-	}
-	KUNIT_ASSERT_EQ(test, ret, 0);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-
-	KUNIT_EXPECT_EQ(test, bridge_priv->enable_count, 2);
-	KUNIT_EXPECT_EQ(test, bridge_priv->disable_count, 1);
-}
-
 /*
  * Test that a bridge using the drm_atomic_helper_bridge_get_hdmi_output_bus_fmts()
  * function for &drm_bridge_funcs.atomic_get_output_bus_fmts behaves as expected
  * for an HDMI connector bridge. Does so by creating an HDMI bridge connector
  * with RGB444, YCBCR444, and YCBCR420 (but not YCBCR422) as supported formats,
@@ -968,11 +865,10 @@ static void drm_test_drm_bridge_helper_hdmi_output_bus_fmts(struct kunit *test)
 }
 
 static struct kunit_case drm_bridge_helper_reset_crtc_tests[] = {
 	KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_atomic),
 	KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_atomic_disabled),
-	KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_legacy),
 	KUNIT_CASE(drm_test_drm_bridge_helper_hdmi_output_bus_fmts),
 	{ }
 };
 
 static struct kunit_suite drm_bridge_helper_reset_crtc_test_suite = {
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 18f3db367dc1..123edad5fa65 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -171,56 +171,10 @@ struct drm_bridge_funcs {
 	 * operation should be rejected.
 	 */
 	bool (*mode_fixup)(struct drm_bridge *bridge,
 			   const struct drm_display_mode *mode,
 			   struct drm_display_mode *adjusted_mode);
-	/**
-	 * @disable:
-	 *
-	 * This callback should disable the bridge. It is called right before
-	 * the preceding element in the display pipe is disabled. If the
-	 * preceding element is a bridge this means it's called before that
-	 * bridge's @disable vfunc. If the preceding element is a &drm_encoder
-	 * it's called right before the &drm_encoder_helper_funcs.disable,
-	 * &drm_encoder_helper_funcs.prepare or &drm_encoder_helper_funcs.dpms
-	 * hook.
-	 *
-	 * The bridge can assume that the display pipe (i.e. clocks and timing
-	 * signals) feeding it is still running when this callback is called.
-	 *
-	 * The @disable callback is optional.
-	 *
-	 * NOTE:
-	 *
-	 * This is deprecated, do not use!
-	 * New drivers shall use &drm_bridge_funcs.atomic_disable.
-	 */
-	void (*disable)(struct drm_bridge *bridge);
-
-	/**
-	 * @post_disable:
-	 *
-	 * This callback should disable the bridge. It is called right after the
-	 * preceding element in the display pipe is disabled. If the preceding
-	 * element is a bridge this means it's called after that bridge's
-	 * @post_disable function. If the preceding element is a &drm_encoder
-	 * it's called right after the encoder's
-	 * &drm_encoder_helper_funcs.disable, &drm_encoder_helper_funcs.prepare
-	 * or &drm_encoder_helper_funcs.dpms hook.
-	 *
-	 * The bridge must assume that the display pipe (i.e. clocks and timing
-	 * signals) feeding it is no longer running when this callback is
-	 * called.
-	 *
-	 * The @post_disable callback is optional.
-	 *
-	 * NOTE:
-	 *
-	 * This is deprecated, do not use!
-	 * New drivers shall use &drm_bridge_funcs.atomic_post_disable.
-	 */
-	void (*post_disable)(struct drm_bridge *bridge);
 
 	/**
 	 * @mode_set:
 	 *
 	 * This callback should set the given mode on the bridge. It is called
@@ -247,59 +201,10 @@ struct drm_bridge_funcs {
 	 * &drm_bridge_funcs.atomic_enable operation.
 	 */
 	void (*mode_set)(struct drm_bridge *bridge,
 			 const struct drm_display_mode *mode,
 			 const struct drm_display_mode *adjusted_mode);
-	/**
-	 * @pre_enable:
-	 *
-	 * This callback should enable the bridge. It is called right before
-	 * the preceding element in the display pipe is enabled. If the
-	 * preceding element is a bridge this means it's called before that
-	 * bridge's @pre_enable function. If the preceding element is a
-	 * &drm_encoder it's called right before the encoder's
-	 * &drm_encoder_helper_funcs.enable, &drm_encoder_helper_funcs.commit or
-	 * &drm_encoder_helper_funcs.dpms hook.
-	 *
-	 * The display pipe (i.e. clocks and timing signals) feeding this bridge
-	 * will not yet be running when this callback is called. The bridge must
-	 * not enable the display link feeding the next bridge in the chain (if
-	 * there is one) when this callback is called.
-	 *
-	 * The @pre_enable callback is optional.
-	 *
-	 * NOTE:
-	 *
-	 * This is deprecated, do not use!
-	 * New drivers shall use &drm_bridge_funcs.atomic_pre_enable.
-	 */
-	void (*pre_enable)(struct drm_bridge *bridge);
-
-	/**
-	 * @enable:
-	 *
-	 * This callback should enable the bridge. It is called right after
-	 * the preceding element in the display pipe is enabled. If the
-	 * preceding element is a bridge this means it's called after that
-	 * bridge's @enable function. If the preceding element is a
-	 * &drm_encoder it's called right after the encoder's
-	 * &drm_encoder_helper_funcs.enable, &drm_encoder_helper_funcs.commit or
-	 * &drm_encoder_helper_funcs.dpms hook.
-	 *
-	 * The bridge can assume that the display pipe (i.e. clocks and timing
-	 * signals) feeding it is running when this callback is called. This
-	 * callback must enable the display link feeding the next bridge in the
-	 * chain if there is one.
-	 *
-	 * The @enable callback is optional.
-	 *
-	 * NOTE:
-	 *
-	 * This is deprecated, do not use!
-	 * New drivers shall use &drm_bridge_funcs.atomic_enable.
-	 */
-	void (*enable)(struct drm_bridge *bridge);
 
 	/**
 	 * @atomic_pre_enable:
 	 *
 	 * This callback should enable the bridge. It is called right before
@@ -1357,18 +1262,10 @@ static inline struct drm_bridge_state *
 drm_bridge_get_current_state(struct drm_bridge *bridge)
 {
 	if (!bridge)
 		return NULL;
 
-	/*
-	 * Only atomic bridges will have bridge->base initialized by
-	 * drm_atomic_private_obj_init(), so we need to make sure we're
-	 * working with one before we try to use the lock.
-	 */
-	if (!bridge->funcs || !bridge->funcs->atomic_create_state)
-		return NULL;
-
 	drm_modeset_lock_assert_held(&bridge->base.lock);
 
 	if (!bridge->base.state)
 		return NULL;
 

-- 
2.54.0

Re: [PATCH v2 37/37] drm/bridge: Remove legacy bridge callback support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unconditional NULL pointer dereference for bridges missing atomic state callbacks.

Pre-existing issues:
- [High] Lockless modifications of `encoder->bridge_chain` and `dev->mode_config.privobj_list` cause list corruption.
--

--- Patch [37]: [PATCH v2 37/37] drm/bridge: Remove legacy bridge callback support ---
commit 7f35866581b3c5c247d4a1cdd1535127f3999993
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: Remove legacy bridge callback support

This commit removes legacy bridge callback support, as all bridge drivers
are expected to have been converted to atomic variants. The private object
state is now initialized unconditionally at attach time.

> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 6ead9b75ae3de..6fb443984dff3 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -629,9 +624,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>  			goto err_reset_bridge;
>  	}
>  
> -	if (drm_bridge_is_atomic(bridge))
> -		drm_atomic_private_obj_init(bridge->dev, &bridge->base,
> -					    &drm_bridge_priv_state_funcs);
> +	drm_atomic_private_obj_init(bridge->dev, &bridge->base,
> +				    &drm_bridge_priv_state_funcs);

[Severity: High]
Does this unconditionally crash for bridges that don't implement atomic state
callbacks?

If a bridge (like aspire_ec_bridge_funcs in
drivers/platform/arm64/acer-aspire1-ec.c) is missing .atomic_create_state,
drm_atomic_private_obj_init() will call drm_bridge_atomic_create_priv_state()
which blindly executes bridge->funcs->atomic_create_state(bridge).

Could this result in a NULL pointer dereference during initialization?

>  	return 0;

[ ... ]

> @@ -664,8 +658,7 @@ void drm_bridge_detach(struct drm_bridge *bridge)
>  	if (WARN_ON(!bridge->dev))
>  		return;
>  
> -	if (drm_bridge_is_atomic(bridge))
> -		drm_atomic_private_obj_fini(&bridge->base);
> +	drm_atomic_private_obj_fini(&bridge->base);

[Severity: High]
This is a pre-existing issue, but does drm_bridge_detach() remove the bridge
from the encoder's bridge chain list locklessly?

Looking at drm_bridge_detach() just below this block:

	list_del(&bridge->chain_node);

It performs this deletion without acquiring encoder->bridge_chain_mutex.
Could this race with list iterations like drm_atomic_bridge_chain_disable()
that do hold the mutex?

Additionally, drm_atomic_private_obj_fini() modifies
dev->mode_config.privobj_list without synchronization. Can this cause list
corruption if a bridge driver is unbound via sysfs during active modesets?

>  	if (bridge->funcs->detach)
>  		bridge->funcs->detach(bridge);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=37

Re: [PATCH v2 00/37] drm/bridge: Convert all bridges to atomic

[Luca Ceresoli]

Il 7 luglio 2026 17:11:07 CEST, Maxime Ripard <mripard@kernel.org> ha scritto:
>Hi,
>
>Over the years, most of the bridges have been converted to atomic
>modesetting and hooks, but not all of them. This forces us to maintain
>two different code path in quite a few places, which is pretty
>bothersome. The switch to atomic modesetting for legacy bridges though
>is pretty trivial, and we don't have a lot of drivers still using the
>legacy path.
>
>This series converts all bridges to atomic modesetting and drops the
>legacy codepaths where relevant.
>
>Let me know what you think,
>Maxime
>
>Signed-off-by: Maxime Ripard <mripard@kernel.org>

I'm out of office so I'm afraid I can only reply from mobile, but I reviewed the series on a real laptop:

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Luca