[PATCH RFC 14/19] drm/bridge: Add the necessary bits to support bus format negotiation

Boris Brezillon boris.brezillon at collabora.com
Thu Aug 8 15:11:45 UTC 2019


This takes the form of various helpers, plus the addition of 2 new
structs:
* drm_bus_caps: describe the bus capabilities of a bridge/encoder. For
  bridges we have one for the input port and one for the output port.
  Encoders just have one output port.
* drm_bus_cfg: added to the drm_bridge_state. It's supposed to store
  bus configuration information. For now we just have format and flags
  but more fields might be added in the future. Again, each
  drm_bridge_state contains 2 drm_bus_cfg elements: one for the output
  port and one for the input port

Helpers can be used from bridge drivers' ->atomic_check() implementation
to negotiate the bus format to use. Negotiation happens in reserve order,
starting from the last element of the bridge chain (the one directly
connected to the display) to the first element of the chain (the one
connected to the encoder).

Negotiation usually takes place in 2 steps:
* make sure the input end of the next element in the chain picked a
  format that's supported by the output end of our bridge. That's done
  with drm_atomic_bridge_choose_output_bus_cfg().
* choose a format for the input end of our bridge based on what's
  supported by the previous bridge in the chain. This is achieved by
  calling drm_atomic_bridge_choose_input_bus_cfg()

Note that those helpers are a bit lax when it comes to unitialized
bus format validation. That's intentional. If we don't do that we'll
basically break things if we start adding support for bus format
negotiation to some elements of the pipeline leaving others on the
side, and that's likely to happen for all external bridges since we
can't necessarily tell where they are used.

Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
---
 drivers/gpu/drm/drm_bridge.c | 187 +++++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h     |  46 +++++++++
 include/drm/drm_encoder.h    |   6 ++
 3 files changed, 239 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 9efb27087e70..ef072df42422 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -602,6 +602,193 @@ void drm_atomic_bridge_chain_enable(struct drm_encoder *encoder,
 }
 EXPORT_SYMBOL(drm_atomic_bridge_chain_enable);
 
