Hi, On Fri, Jan 11, 2019 at 05:51:16AM +0200, Laurent Pinchart wrote: > Hook up drm_bridge support in the omapdrm driver. Despite the recent > extensive preparation work, this is a rather intrusive change, as the > management of outputs needs to be adapted through the driver to handle > both omap_dss_device and drm_bridge. > > Connector creation is skipped when using a drm_bridge, as the bridge > creates the connector internally. This creates issues with systems that > split connector operations (such as modes retrieval and hot-plug > detection) across different bridges. These systems can't be supported > using drm_bridge for now (their support through the omap_dss_device > infrastructure is not affected), this will be fixed in subsequent > changes. > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > Reviewed-by: Sebastian Reichel <sebastian.reic...@collabora.com> > ---
Tested-by: Sebastian Reichel <sebastian.reic...@collabora.com>
-- Sebastian
> Changes since v1:
>
> - Fix typo in function name (updata -> update)
> ---
> drivers/gpu/drm/omapdrm/dss/base.c | 27 ++++++++--
> drivers/gpu/drm/omapdrm/dss/omapdss.h | 1 +
> drivers/gpu/drm/omapdrm/dss/output.c | 21 +++++---
> drivers/gpu/drm/omapdrm/omap_connector.c | 16 ++++--
> drivers/gpu/drm/omapdrm/omap_connector.h | 1 -
> drivers/gpu/drm/omapdrm/omap_crtc.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_drv.c | 69 +++++++++++++++++-------
> drivers/gpu/drm/omapdrm/omap_encoder.c | 69 ++++++++++++++----------
> 8 files changed, 145 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/base.c
> b/drivers/gpu/drm/omapdrm/dss/base.c
> index 81ea0f55cd75..09c9f2971aa2 100644
> --- a/drivers/gpu/drm/omapdrm/dss/base.c
> +++ b/drivers/gpu/drm/omapdrm/dss/base.c
> @@ -19,6 +19,7 @@
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/of_graph.h>
> +#include <linux/platform_device.h>
>
> #include "dss.h"
> #include "omapdss.h"
> @@ -156,7 +157,7 @@ struct omap_dss_device *omapdss_device_next_output(struct
> omap_dss_device *from)
> goto done;
> }
>
> - if (dssdev->id && dssdev->next)
> + if (dssdev->id && (dssdev->next || dssdev->bridge))
> goto done;
> }
>
> @@ -184,7 +185,18 @@ int omapdss_device_connect(struct dss_device *dss,
> {
> int ret;
>
> - dev_dbg(dst->dev, "connect\n");
> + dev_dbg(&dss->pdev->dev, "connect(%s, %s)\n",
> + src ? dev_name(src->dev) : "NULL",
> + dst ? dev_name(dst->dev) : "NULL");
> +
> + if (!dst) {
> + /*
> + * The destination is NULL when the source is connected to a
> + * bridge instead of a DSS device. Stop here, we will attach the
> + * bridge later when we will have a DRM encoder.
> + */
> + return src && src->bridge ? 0 : -EINVAL;
> + }
>
> if (omapdss_device_is_connected(dst))
> return -EBUSY;
> @@ -204,7 +216,16 @@ EXPORT_SYMBOL_GPL(omapdss_device_connect);
> void omapdss_device_disconnect(struct omap_dss_device *src,
> struct omap_dss_device *dst)
> {
> - dev_dbg(dst->dev, "disconnect\n");
> + struct dss_device *dss = src ? src->dss : dst->dss;
> +
> + dev_dbg(&dss->pdev->dev, "disconnect(%s, %s)\n",
> + src ? dev_name(src->dev) : "NULL",
> + dst ? dev_name(dst->dev) : "NULL");
> +
> + if (!dst) {
> + WARN_ON(!src->bridge);
> + return;
> + }
>
> if (!dst->id && !omapdss_device_is_connected(dst)) {
> WARN_ON(!dst->display);
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index ab5467a1e92c..f47e9b94288f 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -410,6 +410,7 @@ struct omap_dss_device {
>
> struct dss_device *dss;
> struct omap_dss_device *next;
> + struct drm_bridge *bridge;
>
> struct list_head list;
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/output.c
> b/drivers/gpu/drm/omapdrm/dss/output.c
> index f25ecfd26534..2a53025c2fde 100644
> --- a/drivers/gpu/drm/omapdrm/dss/output.c
> +++ b/drivers/gpu/drm/omapdrm/dss/output.c
> @@ -20,25 +20,34 @@
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> #include <linux/of.h>
> +#include <linux/of_graph.h>
>
> #include "dss.h"
> #include "omapdss.h"
>
> int omapdss_device_init_output(struct omap_dss_device *out)
> {
> - out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
> - if (IS_ERR(out->next)) {
> - if (PTR_ERR(out->next) != -EPROBE_DEFER)
> - dev_err(out->dev, "failed to find video sink\n");
> - return PTR_ERR(out->next);
> + struct device_node *remote_node;
> +
> + remote_node = of_graph_get_remote_node(out->dev->of_node, 0, 0);
> + if (!remote_node) {
> + dev_dbg(out->dev, "failed to find video sink\n");
> + return 0;
> }
>
> + out->next = omapdss_find_device_by_node(remote_node);
> + out->bridge = of_drm_find_bridge(remote_node);
> +
> + of_node_put(remote_node);
> +
> if (out->next && out->type != out->next->type) {
> dev_err(out->dev, "output type and display type don't match\n");
> + omapdss_device_put(out->next);
> + out->next = NULL;
> return -EINVAL;
> }
>
> - return 0;
> + return out->next || out->bridge ? 0 : -EPROBE_DEFER;
> }
> EXPORT_SYMBOL(omapdss_device_init_output);
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c
> b/drivers/gpu/drm/omapdrm/omap_connector.c
> index 487603c56b08..f528baa80114 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -304,9 +304,16 @@ static const struct drm_connector_helper_funcs
> omap_connector_helper_funcs = {
> .mode_valid = omap_connector_mode_valid,
> };
>
> -static int omap_connector_get_type(struct omap_dss_device *display)
> +static int omap_connector_get_type(struct omap_dss_device *output)
> {
> - switch (display->type) {
> + struct omap_dss_device *display;
> + enum omap_display_type type;
> +
> + display = omapdss_display_get(output);
> + type = display->type;
> + omapdss_device_put(display);
> +
> + switch (type) {
> case OMAP_DISPLAY_TYPE_HDMI:
> return DRM_MODE_CONNECTOR_HDMIA;
> case OMAP_DISPLAY_TYPE_DVI:
> @@ -329,14 +336,13 @@ static int omap_connector_get_type(struct
> omap_dss_device *display)
> /* initialize connector */
> struct drm_connector *omap_connector_init(struct drm_device *dev,
> struct omap_dss_device *output,
> - struct omap_dss_device *display,
> struct drm_encoder *encoder)
> {
> struct drm_connector *connector = NULL;
> struct omap_connector *omap_connector;
> struct omap_dss_device *dssdev;
>
> - DBG("%s", display->name);
> + DBG("%s", output->name);
>
> omap_connector = kzalloc(sizeof(*omap_connector), GFP_KERNEL);
> if (!omap_connector)
> @@ -349,7 +355,7 @@ struct drm_connector *omap_connector_init(struct
> drm_device *dev,
> connector->doublescan_allowed = 0;
>
> drm_connector_init(dev, connector, &omap_connector_funcs,
> - omap_connector_get_type(display));
> + omap_connector_get_type(output));
> drm_connector_helper_add(connector, &omap_connector_helper_funcs);
>
> /*
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.h
> b/drivers/gpu/drm/omapdrm/omap_connector.h
> index 6b7d4d95e32b..608085219336 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.h
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.h
> @@ -31,7 +31,6 @@ struct omap_dss_device;
>
> struct drm_connector *omap_connector_init(struct drm_device *dev,
> struct omap_dss_device *output,
> - struct omap_dss_device *display,
> struct drm_encoder *encoder);
> bool omap_connector_get_hdmi_mode(struct drm_connector *connector);
> void omap_connector_enable_hpd(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 0cea3824d3a6..4486152fd6cc 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -671,7 +671,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
> &omap_crtc_funcs, NULL);
> if (ret < 0) {
> dev_err(dev->dev, "%s(): could not init crtc for: %s\n",
> - __func__, pipe->display->name);
> + __func__, pipe->output->name);
> kfree(omap_crtc);
> return ERR_PTR(ret);
> }
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 6b91f44e5a60..35c4669dc69d 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -140,9 +140,7 @@ static void omap_disconnect_pipelines(struct drm_device
> *ddev)
> omapdss_device_disconnect(NULL, pipe->output);
>
> omapdss_device_put(pipe->output);
> - omapdss_device_put(pipe->display);
> pipe->output = NULL;
> - pipe->display = NULL;
> }
>
> memset(&priv->channels, 0, sizeof(priv->channels));
> @@ -169,7 +167,6 @@ static int omap_connect_pipelines(struct drm_device *ddev)
>
> pipe = &priv->pipes[priv->num_pipes++];
> pipe->output = omapdss_device_get(output);
> - pipe->display = omapdss_display_get(output);
>
> if (priv->num_pipes == ARRAY_SIZE(priv->pipes)) {
> /* To balance the 'for_each_dss_output' loop */
> @@ -207,6 +204,28 @@ static int omap_modeset_init_properties(struct
> drm_device *dev)
> return 0;
> }
>
> +static int omap_display_id(struct omap_dss_device *output)
> +{
> + struct device_node *node = NULL;
> +
> + if (output->next) {
> + struct omap_dss_device *display;
> +
> + display = omapdss_display_get(output);
> + node = display->dev->of_node;
> + omapdss_device_put(display);
> + } else {
> + struct drm_bridge *bridge = output->bridge;
> +
> + while (bridge->next)
> + bridge = bridge->next;
> +
> + node = bridge->of_node;
> + }
> +
> + return node ? of_alias_get_id(node, "display") : -ENODEV;
> +}
> +
> static int omap_modeset_init(struct drm_device *dev)
> {
> struct omap_drm_private *priv = dev->dev_private;
> @@ -262,7 +281,10 @@ static int omap_modeset_init(struct drm_device *dev)
> priv->planes[priv->num_planes++] = plane;
> }
>
> - /* Create the encoders and get the pipelines aliases. */
> + /*
> + * Create the encoders, attach the bridges and get the pipeline alias
> + * IDs.
> + */
> for (i = 0; i < priv->num_pipes; i++) {
> struct omap_drm_pipeline *pipe = &priv->pipes[i];
> int id;
> @@ -271,7 +293,14 @@ static int omap_modeset_init(struct drm_device *dev)
> if (!pipe->encoder)
> return -ENOMEM;
>
> - id = of_alias_get_id(pipe->display->dev->of_node, "display");
> + if (pipe->output->bridge) {
> + ret = drm_bridge_attach(pipe->encoder,
> + pipe->output->bridge, NULL);
> + if (ret < 0)
> + return ret;
> + }
> +
> + id = omap_display_id(pipe->output);
> pipe->alias_id = id >= 0 ? id : i;
> }
>
> @@ -297,16 +326,16 @@ static int omap_modeset_init(struct drm_device *dev)
> for (i = 0; i < priv->num_pipes; i++) {
> struct omap_drm_pipeline *pipe = &priv->pipes[i];
> struct drm_encoder *encoder = pipe->encoder;
> - struct drm_connector *connector;
> struct drm_crtc *crtc;
>
> - connector = omap_connector_init(dev, pipe->output,
> - pipe->display, encoder);
> - if (!connector)
> - return -ENOMEM;
> + if (!pipe->output->bridge) {
> + pipe->connector = omap_connector_init(dev, pipe->output,
> + encoder);
> + if (!pipe->connector)
> + return -ENOMEM;
>
> - drm_connector_attach_encoder(connector, encoder);
> - pipe->connector = connector;
> + drm_connector_attach_encoder(pipe->connector, encoder);
> + }
>
> crtc = omap_crtc_init(dev, pipe, priv->planes[i]);
> if (IS_ERR(crtc))
> @@ -350,10 +379,12 @@ static int omap_modeset_init(struct drm_device *dev)
> static void omap_modeset_enable_external_hpd(struct drm_device *ddev)
> {
> struct omap_drm_private *priv = ddev->dev_private;
> - int i;
> + unsigned int i;
>
> - for (i = 0; i < priv->num_pipes; i++)
> - omap_connector_enable_hpd(priv->pipes[i].connector);
> + for (i = 0; i < priv->num_pipes; i++) {
> + if (priv->pipes[i].connector)
> + omap_connector_enable_hpd(priv->pipes[i].connector);
> + }
> }
>
> /*
> @@ -362,10 +393,12 @@ static void omap_modeset_enable_external_hpd(struct
> drm_device *ddev)
> static void omap_modeset_disable_external_hpd(struct drm_device *ddev)
> {
> struct omap_drm_private *priv = ddev->dev_private;
> - int i;
> + unsigned int i;
>
> - for (i = 0; i < priv->num_pipes; i++)
> - omap_connector_disable_hpd(priv->pipes[i].connector);
> + for (i = 0; i < priv->num_pipes; i++) {
> + if (priv->pipes[i].connector)
> + omap_connector_disable_hpd(priv->pipes[i].connector);
> + }
> }
>
> /*
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c
> b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index e71d359a8f07..76f94cc0c0cf 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -51,6 +51,34 @@ static const struct drm_encoder_funcs omap_encoder_funcs =
> {
> .destroy = omap_encoder_destroy,
> };
>
> +static void omap_encoder_update_videomode_flags(struct videomode *vm,
> + u32 bus_flags)
> +{
> + if (!(vm->flags & (DISPLAY_FLAGS_DE_LOW |
> + DISPLAY_FLAGS_DE_HIGH))) {
> + if (bus_flags & DRM_BUS_FLAG_DE_LOW)
> + vm->flags |= DISPLAY_FLAGS_DE_LOW;
> + else if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> + vm->flags |= DISPLAY_FLAGS_DE_HIGH;
> + }
> +
> + if (!(vm->flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
> + DISPLAY_FLAGS_PIXDATA_NEGEDGE))) {
> + if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> + vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> + else if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> + vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
> + }
> +
> + if (!(vm->flags & (DISPLAY_FLAGS_SYNC_POSEDGE |
> + DISPLAY_FLAGS_SYNC_NEGEDGE))) {
> + if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE)
> + vm->flags |= DISPLAY_FLAGS_SYNC_POSEDGE;
> + else if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE)
> + vm->flags |= DISPLAY_FLAGS_SYNC_NEGEDGE;
> + }
> +}
> +
> static void omap_encoder_hdmi_mode_set(struct drm_encoder *encoder,
> struct drm_display_mode *adjusted_mode)
> {
> @@ -87,7 +115,9 @@ static void omap_encoder_mode_set(struct drm_encoder
> *encoder,
> struct drm_display_mode *adjusted_mode)
> {
> struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
> + struct omap_dss_device *output = omap_encoder->output;
> struct omap_dss_device *dssdev;
> + struct drm_bridge *bridge;
> struct videomode vm = { 0 };
>
> drm_display_mode_to_videomode(adjusted_mode, &vm);
> @@ -101,44 +131,29 @@ static void omap_encoder_mode_set(struct drm_encoder
> *encoder,
> *
> * A better solution is to use DRM's bus-flags through the whole driver.
> */
> - for (dssdev = omap_encoder->output; dssdev; dssdev = dssdev->next) {
> - unsigned long bus_flags = dssdev->bus_flags;
> -
> - if (!(vm.flags & (DISPLAY_FLAGS_DE_LOW |
> - DISPLAY_FLAGS_DE_HIGH))) {
> - if (bus_flags & DRM_BUS_FLAG_DE_LOW)
> - vm.flags |= DISPLAY_FLAGS_DE_LOW;
> - else if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> - vm.flags |= DISPLAY_FLAGS_DE_HIGH;
> - }
> + for (dssdev = output; dssdev; dssdev = dssdev->next)
> + omap_encoder_update_videomode_flags(&vm, dssdev->bus_flags);
>
> - if (!(vm.flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
> - DISPLAY_FLAGS_PIXDATA_NEGEDGE))) {
> - if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> - vm.flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> - else if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> - vm.flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
> - }
> + for (bridge = output->bridge; bridge; bridge = bridge->next) {
> + u32 bus_flags;
>
> - if (!(vm.flags & (DISPLAY_FLAGS_SYNC_POSEDGE |
> - DISPLAY_FLAGS_SYNC_NEGEDGE))) {
> - if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE)
> - vm.flags |= DISPLAY_FLAGS_SYNC_POSEDGE;
> - else if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE)
> - vm.flags |= DISPLAY_FLAGS_SYNC_NEGEDGE;
> - }
> + if (!bridge->timings)
> + continue;
> +
> + bus_flags = bridge->timings->input_bus_flags;
> + omap_encoder_update_videomode_flags(&vm, bus_flags);
> }
>
> /* Set timings for all devices in the display pipeline. */
> - dss_mgr_set_timings(omap_encoder->output, &vm);
> + dss_mgr_set_timings(output, &vm);
>
> - for (dssdev = omap_encoder->output; dssdev; dssdev = dssdev->next) {
> + for (dssdev = output; dssdev; dssdev = dssdev->next) {
> if (dssdev->ops->set_timings)
> dssdev->ops->set_timings(dssdev, adjusted_mode);
> }
>
> /* Set the HDMI mode and HDMI infoframe if applicable. */
> - if (omap_encoder->output->type == OMAP_DISPLAY_TYPE_HDMI)
> + if (output->type == OMAP_DISPLAY_TYPE_HDMI)
> omap_encoder_hdmi_mode_set(encoder, adjusted_mode);
> }
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
