| Message ID | 20190820011721.30136-35-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/omap: Replace custom display drivers with drm_bridge and drm_panel | expand |
On Tue, 20 Aug 2019 04:17:05 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> 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@ideasonboard.com> > --- > Changes since v1: > > - Squash with patch "drm/omap: Detach from panels at remove time" > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 82 +++++++++++++++++++++++++----- > 1 file changed, 70 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c > index de373fd50729..f38d95cb31ba 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -11,10 +11,12 @@ > > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.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> > @@ -290,9 +292,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); I guess the only external bridge connected to an omap display controller in upstream DTs is the TFP410. I was wondering if falling back to !DRM_BRIDGE_ATTACH_NO_CONNECTOR would be acceptable in case one wants to convert a display controller driver which is known to be used in conjunction with various external bridges. > + if (ret < 0) { > + dev_err(priv->dev, > + "unable to attach bridge %pOF\n", > + pipe->output->bridge->of_node); > return ret; > + } > } >
Hi Boris, On Thu, Aug 22, 2019 at 07:00:11PM +0200, Boris Brezillon wrote: > On Tue, 20 Aug 2019 04:17:05 +0300 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@ideasonboard.com> > > --- > > Changes since v1: > > > > - Squash with patch "drm/omap: Detach from panels at remove time" > > --- > > drivers/gpu/drm/omapdrm/omap_drv.c | 82 +++++++++++++++++++++++++----- > > 1 file changed, 70 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c > > index de373fd50729..f38d95cb31ba 100644 > > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > > @@ -11,10 +11,12 @@ > > > > #include <drm/drm_atomic.h> > > #include <drm/drm_atomic_helper.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> > > @@ -290,9 +292,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); > > I guess the only external bridge connected to an omap display > controller in upstream DTs is the TFP410. No, there's also the TI OP362 (analog amplier) and TI TPD12S015 (HDMI ESD protection and level shifter). Those are not really encoders, but they're bridges. > I was wondering if falling back to !DRM_BRIDGE_ATTACH_NO_CONNECTOR > would be acceptable in case one wants to convert a display controller > driver which is known to be used in conjunction with various external > bridges. Yes, a display controller can support both options. I would however recommend addressing bridges first in a conversion, and once all bridges used by a display controller support DRM_BRIDGE_ATTACH_NO_CONNECTOR (and implement the connector bridge ops), switch the display controller to DRM_BRIDGE_ATTACH_NO_CONNECTOR unconditionally. > > + if (ret < 0) { > > + dev_err(priv->dev, > > + "unable to attach bridge %pOF\n", > > + pipe->output->bridge->of_node); > > return ret; > > + } > > } > >
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index de373fd50729..f38d95cb31ba 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -11,10 +11,12 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.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> @@ -290,9 +292,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); @@ -328,8 +335,31 @@ static int omap_modeset_init(struct drm_device *dev) encoder); if (!pipe->connector) return -ENOMEM; + } else { + struct drm_bridge *bridge = pipe->output->bridge; - drm_connector_attach_encoder(pipe->connector, encoder); + pipe->connector = drm_bridge_connector_init(dev, + bridge); + if (IS_ERR(pipe->connector)) { + dev_err(priv->dev, + "unable to create bridge connector for %pOF\n", + bridge->of_node); + return PTR_ERR(pipe->connector); + } + } + + 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. + */ + if (pipe->output->panel) { + ret = drm_panel_attach(pipe->output->panel, + pipe->connector); + if (ret < 0) + return ret; } crtc = omap_crtc_init(dev, pipe, priv->planes[i]); @@ -368,6 +398,23 @@ static int omap_modeset_init(struct drm_device *dev) return 0; } +static void omap_modeset_fini(struct drm_device *ddev) +{ + struct omap_drm_private *priv = ddev->dev_private; + unsigned int i; + + omap_drm_irq_uninstall(ddev); + + for (i = 0; i < priv->num_pipes; i++) { + struct omap_drm_pipeline *pipe = &priv->pipes[i]; + + if (pipe->output->panel) + drm_panel_detach(pipe->output->panel); + } + + drm_mode_config_cleanup(ddev); +} + /* * Enable the HPD in external components if supported */ @@ -377,8 +424,15 @@ static void omap_modeset_enable_external_hpd(struct drm_device *ddev) unsigned int i; for (i = 0; i < priv->num_pipes; i++) { - if (priv->pipes[i].connector) - omap_connector_enable_hpd(priv->pipes[i].connector); + struct drm_connector *connector = priv->pipes[i].connector; + + if (!connector) + continue; + + if (priv->pipes[i].output->next) + omap_connector_enable_hpd(connector); + else + drm_bridge_connector_enable_hpd(connector); } } @@ -391,8 +445,15 @@ static void omap_modeset_disable_external_hpd(struct drm_device *ddev) unsigned int i; for (i = 0; i < priv->num_pipes; i++) { - if (priv->pipes[i].connector) - omap_connector_disable_hpd(priv->pipes[i].connector); + struct drm_connector *connector = priv->pipes[i].connector; + + if (!connector) + continue; + + if (priv->pipes[i].output->next) + omap_connector_disable_hpd(connector); + else + drm_bridge_connector_disable_hpd(connector); } } @@ -615,8 +676,7 @@ static int omapdrm_init(struct omap_drm_private *priv, struct device *dev) omap_fbdev_fini(ddev); err_cleanup_modeset: - drm_mode_config_cleanup(ddev); - omap_drm_irq_uninstall(ddev); + omap_modeset_fini(ddev); err_gem_deinit: omap_gem_deinit(ddev); destroy_workqueue(priv->wq); @@ -641,9 +701,7 @@ static void omapdrm_cleanup(struct omap_drm_private *priv) drm_atomic_helper_shutdown(ddev); - drm_mode_config_cleanup(ddev); - - omap_drm_irq_uninstall(ddev); + omap_modeset_fini(ddev); omap_gem_deinit(ddev); destroy_workqueue(priv->wq);
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@ideasonboard.com> --- Changes since v1: - Squash with patch "drm/omap: Detach from panels at remove time" --- drivers/gpu/drm/omapdrm/omap_drv.c | 82 +++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 12 deletions(-)