Message-ID: <20250804083136.058353f9@booty>
Date: Mon, 4 Aug 2025 08:31:36 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Liu Ying <victor.liu@....com>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong
 <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, Laurent
 Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman
 <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
 <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
 <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Shawn Guo
 <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix
 Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>,
 Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, Philipp Zabel
 <p.zabel@...gutronix.de>, Hui Pu <Hui.Pu@...ealthcare.com>, Thomas
 Petazzoni <thomas.petazzoni@...tlin.com>, linux-kernel@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, imx@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, Andy Shevchenko
 <andriy.shevchenko@...ux.intel.com>, Andrew Morton
 <akpm@...ux-foundation.org>, Zijun Hu <quic_zijuhu@...cinc.com>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v2 0/9] drm/bridge: get/put the bridge returned by
 drm_bridge_get_next_bridge()

Hello Liu,

thanks for having reviewed this series.

On Mon, 4 Aug 2025 12:02:48 +0800
Liu Ying <victor.liu@....com> wrote:

> Hi Luca,
> 
> On 08/01/2025, Luca Ceresoli wrote:
> > Note: the cover in v1 was mentioning by mistake
> > drm_bridge_get_last_bridge() instead of drm_bridge_get_next_bridge().
> >                ^^^^
> > 
> > This series adds drm_bridge_get/put() calls for DRM bridges returned by
> > drm_bridge_get_next_bridge().
> > 
> > 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 (in v6.16-rc1)
> >     B. ✔ convert all bridge drivers to new API (now in drm-misc-next)
> >     C. ✔ kunit tests (now in drm-misc-next)
> >     D. ✔ add get/put to drm_bridge_add/remove() + attach/detach()
> >          and warn on old allocation pattern (now in drm-misc-next)
> >     E. ➜ add get/put on drm_bridge accessors
> >        1. ✔ drm_bridge_chain_get_first_bridge() + add a cleanup action
> >        2. ✔ drm_bridge_get_prev_bridge()
> >        3. ➜ drm_bridge_get_next_bridge()
> >        4. … drm_for_each_bridge_in_chain()
> >        5. drm_bridge_connector_init
> >        6. of_drm_find_bridge
> >        7. drm_of_find_panel_or_bridge, *_of_get_bridge
> >     F. debugfs improvements
> >  2. handle gracefully atomic updates during bridge removal
> >  3. … avoid DSI host drivers to have dangling pointers to DSI devices
> >  4. finish the hotplug bridge work, removing the "always-disconnected"
> >     connector, moving code to the core and potentially removing the
> >     hotplug-bridge itself (this needs to be clarified as points 1-3 are
> >     developed)
> > 
> > There are various users of drm_bridge_get_next_bridge() which cannot be
> > converted easily. Luckily they are not really looking for the next bridge,
> > but for something else, such as getting the last bridge in the encoder
> > chain or checking whether a bridge is the last in the encoder chain. So
> > introduce better functions for those users and use them instead of
> > drm_bridge_get_next_bridge(), making the code cleaner at the same time.
> > 
> > Finally add a drm_bridge_get() to drm_bridge_get_next_bridge() and
> > drm_bridge_put() to the remaining, and legitimate, calls.
> > 
> > [0] https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/0cc6aadd7fc1e629b715ea3d1ba537ef2da95eec
> > [1] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/t/#u
> > 
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
> > ---
> > Changes in v2:
> > - Use cleanup action in patches 4, 8 and 9
> > - Fix: replace get_last -> get_next in cover
> > - Link to v1: https://lore.kernel.org/r/20250709-drm-bridge-alloc-getput-drm_bridge_get_next_bridge-v1-0-48920b9cf369@bootlin.com
> > 
> > ---
> > Luca Ceresoli (9):
> >       list: add list_last_entry_or_null()
> >       drm/bridge: add drm_bridge_chain_get_last_bridge()
> >       drm/bridge: imx93-mipi-dsi: use drm_bridge_chain_get_last_bridge()
> >       drm/omapdrm: use drm_bridge_chain_get_last_bridge()
> >       drm/bridge: add drm_bridge_is_last()
> >       drm/display: bridge_connector: use drm_bridge_is_last()
> >       drm/bridge: get the bridge returned by drm_bridge_get_next_bridge()
> >       drm/bridge: put the bridge returned by drm_bridge_get_next_bridge()
> >       drm/imx: parallel-display: put the bridge returned by drm_bridge_get_next_bridge()
> > 
> >  drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c    | 12 ++++------
> >  drivers/gpu/drm/display/drm_bridge_connector.c |  5 ++--
> >  drivers/gpu/drm/drm_bridge.c                   |  3 +--
> >  drivers/gpu/drm/imx/ipuv3/parallel-display.c   |  4 ++--
> >  drivers/gpu/drm/omapdrm/omap_drv.c             |  6 ++---
> >  include/drm/drm_bridge.h                       | 32 +++++++++++++++++++++++++-
> >  include/linux/list.h                           | 14 +++++++++++
> >  7 files changed, 57 insertions(+), 19 deletions(-)  
> 
> Looks like this one needs to be handled by this patch series?
> 
> drivers/gpu/drm/omapdrm/omap_encoder.c:101:
> bridge = drm_bridge_get_next_bridge(bridge)) {

This is handled in a better way in a separate series, converting the
for_each loops around the bridges in an encoder chain:

https://lore.kernel.org/all/20250723-drm-bridge-alloc-getput-for_each_bridge-v1-9-be8f4ae006e9@bootlin.com/

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Powered by blists - more mailing lists