The drm_bridge_connector is nowadays the recommended way to implement DRM
connectors when a chain of bridges is used, and as such it is the ideal
component to implement DRM bridge hotplug. Thus, let the
drm_bridge_connector be created once for the whole drm_encoder lifetime (as
it is now), but make it able of creating and destroying the DRM connector
reacting on bridge hot(un)plug events.

Unfortunately the current drm_bridge_connector_init() API is unable to
support DRM bridge hotplug because it returns a drm_connector, and with
hotplug the connector might start existing only later, and also be
destroyed and recreated multiple times.

So add a new API which is similar but returns a pointer to the
drm_bridge_connector, which is persistent, and supports bridge hotplug by
creating and destroying the drm_connector based on hotplug events.

Because drm_bridge_connector_init() is really a drmm function (it does only
drmm allocations), but without a drmm_ prefix, take this as an opportunity
to call the new function with the same name but a drmm_ prefix:
drmm_bridge_connector_init(). The old API can be replaced over time when
needed to support hotplug, and eventually be removed if unused.

The new drmm_bridge_connector_init() API differs from the old one in a few
aspects in order to support bridge hotplug:

 * if the bridge chain is not (yet) complete, returns successfully without
   adding a drm_connector (in this case the old API returns an error)
 * registers an event notifier block to be able of reacting to events
   related to bridge hotplug and create/destroy the drm_connector

Signed-off-by: Luca Ceresoli <[email protected]>

---

I'm tempted to let drmm_bridge_connector_init() return int (0 or negative
error) instead of a struct drm_bridge_connector *, but I'm not sure whether
the drm_bridge_connector pointer may be useful in the future to invoke
actions on the bridge_connector after its creation. Opnions about this
would be welcome.
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 157 +++++++++++++++++++++++++
 include/drm/drm_bridge_connector.h             |   2 +
 2 files changed, 159 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c 
b/drivers/gpu/drm/display/drm_bridge_connector.c
index b83da529cc7a..2ce6c04e2fc7 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -20,6 +20,7 @@
 #include <drm/drm_device.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_managed.h>
+#include <drm/drm_event_notifier.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
@@ -158,6 +159,10 @@ struct drm_bridge_connector {
         * destruction
         */
        struct mutex dynconn_mutex;
+       /**
+        * @drm_event_nb: notifier to receive DRM hotplug-related events
+        */
+       struct notifier_block drm_event_nb;
 };
 
 static struct drm_bridge_connector_dynconn *
@@ -1037,6 +1042,19 @@ static int 
drm_bridge_connector_init_hdmi_audio_cec(struct drm_bridge_connector_
        return 0;
 }
 
