[PATCH v3 35/50] drm/omap: Create connector for bridges

[Laurent Pinchart]

Hi Sam,

Thank you for the review.

On Sun, Dec 15, 2019 at 11:00:18AM +0100, Sam Ravnborg wrote:
> On Wed, Dec 11, 2019 at 12:57:35AM +0200, Laurent Pinchart wrote:
> > Use the drm_bridge_connector helper to create a connector for pipelines
> > that use drm_bridge. This allows splitting connector operations across
> > multiple bridges when necessary, instead of having the last bridge in
> > the chain creating the connector and handling all connector operations
> > internally.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Squash with patch "drm/omap: Detach from panels at remove time"
> > ---
> >  drivers/gpu/drm/omapdrm/omap_drv.c | 79 +++++++++++++++++++++++++-----
> >  1 file changed, 67 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> > index 1df509342b5d..097fbbaa5df0 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> > @@ -12,10 +12,12 @@
> >  #include <drm/drm_atomic.h>
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_bridge.h>
> > +#include <drm/drm_bridge_connector.h>
> >  #include <drm/drm_drv.h>
> >  #include <drm/drm_fb_helper.h>
> >  #include <drm/drm_file.h>
> >  #include <drm/drm_ioctl.h>
> > +#include <drm/drm_panel.h>
> >  #include <drm/drm_prime.h>
> >  #include <drm/drm_probe_helper.h>
> >  #include <drm/drm_vblank.h>
> > @@ -291,9 +293,14 @@ static int omap_modeset_init(struct drm_device *dev)
> >  
> >  		if (pipe->output->bridge) {
> >  			ret = drm_bridge_attach(pipe->encoder,
> > -						pipe->output->bridge, NULL, 0);
> > -			if (ret < 0)
> > +						pipe->output->bridge, NULL,
> > +						DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> > +			if (ret < 0) {
> > +				dev_err(priv->dev,
> > +					"unable to attach bridge %pOF\n",
> > +					pipe->output->bridge->of_node);
> >  				return ret;
> > +			}
> >  		}
> >  
> >  		id = omap_display_id(pipe->output);
> > @@ -329,8 +336,28 @@ static int omap_modeset_init(struct drm_device *dev)
> >  							      encoder);
> >  			if (!pipe->connector)
> >  				return -ENOMEM;
> > +		} else {
> > +			pipe->connector = drm_bridge_connector_init(dev, encoder);
> > +			if (IS_ERR(pipe->connector)) {
> > +				dev_err(priv->dev,
> > +					"unable to create bridge connector for %s\n",
> > +					pipe->output->name);
> > +				return PTR_ERR(pipe->connector);
> > +			}
> > +		}
> >  
> > -			drm_connector_attach_encoder(pipe->connector, encoder);
> > +		drm_connector_attach_encoder(pipe->connector, encoder);
> > +
> > +		/*
> > +		 * FIXME: drm_panel should not store the drm_connector pointer
> > +		 * internally but should receive it in its .get_modes()
> > +		 * operation.
> > +		 */
>
> This FIXME is not fully up-to-date.
> drm_panel_attach ignores the connector argumant, and we could also pass
> NULL here.

You're right, I'll remove the comment.

> I am not convinced we need the drm_panel_(attach|detach) anymore, but
> this is not the thread to take that discussion.

Indeed :-) I think we should replace all direct panel usage with the DRM
panel bridge, then we'll be able to refactor the panel API freely.

-- 
Regards,

Laurent Pinchart