| Message ID | 20191210225750.15709-36-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 |
Hi Laurent. 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@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. I am not convinced we need the drm_panel_(attach|detach) anymore, but this is not the thread to take that discussion. Sam
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@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.
On 11/12/2019 00:57, 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 | 79 +++++++++++++++++++++++++----- > 1 file changed, 67 insertions(+), 12 deletions(-) Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Tomi
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. + */ + 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]); @@ -369,6 +396,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 */ @@ -378,8 +422,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); } } @@ -392,8 +443,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); } } @@ -616,8 +674,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); @@ -642,9 +699,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 | 79 +++++++++++++++++++++++++----- 1 file changed, 67 insertions(+), 12 deletions(-)