+int drm_find_best_bus_format(const struct drm_bus_caps *a,
+			     const struct drm_bus_caps *b,
+			     const struct drm_display_mode *mode,
+			     u32 *selected_bus_fmt)
+{
+	unsigned int i, j;
+
+	/*
+	 * Some drm_bridge/drm_encoder don't care about the input/output bus
+	 * format, let's set the format to zero in that case (this is only
+	 * valid if both side of the link don't care).
+	 */
+	if (!a->num_supported_fmts && !b->num_supported_fmts) {
+		*selected_bus_fmt = 0;
+		return 0;
+	} else if (b->num_supported_fmts > 1 && b->supported_fmts) {
+		*selected_bus_fmt = b->supported_fmts[0];
+		return 0;
+	} else if (a->num_supported_fmts > 1 && a->supported_fmts) {
+		*selected_bus_fmt = a->supported_fmts[0];
+		return 0;
+	} else if (!a->num_supported_fmts || !a->supported_fmts ||
+		   !b->num_supported_fmts || !b->supported_fmts) {
+		return -EINVAL;
+	}
+
+	/*
+	 * TODO:
+	 * Dummy algo picking the first match. We probably want something
+	 * smarter where the narrowest format (in term of bus width) that
+	 * does not incur data loss is picked, and if all possible formats
+	 * are lossy, pick the one that's less lossy among all the choices
+	 * we have. In order to do that we'd need to convert MEDIA_BUS_FMT_
+	 * modes into something like drm_format_info.
+	 */
+	for (i = 0; i < a->num_supported_fmts; i++) {
+		for (j = 0; j < b->num_supported_fmts; j++) {
+			if (a->supported_fmts[i] == b->supported_fmts[j]) {
+				*selected_bus_fmt = a->supported_fmts[i];
+				return 0;
+			}
+		}
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL(drm_find_best_bus_format);
+
+/**
+ * drm_atomic_bridge_choose_input_bus_cfg() - Choose bus config for the input
+ *					      end
+ * @bridge_state: bridge state
+ * @crtc_state: CRTC state
+ * @conn_state: connector state
+ *
+ * Choose a bus format for the input side of a bridge based on what the
+ * previous bridge in the chain and this bridge support. Can be called from
+ * bridge drivers' &drm_bridge_funcs.atomic_check() implementation.
+ *
+ * RETURNS:
+ * 0 if a matching format was found, a negative error code otherwise
+ */
+int
+drm_atomic_bridge_choose_input_bus_cfg(struct drm_bridge_state *bridge_state,
+				       struct drm_crtc_state *crtc_state,
+				       struct drm_connector_state *conn_state)
+{
+	struct drm_bridge *self = bridge_state->bridge;
+	struct drm_bus_caps *prev_output_bus_caps;
+	struct drm_bridge *prev;
+	int ret;
+
+	prev = drm_bridge_chain_get_prev_bridge(self);
+	if (!prev)
+		prev_output_bus_caps = &self->encoder->output_bus_caps;
+	else
+		prev_output_bus_caps = &prev->output_bus_caps;
+
+	ret = drm_find_best_bus_format(prev_output_bus_caps,
+				       &self->input_bus_caps, &crtc_state->mode,
+				       &bridge_state->input_bus_cfg.fmt);
+	if (ret)
+		return ret;
+
+	/*
+	 * TODO:
+	 * Should we fill/check the ->flag field too?
+	 */
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_bridge_choose_input_bus_cfg);
+
+/**
+ * drm_atomic_bridge_choose_output_bus_cfg() - Choose bus config for the output
+ *					       end
+ * @bridge_state: bridge state
+ * @crtc_state: CRTC state
+ * @conn_state: connector state
+ *
+ * Choose a bus format for the output side of a bridge based on what the next
+ * bridge in the chain and this bridge support. Can be called from bridge
+ * drivers' &drm_bridge_funcs.atomic_check() implementation.
+ *
+ * RETURNS:
+ * 0 if a matching format was found, a negative error code otherwise
+ */
+int
+drm_atomic_bridge_choose_output_bus_cfg(struct drm_bridge_state *bridge_state,
+					struct drm_crtc_state *crtc_state,
+					struct drm_connector_state *conn_state)
+{
+	struct drm_atomic_state *state = crtc_state->state;
+	struct drm_bridge *self = bridge_state->bridge;
+	struct drm_bridge_state *next_bridge_state;
+	struct drm_bridge *next;
+	u32 fmt;
+
+	next = drm_bridge_chain_get_next_bridge(self);
+	if (!next) {
+		struct drm_connector *conn = conn_state->connector;
+		struct drm_display_info *di = &conn->display_info;
+
+		/*
+		 * FIXME:
+		 * We currently pick the first supported format
+		 * unconditionally. It seems to fit all current use cases but
+		 * might be too limited for panels/displays that can configure
+		 * the bus format dynamically.
+		 */
+		if (di->num_bus_formats && di->bus_formats)
+			bridge_state->output_bus_cfg.fmt = di->bus_formats[0];
+		else
+			bridge_state->output_bus_cfg.fmt = 0;
+
+		bridge_state->output_bus_cfg.flags = di->bus_flags;
+		return 0;
+	}
+
+	/*
+	 * We expect output_bus_caps to contain at least one format. Note
+	 * that don't care about bus format negotiation can simply not
+	 * call this helper.
+	 */
+	if (!self->output_bus_caps.num_supported_fmts ||!
+	    !self->output_bus_caps.supported_fmts)
+		return -EINVAL;
+
+	next_bridge_state = drm_atomic_get_new_bridge_state(state, next);
+
+	/*
+	 * The next bridge is expected to have called
+	 * &drm_atomic_bridge_choose_input_bus_cfg() as part of its
+	 * &drm_bridge_funcs.atomic_check() implementation, but this hook is
+	 * optional, and even if it's implemented, calling
+	 * &drm_atomic_bridge_choose_input_bus_cfg() is not mandated.
+	 * If fmt is zero, that means the next element in the chain doesn't
+	 * care about bus format negotiation (probably because there's only
+	 * one possible setting). In that case, we still have to select one
+	 * bus format for the output port of our bridge, and this is only
+	 * possible if the bridge supports only one format.
+	 */
+	fmt = next_bridge_state->input_bus_cfg.fmt;
+	if (fmt) {
+		unsigned int i;
+
+		for (i = 0; i < self->output_bus_caps.num_supported_fmts; i++) {
+			if (self->output_bus_caps.supported_fmts[i] == fmt)
+				break;
+		}
+
+		if (i == self->output_bus_caps.num_supported_fmts)
+			return -ENOTSUPP;
+	} else if (self->output_bus_caps.num_supported_fmts == 1) {
+		fmt = self->output_bus_caps.supported_fmts[0];
+	} else {
+		return -EINVAL;
+	}
+
+	/*
+	 * TODO:
+	 * Should we fill/check the ->flag field too?
+	 */
+	bridge_state->output_bus_cfg.fmt = fmt;
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_bridge_choose_output_bus_cfg);
+
 static int drm_atomic_bridge_check(struct drm_bridge *bridge,
 				   struct drm_crtc_state *crtc_state,
 				   struct drm_connector_state *conn_state)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 2f69adb7b0f3..c578800a05e0 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -33,15 +33,45 @@ struct drm_bridge;
 struct drm_bridge_timings;
 struct drm_panel;
 
+/**
+ * struct drm_bus_cfg - bus configuration
+ * @fmt: format used on this bus
+ * @flags: DRM_BUS_ flags used on this bus
+ *
+ * Encodes the bus format and bus flags used by one end of the bridge or
+ * by the encoder output.
+ */
+struct drm_bus_cfg {
+	u32 fmt;
+	u32 flags;
+};
+
+/**
+ * struct drm_bus_caps - bus capabilities
+ * @supported_fmts: array of MEDIA_BUS_FMT_ formats
+ * @num_supported_fmts: size of the supported_fmts array
+ *
+ * Used by the core to negotiate the bus format at runtime.
+ */
+struct drm_bus_caps {
+	const u32 *supported_fmts;
+	unsigned int num_supported_fmts;
+};
+
 /**
  * struct drm_bridge_state - Atomic bridge state object
  * @base: inherit from &drm_private_state
  * @bridge: the bridge this state refers to
+ * @input_bus_info: input bus information
+ * @output_bus_info: output bus information
  */
 struct drm_bridge_state {
 	struct drm_private_state base;
 
 	struct drm_bridge *bridge;
+
+	struct drm_bus_cfg input_bus_cfg;
+	struct drm_bus_cfg output_bus_cfg;
 };
 
 static inline struct drm_bridge_state *
@@ -465,6 +495,9 @@ struct drm_bridge {
 	const struct drm_bridge_funcs *funcs;
 	/** @driver_private: pointer to the bridge driver's internal context */
 	void *driver_private;
+
+	struct drm_bus_caps input_bus_caps;
+	struct drm_bus_caps output_bus_caps;
 };
 
 static inline struct drm_bridge *
@@ -479,6 +512,14 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np);
 int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
 		      struct drm_bridge *previous);
 
