[PATCH v3 05/50] drm/bridge: Extend bridge API to disable connector creation

[Laurent Pinchart]

Hi Tomi,

On Tue, Dec 17, 2019 at 02:12:59PM +0200, Tomi Valkeinen wrote:
> On 11/12/2019 00:57, Laurent Pinchart wrote:
> > Most bridge drivers create a DRM connector to model the connector at the
> > output of the bridge. This model is historical and has worked pretty
> > well so far, but causes several issues:
> > 
> > - It prevents supporting more complex display pipelines where DRM
> > connector operations are split over multiple components. For instance a
> > pipeline with a bridge connected to the DDC signals to read EDID data,
> > and another one connected to the HPD signal to detect connection and
> > disconnection, will not be possible to support through this model.
> > 
> > - It requires every bridge driver to implement similar connector
> > handling code, resulting in code duplication.
> > 
> > - It assumes that a bridge will either be wired to a connector or to
> > another bridge, but doesn't support bridges that can be used in both
> > positions very well (although there is some ad-hoc support for this in
> > the analogix_dp bridge driver).
> > 
> > In order to solve these issues, ownership of the connector should be
> > moved to the display controller driver (where it can be implemented
> > using helpers provided by the core).
> > 
> > Extend the bridge API to allow disabling connector creation in bridge
> > drivers as a first step towards the new model. The new flags argument to
> > the bridge .attach() operation allows instructing the bridge driver to
> > skip creating a connector. Unconditionally set the new flags argument to
> > 0 for now to keep the existing behaviour, and modify all existing bridge
> > drivers to return an error when connector creation is not requested as
> > they don't support this feature yet.
> > 
> > The change is based on the following semantic patch, with manual review
> > and edits.
> > 
> > @ rule1 @
> > identifier funcs;
> > identifier fn;
> > @@
> >   struct drm_bridge_funcs funcs = {
> >   	...,
> >   	.attach = fn
> >   };
> > 
> > @ depends on rule1 @
> > identifier rule1.fn;
> > identifier bridge;
> > statement S, S1;
> > @@
> >   int fn(
> >   	struct drm_bridge *bridge
> > +	, enum drm_bridge_attach_flags flags
> >   )
> >   {
> >   	... when != S
> > +	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> > +		return -EINVAL;
> > +
> 
> Should there be a dev_err or such here? This should only happen when your setting up a new board, 
> and you try to use a bridge that doesn't support no-connector-mode yet, right?
> 
> I hit this when trying out AM4 EVM with SiI9022 driver. It wasn't too difficult to pinpoint where 
> the failure happens, but an error would have made it immediately obvious.

I hate that you're right as it forces me to re-review the change
manually :-) I'll fix this.

-- 
Regards,

Laurent Pinchart