All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Maxime Ripard" <mripard@kernel.org>,
	"Luca Ceresoli" <luca.ceresoli@bootlin.com>
Cc: "Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"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>,
	"Jonas Karlman" <jonas@kwiboo.se>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Inki Dae" <inki.dae@samsung.com>,
	"Jagan Teki" <jagan@amarulasolutions.com>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Marek Vasut" <marex@denx.de>, "Stefan Agner" <stefan@agner.ch>,
	"Frank Li" <Frank.Li@nxp.com>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Fabio Estevam" <festevam@gmail.com>,
	"Hui Pu" <Hui.Pu@gehealthcare.com>,
	"Ian Ray" <ian.ray@gehealthcare.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
	<imx@lists.linux.dev>, <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 05/37] drm/display: bridge-connector: split code creating the connector to a subfunction
Date: Fri, 17 Jul 2026 11:41:33 +0200	[thread overview]
Message-ID: <DK0QXI6KF7PC.3DJW38KB8MJED@bootlin.com> (raw)
In-Reply-To: <20260716-stoic-muskox-from-asgard-c12b65@houat>

Hi Maxime,

On Thu Jul 16, 2026 at 3:22 PM CEST, Maxime Ripard wrote:
> On Thu, Jul 16, 2026 at 10:37:24AM +0200, Luca Ceresoli wrote:
>> >> > Now if the bridges start doing it themselves we should go back to
>> >> > those encoder drivers and ditch all the drm_bridge_connector from
>> >> > there?
>> >> >
>> >> > I must be missing something. Can you elaborate on this?
>> >>
>> >> drm_bridge_connectors bring together a (complete) bridge chain and a
>> >> connector. If you don't have either anymore, then we shouldn't keep it
>> >> around.
>> >>
>> >> What I was suggesting before was only a suggestion. I guess we could
>> >> also make the encoder own the hotplug handling code and create the
>> >> drm_bridge_connector when the chain is complete, and remove it when it's
>> >> no longer the case.
>> >
>> > That's an interesting option. We don't have to keep drm_bridge_connector
>> > in its current form, but I don't think we should go back to individual
>> > bridge driver creating connectors, especially now that we have bridge
>> > chains where the connector ops are implemented collectively by multiple
>> > bridges.
>>
>> I definitely agree we don't want to add burden back on the encoder.
>
> I don't think Laurent mentioned the encoder anywhere.

Ah, indeed, sorry! However, I think both the bridges and the encoder
drivers should equally have the minimum burden on them.

Right now the recommended practice is:

 - bridges do not create connectors (thanks to DRM_BRIDGE_ATTACH_NO_CONNECTOR)
 - encoders just call drm_bridge_connector_init(), which does all the
   common operations to populate a suitable drm_connector

So all common operations involved in connector creation and bridge chain
analysis are implemented in common code, not per-bridge or
per-encoder. That's good.

>> >> We can discuss alternatives too. But either way, we shouldn't have it
>> >> stick around.
>>
>> Bottom line, I roughly see three ideas mentioned:
>>
>>  1. (this series) extend the drm_bridge_connector to create the
>>     drm_connector based on bridge hotplug events [+rename it]
>>  2. - keep the drm_bridge_connector (mostly) as is
>>     - let each encoder driver add/remove it based on bridge hotplug events
>>     => more burden on encoder drivers -> no
>
> Can you motivate that with *any* reason? Because I really feel like it's
> the best solution going forward.

My understanding of your idea (maybe a bit overstressed just to ensure it's
clear) is that:

 - the drm_bridge_connector should stay (almost) unmodified
 - there should be no new "manager" component (not sure this is actually
   your opinion, can you comment on this specifically?)
 - every encoder driver would have to:
   - register to receive hotplug events
   - when receiving one such event, find out whether the hardware is
     complete or not (by calling drm_bridge_connector_pipeline_is_complete()
     or so)
   - create/destroy a drm_bridge_connector based on hotplug events

Is this somewhat close to what you have in mind?