+int
+drm_atomic_bridge_choose_input_bus_cfg(struct drm_bridge_state *bridge_state,
+				       struct drm_crtc_state *crtc_state,
+				       struct drm_connector_state *conn_state);
+int
+drm_atomic_bridge_choose_output_bus_cfg(struct drm_bridge_state *bridge_state,
+					struct drm_crtc_state *crtc_state,
+					struct drm_connector_state *conn_state);
 struct drm_bridge *
 drm_bridge_chain_get_first_bridge(struct drm_encoder *encoder);
 struct drm_bridge *
@@ -562,6 +603,11 @@ drm_atomic_get_new_bridge_state(struct drm_atomic_state *state,
 	return drm_priv_to_bridge_state(obj_state);
 }
 
+int drm_find_best_bus_format(const struct drm_bus_caps *a,
+			     const struct drm_bus_caps *b,
+			     const struct drm_display_mode *mode,
+			     u32 *selected_bus_fmt);
+
 #ifdef CONFIG_DRM_PANEL_BRIDGE
 struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
 					u32 connector_type);
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 0899f7f34e3a..e891921b3aed 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -25,6 +25,7 @@
 
 #include <linux/list.h>
 #include <linux/ctype.h>
+#include <drm/drm_bridge.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_mode.h>
 #include <drm/drm_mode_object.h>
@@ -184,6 +185,11 @@ struct drm_encoder {
 	 * functions as part of its encoder enable/disable handling.
 	 */
 	uint32_t custom_bridge_enable_disable_seq : 1;
+
+	/**
+	 * @output_bus_caps: Supported output bus caps
+	 */
+	struct drm_bus_caps output_bus_caps;
 };
 
 #define obj_to_encoder(x) container_of(x, struct drm_encoder, base)
-- 
2.21.0



More information about the dri-devel mailing list