Re: [PATCH v9 12/19] drm/bridge: dw-hdmi-qp: Implement atomic_get_output_bus_fmts

Dmitry Baryshkov
2026-03-01T16:42:48Z

On Fri, Feb 27, 2026 at 08:20:17PM +0100, Nicolas Frattaroli wrote:
> The atomic_get_output_bus_fmts callback is used by the DRM bridge layer
> to recursively select a suitable output format in a bridge chain.
>
> As a bridge that outputs to HDMI, dw-hdmi-qp will have its output
> formats determined by which formats the platform-specific integration of
> the hardware supports, and the chosen HDMI output bit depth.
>
> Implement this callback. The returned u32* buffer is supposed to be
> freed by the caller of this callback, as specified by the callback's
> documentation.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 64 ++++++++++++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> index d649a1cf07f5..4556494f9bb1 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> @@ -11,6 +11,7 @@
> #include <linux/export.h>
> #include <linux/i2c.h>
> #include <linux/irq.h>
> +#include <linux/media-bus-format.h>
> #include <linux/minmax.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> @@ -749,6 +750,68 @@ static struct i2c_adapter *dw_hdmi_qp_i2c_adapter(struct dw_hdmi_qp *hdmi)
> return adap;
> }
>
> +static u32*
> +dw_hdmi_qp_bridge_get_output_bus_fmts(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state,
> + unsigned int *num_output_fmts)
> +{
> + unsigned int num_fmts = 0;
> + u32 *out_fmts;
> +
> + /*
> + * bridge->supported_formats is a bit field of the HDMI_COLORSPACE_* enums.
> + * These enums are defined by the HDMI standard, and currently top out at
> + * 7. Consequently, BIT(7) is the highest bit that will be set here, unless
> + * the standard runs out of reserved pixel formats. Therefore, hweight8()
> + * will give an accurate count of how many bus formats we'll output.
> + */
> + out_fmts = kmalloc_array(hweight8(bridge->supported_formats), sizeof(u32),
> + GFP_KERNEL);
> + if (!out_fmts) {
> + *num_output_fmts = 0;
> + return NULL;
> + }
> +
> + switch (conn_state->hdmi.output_bpc) {
> + case 12:
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_RGB121212_1X36;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_YUV12_1X36;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYVY12_1X24;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYYVYY12_0_5X36;

This looks like very generic code. It uses bridge->supported_formats
only. Can we move this to the helpers library?

> + break;
> + case 10:
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_RGB101010_1X30;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_YUV10_1X30;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYVY10_1X20;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYYVYY10_0_5X30;
> + break;
> + default:
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_RGB888_1X24;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_YUV8_1X24;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYVY8_1X16;
> + if (bridge->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420))
> + out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
> + break;
> + }
> +
> + *num_output_fmts = num_fmts;
> +
> + return out_fmts;
> +}
> +
> static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
> struct drm_atomic_state *state)
> {
> @@ -1192,6 +1255,7 @@ static int dw_hdmi_qp_cec_transmit(struct drm_bridge *bridge, u8 attempts,
> #endif /* CONFIG_DRM_DW_HDMI_QP_CEC */
>
> static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
> + .atomic_get_output_bus_fmts = dw_hdmi_qp_bridge_get_output_bus_fmts,
> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> .atomic_reset = drm_atomic_helper_bridge_reset,
>
> --
> 2.53.0
>

--
With best wishes
Dmitry