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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
