From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1F437C4453A for ; Wed, 22 Jul 2026 09:11:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:References: Content-Transfer-Encoding:Mime-Version:From:To:Cc:Subject:Message-Id:Date: Content-Type:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=CVMvqT2UQr1txHFCMAIcwU7vZqub1auWVL1fnTiKFZE=; b=IfccrStoLdIEPUwaZsGq/N5Kw6 J5bA0KicyiUv3GsyHYmNvBGJP042UZ42VU/g+9LuHRqv/+58qW5QfzCTtxRnxuy1S82nMEtJtX/y/ 92yvJG5mKbT8tFQd6TxZsq5/8t1dpcXan2e0pRTsLsjtV26rLL+kTFmwXOfhptHhMIhahRM/3LPn+ YX/HjbeMpHOPOcIQaB47Hc/sT9KQa5+Rut5Nw/bA4AZtWRM9JXRsdJtoOc9rv6q/Dx6VMvNtpWzyR RrLODEHJJof9uG4JNxfPKDr5apNVFOjpcddlqe54eQwJ6AWkllMvwcf92WeStmtwwhW8Ln6xzDCVS 9DbQkxOw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wmSz4-0000000BLkr-3UW5; Wed, 22 Jul 2026 09:11:22 +0000 Received: from smtpout-02.galae.net ([185.246.84.56]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1wmSz2-0000000BLk5-0gbe for linux-arm-kernel@lists.infradead.org; Wed, 22 Jul 2026 09:11:22 +0000 Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-02.galae.net (Postfix) with ESMTPS id 1C60A1A1144; Wed, 22 Jul 2026 09:11:15 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id DF2FE60388; Wed, 22 Jul 2026 09:11:14 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 2FAFF11BD0ADC; Wed, 22 Jul 2026 11:11:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1784711473; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=CVMvqT2UQr1txHFCMAIcwU7vZqub1auWVL1fnTiKFZE=; b=z2mUyKFafy0l5dpLU+0DGap45Kxpu05bFTufLn+AejhY8o/GNw+bsMIGqsZ+wiE4M52cUb tJ2kf9T2YiRxOqhr2rWzY6HX0HaZTrXAkwRwYkiIApKjq6ixTgX+STS1gkAJGd5e/6BVDK GToI/hX/BbykAokOk8TWHpHPFG8V07FtMaqxnbrhnkb/T4rB9QWpMae8QNrsMiy7H/LA64 CBxKwBYvzKbIdeqwP++7cQtAxs+z3x9pM+K4I1TBW6Z2Gjrq8TuQOPUI1fgOKPRLrxeDV9 sIjNA1KMA98U1kfDDJO4VYRUgzaOUz1xNXwlEr4Ixh4tdw5zVeqvwFdXq3uDLw== Content-Type: text/plain; charset=UTF-8 Date: Wed, 22 Jul 2026 11:10:53 +0200 Message-Id: Subject: Re: [PATCH 05/37] drm/display: bridge-connector: split code creating the connector to a subfunction Cc: "Laurent Pinchart" , "Maarten Lankhorst" , "Thomas Zimmermann" , "David Airlie" , "Simona Vetter" , "Andrzej Hajda" , "Neil Armstrong" , "Robert Foss" , "Jonas Karlman" , "Jernej Skrabec" , "Inki Dae" , "Jagan Teki" , "Marek Szyprowski" , "Marek Vasut" , "Stefan Agner" , "Frank Li" , "Sascha Hauer" , "Pengutronix Kernel Team" , "Fabio Estevam" , "Hui Pu" , "Ian Ray" , "Thomas Petazzoni" , , , , To: "Maxime Ripard" , "Luca Ceresoli" From: "Luca Ceresoli" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.21.0 References: <20260626-polite-hairy-perch-25e1aa@houat> <20260626-classy-nightingale-of-romance-1cacad@houat> <20260707-warping-goshawk-of-swiftness-beea4a@penduick> <20260707123205.GB211515@killaraus.ideasonboard.com> <20260716-stoic-muskox-from-asgard-c12b65@houat> <20260720-eel-of-immense-triumph-3e24e0@penduick> In-Reply-To: <20260720-eel-of-immense-triumph-3e24e0@penduick> X-Last-TLS-Session-Version: TLSv1.3 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260722_021120_352955_7815390B X-CRM114-Status: GOOD ( 41.86 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Maxime, On Mon Jul 20, 2026 at 4:28 PM CEST, Maxime Ripard wrote: > On Fri, Jul 17, 2026 at 11:41:33AM +0200, Luca Ceresoli wrote: >> 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 t= o >> >> >> > those encoder drivers and ditch all the drm_bridge_connector fro= m >> >> >> > 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 kee= p it >> >> >> around. >> >> >> >> >> >> What I was suggesting before was only a suggestion. I guess we cou= ld >> >> >> also make the encoder own the hotplug handling code and create the >> >> >> drm_bridge_connector when the chain is complete, and remove it whe= n it's >> >> >> no longer the case. >> >> > >> >> > That's an interesting option. We don't have to keep drm_bridge_conn= ector >> >> > in its current form, but I don't think we should go back to individ= ual >> >> > bridge driver creating connectors, especially now that we have brid= ge >> >> > chains where the connector ops are implemented collectively by mult= iple >> >> > 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_CONN= ECTOR) >> - 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. > > I agree, but another way to phrase it is: bridges aren't aware of how > the chain is setup, the encoder ties it all together. > >> >> >> 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 e= vents >> >> =3D> 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 i= t's >> clear) is that: >> >> - the drm_bridge_connector should stay (almost) unmodified > > Yes, and bridges should ideally remain as lightly affected as possible. I fully agree. > We have probably around 100 bridge drivers at the moment, having some > kind of opt-in to enable hotplug would mean that we can't expect hotplug > to work on a new platform, which should be a last resort. A few changes to each bridge wanting to support hotplug will unavoidably be needed. The .get_next_bridge callback we mentioned in the discussion for patch 30 at least. I'm keeping any other changes, if any, to a minimum. >> - 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_comple= te() >> or so) >> - create/destroy a drm_bridge_connector based on hotplug events >> >> Is this somewhat close to what you have in mind? > > Yes. To make things a bit more precise, we need two things: the encoder > to put the chain together, and "something" (that you used to call > manager) to react to hotplug events and handle the bridge > detach/destruction, connector creation/destruction, etc and should stick > around when we enable hotplug. > > What I'm suggesting is that, since the encoder already owns and creates > the chain in the first place, and is there forever, it's only natural > for the encoder to be that "something", and we don't necessarily mean > creating a new entity or piece of code. A bunch of helpers and hooks a > probably going to be enough. That's the idea I had reached too, yes. Except the "bunch of helpers and hooks" could be perhaps as small as one single helper function or little more. > This is where the opt-in part should be, and I'd like, if possible, for > hotplug-enabled encoders to work with any bridge. > >> 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 t= o >> something new (let's call it a hotplug manager), which takes care of all >> the common aspects: registering to receive bridge hotplug events, findin= g >> 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 =3D drm_bridge_connector_init(lcdif->drm, encoder); >> + drm_hotplug_manager =3D 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? > > From a high level point-of-view, I think we mostly agree. Good. > We can argue > on the name, and if we should merge it with something else > (drm_encoder_init, drm_bridge_attach, something else?) but that's the > path forward I think. OK, let's see what I can come up with in v2. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com