Hi

with my comments addressed:

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

Am 17.03.21 um 16:43 schrieb Maxime Ripard:
> The new bridge rework to support the input and output formats introduced
> some boilerplate code that will need to be shared across drivers.
> 
> Since dw-hdmi is the only driver so far, let's introduce those helpers
> based on that code.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/Makefile   |   2 +-
>   drivers/gpu/drm/drm_hdmi.c | 170 +++++++++++++++++++++++++++++++++++++
>   include/drm/drm_hdmi.h     |  24 ++++++
>   3 files changed, 195 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/gpu/drm/drm_hdmi.c
>   create mode 100644 include/drm/drm_hdmi.h
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 5eb5bf7c16e3..1b77bd64a37e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -17,7 +17,7 @@ drm-y       :=	drm_auth.o drm_cache.o \
>   		drm_plane.o drm_color_mgmt.o drm_print.o \
>   		drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
>   		drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> -		drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o \
> +		drm_client_modeset.o drm_atomic_uapi.o drm_hdmi.o drm_hdcp.o \
>   		drm_managed.o drm_vblank_work.o
>   
>   drm-$(CONFIG_DRM_LEGACY) += drm_bufs.o drm_context.o drm_dma.o drm_legacy_misc.o drm_lock.o \
> diff --git a/drivers/gpu/drm/drm_hdmi.c b/drivers/gpu/drm/drm_hdmi.c
> new file mode 100644
> index 000000000000..3834d5dd6d88
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_hdmi.c
> @@ -0,0 +1,170 @@

The SPDX tag is missing from this file.

