Re: [PATCHv2 1/6] drm: drm_bridge: add connector_attach/detach bridge ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

On 02/03/2021 18:23, Hans Verkuil wrote:
Add bridge connector_attach/detach ops. These ops are called when a
bridge is attached or detached to a drm_connector. These ops can be
used to register and unregister an HDMI CEC adapter for a bridge that
supports CEC.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
---
  drivers/gpu/drm/drm_bridge_connector.c |  9 +++++++++
  include/drm/drm_bridge.h               | 27 ++++++++++++++++++++++++++
  2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
index 791379816837..07db71d4f5b3 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -203,6 +203,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector)
  {
  	struct drm_bridge_connector *bridge_connector =
  		to_drm_bridge_connector(connector);
+	struct drm_bridge *bridge;
+
+	drm_for_each_bridge_in_chain(bridge_connector->encoder, bridge)
+		if (bridge->funcs->connector_detach)
+			bridge->funcs->connector_detach(bridge, connector);
if (bridge_connector->bridge_hpd) {
  		struct drm_bridge *hpd = bridge_connector->bridge_hpd;
@@ -375,6 +380,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
  		connector->polled = DRM_CONNECTOR_POLL_CONNECT
  				  | DRM_CONNECTOR_POLL_DISCONNECT;
+ drm_for_each_bridge_in_chain(encoder, bridge)
+		if (bridge->funcs->connector_attach)
+			bridge->funcs->connector_attach(bridge, connector);
+
  	return connector;
  }
  EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 2195daa289d2..3320a6ebd253 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -629,6 +629,33 @@ struct drm_bridge_funcs {
  	 * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops.
  	 */
  	void (*hpd_disable)(struct drm_bridge *bridge);
+
+	/**
+	 * @connector_attach:
+	 *
+	 * This callback is invoked whenever our bridge is being attached to a
+	 * &drm_connector. This is where an HDMI CEC adapter can be registered.
+	 * Note that this callback expects that this op always succeeds. Since
+	 * HDMI CEC support is an optional feature, any failure to register a
+	 * CEC adapter must be ignored since video output will still work
+	 * without CEC.
+	 *

Even if CEC support is optional, the callback itself is generic. Wouldn't it be better to make this function return an error, and for CEC, just return 0 if CEC won't get registered correctly?

Also, I personally like things to fail if something doesn't go right, instead of continuing, if that thing is never supposed to happen in normal situations. E.g. if CEC registration fails because we're out of memory, I think the op should fail too.

 Tomi



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux