Re: [PATCH 3/3] drm/bridge: Introduce LT9611 DSI to HDMI bridge

[Laurent Pinchart]

Hi Vinod,

On Thu, Jun 04, 2020 at 12:55:48PM +0530, Vinod Koul wrote:
> On 28-05-20, 04:52, Laurent Pinchart wrote:
> 
> > > +static int lt9611_bridge_attach(struct drm_bridge *bridge,
> > > +				enum drm_bridge_attach_flags flags)
> > > +{
> > > +	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
> > > +	int ret;
> > > +
> > > +	dev_dbg(lt9611->dev, "bridge attach\n");
> > 
> > 
> > Connector creation in bridge drivers is deprecated. Please at least add
> 
> Okay what is the right way for connector creation? I can add support for
> that.

Historically bridge drivers have created connectors. With support for
bridge chaining, this approach was considered not to scale. For
instance, I have a board where the SoC has an internal LVDS encoder, and
the board itself has an LVDS-to-DPI decoder followed by a DPI-to-HDMI
encoder. All three components are supported by bridge drivers, and only
the last one should create a connector. Furthermore, different
operations of the connector may be implemented by different bridges, for
instance with one bridge connected to the DDC lines to read EDID, and
another bridge connected to the HPD line to detect hotplug.

To support these systems, we have deprecated connector creation in
bridges, in favour of implementing new bridge callback functions for
connector-related operations (see .get_modes(), .get_edid() and
.detect() in struct drm_bridge_funcs). With this new model, each bridge
implements the operations it supports, and the display controller driver
binds the bridges together to create a connector that delegates the
connector operations to the appropriate bridge. A helper function,
drm_bridge_connector_init(), can be used to automate that.

To transition to this model, we require all new bridge to at least
optionally support disabling connector creation (as requested by the
DRM_BRIDGE_ATTACH_NO_CONNECTOR), and implement the drm_bridge_funcs
functions related to connector operations. Existing bridges are also
converted to the new model. Once all bridges used by a display
controller support the new model, the display controller is then
converted to use DRM_BRIDGE_ATTACH_NO_CONNECTOR and
drm_bridge_connector_init() (or implement the latter manually if the
helper doesn't support all the display controller's needs). Once all
display controllers using a bridge have been converted to the new model,
support for creating a connector (the !DRM_BRIDGE_ATTACH_NO_CONNECTOR
case) is removed from the bridge driver. Finally, once everybody will
use the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, we will simply drop it.

> > support for the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, to make connector
> > creation optional. Ideally the !DRM_BRIDGE_ATTACH_NO_CONNECTOR case
> 
> will add that
> 
> > should not be implemented at all. This will require the display
> > controller driver to use DRM_BRIDGE_ATTACH_NO_CONNECTOR. Which display
> > controller(s) do you use this driver with ?
> 
> I am using with msm display driver, this was tested on dragon-board
> db845c board.

-- 
Regards,

Laurent Pinchart