Re: [PATCH v5 4/4] drm/bridge: Add the necessary bits to support bus format negotiation
Hi!
Dne četrtek, 19. december 2019 ob 11:11:51 CET je Neil Armstrong napisal(a):
> From: Boris Brezillon
>
> drm_bridge_state is extended to describe the input and output bus
> configurations. These bus configurations are exposed through the
> drm_bus_cfg struct which encodes the configuration of a physical
> bus between two components in an output pipeline, usually between
> two bridges, an encoder and a bridge, or a bridge and a connector.
>
> The bus configuration is stored in drm_bridge_state separately for
> the input and output buses, as seen from the point of view of each
> bridge. The bus configuration of a bridge output is usually identical
> to the configuration of the next bridge's input, but may differ if
> the signals are modified between the two bridges, for instance by an
> inverter on the board. The input and output configurations of a
> bridge may differ if the bridge modifies the signals internally,
> for instance by performing format conversion, or*modifying signals
> polarities.
>
> Bus format negotiation is automated by the core, drivers just have
> to implement the ->atomic_get_{output,input}_bus_fmts() hooks if they
> want to take part to this negotiation. Negotiation happens in reverse
> order, starting from the last element of the chain (the one directly
> connected to the display) up to the first element of the chain (the one
> connected to the encoder).
> During this negotiation all supported formats are tested until we find
> one that works, meaning that the formats array should be in decreasing
> preference order (assuming the driver has a preference order).
>
> Note that the bus format negotiation works even if some elements in the
> chain don't implement the ->atomic_get_{output,input}_bus_fmts() hooks.
> In that case, the core advertises only MEDIA_BUS_FMT_FIXED and lets
> the previous bridge element decide what to do (most of the time, bridge
> drivers will pick a default bus format or extract this piece of
> information from somewhere else, like a FW property).
>
> Signed-off-by: Boris Brezillon
> Signed-off-by: Neil Armstrong
> ---
Thanks a lot for this work! Just one small nit bellow. Otherwise:
Reviewed by: Jernej Skrabec
> Changes in v5:
> * None
>
> Changes in v4:
> * Enhance the doc
> * Fix typos
> * Rename some parameters/fields
> * Reword the commit message
>
> Changes in v3:
> * Fix the commit message (Reported by Laurent)
> * Document the fact that bus formats should not be directly modified by
> drivers (Suggested by Laurent)
> * Document the fact that format order matters (Suggested by Laurent)
> * Propagate bus flags by default
> * Document the fact that drivers can tweak bus flags if needed
> * Let ->atomic_get_{output,input}_bus_fmts() allocate the bus format
> array (Suggested by Laurent)
> * Add a drm_atomic_helper_bridge_propagate_bus_fmt()
> * Mandate that bridge drivers return accurate input_fmts even if they
> are known to be the first element in the bridge chain
>
> Changes in v2:
> * Rework things to support more complex use cases
> ---
> drivers/gpu/drm/drm_bridge.c | 267 ++-
> include/drm/drm_bridge.h | 124
> 2 files changed, 390 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 442804598f60..9cc4b0181f85 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -635,13 +635,261 @@ static int drm_atomic_bridge_check(struct drm_bridge
> *bridge, return 0;
> }
>
> +/**
> + * drm_atomic_helper_bridge_propagate_bus_fmt() - Propagate output format
> to + * the
input end of a bridge
> + * @bridge: bridge control structure
> + * @bridge_state: new bridge state
> + * @crtc_state: new CRTC state
> + * @conn_state: new connector state
> + * @output_fmt: tested output bus format
> + * @num_input_fmts: will contain the size of the returned array
> + *
> + * This helper is a pluggable implementation of the
> + * _bridge_funcs.atomic_get_input_bus_fmts operation for bridges that
> don't + * modify the bus configuration between their input and their
> output. It + * returns an array of input formats with a single element set
> to @output_fmt. + *
> + * RETURNS:
> + * a valid format array of size @num_input_fmts, or NULL if the allocation
> + * failed
> + */
> +u32 *
> +drm_atomic_helper_bridge_propagate_bus_fmt(struct drm_bridge *bridge,
> + struct drm_bridge_state
*bridge_state,
> + struct drm_crtc_state
*crtc_state,
> + struct
drm_connector_state *conn_state,
> + u32 output_fmt,
> + unsigned int
*num_input_fmts)
> +{
> + u32 *input_fmts;
> +
> + input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
> + if (!input_fmts) {
> +
[PATCH v5 4/4] drm/bridge: Add the necessary bits to support bus format negotiation
From: Boris Brezillon
drm_bridge_state is extended to describe the input and output bus
configurations. These bus configurations are exposed through the
drm_bus_cfg struct which encodes the configuration of a physical
bus between two components in an output pipeline, usually between
two bridges, an encoder and a bridge, or a bridge and a connector.
The bus configuration is stored in drm_bridge_state separately for
the input and output buses, as seen from the point of view of each
bridge. The bus configuration of a bridge output is usually identical
to the configuration of the next bridge's input, but may differ if
the signals are modified between the two bridges, for instance by an
inverter on the board. The input and output configurations of a
bridge may differ if the bridge modifies the signals internally,
for instance by performing format conversion, or*modifying signals
polarities.
Bus format negotiation is automated by the core, drivers just have
to implement the ->atomic_get_{output,input}_bus_fmts() hooks if they
want to take part to this negotiation. Negotiation happens in reverse
order, starting from the last element of the chain (the one directly
connected to the display) up to the first element of the chain (the one
connected to the encoder).
During this negotiation all supported formats are tested until we find
one that works, meaning that the formats array should be in decreasing
preference order (assuming the driver has a preference order).
Note that the bus format negotiation works even if some elements in the
chain don't implement the ->atomic_get_{output,input}_bus_fmts() hooks.
In that case, the core advertises only MEDIA_BUS_FMT_FIXED and lets
the previous bridge element decide what to do (most of the time, bridge
drivers will pick a default bus format or extract this piece of
information from somewhere else, like a FW property).
Signed-off-by: Boris Brezillon
Signed-off-by: Neil Armstrong
---
Changes in v5:
* None
Changes in v4:
* Enhance the doc
* Fix typos
* Rename some parameters/fields
* Reword the commit message
Changes in v3:
* Fix the commit message (Reported by Laurent)
* Document the fact that bus formats should not be directly modified by
drivers (Suggested by Laurent)
* Document the fact that format order matters (Suggested by Laurent)
* Propagate bus flags by default
* Document the fact that drivers can tweak bus flags if needed
* Let ->atomic_get_{output,input}_bus_fmts() allocate the bus format
array (Suggested by Laurent)
* Add a drm_atomic_helper_bridge_propagate_bus_fmt()
* Mandate that bridge drivers return accurate input_fmts even if they
are known to be the first element in the bridge chain
Changes in v2:
* Rework things to support more complex use cases
---
drivers/gpu/drm/drm_bridge.c | 267 ++-
include/drm/drm_bridge.h | 124
2 files changed, 390 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 442804598f60..9cc4b0181f85 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -635,13 +635,261 @@ static int drm_atomic_bridge_check(struct drm_bridge
*bridge,
return 0;
}
+/**
+ * drm_atomic_helper_bridge_propagate_bus_fmt() - Propagate output format to
+ * the input end of a bridge
+ * @bridge: bridge control structure
+ * @bridge_state: new bridge state
+ * @crtc_state: new CRTC state
+ * @conn_state: new connector state
+ * @output_fmt: tested output bus format
+ * @num_input_fmts: will contain the size of the returned array
+ *
+ * This helper is a pluggable implementation of the
+ * _bridge_funcs.atomic_get_input_bus_fmts operation for bridges that don't
+ * modify the bus configuration between their input and their output. It
+ * returns an array of input formats with a single element set to @output_fmt.
+ *
+ * RETURNS:
+ * a valid format array of size @num_input_fmts, or NULL if the allocation
+ * failed
+ */
+u32 *
+drm_atomic_helper_bridge_propagate_bus_fmt(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
+{
+ u32 *input_fmts;
+
+ input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
+ if (!input_fmts) {
+ *num_input_fmts = 0;
+ return NULL;
+ }
+
+ *num_input_fmts = 1;
+ input_fmts[0] = output_fmt;
+ return input_fmts;
+}
+EXPORT_SYMBOL(drm_atomic_helper_bridge_propagate_bus_fmt);
+
+static int select_bus_fmt_recursive(struct drm_bridge *first_bridge,
+ struct drm_bridge *cur_bridge,
+
