Re: [PATCH v4 08/11] drm/bridge: Add a drm_bridge_state object

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: dri-devel@lists.freedesktop.org,
	Lucas Stach <l.stach@pengutronix.de>,
	Chris Healy <cphealy@gmail.com>,
	Andrey Smirnov <andrew.smirnov@gmail.com>,
	Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	kernel@collabora.com, Daniel Vetter <daniel@ffwll.ch>,
	Inki Dae <inki.dae@samsung.com>,
	Joonyoung Shim <jy0922.shim@samsung.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Rob Clark <robdclark@gmail.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, Eric Anholt <eric@anholt.net>
Subject: Re: [PATCH v4 08/11] drm/bridge: Add a drm_bridge_state object
Date: Wed, 4 Dec 2019 12:38:06 +0200	[thread overview]
Message-ID: <20191204103806.GD6705@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20191204104207.38367944@collabora.com>

Hi Boris,

On Wed, Dec 04, 2019 at 10:42:07AM +0100, Boris Brezillon wrote:
> On Wed, 4 Dec 2019 11:12:55 +0200 Laurent Pinchart wrote:
> > On Wed, Dec 04, 2019 at 10:03:02AM +0100, Boris Brezillon wrote:
> > > On Tue, 3 Dec 2019 20:17:05 +0200 Laurent Pinchart wrote:  
> > > > On Tue, Dec 03, 2019 at 03:15:12PM +0100, Boris Brezillon wrote:  
> > > > > One of the last remaining objects to not have its atomic state.
> > > > > 
> > > > > This is being motivated by our attempt to support runtime bus-format
> > > > > negotiation between elements of the bridge chain.
> > > > > This patch just paves the road for such a feature by adding a new
> > > > > drm_bridge_state object inheriting from drm_private_obj so we can
> > > > > re-use some of the existing state initialization/tracking logic.
> > > > > 
> > > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > > > > Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> > > > > ---
> > > > > Changes in v4:
> > > > > * Fix the doc
> > > > > * Kill default helpers (inlined)    
> > > > 
> > > > I liked the default helpers, inlining their content makes the code more
> > > > difficult to follow in my opinion.  
> > > 
> > > I'll go back to this approach then. Should I keep the original helper
> > > names even though they're not globally visible (and should probably
> > > never be)?  
> > 
> > I agree they should probably never be visible, and I trust your
> > judgement on naming. Please double-check the documentation though, to
> > ensure that it matches the implementation.
> 
> Is there any point keeping the documentation if they're not exposed?

I'll let you decide on that, depending on if the documentation could
bring value or if the functions would be so trivial that it would be
overkill.

-- 
Regards,

Laurent Pinchart