linux-kernel - Re: [PATCH v5 08/10] drm/bridge: samsung-dsim: use supporting variable for out

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <zave5l4djfl6i2qyxcpfn2dn3pip2sjaqefrkct47qzdd653jf@z4zhcpptbt4x>
Date: Tue, 28 Jan 2025 16:52:18 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Luca Ceresoli <luca.ceresoli@...tlin.com>, 
	Simona Vetter <simona@...ll.ch>, Inki Dae <inki.dae@...sung.com>, 
	Jagan Teki <jagan@...rulasolutions.com>, Marek Szyprowski <m.szyprowski@...sung.com>, 
	Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, Shawn Guo <shawnguo@...nel.org>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Fabio Estevam <festevam@...il.com>, Daniel Thompson <danielt@...nel.org>, 
	Andrzej Hajda <andrzej.hajda@...el.com>, Jonathan Corbet <corbet@....net>, 
	Paul Kocialkowski <contact@...lk.fr>, 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>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Hervé Codina <herve.codina@...tlin.com>, 
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>, linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	linux-doc@...r.kernel.org, Paul Kocialkowski <paul.kocialkowski@...tlin.com>
Subject: Re: [PATCH v5 08/10] drm/bridge: samsung-dsim: use supporting
 variable for out_bridge

On Tue, Jan 21, 2025 at 01:57:47PM +0200, Dmitry Baryshkov wrote:
> On Tue, Jan 21, 2025 at 12:27:18PM +0100, Luca Ceresoli wrote:
> > Hi Dmitry,
> > 
> > On Thu, 16 Jan 2025 12:56:25 +0200
> > Dmitry Baryshkov <dmitry.baryshkov@...aro.org> wrote:
> > 
> > [...]
> > 
> > > > Idea 3: 
> > > > 
> > > > The idea is that if the panel driver framework always creates a panel
> > > > bridge, it will never need to be created on the fly automagically by
> > > > its consumers, so the whole problem would disappear. It also would be
> > > > better modeling the hardware: still wrapping a panel with a drm_bridge
> > > > that does not exist in the hardware, but at least having it created by
> > > > the provider driver and not by the consumer driver which happens to
> > > > look for it.  
> > > 
> > > I think this is the best option up to now.
> > 
> > Thanks for sharing your opinion. However a few points are not clear to
> > me, see below.
> > 
> > > > This looks like a promising and simple idea, so I tried a quick
> > > > implementation:
> > > > 
> > > >  void drm_panel_init(struct drm_panel *panel, struct device *dev,
> > > >                     const struct drm_panel_funcs *funcs, int connector_type)
> > > >  {
> > > > +       struct drm_bridge *bridge;
> > > > +
> > > >         INIT_LIST_HEAD(&panel->list);
> > > >         INIT_LIST_HEAD(&panel->followers);
> > > >         mutex_init(&panel->follower_lock);
> > > >         panel->dev = dev;
> > > >         panel->funcs = funcs;
> > > >         panel->connector_type = connector_type;
> > > > +
> > > > +       bridge = devm_drm_panel_bridge_add(panel->dev, panel);
> > > > +       WARN_ON(!bridge);
> > > >  }
> > > > 
> > > > This is somewhat working but it requires more work because:
> > > > 
> > > >  * as it is, it creates a circular dependency between drm_panel and the
> > > >    panel bridge, and modular builds will fail:
> > > > 
> > > >      depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
> > > > 
> > > >  * The panel bridge implementation should be made private to the panel
> > > >    driver only (possibly helping to solve the previous issue?)  
> > > 
> > > Or merge drm_panel.c into bridge/panel.c.
> > 
> > Not sure here you mean exactly what you wrote, or the other way around.
> > It looks more correct to me that the panel core (drm_panel.c) starts
> > exposing a bridge now, and not that the panel bridge which is just one
> > of many bridge drivers starts handling all the panel-related stuff.
> 
> No, I actually meant what I wrote: merge drm_panel.c into
> bridge/panel.c. Indeed we have some drivers that use panel directly.
> However drm_bridge is a more generic interface. So, yes, I propose to
> have a bridge driver which incorporates panel support.

It's a legitimate subject to discuss, but I'm not sure it's worth
focusing on this at this point though.

We should probably split it out into smaller chunks. Figuring out the
drivers lifetime, and reference counting API is hard enough as it is,
throwing panels into the mix at this point just adds more complexity.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)