[PATCH v2 13/50] drm/bridge: panel: Implement bridge connector operations

[Laurent Pinchart]

Hi Boris,

On Thu, Aug 22, 2019 at 06:29:09PM +0200, Boris Brezillon wrote:
> On Tue, 20 Aug 2019 04:16:44 +0300 Laurent Pinchart wrote:
> 
> > Implement the newly added bridge connector operations, allowing the
> > usage of drm_bridge_panel with drm_bridge_connector.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  drivers/gpu/drm/bridge/panel.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> > index f5b8e55301ac..1c7f5b648f05 100644
> > --- a/drivers/gpu/drm/bridge/panel.c
> > +++ b/drivers/gpu/drm/bridge/panel.c
> > @@ -60,7 +60,7 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
> >  	int ret;
> >  
> >  	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> > -		return -EINVAL;
> > +		return 0;
> >  
> >  	if (!bridge->encoder) {
> >  		DRM_ERROR("Missing encoder\n");
> > @@ -123,6 +123,18 @@ static void panel_bridge_post_disable(struct drm_bridge *bridge)
> >  	drm_panel_unprepare(panel_bridge->panel);
> >  }
> >  
> > +static int panel_bridge_get_modes(struct drm_bridge *bridge,
> > +				  struct drm_connector *connector)
> > +{
> > +	struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
> > +
> > +	/*
> > +	 * FIXME: drm_panel_get_modes() should take the connector as an
> > +	 * argument.
> > +	 */
> > +	return drm_panel_get_modes(panel_bridge->panel);
> > +}
> > +
> >  static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
> >  	.attach = panel_bridge_attach,
> >  	.detach = panel_bridge_detach,
> > @@ -130,6 +142,7 @@ static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
> >  	.enable = panel_bridge_enable,
> >  	.disable = panel_bridge_disable,
> >  	.post_disable = panel_bridge_post_disable,
> > +	.get_modes = panel_bridge_get_modes,
> >  };
> >  
> >  /**
> > @@ -175,6 +188,9 @@ struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
> >  #ifdef CONFIG_OF
> >  	panel_bridge->bridge.of_node = panel->dev->of_node;
> >  #endif
> > +	panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES;
> > +	/* FIXME: The panel should report its type. */
> > +	panel_bridge->bridge.type = DRM_MODE_CONNECTOR_DPI;
> 
> Shouldn't we patch all panel drivers to expose this type before doing
> this change? I mean, the connector type is exposed to userspace, and I
> wouldn't be surprised if some userspace apps/libs decided to base their
> output selection logic on this field.

Note that this type will only make it to userspace for drivers that use
the bridge->type field, likely through the drm bridge connector helper.
I do agree that panel drivers should be updated, but given the number of
panels in panel-simple and the fact that the information would need to
be researched for most of them, this will be significant work. Can't
this be done when converting display controller drivers on a need basis
?

Or maybe we could, as an interim measure, derive the type from the bus
formats reported by the panel if the panel type is not set ? If the
panel reports MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
MEDIA_BUS_FMT_RGB666_1X7X4_SPWG or MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA then
we can set the type to LVDS, otherwise we set it to DPI. I can submit a
patch to add a type field to the panel structure and implement this
logic.

> >  	drm_bridge_add(&panel_bridge->bridge);

-- 
Regards,

Laurent Pinchart