dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Luca Ceresoli <luca.ceresoli@bootlin.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	 Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	 David Airlie <airlied@gmail.com>,
	Simona Vetter <simona@ffwll.ch>,
	 Andrzej Hajda <andrzej.hajda@intel.com>,
	 Neil Armstrong <neil.armstrong@linaro.org>,
	Robert Foss <rfoss@kernel.org>,
	 Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	 Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Hui Pu <Hui.Pu@gehealthcare.com>,
	 Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	 dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	 Luca Ceresoli <luca.ceresoli@bootlin.com>
Subject: [PATCH v2 0/7] drm/bridge: protect encoder bridge chain with a mutex
Date: Fri, 03 Oct 2025 12:39:22 +0200	[thread overview]
Message-ID: <20251003-drm-bridge-alloc-encoder-chain-mutex-v2-0-78bf61580a06@bootlin.com> (raw)

This series ensures that the bridge chain of the encoder will not be
modified while some other concurrent code flows are iterating over it.

This is part of the work towards removal of bridges from a still existing
DRM pipeline without use-after-free. The grand plan was discussed in [1].
Here's the work breakdown (➜ marks the current series):

 1. ➜ add refcounting to DRM bridges (struct drm_bridge)
    (based on devm_drm_bridge_alloc() [0])
    A. ✔ add new alloc API and refcounting (v6.16)
    B. ✔ convert all bridge drivers to new API (v6.17)
    C. ✔ kunit tests (v6.17)
    D. ✔ add get/put to drm_bridge_add/remove() + attach/detach()
         and warn on old allocation pattern (v6.17)
    E. ➜ add get/put on drm_bridge accessors
       1. ✔ drm_bridge_chain_get_first_bridge() + add a cleanup action
            (drm-misc-next)
       2. ✔ drm_bridge_get_prev_bridge() (drm-misc-next)
       3. ✔ drm_bridge_get_next_bridge() (drm-misc-next)
       4. ✔ drm_for_each_bridge_in_chain() (drm-misc-next)
       5. ✔ drm_bridge_connector_init (drm-misc-next)
       6. ➜ protect encoder bridge chain with a mutex
       7. of_drm_find_bridge
       8. drm_of_find_panel_or_bridge, *_of_get_bridge
       9. … enforce drm_bridge_add before drm_bridge_attach
    F. ✔ debugfs improvements
       1. ✔ add top-level 'bridges' file (v6.16)
       2. ✔ show refcount and list removed bridges (drm-misc-next)
 2. … handle gracefully atomic updates during bridge removal
 3. … DSI host-device driver interaction
 4. ✔ removing the need for the "always-disconnected" connector
 5. finish the hotplug bridge work, moving code to the core and potentially
    removing the hotplug-bridge itself (this needs to be clarified as
    points 1-3 are developed)

The per-encoder bridge chain is currently assumed to be static once it is
fully initialized. Work is in progress to add hot-pluggable bridges,
breaking that assumption.

With hotplug and especially hot-unplug, bridges will be added and removed
without notice, and thus be added/removed to/from the encoder chain in
drm_bridge_attach/detach(), concurrently to the code iterating on the
chain. This can result in disruption of the code iterating over the
chain. The rationale is explained by a detailed example in patch 2.

Avoid bugs by introducing a mutex to make list insertion, removal and
iterations mutually exclusive.

[1] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/#t

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v2:
- Improve commit messages and add documentation as per v1 review
- Patch 4: fixed infinite loop when encoder->bridge_chain is empty
- Link to v1: https://lore.kernel.org/r/20250926-drm-bridge-alloc-encoder-chain-mutex-v1-0-23b62c47356a@bootlin.com

---
Luca Ceresoli (7):
      drm/encoder: add mutex to protect the bridge chain
      drm/encoder: drm_encoder_cleanup: take chain mutex while tearing down
      drm/bridge: lock the encoder bridge chain mutex during insertion
      drm/bridge: lock the encoder chain in scoped for_each loops
      drm/bridge: prevent encoder chain changes while iterating with list_for_each_entry_from()
      drm/bridge: prevent encoder chain changes while iterating with list_for_each_entry_reverse()
      drm/bridge: prevent encoder chain changes in pre_enable/post_disable

 drivers/gpu/drm/drm_bridge.c  | 51 ++++++++++++++++++-----------------
 drivers/gpu/drm/drm_encoder.c |  4 +++
 include/drm/drm_bridge.h      | 62 +++++++++++++++++++++++--------------------
 include/drm/drm_encoder.h     | 39 +++++++++++++++++++++++++++
 4 files changed, 103 insertions(+), 53 deletions(-)
---
base-commit: 0f2efbe6d8305b91c9b2c92ebaf8c24a614bc305
change-id: 20250925-drm-bridge-alloc-encoder-chain-mutex-b78d62085ee5

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>


             reply	other threads:[~2025-10-03 10:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-03 10:39 Luca Ceresoli [this message]
2025-10-03 10:39 ` [PATCH v2 1/7] drm/encoder: add mutex to protect the bridge chain Luca Ceresoli
2025-10-04  9:47   ` Dmitry Baryshkov
2025-10-07  9:45     ` Luca Ceresoli
2025-10-07 14:02       ` Luca Ceresoli
2025-10-03 10:39 ` [PATCH v2 2/7] drm/encoder: drm_encoder_cleanup: take chain mutex while tearing down Luca Ceresoli
2025-10-03 10:39 ` [PATCH v2 3/7] drm/bridge: lock the encoder bridge chain mutex during insertion Luca Ceresoli
2025-10-03 10:39 ` [PATCH v2 4/7] drm/bridge: lock the encoder chain in scoped for_each loops Luca Ceresoli
2025-10-03 14:04   ` Maxime Ripard
2025-10-03 16:39     ` Luca Ceresoli
2025-10-03 10:39 ` [PATCH v2 5/7] drm/bridge: prevent encoder chain changes while iterating with list_for_each_entry_from() Luca Ceresoli
2025-10-03 10:39 ` [PATCH v2 6/7] drm/bridge: prevent encoder chain changes while iterating with list_for_each_entry_reverse() Luca Ceresoli
2025-10-03 10:39 ` [PATCH v2 7/7] drm/bridge: prevent encoder chain changes in pre_enable/post_disable Luca Ceresoli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251003-drm-bridge-alloc-encoder-chain-mutex-v2-0-78bf61580a06@bootlin.com \
    --to=luca.ceresoli@bootlin.com \
    --cc=Hui.Pu@gehealthcare.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@gmail.com \
    --cc=andrzej.hajda@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=rfoss@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).