Re: [PATCH v2 1/4] drm: rcar-du: lvds: Convert to DRM panel bridge helper

From: Jacopo Mondi <jacopo@jmondi.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	dri-devel@lists.freedesktop.org,
	linux-renesas-soc@vger.kernel.org,
	Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Subject: Re: [PATCH v2 1/4] drm: rcar-du: lvds: Convert to DRM panel bridge helper
Date: Wed, 16 Dec 2020 15:44:15 +0100	[thread overview]
Message-ID: <20201216144415.dobj5pn54ceres7j@uno.localdomain> (raw)
In-Reply-To: <X9oQZAEB3pbfGSZc@pendragon.ideasonboard.com>

Hi Laurent,

On Wed, Dec 16, 2020 at 03:49:24PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> > > +	if (lvds->panel) {
> > > +		lvds->next_bridge = devm_drm_panel_bridge_add(lvds->dev,
> > > +							      lvds->panel);
> >
> > Reading the devm_drm_panel_bridge_add() function documentation:
> >
> >  * devm_drm_panel_bridge_add - Creates a managed &drm_bridge and &drm_connector
> >
> > Doesn't this conflict with the drm_bridge_connector_init() called by
> > the encoder in [4/4] ?
>
> It would, if the documentation was right :-) The function only creates a
> bridge. A connector will only be created when the bridge is attached
> without DRM_BRIDGE_ATTACH_NO_CONNECTOR.

Well, reading it again, it is kind of implied that if NO_CONNECTOR is
given to the bridge, no connector will be registered at all.

>
> Would you like to send a patch to fix the documentation ?
>
> > > +		if (IS_ERR_OR_NULL(lvds->next_bridge)) {
> > > +			ret = -EINVAL;
> > > +			goto done;
> > > +		}
> > > +	}
> > > +
> > >  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> > >  		ret = rcar_lvds_parse_dt_companion(lvds);
> > >
>
> --
> Regards,
>
> Laurent Pinchart