+static bool drm_bridge_connector_pipeline_is_complete(struct 
drm_bridge_connector *bridge_connector)
+{
+       struct drm_bridge *last_bridge __free(drm_bridge_put) =
+               drm_bridge_chain_get_last_bridge(bridge_connector->encoder);
+
+       if (!last_bridge || !drm_bridge_is_tail(last_bridge)) {
+               drm_dbg_driver(bridge_connector->drm, "pipeline not (yet) fully 
connected");
+               return false;
+       }
+
+       return true;
+}
+
 /**
  * drm_bridge_connector_add_connector - add the drm_connector
  * @bridge_connector: drm_bridge_connector to add the drm_connector to
@@ -1153,6 +1171,79 @@ static int drm_bridge_connector_add_connector(struct 
drm_bridge_connector *bridg
        return ret;
 }
 
+/*
+ * Propagate the attach chain and possibly add a drm_connector after a new
+ * drm_bridge is hot-plugged.
+ *
+ * The connector is added only if the pipeline is now complete. This could
+ * not be the case for various reasons:
+ *
+ * - the new bridge is just unrelated to our encoder
+ * - the new bridge is not be the next one in the pipeline
+ * - the new bridge is the next in the pipeline but the pipeline is not yet
+ *   complete
+ *
+ * All these cases are normal, not an error.
+ */
+static void drm_bridge_connector_try_complete(struct drm_bridge_connector 
*bridge_connector)
+{
+       int err;
+
+       /*
+        * drm_connector already present, the new bridge must be for
+        * another card
+        */
+       if (bridge_connector->dynconn)
+               return;
+
+       /* Propagate the attach call chain to newly hotplugged bridge(s) */
+       struct drm_bridge *last_bridge __free(drm_bridge_put) =
+               drm_bridge_chain_get_last_bridge(bridge_connector->encoder);
+
+       /* Encoder chain empty? */
+       if (!last_bridge)
+               return;
+
+       err = last_bridge->funcs->attach(last_bridge, bridge_connector->encoder,
+                                        DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+       if (err)
+               return;
+
+       /* Add the connector if the pipeline is now complete */
+       if (drm_bridge_connector_pipeline_is_complete(bridge_connector))
+               drm_bridge_connector_add_connector(bridge_connector);
+}
+
+static int drm_bridge_connector_handle_event(struct notifier_block *nb,
+                                            unsigned long event, void *data)
+{
+       struct drm_bridge_connector *bridge_connector =
+               container_of(nb, struct drm_bridge_connector, drm_event_nb);
+
+       switch (event) {
+       case DRM_MIPI_DSI_ATTACHED:
+               /* One or more bridges hot-plugged, try adding the 
drm_connector */
+               drm_bridge_connector_try_complete(bridge_connector);
+               break;
+       case DRM_BRIDGE_DETACHED:
+       {
+               /*
+                * A bridge was unplugged, remove the drm_connector if it's
+                * part of the same pipeline
+                */
+               struct drm_bridge *bridge = (struct drm_bridge *)data;
+
+               if (bridge_connector->dynconn &&
+                   bridge->encoder == bridge_connector->encoder)
+                       drm_bridge_connector_dynconn_release(bridge_connector);
+               break;
+       }
+       default:
+       }
+
+       return NOTIFY_DONE;
+}
+
 static void drm_bridge_connector_fini(struct drm_device *dev, void *res)
 {
        struct drm_bridge_connector *bridge_connector = (struct 
drm_bridge_connector *)res;
@@ -1205,3 +1296,69 @@ struct drm_connector *drm_bridge_connector_init(struct 
drm_device *drm,
        return &bridge_connector->dynconn->connector;
 }
 EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
+
+static void drm_bridge_connector_notifier_unregister(struct drm_device *dev, 
void *res)
+{
+       struct notifier_block *nb = (struct notifier_block *)res;
+
+       drm_event_notifier_unregister(nb);
+}
+
+/**
+ * drmm_bridge_connector_init - Initialise the bridge_connector for a chain
+ *                              of bridges, with bridge hotplug support
+ * @drm: the DRM device
+ * @encoder: the encoder where the bridge chain starts
+ *
+ * Allocate, initialise and register a &drm_bridge_connector with the @drm
+ * device. The connector is associated with a chain of bridges that starts at
+ * the @encoder. All bridges in the chain shall report bridge operation flags
+ * (&drm_bridge->ops) and bridge output type (&drm_bridge->type), and none of
+ * them may create a DRM connector directly.
+ *
+ * Also adds a drm_connector if the bridge chain is complete, otherwise the
+ * &drm_bridge_connector will still be successfully created but without
+ * adding a drm_connector. In both cases the &drm_bridge_connector will
+ * receive events notifying bridge hot-plug and hot-unplug and add or
+ * destroy the drm_connector accordingly.
+ *
+ * Returns a pointer to the new &drm_bridge_connector on success, or a
+ * negative error pointer otherwise.
+ */
+struct drm_bridge_connector *drmm_bridge_connector_init(struct drm_device *drm,
+                                                       struct drm_encoder 
*encoder)
+{
+       struct drm_bridge_connector *bridge_connector;
+       int ret;
+
+       bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector), 
GFP_KERNEL);
+       if (!bridge_connector)
+               return ERR_PTR(-ENOMEM);
+
+       mutex_init(&bridge_connector->dynconn_mutex);
+       bridge_connector->drm = drm;
+       bridge_connector->encoder = encoder;
+       bridge_connector->drm_event_nb.notifier_call = 
drm_bridge_connector_handle_event;
+
+       if (drm_bridge_connector_pipeline_is_complete(bridge_connector)) {
+               ret = drm_bridge_connector_add_connector(bridge_connector);
+               if (ret)
+                       return ERR_PTR(ret);
+       }
+
+       ret = drmm_add_action_or_reset(drm, drm_bridge_connector_fini, 
bridge_connector);
+       if (ret)
+               return ERR_PTR(ret);
+
+       ret = drm_event_notifier_register(&bridge_connector->drm_event_nb);
+       if (ret)
+               return ERR_PTR(ret);
+
+       ret = drmm_add_action_or_reset(drm, 
drm_bridge_connector_notifier_unregister,
+                                      &bridge_connector->drm_event_nb);
+       if (ret)
+               return ERR_PTR(ret);
+
+       return 0;
+}
+EXPORT_SYMBOL_GPL(drmm_bridge_connector_init);
diff --git a/include/drm/drm_bridge_connector.h 
b/include/drm/drm_bridge_connector.h
index 69630815fb09..1c78221c0857 100644
--- a/include/drm/drm_bridge_connector.h
+++ b/include/drm/drm_bridge_connector.h
@@ -10,6 +10,8 @@ struct drm_connector;
 struct drm_device;
 struct drm_encoder;
 
+struct drm_bridge_connector *drmm_bridge_connector_init(struct drm_device *drm,
+                                                       struct drm_encoder 
*encoder);
 struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
                                                struct drm_encoder *encoder);
 

-- 
2.54.0

Reply via email to