On 08/08/2019 17:11, Boris Brezillon wrote:
> 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.brezil...@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;
Here, `!a->num_supported_fmts &&` is missing otherwise this code will
select b->supported_fmts[0] whatever the supported formats of a.
> + } else if (a->num_supported_fmts > 1 && a->supported_fmts) {
> + *selected_bus_fmt = a->supported_fmts[0];
> + return 0;
Here, `!b->num_supported_fmts &&` is missing otherwise this code will
select a->supported_fmts[0] whatever the supported formats of b.
> + } 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);
[...]
With that fixed, this works with basic negociation of the single
MEDIA_BUS_FMT_YUV8_1X24
exposed by the meson hdmi encoder, otherwise it takes the first default
MEDIA_BUS_FMT_RGB888_1X24
format exposed by dw-hdmi.
Neil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