To me the best solution to add hotplug support is that encoder drivers
replace the single drm_bridge_connector_init() call with a single call to
something new (let's call it a hotplug manager), which takes care of all
the common aspects: registering to receive bridge hotplug events, finding
out whether the hardware pipeline is complete or not, and add/remove the
drm_connector based on that.

In other words, the changes on encoder drivers would be similar to patch
37. In a nutshell:

  -	connector = drm_bridge_connector_init(lcdif->drm, encoder);
  +	drm_hotplug_manager = drm_hotplug_manager_init(lcdif->drm, encoder);

All the hotplug logic would be in common code, and any maintenance and
future improvements to it would stay in a single place, benefitting all
encoders at once.

What do you think about this?

Luca

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

  reply	other threads:[~2026-07-17  9:41 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19 10:37 [PATCH 00/37] drm bridge hotplug Luca Ceresoli
2026-05-19 10:37 ` [PATCH 01/37] drm/connector: split drmm_connector_hdmi_init() in 3 parts Luca Ceresoli
2026-06-08 11:37   ` Maxime Ripard
2026-05-19 10:37 ` [PATCH 02/37] drm/connector: add drm_connector_hdmi_dynamic_init() Luca Ceresoli
2026-05-19 11:04   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 03/37] drm/display: bridge-connector: rename variable for consistency Luca Ceresoli
2026-05-19 10:37 ` [PATCH 04/37] drm/display: bridge-connector: store the drm_device pointer Luca Ceresoli
2026-06-08 11:34   ` Maxime Ripard
2026-06-12 13:12     ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 05/37] drm/display: bridge-connector: split code creating the connector to a subfunction Luca Ceresoli
2026-06-08 11:40   ` Maxime Ripard
2026-06-12 12:56     ` Luca Ceresoli
2026-06-24 11:41       ` Maxime Ripard
2026-06-24 15:47         ` Luca Ceresoli
2026-06-26 10:09           ` Maxime Ripard
2026-06-26 14:16             ` Luca Ceresoli
2026-06-26 14:38               ` Maxime Ripard
2026-06-26 16:51                 ` Luca Ceresoli
2026-06-29 14:44                   ` Laurent Pinchart
2026-06-29 15:23                     ` Luca Ceresoli
2026-07-07  9:55                   ` Maxime Ripard
2026-07-07 12:32                     ` Laurent Pinchart
2026-07-16  8:37                       ` Luca Ceresoli
2026-07-16 13:22                         ` Maxime Ripard
2026-07-17  9:41                           ` Luca Ceresoli [this message]
2026-05-19 10:37 ` [PATCH 06/37] drm/display: bridge-connector: use a drm_bridge_connector internally, not a drm_connector Luca Ceresoli
2026-06-08 11:41   ` Maxime Ripard
2026-06-12 12:57     ` Luca Ceresoli
2026-06-24 11:47       ` Maxime Ripard
2026-06-24 15:39         ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 07/37] drm/display: bridge-connector: extract drm_bridge_connector_get_bridges() Luca Ceresoli
2026-05-19 11:01   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 08/37] drm/display: bridge-connector: return int from drm_bridge_connector_get_bridges() Luca Ceresoli
2026-05-19 10:58   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 09/37] drm/display: bridge-connector: extract drm_bridge_connector_init_hdmi_audio_cec() Luca Ceresoli
2026-05-19 10:37 ` [PATCH 10/37] drm/display: bridge-connector: return int from drm_bridge_connector_init_hdmi_audio_cec() Luca Ceresoli
2026-05-19 10:37 ` [PATCH 11/37] drm/display: bridge-connector: return int from drm_bridge_connector_add_connector() Luca Ceresoli
2026-05-19 10:37 ` [PATCH 12/37] drm/display: bridge-connector: hoist error management to common code Luca Ceresoli
2026-05-19 10:37 ` [PATCH 13/37] drm/display: bridge-connector: move drm_bridge_connector_put_bridges() definition eariler Luca Ceresoli
2026-05-19 10:37 ` [PATCH 14/37] drm/display: bridge-connector: add non-drmm variant of drm_bridge_connector_put_bridges() Luca Ceresoli
2026-05-19 10:37 ` [PATCH 15/37] drm/display: bridge-connector: allocate the connector dynamically Luca Ceresoli
2026-05-19 11:15   ` sashiko-bot
2026-06-08 11:46   ` Maxime Ripard
2026-06-12 12:44     ` Luca Ceresoli
2026-06-24 11:48       ` Maxime Ripard
2026-06-24 15:34         ` Luca Ceresoli
2026-06-25  9:32           ` Luca Ceresoli
2026-06-26 11:44             ` Maxime Ripard
2026-05-19 10:37 ` [PATCH 16/37] drm/display: bridge-connector: move per-connector fields to the dynamic connector Luca Ceresoli
2026-05-19 11:17   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 17/37] drm/display: bridge-connector: protect dynconn creation and destruction with a mutex Luca Ceresoli
2026-05-19 11:18   ` sashiko-bot
2026-06-08 11:49   ` Maxime Ripard
2026-06-10 13:30     ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 18/37] drm/bridge: samsung-dsim: remove the panel_bridge on host_detach Luca Ceresoli
2026-06-08 11:53   ` Maxime Ripard
2026-06-10 13:24     ` Luca Ceresoli
2026-06-24 12:26       ` Maxime Ripard
2026-05-19 10:37 ` [PATCH 19/37] drm/bridge: samsung-dsim: move drm_bridge_add() call to probe Luca Ceresoli
2026-05-19 11:16   ` sashiko-bot
2026-06-08 11:58   ` Maxime Ripard
2026-06-11  8:54     ` Luca Ceresoli
2026-06-24 15:28       ` Maxime Ripard
2026-06-25 17:06         ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 20/37] drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available Luca Ceresoli
2026-05-19 11:13   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 21/37] drm/bridge: initialize chain_node list head on allocation Luca Ceresoli
2026-05-19 10:37 ` [PATCH 22/37] drm/bridge: initialize chain_node list head on detach and attach errors Luca Ceresoli
2026-05-19 11:17   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 23/37] drm/encoder: add drm_encoder_cleanup_from() Luca Ceresoli
2026-05-19 11:14   ` sashiko-bot
2026-06-08 12:10   ` Maxime Ripard
2026-06-09 10:10     ` Luca Ceresoli
2026-06-09 12:43       ` Maxime Ripard
2026-05-19 10:37 ` [PATCH 24/37] drm/atomic: move drm_atomic_helper_disable_all() and drm_atomic_helper_shutdown() from drm_atomic_helper to drm_atomic Luca Ceresoli
2026-05-19 10:57   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 25/37] drm/bridge: shutdown and cleanup on bridge unplug Luca Ceresoli
2026-05-19 11:09   ` sashiko-bot
2026-06-08 12:07   ` Maxime Ripard
2026-06-09  9:31     ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 26/37] drm: event-notifier: add mechanism to notify about hotplug events Luca Ceresoli
2026-05-19 11:06   ` sashiko-bot
2026-06-08 12:13   ` Maxime Ripard
2026-06-09  9:30     ` Luca Ceresoli
2026-06-24 15:09       ` Maxime Ripard
2026-05-19 10:37 ` [PATCH 27/37] drm/bridge: notify about detached bridges Luca Ceresoli
2026-05-19 11:32   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 28/37] drm/mipi-dsi: turn DRM_MIPI_DSI into a tristate Luca Ceresoli
2026-05-19 11:07   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 29/37] drm/mipi-dsi: notify about DSI attach Luca Ceresoli
2026-05-19 11:13   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 30/37] drm/bridge: add drm_bridge_is_tail() to know whether a bridge completes the pipeline Luca Ceresoli
2026-05-19 10:59   ` sashiko-bot
2026-06-08 12:34   ` Maxime Ripard
2026-06-09  8:23     ` Luca Ceresoli
2026-06-24 13:04       ` Maxime Ripard
2026-06-24 16:06         ` Luca Ceresoli
2026-05-19 10:37 ` [PATCH 31/37] drm/bridge: panel: implement .is_tail Luca Ceresoli
2026-05-19 15:12   ` Neil Armstrong
2026-05-19 10:37 ` [PATCH 32/37] drm/bridge: display-connector: " Luca Ceresoli
2026-05-19 10:37 ` [PATCH 33/37] drm/bridge: samsung-dsim: " Luca Ceresoli
2026-05-19 10:37 ` [PATCH 34/37] drm/bridge: ti-sn65dsi83: " Luca Ceresoli
2026-05-19 10:37 ` [PATCH 35/37] drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER Luca Ceresoli
2026-05-19 11:21   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 36/37] drm/display: bridge-connector: handle bridge hotplug Luca Ceresoli
2026-05-19 11:15   ` sashiko-bot
2026-05-19 10:37 ` [PATCH 37/37] drm/mxsfb/lcdif: enable " Luca Ceresoli
2026-05-19 11:33   ` sashiko-bot
2026-06-01 15:44 ` [PATCH 00/37] drm " Luca Ceresoli
2026-06-09  7:47   ` 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=DK0QXI6KF7PC.3DJW38KB8MJED@bootlin.com \
    --to=luca.ceresoli@bootlin.com \
    --cc=Frank.Li@nxp.com \
    --cc=Hui.Pu@gehealthcare.com \
    --cc=airlied@gmail.com \
    --cc=andrzej.hajda@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=festevam@gmail.com \
    --cc=ian.ray@gehealthcare.com \
    --cc=imx@lists.linux.dev \
    --cc=inki.dae@samsung.com \
    --cc=jagan@amarulasolutions.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=kernel@pengutronix.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=marex@denx.de \
    --cc=mripard@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=rfoss@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=simona@ffwll.ch \
    --cc=stefan@agner.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.