> +#include <linux/errno.h>
> +#include <linux/hdmi.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/types.h>
> +
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_hdmi.h>
> +
> +/**
> + * drm_hdmi_bus_fmt_is_rgb() - Is the media bus format an RGB format?
> + * @bus_format: MEDIA_BUS_FMT* to test
> + *
> + * Checks if the media bus format is an RGB one
> + *
> + * RETURNS:

Just a question on this. I always use 'Returns:' Is this supposed to be 
in capital letters? And does it make a difference?

> + * True if the format is an RGB one, false otherwise
> + */
> +bool drm_hdmi_bus_fmt_is_rgb(u32 bus_format)
> +{
> +	switch (bus_format) {
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_RGB101010_1X30:
> +	case MEDIA_BUS_FMT_RGB121212_1X36:
> +	case MEDIA_BUS_FMT_RGB161616_1X48:
> +		return true;
> +

No empty line here and in similar places.

> +	default:
> +		return false;
> +	}
> +}
> +EXPORT_SYMBOL(drm_hdmi_bus_fmt_is_rgb);
> +
> +/**
> + * drm_hdmi_bus_fmt_is_yuv444() - Is the media bus format an YUV444 format?
> + * @bus_format: MEDIA_BUS_FMT* to test
> + *
> + * Checks if the media bus format is an YUV444 one
> + *
> + * RETURNS:
> + * True if the format is an YUV444 one, false otherwise
> + */
> +bool drm_hdmi_bus_fmt_is_yuv444(u32 bus_format)
> +{
> +	switch (bus_format) {
> +	case MEDIA_BUS_FMT_YUV8_1X24:
> +	case MEDIA_BUS_FMT_YUV10_1X30:
> +	case MEDIA_BUS_FMT_YUV12_1X36:
> +	case MEDIA_BUS_FMT_YUV16_1X48:
> +		return true;
> +
> +	default:
> +		return false;
> +	}
> +}
> +EXPORT_SYMBOL(drm_hdmi_bus_fmt_is_yuv444);
> +
> +/**
> + * drm_hdmi_bus_fmt_is_yuv422() - Is the media bus format an YUV422 format?
> + * @bus_format: MEDIA_BUS_FMT* to test
> + *
> + * Checks if the media bus format is an YUV422 one
> + *
> + * RETURNS:
> + * True if the format is an YUV422 one, false otherwise
> + */
> +bool drm_hdmi_bus_fmt_is_yuv422(u32 bus_format)
> +{
> +	switch (bus_format) {
> +	case MEDIA_BUS_FMT_UYVY8_1X16:
> +	case MEDIA_BUS_FMT_UYVY10_1X20:
> +	case MEDIA_BUS_FMT_UYVY12_1X24:
> +		return true;
> +
> +	default:
> +		return false;
> +	}
> +}
> +EXPORT_SYMBOL(drm_hdmi_bus_fmt_is_yuv422);
> +
> +/**
> + * drm_hdmi_bus_fmt_is_yuv420() - Is the media bus format an YUV420 format?
> + * @bus_format: MEDIA_BUS_FMT* to test
> + *
> + * Checks if the media bus format is an YUV420 one
> + *
> + * RETURNS:
> + * True if the format is an YUV420 one, false otherwise
> + */
> +bool drm_hdmi_bus_fmt_is_yuv420(u32 bus_format)
> +{
> +	switch (bus_format) {
> +	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
> +	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
> +	case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
> +	case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
> +		return true;
> +
> +	default:
> +		return false;
> +	}
> +}
> +EXPORT_SYMBOL(drm_hdmi_bus_fmt_is_yuv420);
> +
> +/**
> + * drm_hdmi_bus_fmt_color_depth() - Returns the color depth in bits
> + * @bus_format: MEDIA_BUS_FMT* to test
> + *
> + * Computes the number of bits per color for a given media bus format
> + *
> + * RETURNS:
> + * The number of bits per color
> + */
> +int drm_hdmi_bus_fmt_color_depth(u32 bus_format)
> +{
> +	switch (bus_format) {
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_YUV8_1X24:
> +	case MEDIA_BUS_FMT_UYVY8_1X16:
> +	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
> +		return 8;
> +
> +	case MEDIA_BUS_FMT_RGB101010_1X30:
> +	case MEDIA_BUS_FMT_YUV10_1X30:
> +	case MEDIA_BUS_FMT_UYVY10_1X20:
> +	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
> +		return 10;
> +
> +	case MEDIA_BUS_FMT_RGB121212_1X36:
> +	case MEDIA_BUS_FMT_YUV12_1X36:
> +	case MEDIA_BUS_FMT_UYVY12_1X24:
> +	case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
> +		return 12;
> +
> +	case MEDIA_BUS_FMT_RGB161616_1X48:
> +	case MEDIA_BUS_FMT_YUV16_1X48:
> +	case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
> +		return 16;
> +
> +	default:
> +		return 0;
> +	}
> +}
> +EXPORT_SYMBOL(drm_hdmi_bus_fmt_color_depth);
> +
> +/**
> + * drm_hdmi_bus_fmt_color_depth() - Returns the color depth in bits
> + * @bus_format: MEDIA_BUS_FMT* to test
> + *
> + * Computes the number of bits per color for a given media bus format
> + *
> + * RETURNS:
> + * The number of bits per color
> + */
> +int drm_hdmi_avi_infoframe_output_colorspace(struct hdmi_avi_infoframe *frame,
> +					     struct drm_bus_cfg *out_bus_cfg)
> +{
> +	if (drm_hdmi_bus_fmt_is_yuv444(out_bus_cfg->format))
> +		frame->colorspace = HDMI_COLORSPACE_YUV444;
> +	else if (drm_hdmi_bus_fmt_is_yuv422(out_bus_cfg->format))
> +		frame->colorspace = HDMI_COLORSPACE_YUV422;
> +	else if (drm_hdmi_bus_fmt_is_yuv420(out_bus_cfg->format))
> +		frame->colorspace = HDMI_COLORSPACE_YUV420;
> +	else if (drm_hdmi_bus_fmt_is_rgb(out_bus_cfg->format))
> +		frame->colorspace = HDMI_COLORSPACE_RGB;
> +	else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_output_colorspace);
> diff --git a/include/drm/drm_hdmi.h b/include/drm/drm_hdmi.h
> new file mode 100644
> index 000000000000..8cd281699ea0
> --- /dev/null
> +++ b/include/drm/drm_hdmi.h
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2013-2015 Mentor Graphics Inc.
> + * Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
> + * Copyright (C) 2010, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + */
> +
> +#ifndef __DRM_HDMI_H_
> +#define __DRM_HDMI_H_
> +
> +#include <linux/types.h>
> +
> +struct drm_bus_cfg;
> +struct hdmi_avi_infoframe;
> +
> +bool drm_hdmi_bus_fmt_is_rgb(u32 bus_format);
> +bool drm_hdmi_bus_fmt_is_yuv444(u32 bus_format);
> +bool drm_hdmi_bus_fmt_is_yuv422(u32 bus_format);
> +bool drm_hdmi_bus_fmt_is_yuv420(u32 bus_format);
> +int drm_hdmi_bus_fmt_color_depth(u32 bus_format);
> +int drm_hdmi_avi_infoframe_output_colorspace(struct hdmi_avi_infoframe *frame,
> +					     struct drm_bus_cfg *out_bus_cfg);
> +
> +#endif // __DRM_HDMI_H_
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer