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_

Hi

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

The atomic_get_output_bus_fmts bridge callback is there to list the available formats for output by decreasing order of preference.

On HDMI controllers, we have a fairly static list that will depend on what the HDMI sink is capable of and the BPC our controller can output.

The dw-hdmi driver already has that code done in a fairly generic manner, so let's turn that code into an helper for all the HDMI controllers to reuse.

Signed-off-by: Maxime Ripard maxime@cerno.tech

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 127 ---------------------- drivers/gpu/drm/drm_bridge.c | 118 ++++++++++++++++++++ include/drm/drm_bridge.h | 6 + 3 files changed, 124 insertions(+), 127 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..d010c9c525d9 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2514,133 +2514,6 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)

• DRM Bridge Operations

*/

-/*

• • Possible output formats :
• • • MEDIA_BUS_FMT_UYYVYY16_0_5X48,
• • • MEDIA_BUS_FMT_UYYVYY12_0_5X36,
• • • MEDIA_BUS_FMT_UYYVYY10_0_5X30,
• • • MEDIA_BUS_FMT_UYYVYY8_0_5X24,
• • • MEDIA_BUS_FMT_YUV16_1X48,
• • • MEDIA_BUS_FMT_RGB161616_1X48,
• • • MEDIA_BUS_FMT_UYVY12_1X24,
• • • MEDIA_BUS_FMT_YUV12_1X36,
• • • MEDIA_BUS_FMT_RGB121212_1X36,
• • • MEDIA_BUS_FMT_UYVY10_1X20,
• • • MEDIA_BUS_FMT_YUV10_1X30,
• • • MEDIA_BUS_FMT_RGB101010_1X30,
• • • MEDIA_BUS_FMT_UYVY8_1X16,
• • • MEDIA_BUS_FMT_YUV8_1X24,
• • • MEDIA_BUS_FMT_RGB888_1X24,
• */

-/* Can return a maximum of 11 possible output formats for a mode/connector */ -#define MAX_OUTPUT_SEL_FORMATS 11

-static u32 *dw_hdmi_bridge_atomic_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)
  

-{

• struct drm_connector *conn = conn_state->connector;
• struct drm_display_info *info = &conn->display_info;
• struct drm_display_mode *mode = &crtc_state->mode;
• u8 max_bpc = conn_state->max_requested_bpc;
• bool is_hdmi2_sink = info->hdmi.scdc.supported ||

• 	     (info->color_formats & DRM_COLOR_FORMAT_YCRCB420);
  

• u32 *output_fmts;
• unsigned int i = 0;
• *num_output_fmts = 0;
• output_fmts = kcalloc(MAX_OUTPUT_SEL_FORMATS, sizeof(*output_fmts),

• 	      GFP_KERNEL);
  

• if (!output_fmts)

• return NULL;
  

• /* If dw-hdmi is the only bridge, avoid negociating with ourselves */
• if (list_is_singular(&bridge->encoder->bridge_chain)) {

• *num_output_fmts = 1;
  

• output_fmts[0] = MEDIA_BUS_FMT_FIXED;
  

• return output_fmts;
  

• }
• /*

• * If the current mode enforces 4:2:0, force the output but format
  

• * to 4:2:0 and do not add the YUV422/444/RGB formats
  

• */
  

• if (conn->ycbcr_420_allowed &&

•    (drm_mode_is_420_only(info, mode) ||
  

•     (is_hdmi2_sink && drm_mode_is_420_also(info, mode)))) {
  

• /* Order bus formats from 16bit to 8bit if supported */
  

• if (max_bpc >= 16 && info->bpc == 16 &&
  

•     (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_48))
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY16_0_5X48;
  

• if (max_bpc >= 12 && info->bpc >= 12 &&
  

•     (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY12_0_5X36;
  

• if (max_bpc >= 10 && info->bpc >= 10 &&
  

•     (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_30))
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY10_0_5X30;
  

• /* Default 8bit fallback */
  

• output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
  

• *num_output_fmts = i;
  

• return output_fmts;
  

• }
• /*

• * Order bus formats from 16bit to 8bit and from YUV422 to RGB
  

• * if supported. In any case the default RGB888 format is added
  

• */
  

• if (max_bpc >= 16 && info->bpc == 16) {

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
  

• output_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
  

• }
• if (max_bpc >= 12 && info->bpc >= 12) {

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
  

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
  

• output_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
  

• }
• if (max_bpc >= 10 && info->bpc >= 10) {

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
  

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
  

• output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
  

• }
• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)

• output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
  

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)

• output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
  

• /* Default 8bit RGB fallback */
• output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
• *num_output_fmts = i;
• return output_fmts;

-}

• /*
  • Possible input formats :
  • • MEDIA_BUS_FMT_RGB888_1X24

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 64f0effb52ac..253cbca1c19e 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1035,6 +1035,124 @@ int drm_atomic_bridge_chain_check(struct drm_bridge *bridge, } EXPORT_SYMBOL(drm_atomic_bridge_chain_check);

+/* Can return a maximum of 11 possible output formats for a mode/connector */ +#define MAX_OUTPUT_SEL_FORMATS 11

Where does this number come from? In patch 1, there are 13 formats. Now there are only 11.

+/**

• • drm_bridge_helper_hdmi_atomic_get_output_bus_fmts() - Lists the output formats for an HDMI sink
• • @bridge: bridge control structure
• • @bridge_state: bridge state
• • @crtc_state: CRTC state
• • @conn_state: connector state
• • @num_output_fmts: number of formats returned
• • Returns the supported bus formats on the output end of an HDMI
• • bridge. The returned array is allocated with kmalloc and will thus
• • need to be freed. Formats will be listed in decreasing preference
• • order, the framework eventually picking the highest preference
• • available across all the bridges.
• • RETURNS:
• • an array of MEDIA_FMT_* on success, NULL on failure
• */

+u32 *drm_atomic_helper_bridge_hdmi_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)
  

+{

• struct drm_connector *conn = conn_state->connector;
• struct drm_display_info *info = &conn->display_info;
• struct drm_display_mode *mode = &crtc_state->mode;
• u8 max_bpc = conn_state->max_requested_bpc;
• bool is_hdmi2_sink = info->hdmi.scdc.supported ||

• 	     (info->color_formats & DRM_COLOR_FORMAT_YCRCB420);
  

• u32 *output_fmts;
• unsigned int i = 0;
• *num_output_fmts = 0;
• output_fmts = kcalloc(MAX_OUTPUT_SEL_FORMATS, sizeof(*output_fmts),

• 	      GFP_KERNEL);
  

• if (!output_fmts)

• return NULL;
  

• /*

• * If the current mode enforces 4:2:0, force the output but format
  

• * to 4:2:0 and do not add the YUV422/444/RGB formats
  

• */
  

• if (conn->ycbcr_420_allowed &&

•    (drm_mode_is_420_only(info, mode) ||
  

•     (is_hdmi2_sink && drm_mode_is_420_also(info, mode)))) {
  

• /* Order bus formats from 16bit to 8bit if supported */
  

• if (max_bpc >= 16 && info->bpc == 16 &&
  

•     (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_48))
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY16_0_5X48;
  

• if (max_bpc >= 12 && info->bpc >= 12 &&
  

•     (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY12_0_5X36;
  

• if (max_bpc >= 10 && info->bpc >= 10 &&
  

•     (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_30))
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY10_0_5X30;
  

• /* Default 8bit fallback */
  

• output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
  

• *num_output_fmts = i;
  

• return output_fmts;
  

• }
• /*

• * Order bus formats from 16bit to 8bit and from YUV422 to RGB
  

• * if supported. In any case the default RGB888 format is added
  

• */
  

• if (max_bpc >= 16 && info->bpc == 16) {

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
  

• output_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
  

• }
• if (max_bpc >= 12 && info->bpc >= 12) {

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
  

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
  

• output_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
  

• }
• if (max_bpc >= 10 && info->bpc >= 10) {

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
  

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
  

• 	output_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
  

• output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
  

• }
• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)

• output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
  

• if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)

• output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
  

• /* Default 8bit RGB fallback */
• output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
• *num_output_fmts = i;
• return output_fmts;

+} +EXPORT_SYMBOL_GPL(drm_atomic_helper_bridge_hdmi_get_output_bus_fmts);

• /**
  • drm_bridge_detect - check if anything is attached to the bridge output
  • @bridge: bridge control structure

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 2195daa289d2..1d801d77e90a 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -868,6 +868,12 @@ drm_atomic_helper_bridge_propagate_bus_fmt(struct drm_bridge *bridge, u32 output_fmt, unsigned int *num_input_fmts);

+u32 *drm_atomic_helper_bridge_hdmi_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);
  

• enum drm_connector_status drm_bridge_detect(struct drm_bridge *bridge); int drm_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector);

There was some objection in patch 2, but at least this conversion patch looks good.

Acked-by: Thomas Zimmremann tzimmermann@suse.de

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

Signed-off-by: Maxime Ripard maxime@cerno.tech

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 141 +++++----------------- 1 file changed, 28 insertions(+), 113 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index d010c9c525d9..39b380453183 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -29,6 +29,7 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> #include <drm/drm_edid.h> +#include <drm/drm_hdmi.h> #include <drm/drm_of.h> #include <drm/drm_print.h> #include <drm/drm_probe_helper.h> @@ -801,92 +802,6 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi) } EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);

-static bool hdmi_bus_fmt_is_rgb(unsigned int 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;
  

• default:

• return false;
  

• }

-}

-static bool hdmi_bus_fmt_is_yuv444(unsigned int 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;
  

• }

-}

-static bool hdmi_bus_fmt_is_yuv422(unsigned int 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;
  

• }

-}

-static bool hdmi_bus_fmt_is_yuv420(unsigned int 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;
  

• }

-}

-static int hdmi_bus_fmt_color_depth(unsigned int 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;
  

• }

-}

• /*
  • this submodule is responsible for the video data synchronization.
  • for example, for RGB 4:4:4 input, the data map is defined as

@@ -967,8 +882,8 @@ static int is_color_space_conversion(struct dw_hdmi *hdmi) struct hdmi_data_info *hdmi_data = &hdmi->hdmi_data; bool is_input_rgb, is_output_rgb;

• is_input_rgb = hdmi_bus_fmt_is_rgb(hdmi_data->enc_in_bus_format);
• is_output_rgb = hdmi_bus_fmt_is_rgb(hdmi_data->enc_out_bus_format);

• is_input_rgb = drm_hdmi_bus_fmt_is_rgb(hdmi_data->enc_in_bus_format);

• is_output_rgb = drm_hdmi_bus_fmt_is_rgb(hdmi_data->enc_out_bus_format);

  return (is_input_rgb != is_output_rgb) || (is_input_rgb && is_output_rgb && hdmi_data->rgb_limited_range);

@@ -976,11 +891,11 @@ static int is_color_space_conversion(struct dw_hdmi *hdmi)

static int is_color_space_decimation(struct dw_hdmi *hdmi) {

• if (!hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format))

• if (!drm_hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format)) return 0;

• if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format) ||

•    hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_in_bus_format))
  

• if (drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format) ||

•    drm_hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_in_bus_format))
  

  return 1;

  return 0;

@@ -988,11 +903,11 @@ static int is_color_space_decimation(struct dw_hdmi *hdmi)

static int is_color_space_interpolation(struct dw_hdmi *hdmi) {

• if (!hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_in_bus_format))

• if (!drm_hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_in_bus_format)) return 0;

• if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) ||

•    hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
  

• if (drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) ||

•    drm_hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
  

  return 1;

  return 0;

@@ -1012,8 +927,8 @@ static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi) unsigned i; u32 csc_scale = 1;

• is_input_rgb = hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format);
• is_output_rgb = hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format);

• is_input_rgb = drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format);

• is_output_rgb = drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format);

  if (!is_input_rgb && is_output_rgb) { if (hdmi->hdmi_data.enc_out_encoding == V4L2_YCBCR_ENC_601)

@@ -1061,7 +976,7 @@ static void hdmi_video_csc(struct dw_hdmi *hdmi) else if (is_color_space_decimation(hdmi)) decimation = HDMI_CSC_CFG_DECMODE_CHROMA_INT_FORMULA3;

• switch (hdmi_bus_fmt_color_depth(hdmi->hdmi_data.enc_out_bus_format)) {

• switch (drm_hdmi_bus_fmt_color_depth(hdmi->hdmi_data.enc_out_bus_format)) { case 8: color_depth = HDMI_CSC_SCALE_CSC_COLORDE_PTH_24BPP; break;

@@ -1100,10 +1015,10 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi) struct hdmi_data_info *hdmi_data = &hdmi->hdmi_data; u8 val, vp_conf;

• if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) ||

•    hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format) ||
  

•    hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format)) {
  

• switch (hdmi_bus_fmt_color_depth(
  

• if (drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) ||

•    drm_hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format) ||
  

•    drm_hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format)) {
  

• switch (drm_hdmi_bus_fmt_color_depth(
  		hdmi->hdmi_data.enc_out_bus_format)) {
  

  case 8: color_depth = 4;

@@ -1121,8 +1036,8 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi) default: output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS; }

• } else if (hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format)) {

• switch (hdmi_bus_fmt_color_depth(
  

• } else if (drm_hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format)) {

• switch (drm_hdmi_bus_fmt_color_depth(
  		hdmi->hdmi_data.enc_out_bus_format)) {
  

  case 0: case 8:

@@ -1641,7 +1556,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, /* Initialise info frame from DRM mode */ drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);

• if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format)) {

• if (drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format)) { drm_hdmi_avi_infoframe_quant_range(&frame, connector, mode, hdmi->hdmi_data.rgb_limited_range ? HDMI_QUANTIZATION_RANGE_LIMITED :

@@ -1652,17 +1567,17 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, HDMI_YCC_QUANTIZATION_RANGE_LIMITED; }

• if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))

• if (drm_hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format)) frame.colorspace = HDMI_COLORSPACE_YUV444;

• else if (hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format))

• else if (drm_hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format)) frame.colorspace = HDMI_COLORSPACE_YUV422;

• else if (hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format))

• else if (drm_hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format)) frame.colorspace = HDMI_COLORSPACE_YUV420; else frame.colorspace = HDMI_COLORSPACE_RGB;

  /* Set up colorimetry */

• if (!hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format)) {

• if (!drm_hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format)) { switch (hdmi->hdmi_data.enc_out_encoding) { case V4L2_YCBCR_ENC_601: if (hdmi->hdmi_data.enc_in_encoding == V4L2_YCBCR_ENC_XV601)

@@ -1864,8 +1779,8 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,

vmode->mtmdsclock = vmode->mpixelclock;

• if (!hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format)) {

• switch (hdmi_bus_fmt_color_depth(
  

• if (!drm_hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format)) {

• switch (drm_hdmi_bus_fmt_color_depth(
  	hdmi->hdmi_data.enc_out_bus_format)) {
  

  case 16: vmode->mtmdsclock = vmode->mpixelclock * 2;

@@ -1879,7 +1794,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, } }

• if (hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format))

• if (drm_hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format)) vmode->mtmdsclock /= 2;

  dev_dbg(hdmi->dev, "final tmdsclock = %d\n", vmode->mtmdsclock);

@@ -1930,7 +1845,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, * When we're setting a YCbCr420 mode, we need * to adjust the horizontal timing to suit. */

• if (hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format)) {

• if (drm_hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format)) { hdisplay /= 2; hblank /= 2; h_de_hs /= 2;

@@ -2766,7 +2681,7 @@ static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = { .attach = dw_hdmi_bridge_attach, .detach = dw_hdmi_bridge_detach, .atomic_check = dw_hdmi_bridge_atomic_check,

• .atomic_get_output_bus_fmts = dw_hdmi_bridge_atomic_get_output_bus_fmts,

• .atomic_get_output_bus_fmts = drm_atomic_helper_bridge_hdmi_get_output_bus_fmts, .atomic_get_input_bus_fmts = dw_hdmi_bridge_atomic_get_input_bus_fmts, .atomic_enable = dw_hdmi_bridge_atomic_enable, .atomic_disable = dw_hdmi_bridge_atomic_disable,

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

The current code does a binary or on the possible_crtcs variable of the

s/binary or/binary OR/

I had to read this twice to get it. Otherwise

Acked-by: Thomas Zimmermann tzimmermann@suse.de

TXP encoder, while we want to set it to that value instead.

Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own") Signed-off-by: Maxime Ripard maxime@cerno.tech

---

drivers/gpu/drm/vc4/vc4_txp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index c0122d83b651..2fc7f4b5fa09 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -507,7 +507,7 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) return ret;

encoder = &txp->connector.encoder;

• encoder->possible_crtcs |= drm_crtc_mask(crtc);

• encoder->possible_crtcs = drm_crtc_mask(crtc);

  ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0, dev_name(dev), txp);

Hi Thomas,

On Mon, Apr 12, 2021 at 11:44:05AM +0200, Thomas Zimmermann wrote:

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

...

We're going to need to tell whether we want to run with a full or limited range RGB output in multiple places in the code, so let's create a helper that will return whether we need with full range or not.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

although with a comments

...

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index eee9751009c2..fc545072b173 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -95,6 +95,15 @@ #define HDMI_14_MAX_TMDS_CLK (340 * 1000 * 1000) +static bool vc4_hdmi_is_full_range_rgb(struct vc4_hdmi *vc4_hdmi,

• 		       const struct drm_display_mode *mode)
  

+{

• struct vc4_hdmi_encoder *vc4_encoder = &vc4_hdmi->encoder;
• return !vc4_encoder->hdmi_monitor ||

Is this ever being called from non-HDMI code? If not, I'd put an drm_WARN_ONCE around this check.

I'm not sure we need to worry about this, it's a static function in the HDMI controller driver so it can't be called from anywhere else

Maxime

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

The CSC callbacks takes a boolean as an argument to tell whether we're using the full range or limited range RGB.

However, with the upcoming YUV support, the logic will be a bit more complex. In order to address this, let's make the callbacks take the entire mode, and call our new helper to tell whether the full or limited range RGB should be used.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 14 ++++++++------ drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ++- 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index fc545072b173..bb2fffa2d495 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -494,14 +494,15 @@ static void vc4_hdmi_encoder_disable(struct drm_encoder *encoder) { }

-static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, bool enable) +static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,

• 	       const struct drm_display_mode *mode)
  

  { u32 csc_ctl;

  csc_ctl = VC4_SET_FIELD(VC4_HD_CSC_CTL_ORDER_BGR, VC4_HD_CSC_CTL_ORDER);

• if (enable) {

• if (!vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode)) { /* CEA VICs other than #1 requre limited range RGB
  • output unless overridden by an AVI infoframe.
  • Apply a colorspace conversion to squash 0-255 down

@@ -529,13 +530,14 @@ static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, bool enable) HDMI_WRITE(HDMI_CSC_CTL, csc_ctl); }

-static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, bool enable) +static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,

• 	       const struct drm_display_mode *mode)
  

  { u32 csc_ctl;

  csc_ctl = 0x07; /* RGB_CONVERT_MODE = custom matrix, || USE_RGB_TO_YCBCR */

• if (enable) {

• if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode)) { /* CEA VICs other than #1 requre limited range RGB
  • output unless overridden by an AVI infoframe.
  • Apply a colorspace conversion to squash 0-255 down

@@ -844,12 +846,12 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder,

if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode) { if (vc4_hdmi->variant->csc_setup)

• 	vc4_hdmi->variant->csc_setup(vc4_hdmi, true);
  

• 	vc4_hdmi->variant->csc_setup(vc4_hdmi, mode);
  
  

  vc4_encoder->limited_rgb_range = true; } else { if (vc4_hdmi->variant->csc_setup)

• 	vc4_hdmi->variant->csc_setup(vc4_hdmi, false);
  

• 	vc4_hdmi->variant->csc_setup(vc4_hdmi, mode);
  
  

  vc4_encoder->limited_rgb_range = false; }

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 3cebd1fd00fc..3d88261d463e 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -77,7 +77,8 @@ struct vc4_hdmi_variant { void (*reset)(struct vc4_hdmi *vc4_hdmi);

/* Callback to enable / disable the CSC */

• void (*csc_setup)(struct vc4_hdmi *vc4_hdmi, bool enable);

• void (*csc_setup)(struct vc4_hdmi *vc4_hdmi,

• 	  const struct drm_display_mode *mode);
  
  

  /* Callback to configure the video timings in the HDMI block */ void (*set_timings)(struct vc4_hdmi *vc4_hdmi,

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

The limited_rgb_range field in the vc4_hdmi_encoder structure is used to tell whether we're supposed to output with a full or limited RGB range.

This is redundant with the new helper we introduced, so let's convert to that helper and drop that field.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 21 +++++---------------- drivers/gpu/drm/vc4/vc4_hdmi.h | 1 - 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index bb2fffa2d495..8f0af246f18f 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -384,7 +384,6 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

• struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder); struct drm_connector *connector = &vc4_hdmi->connector; struct drm_connector_state *cstate = connector->state; struct drm_crtc *crtc = encoder->crtc;

@@ -401,9 +400,9 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)

drm_hdmi_avi_infoframe_quant_range(&frame.avi, connector, mode,

• 			   vc4_encoder->limited_rgb_range ?
  

• 			   HDMI_QUANTIZATION_RANGE_LIMITED :
  

• 			   HDMI_QUANTIZATION_RANGE_FULL);
  

• 			   vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode) ?
  

• 			   HDMI_QUANTIZATION_RANGE_FULL :
  

• 			   HDMI_QUANTIZATION_RANGE_LIMITED);
  
  

  drm_hdmi_avi_infoframe_bars(&frame.avi, cstate);

@@ -841,20 +840,10 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, struct drm_atomic_state *state) { struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;

• struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder); struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

• if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode) {

• if (vc4_hdmi->variant->csc_setup)
  

• 	vc4_hdmi->variant->csc_setup(vc4_hdmi, mode);
  

• vc4_encoder->limited_rgb_range = true;
  

• } else {

• if (vc4_hdmi->variant->csc_setup)
  

• 	vc4_hdmi->variant->csc_setup(vc4_hdmi, mode);
  

• vc4_encoder->limited_rgb_range = false;
  

• }

• if (vc4_hdmi->variant->csc_setup)

• vc4_hdmi->variant->csc_setup(vc4_hdmi, mode);
  
  

  HDMI_WRITE(HDMI_FIFO_CTL, VC4_HDMI_FIFO_CTL_MASTER_SLAVE_N); }

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 3d88261d463e..8e42f9e7b3e2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -12,7 +12,6 @@ struct vc4_hdmi_encoder { struct vc4_encoder base; bool hdmi_monitor;

• bool limited_rgb_range; };

  static inline struct vc4_hdmi_encoder *

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

Converting the HDMI controller to a bridge seems like the preferred way to support an YUV output, so let's do this.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_crtc.c | 37 ++++++----- drivers/gpu/drm/vc4/vc4_drv.c | 15 +++-- drivers/gpu/drm/vc4/vc4_drv.h | 27 +++++--- drivers/gpu/drm/vc4/vc4_hdmi.c | 111 +++++++++++++++++++++------------ drivers/gpu/drm/vc4/vc4_hdmi.h | 8 +++ 5 files changed, 131 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index e2607e1f2520..8c13d31827bc 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -283,14 +283,19 @@ static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc) drm_connector_list_iter_begin(crtc->dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter) { struct drm_encoder *encoder;

• struct vc4_encoder *vc4_encoder;
  

• struct drm_bridge *bridge;
  

• struct vc4_bridge *vc4_bridge;
  
  

  encoder = vc4_get_connector_encoder(connector); if (!encoder) continue;

• vc4_encoder = to_vc4_encoder(encoder);
  

• if (vc4_encoder->crtc == crtc) {
  

• bridge = drm_bridge_chain_get_first_bridge(encoder);
  

• if (!bridge)
  

• 	continue;
  

• vc4_bridge = to_vc4_bridge(bridge);
  

• if (vc4_bridge->crtc == crtc) {
  drm_connector_list_iter_end(&conn_iter);
  return encoder;
  

  }

@@ -429,7 +434,8 @@ static int vc4_crtc_disable(struct drm_crtc *crtc, unsigned int channel) { struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);

• struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);

• struct drm_bridge *bridge = drm_bridge_chain_get_first_bridge(encoder);
• struct vc4_bridge *vc4_bridge = to_vc4_bridge(bridge); struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); struct drm_device *dev = crtc->dev; int ret;

@@ -457,14 +463,14 @@ static int vc4_crtc_disable(struct drm_crtc *crtc, */ mdelay(20);

• if (vc4_encoder && vc4_encoder->post_crtc_disable)

• vc4_encoder->post_crtc_disable(encoder, state);
  

• if (vc4_bridge && vc4_bridge->post_crtc_disable)

• vc4_bridge->post_crtc_disable(bridge, state);
  
  

  vc4_crtc_pixelvalve_reset(crtc); vc4_hvs_stop_channel(dev, channel);

• if (vc4_encoder && vc4_encoder->post_crtc_powerdown)

• vc4_encoder->post_crtc_powerdown(encoder, state);
  

• if (vc4_bridge && vc4_bridge->post_crtc_powerdown)

• vc4_bridge->post_crtc_powerdown(bridge, state);
  
  

  return 0; }

@@ -529,7 +535,8 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);

• struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);

• struct drm_bridge *bridge = drm_bridge_chain_get_first_bridge(encoder);

• struct vc4_bridge *vc4_bridge = to_vc4_bridge(bridge);

  require_hvs_enabled(dev);

@@ -540,15 +547,15 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,

vc4_hvs_atomic_enable(crtc, state);

• if (vc4_encoder->pre_crtc_configure)

• vc4_encoder->pre_crtc_configure(encoder, state);
  

• if (vc4_bridge->pre_crtc_configure)

• vc4_bridge->pre_crtc_configure(bridge, state);
  
  

  vc4_crtc_config_pv(crtc);

  CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_EN);

• if (vc4_encoder->pre_crtc_enable)

• vc4_encoder->pre_crtc_enable(encoder, state);
  

• if (vc4_bridge->pre_crtc_enable)

• vc4_bridge->pre_crtc_enable(bridge, state);
  
  

  /* When feeding the transposer block the pixelvalve is unneeded and

  • should not be enabled.

@@ -556,8 +563,8 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, CRTC_WRITE(PV_V_CONTROL, CRTC_READ(PV_V_CONTROL) | PV_VCONTROL_VIDEN);

• if (vc4_encoder->post_crtc_enable)

• vc4_encoder->post_crtc_enable(encoder, state);
  

• if (vc4_bridge->post_crtc_enable)

• vc4_bridge->post_crtc_enable(bridge, state);
  

  }

  static enum drm_mode_status vc4_crtc_mode_valid(struct drm_crtc *crtc,

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index cd1fb75c66a7..cee54f3b64e9 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -217,20 +217,25 @@ static struct drm_crtc *vc4_drv_find_crtc(struct drm_device *drm, return NULL; }

-static void vc4_drv_set_encoder_data(struct drm_device *drm) +static void vc4_drv_set_bridge_data(struct drm_device *drm) { struct drm_encoder *encoder;

drm_for_each_encoder(encoder, drm) {

• struct vc4_encoder *vc4_encoder;
  

• struct vc4_bridge *vc4_bridge;
  

• struct drm_bridge *bridge;
  

  struct drm_crtc *crtc;

  crtc = vc4_drv_find_crtc(drm, encoder); if (WARN_ON(!crtc)) return;

• vc4_encoder = to_vc4_encoder(encoder);
  

• vc4_encoder->crtc = crtc;
  

• bridge = drm_bridge_chain_get_first_bridge(encoder);
  

• if (!bridge)
  

• 	continue;
  

• vc4_bridge = to_vc4_bridge(bridge);
  

• vc4_bridge->crtc = crtc;
  

  } }

@@ -296,7 +301,7 @@ static int vc4_drm_bind(struct device *dev) ret = component_bind_all(dev, drm); if (ret) return ret;

• vc4_drv_set_encoder_data(drm);

• vc4_drv_set_bridge_data(drm);

  ret = vc4_plane_create_additional_planes(drm); if (ret)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 1b569dcc2154..a5721ffc6529 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -10,6 +10,7 @@ #include <linux/uaccess.h>

#include <drm/drm_atomic.h> +#include <drm/drm_bridge.h> #include <drm/drm_debugfs.h> #include <drm/drm_device.h> #include <drm/drm_encoder.h> @@ -438,16 +439,8 @@ enum vc4_encoder_type {

struct vc4_encoder { struct drm_encoder base;

• struct drm_crtc *crtc; enum vc4_encoder_type type; u32 clock_select;

• void (*pre_crtc_configure)(struct drm_encoder *encoder, struct drm_atomic_state *state);

• void (*pre_crtc_enable)(struct drm_encoder *encoder, struct drm_atomic_state *state);

• void (*post_crtc_enable)(struct drm_encoder *encoder, struct drm_atomic_state *state);

• void (*post_crtc_disable)(struct drm_encoder *encoder, struct drm_atomic_state *state);

• void (*post_crtc_powerdown)(struct drm_encoder *encoder, struct drm_atomic_state *state); };

  static inline struct vc4_encoder *

@@ -456,6 +449,24 @@ to_vc4_encoder(struct drm_encoder *encoder) return container_of(encoder, struct vc4_encoder, base); }

+struct vc4_bridge {

• struct drm_bridge base;
• struct drm_crtc *crtc;
• void (*pre_crtc_configure)(struct drm_bridge *bridge, struct drm_atomic_state *state);
• void (*pre_crtc_enable)(struct drm_bridge *bridge, struct drm_atomic_state *state);
• void (*post_crtc_enable)(struct drm_bridge *bridge, struct drm_atomic_state *state);
• void (*post_crtc_disable)(struct drm_bridge *bridge, struct drm_atomic_state *state);
• void (*post_crtc_powerdown)(struct drm_bridge *bridge, struct drm_atomic_state *state);

+};

+static inline struct vc4_bridge * +to_vc4_bridge(struct drm_bridge *bridge) +{

• return container_of(bridge, struct vc4_bridge, base);

+}

• struct vc4_crtc_data { /* Bitmask of channels (FIFOs) of the HVS that the output can source

from */

unsigned int hvs_available_channels; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 8f0af246f18f..4ce0aea6ba17 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -454,10 +454,10 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) vc4_hdmi_set_audio_infoframe(encoder); }

-static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,

• 			       struct drm_atomic_state *state)
  

+static void vc4_hdmi_bridge_post_crtc_disable(struct drm_bridge *bridge,

• 			      struct drm_atomic_state *state)
  

  {

• struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge);

  HDMI_WRITE(HDMI_RAM_PACKET_CONFIG, 0);

@@ -468,10 +468,10 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder, HDMI_READ(HDMI_VID_CTL) | VC4_HD_VID_CTL_BLANKPIX); }

-static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder,

• 				 struct drm_atomic_state *state)
  

+static void vc4_hdmi_bridge_post_crtc_powerdown(struct drm_bridge *bridge,

• 				struct drm_atomic_state *state)
  

  {

• struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge); int ret;

  if (vc4_hdmi->variant->phy_disable)

@@ -489,10 +489,6 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, DRM_ERROR("Failed to release power domain: %d\n", ret); }

-static void vc4_hdmi_encoder_disable(struct drm_encoder *encoder) -{ -}

• static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, const struct drm_display_mode *mode) {

@@ -740,9 +736,10 @@ vc4_hdmi_encoder_get_connector_state(struct drm_encoder *encoder, return NULL; }

-static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder,

• 				struct drm_atomic_state *state)
  

+static void vc4_hdmi_bridge_pre_crtc_configure(struct drm_bridge *bridge,

• 			       struct drm_atomic_state *state)
  

  {
• struct drm_encoder *encoder = bridge->encoder; struct drm_connector_state *conn_state = vc4_hdmi_encoder_get_connector_state(encoder, state); struct vc4_hdmi_connector_state *vc4_conn_state =

@@ -836,9 +833,10 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, vc4_hdmi->variant->set_timings(vc4_hdmi, conn_state, mode); }

-static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder,

• 			     struct drm_atomic_state *state)
  

+static void vc4_hdmi_bridge_pre_crtc_enable(struct drm_bridge *bridge,

• 			    struct drm_atomic_state *state)
  

  {
• struct drm_encoder *encoder = bridge->encoder; struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

@@ -848,9 +846,10 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, HDMI_WRITE(HDMI_FIFO_CTL, VC4_HDMI_FIFO_CTL_MASTER_SLAVE_N); }

-static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,

• 			      struct drm_atomic_state *state)
  

+static void vc4_hdmi_bridge_post_crtc_enable(struct drm_bridge *bridge,

• 			     struct drm_atomic_state *state)
  

  {
• struct drm_encoder *encoder = bridge->encoder; struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);

@@ -907,20 +906,17 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, vc4_hdmi_recenter_fifo(vc4_hdmi); }

-static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder) -{ -}

• #define WIFI_2_4GHz_CH1_MIN_FREQ 2400000000ULL #define WIFI_2_4GHz_CH1_MAX_FREQ 2422000000ULL

-static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder,

• 			 struct drm_crtc_state *crtc_state,
  

• 			 struct drm_connector_state *conn_state)
  

+static int vc4_hdmi_bridge_atomic_check(struct drm_bridge *bridge,

• 			struct drm_bridge_state *bridge_state,
  

• 			struct drm_crtc_state *crtc_state,
  

• 			struct drm_connector_state *conn_state)
  

  { struct vc4_hdmi_connector_state *vc4_state = conn_state_to_vc4_hdmi_conn_state(conn_state); struct drm_display_mode *mode = &crtc_state->adjusted_mode;

• struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge); unsigned long long pixel_rate = mode->clock * 1000; unsigned long long tmds_rate;

@@ -963,10 +959,11 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, }

static enum drm_mode_status -vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder,

• 	    const struct drm_display_mode *mode)
  

+vc4_hdmi_bridge_mode_valid(struct drm_bridge *bridge,

• 	   const struct drm_display_info *info,
  

• 	   const struct drm_display_mode *mode)
  

  {

• struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge);

  if (vc4_hdmi->variant->unsupported_odd_h_timings && ((mode->hdisplay % 2) || (mode->hsync_start % 2) ||

@@ -979,13 +976,49 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder, return MODE_OK; }

-static const struct drm_encoder_helper_funcs vc4_hdmi_encoder_helper_funcs = {

• .atomic_check = vc4_hdmi_encoder_atomic_check,
• .mode_valid = vc4_hdmi_encoder_mode_valid,
• .disable = vc4_hdmi_encoder_disable,
• .enable = vc4_hdmi_encoder_enable,

+static int vc4_hdmi_bridge_attach(struct drm_bridge *bridge,

• 		  enum drm_bridge_attach_flags flags)
  

+{

• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge);
• if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)

• return 0;
  

• return vc4_hdmi_connector_init(bridge->dev, vc4_hdmi);

+}

+static const struct drm_bridge_funcs vc4_hdmi_bridge_funcs = {

• .attach = vc4_hdmi_bridge_attach,
• .atomic_check = vc4_hdmi_bridge_atomic_check,
• .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
• .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
• .atomic_reset = drm_atomic_helper_bridge_reset,
• .mode_valid = vc4_hdmi_bridge_mode_valid, };

+static int vc4_hdmi_bridge_init(struct drm_device *drm,

• 		struct vc4_hdmi *vc4_hdmi)
  

+{

• struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
• struct drm_bridge *bridge = &vc4_hdmi->bridge.base;
• struct device *dev = &vc4_hdmi->pdev->dev;
• int ret;
• bridge->funcs = &vc4_hdmi_bridge_funcs;
• bridge->of_node = dev->of_node;
• bridge->type = DRM_MODE_CONNECTOR_HDMIA;
• drm_bridge_add(bridge);
• ret = drm_bridge_attach(encoder, bridge, NULL, 0);
• if (ret) {

• drm_bridge_remove(bridge);
  

• return ret;
  

• }
• return 0;

+}

• static u32 vc4_hdmi_channel_map(struct vc4_hdmi *vc4_hdmi, u32 channel_mask) { int i;

@@ -1945,14 +1978,15 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) dev_set_drvdata(dev, vc4_hdmi); encoder = &vc4_hdmi->encoder.base.base; vc4_hdmi->encoder.base.type = variant->encoder_type;

• vc4_hdmi->encoder.base.pre_crtc_configure = vc4_hdmi_encoder_pre_crtc_configure;
• vc4_hdmi->encoder.base.pre_crtc_enable = vc4_hdmi_encoder_pre_crtc_enable;
• vc4_hdmi->encoder.base.post_crtc_enable = vc4_hdmi_encoder_post_crtc_enable;
• vc4_hdmi->encoder.base.post_crtc_disable = vc4_hdmi_encoder_post_crtc_disable;
• vc4_hdmi->encoder.base.post_crtc_powerdown = vc4_hdmi_encoder_post_crtc_powerdown; vc4_hdmi->pdev = pdev; vc4_hdmi->variant = variant;

• vc4_hdmi->bridge.pre_crtc_configure = vc4_hdmi_bridge_pre_crtc_configure;
• vc4_hdmi->bridge.pre_crtc_enable = vc4_hdmi_bridge_pre_crtc_enable;
• vc4_hdmi->bridge.post_crtc_enable = vc4_hdmi_bridge_post_crtc_enable;
• vc4_hdmi->bridge.post_crtc_disable = vc4_hdmi_bridge_post_crtc_disable;
• vc4_hdmi->bridge.post_crtc_powerdown = vc4_hdmi_bridge_post_crtc_powerdown;
• ret = variant->init_resources(vc4_hdmi); if (ret) return ret;

@@ -1996,9 +2030,8 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) pm_runtime_enable(dev);

drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);

• drm_encoder_helper_add(encoder, &vc4_hdmi_encoder_helper_funcs);

• ret = vc4_hdmi_connector_init(drm, vc4_hdmi);

• ret = vc4_hdmi_bridge_init(drm, vc4_hdmi); if (ret) goto err_destroy_encoder;

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 8e42f9e7b3e2..d03c849d6ea0 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -1,6 +1,7 @@ #ifndef _VC4_HDMI_H_ #define _VC4_HDMI_H_

+#include <drm/drm_bridge.h> #include <drm/drm_connector.h> #include <media/cec.h> #include <sound/dmaengine_pcm.h> @@ -125,6 +126,7 @@ struct vc4_hdmi {

struct vc4_hdmi_encoder encoder; struct drm_connector connector;

• struct vc4_bridge bridge;

  struct i2c_adapter *ddc; void __iomem *hdmicore_regs;

@@ -171,6 +173,12 @@ struct vc4_hdmi { struct debugfs_regset32 hd_regset; };

+static inline struct vc4_hdmi * +bridge_to_vc4_hdmi(struct drm_bridge *bridge) +{

• return container_of(bridge, struct vc4_hdmi, bridge.base);

+}

• static inline struct vc4_hdmi * connector_to_vc4_hdmi(struct drm_connector *connector) {

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

On the BCM2711, the HDMI_VEC_INTERFACE_XBAR register configuration depends on whether we're using an RGB or YUV output. Let's move that configuration to the CSC setup.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 4ce0aea6ba17..9ba555d24187 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -530,6 +530,8 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, { u32 csc_ctl;

• HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, 0x354021);

• csc_ctl = 0x07; /* RGB_CONVERT_MODE = custom matrix, || USE_RGB_TO_YCBCR */

  if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode)) {

@@ -636,7 +638,6 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, bool gcp_en; u32 reg;

• HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, 0x354021); HDMI_WRITE(HDMI_HORZA, (vsync_pos ? VC5_HDMI_HORZA_VPOS : 0) | (hsync_pos ? VC5_HDMI_HORZA_HPOS : 0) |

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

On BCM2711, the HDMI_CSC_CTL register value has been hardcoded to an opaque value. Let's replace it with properly defined values.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 5 ++--- drivers/gpu/drm/vc4/vc4_regs.h | 3 +++ 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 9ba555d24187..b0e0cb533944 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -528,12 +528,11 @@ static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, const struct drm_display_mode *mode) {

• u32 csc_ctl;

• u32 csc_ctl = VC5_MT_CP_CSC_CTL_ENABLE | VC4_SET_FIELD(VC4_HD_CSC_CTL_MODE_CUSTOM,

• 					       VC5_MT_CP_CSC_CTL_MODE);
  
  

  HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, 0x354021);

• csc_ctl = 0x07; /* RGB_CONVERT_MODE = custom matrix, || USE_RGB_TO_YCBCR */
• if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode)) { /* CEA VICs other than #1 requre limited range RGB
  • output unless overridden by an AVI infoframe.

diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h index be2c32a519b3..9d7c034c8b4f 100644 --- a/drivers/gpu/drm/vc4/vc4_regs.h +++ b/drivers/gpu/drm/vc4/vc4_regs.h @@ -744,6 +744,9 @@ # define VC4_HD_CSC_CTL_RGB2YCC BIT(1) # define VC4_HD_CSC_CTL_ENABLE BIT(0)

+# define VC5_MT_CP_CSC_CTL_ENABLE BIT(2) +# define VC5_MT_CP_CSC_CTL_MODE_MASK VC4_MASK(1, 0)

• # define VC4_DVP_HT_CLOCK_STOP_PIXEL BIT(1)

  /* HVS display list information. */

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

The current CSC setup code for the BCM2711 uses a sequence of register writes to configure the CSC depending on whether we output using a full or limited range.

However, with the upcoming introduction of the YUV output, we're going to add new matrices to perform the conversions, so we should switch to something a bit more flexible that takes the matrix as an argument and programs the CSC accordingly.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 77 +++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index b0e0cb533944..9614de7303b8 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -525,6 +525,50 @@ static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_CSC_CTL, csc_ctl); }

+/*

• • If we need to output Full Range RGB, then use the unity matrix
• • [ 1 0 0 0]
• • [ 0 1 0 0]
• • [ 0 0 1 0]
• • Matrix is signed 2p13 fixed point, with signed 9p6 offsets
• */

+static const u16 vc5_hdmi_csc_full_rgb_unity[3][4] = {

• { 0x2000, 0x0000, 0x0000, 0x0000 },
• { 0x0000, 0x2000, 0x0000, 0x0000 },
• { 0x0000, 0x0000, 0x2000, 0x0000 },

+};

+/*

• • CEA VICs other than #1 require limited range RGB output unless
• • overridden by an AVI infoframe. Apply a colorspace conversion to
• • squash 0-255 down to 16-235. The matrix here is:
• • [ 0.8594 0 0 16]
• • [ 0 0.8594 0 16]
• • [ 0 0 0.8594 16]
• • Matrix is signed 2p13 fixed point, with signed 9p6 offsets
• */

+static const u16 vc5_hdmi_csc_full_rgb_to_limited_rgb[3][4] = {

• { 0x1b80, 0x0000, 0x0000, 0x0400 },
• { 0x0000, 0x1b80, 0x0000, 0x0400 },
• { 0x0000, 0x0000, 0x1b80, 0x0400 },

+};

+static void vc5_hdmi_set_csc_coeffs(struct vc4_hdmi *vc4_hdmi,

• 		    const u16 coeffs[3][4])
  

+{

• HDMI_WRITE(HDMI_CSC_12_11, (coeffs[0][1] << 16) | coeffs[0][0]);
• HDMI_WRITE(HDMI_CSC_14_13, (coeffs[0][3] << 16) | coeffs[0][2]);
• HDMI_WRITE(HDMI_CSC_22_21, (coeffs[1][1] << 16) | coeffs[1][0]);
• HDMI_WRITE(HDMI_CSC_24_23, (coeffs[1][3] << 16) | coeffs[1][2]);
• HDMI_WRITE(HDMI_CSC_32_31, (coeffs[2][1] << 16) | coeffs[2][0]);
• HDMI_WRITE(HDMI_CSC_34_33, (coeffs[2][3] << 16) | coeffs[2][2]);

+}

• static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, const struct drm_display_mode *mode) {

@@ -533,35 +577,10 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,

HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, 0x354021);

• if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode)) {

• /* CEA VICs other than #1 requre limited range RGB
  

•  * output unless overridden by an AVI infoframe.
  

•  * Apply a colorspace conversion to squash 0-255 down
  

•  * to 16-235.  The matrix here is:
  

•  *
  

•  * [ 0.8594 0      0      16]
  

•  * [ 0      0.8594 0      16]
  

•  * [ 0      0      0.8594 16]
  

•  * [ 0      0      0       1]
  

•  * Matrix is signed 2p13 fixed point, with signed 9p6 offsets
  

•  */
  

• HDMI_WRITE(HDMI_CSC_12_11, (0x0000 << 16) | 0x1b80);
  

• HDMI_WRITE(HDMI_CSC_14_13, (0x0400 << 16) | 0x0000);
  

• HDMI_WRITE(HDMI_CSC_22_21, (0x1b80 << 16) | 0x0000);
  

• HDMI_WRITE(HDMI_CSC_24_23, (0x0400 << 16) | 0x0000);
  

• HDMI_WRITE(HDMI_CSC_32_31, (0x0000 << 16) | 0x0000);
  

• HDMI_WRITE(HDMI_CSC_34_33, (0x0400 << 16) | 0x1b80);
  

• } else {

• /* Still use the matrix for full range, but make it unity.
  

•  * Matrix is signed 2p13 fixed point, with signed 9p6 offsets
  

•  */
  

• HDMI_WRITE(HDMI_CSC_12_11, (0x0000 << 16) | 0x2000);
  

• HDMI_WRITE(HDMI_CSC_14_13, (0x0000 << 16) | 0x0000);
  

• HDMI_WRITE(HDMI_CSC_22_21, (0x2000 << 16) | 0x0000);
  

• HDMI_WRITE(HDMI_CSC_24_23, (0x0000 << 16) | 0x0000);
  

• HDMI_WRITE(HDMI_CSC_32_31, (0x0000 << 16) | 0x0000);
  

• HDMI_WRITE(HDMI_CSC_34_33, (0x0000 << 16) | 0x2000);
  

• }

• if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode))

• vc5_hdmi_set_csc_coeffs(vc4_hdmi, &vc5_hdmi_csc_full_rgb_unity);
  

• else

• vc5_hdmi_set_csc_coeffs(vc4_hdmi, &vc5_hdmi_csc_full_rgb_to_limited_rgb);
  
  

  HDMI_WRITE(HDMI_CSC_CTL, csc_ctl); }

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

In order to support the YUV output, we'll need the atomic state to know what is the state of the associated property in the CSC setup callback.

Let's change the prototype of that callback to allow us to access it.

Signed-off-by: Maxime Ripard maxime@cerno.tech

drivers/gpu/drm/vc4/vc4_hdmi.c | 4 +++- drivers/gpu/drm/vc4/vc4_hdmi.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 9614de7303b8..56b5654c820f 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -490,6 +490,7 @@ static void vc4_hdmi_bridge_post_crtc_powerdown(struct drm_bridge *bridge, }

static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,

• 	       struct drm_atomic_state *state,
  

With the line length updated to 100 characters, this might just fit onto the previous line. Anyway

Acked-by: Thomas Zimmermann tzimmermann@suse.de

	       const struct drm_display_mode *mode)

{ u32 csc_ctl; @@ -570,6 +571,7 @@ static void vc5_hdmi_set_csc_coeffs(struct vc4_hdmi

*vc4_hdmi,

}

static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,

• 	       struct drm_atomic_state *state,
         const struct drm_display_mode *mode)
  

  { u32 csc_ctl = VC5_MT_CP_CSC_CTL_ENABLE | VC4_SET_FIELD(VC4_HD_CSC_CTL_MODE_CUSTOM,

@@ -860,7 +862,7 @@ static void vc4_hdmi_bridge_pre_crtc_enable(struct drm_bridge *bridge, struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

if (vc4_hdmi->variant->csc_setup)

• vc4_hdmi->variant->csc_setup(vc4_hdmi, mode);
  

• vc4_hdmi->variant->csc_setup(vc4_hdmi, state, mode);
  
  

  HDMI_WRITE(HDMI_FIFO_CTL, VC4_HDMI_FIFO_CTL_MASTER_SLAVE_N); }

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index d03c849d6ea0..cf5e58a08eb4 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -78,6 +78,7 @@ struct vc4_hdmi_variant {

/* Callback to enable / disable the CSC */ void (*csc_setup)(struct vc4_hdmi *vc4_hdmi,

• 	  struct drm_atomic_state *state,
    const struct drm_display_mode *mode);
  
  

  /* Callback to configure the video timings in the HDMI block */

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

In order to support a YUV output, we're going to need to have access to the bridge state in the vc4_hdmi_set_avi_infoframe function. Since we also need the connector state in that function, let's pass the full atomic state.

While we're at it, since all those functions actually need the vc4_hdmi structure, let's pass it instead of the drm_encoder.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 56b5654c820f..83e44cf44d65 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -330,10 +330,10 @@ static int vc4_hdmi_stop_packet(struct drm_encoder *encoder, BIT(packet_id)), 100); }

-static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, +static void vc4_hdmi_write_infoframe(struct vc4_hdmi *vc4_hdmi, union hdmi_infoframe *frame) {

• struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

• struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base; u32 packet_id = frame->any.type - 0x80; const struct vc4_hdmi_register *ram_packet_start = &vc4_hdmi->variant->registers[HDMI_RAM_PACKET_START];

@@ -381,11 +381,13 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, DRM_ERROR("Failed to wait for infoframe to start: %d\n", ret); }

-static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder) +static void vc4_hdmi_set_avi_infoframe(struct vc4_hdmi *vc4_hdmi,

• 		       struct drm_atomic_state *state)
  

  {

• struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

• struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base; struct drm_connector *connector = &vc4_hdmi->connector;

• struct drm_connector_state *cstate = connector->state;

• struct drm_connector_state *cstate =

• drm_atomic_get_new_connector_state(state, connector);
  

  struct drm_crtc *crtc = encoder->crtc; const struct drm_display_mode *mode = &crtc->state->adjusted_mode; union hdmi_infoframe frame;

@@ -406,10 +408,10 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)

drm_hdmi_avi_infoframe_bars(&frame.avi, cstate);

• vc4_hdmi_write_infoframe(encoder, &frame);

• vc4_hdmi_write_infoframe(vc4_hdmi, &frame); }

-static void vc4_hdmi_set_spd_infoframe(struct drm_encoder *encoder) +static void vc4_hdmi_set_spd_infoframe(struct vc4_hdmi *vc4_hdmi) { union hdmi_infoframe frame; int ret; @@ -422,12 +424,11 @@ static void vc4_hdmi_set_spd_infoframe(struct drm_encoder *encoder)

frame.spd.sdi = HDMI_SPD_SDI_PC;

• vc4_hdmi_write_infoframe(encoder, &frame);

• vc4_hdmi_write_infoframe(vc4_hdmi, &frame); }

-static void vc4_hdmi_set_audio_infoframe(struct drm_encoder *encoder) +static void vc4_hdmi_set_audio_infoframe(struct vc4_hdmi *vc4_hdmi) {

• struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); union hdmi_infoframe frame;

  hdmi_audio_infoframe_init(&frame.audio);

@@ -437,21 +438,19 @@ static void vc4_hdmi_set_audio_infoframe(struct drm_encoder *encoder) frame.audio.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM; frame.audio.channels = vc4_hdmi->audio.channels;

• vc4_hdmi_write_infoframe(encoder, &frame);

• vc4_hdmi_write_infoframe(vc4_hdmi, &frame); }

-static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) +static void vc4_hdmi_set_infoframes(struct vc4_hdmi *vc4_hdmi, struct drm_atomic_state *state) {

• struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
• vc4_hdmi_set_avi_infoframe(encoder);
• vc4_hdmi_set_spd_infoframe(encoder);

• vc4_hdmi_set_avi_infoframe(vc4_hdmi, state);
• vc4_hdmi_set_spd_infoframe(vc4_hdmi); /*
  • If audio was streaming, then we need to reenabled the audio
  • infoframe here during encoder_enable.
  */ if (vc4_hdmi->audio.streaming)

• vc4_hdmi_set_audio_infoframe(encoder);
  

• vc4_hdmi_set_audio_infoframe(vc4_hdmi);
  

  }

  static void vc4_hdmi_bridge_post_crtc_disable(struct drm_bridge *bridge,

@@ -921,7 +920,7 @@ static void vc4_hdmi_bridge_post_crtc_enable(struct

drm_bridge *bridge,

HDMI_WRITE(HDMI_RAM_PACKET_CONFIG,
	   VC4_HDMI_RAM_PACKET_ENABLE);

• vc4_hdmi_set_infoframes(encoder);
  

• vc4_hdmi_set_infoframes(vc4_hdmi, state);
  

  }

  vc4_hdmi_recenter_fifo(vc4_hdmi);

@@ -1184,7 +1183,6 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);

• struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base; struct device *dev = &vc4_hdmi->pdev->dev; u32 audio_packet_config, channel_mask; u32 channel_map;

@@ -1244,7 +1242,7 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream, HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config); vc4_hdmi_set_n_cts(vc4_hdmi);

• vc4_hdmi_set_audio_infoframe(encoder);

• vc4_hdmi_set_audio_infoframe(vc4_hdmi);

  return 0; }

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

The HDMI controllers in the BCM2711 support YUV444 and YUV420 outputs, let's add support for it.

Signed-off-by: Maxime Ripard maxime@cerno.tech

drivers/gpu/drm/vc4/vc4_hdmi.c | 73 +++++++++++++++++++++++++++-- drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 6 +++ drivers/gpu/drm/vc4/vc4_regs.h | 16 +++++++ 3 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 83e44cf44d65..407b468dab67 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -33,6 +33,7 @@

#include <drm/drm_atomic_helper.h> #include <drm/drm_edid.h> +#include <drm/drm_hdmi.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> #include <linux/clk.h> @@ -388,6 +389,8 @@ static void vc4_hdmi_set_avi_infoframe(struct vc4_hdmi *vc4_hdmi, struct drm_connector *connector = &vc4_hdmi->connector; struct drm_connector_state *cstate = drm_atomic_get_new_connector_state(state, connector);

• struct drm_bridge_state *bstate =

• drm_atomic_get_new_bridge_state(state, &vc4_hdmi->bridge.base);
  

  struct drm_crtc *crtc = encoder->crtc; const struct drm_display_mode *mode = &crtc->state->adjusted_mode; union hdmi_infoframe frame;

@@ -407,6 +410,7 @@ static void vc4_hdmi_set_avi_infoframe(struct vc4_hdmi *vc4_hdmi, HDMI_QUANTIZATION_RANGE_LIMITED);

drm_hdmi_avi_infoframe_bars(&frame.avi, cstate);

• drm_hdmi_avi_infoframe_output_colorspace(&frame.avi, &bstate->output_bus_cfg);

  vc4_hdmi_write_infoframe(vc4_hdmi, &frame); }

@@ -558,6 +562,38 @@ static const u16 vc5_hdmi_csc_full_rgb_to_limited_rgb[3][4] = { { 0x0000, 0x0000, 0x1b80, 0x0400 }, };

+/*

• • Conversion between Full Range RGB and Full Range YUV422 using the
• • BT.709 Colorspace
• • [ 0.212639 0.715169 0.072192 0 ]
• • [ -0.117208 -0.394207 0.511416 128 ]
• • [ 0.511416 -0.464524 -0.046891 128 ]
• • Matrix is signed 2p13 fixed point, with signed 9p6 offsets
• */

+static const u16 vc5_hdmi_csc_full_rgb_to_full_yuv422_bt709[3][4] = {

• { 0x06ce, 0x16e3, 0x024f, 0x0000 },
• { 0xfc41, 0xf364, 0x105e, 0x2000 },
• { 0x105e, 0xf124, 0xfe81, 0x2000 },

+};

+/*

• • Conversion between Full Range RGB and Full Range YUV444 using the
• • BT.709 Colorspace
• • [ -0.117208 -0.394207 0.511416 128 ]
• • [ 0.511416 -0.464524 -0.046891 128 ]
• • [ 0.212639 0.715169 0.072192 0 ]
• • Matrix is signed 2p13 fixed point, with signed 9p6 offsets
• */

+static const u16 vc5_hdmi_csc_full_rgb_to_full_yuv444_bt709[3][4] = {

• { 0xfc41, 0xf364, 0x105e, 0x2000 },
• { 0x105e, 0xf124, 0xfe81, 0x2000 },
• { 0x06ce, 0x16e3, 0x024f, 0x0000 },

+};

• static void vc5_hdmi_set_csc_coeffs(struct vc4_hdmi *vc4_hdmi, const u16 coeffs[3][4]) {

@@ -573,16 +609,42 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, struct drm_atomic_state *state, const struct drm_display_mode *mode) {

• struct drm_bridge *bridge = &vc4_hdmi->bridge.base;
• struct drm_bridge_state *bridge_state =

• drm_atomic_get_new_bridge_state(state, bridge);
  

• u32 if_cfg = 0;
• u32 if_xbar = 0x543210;
• u32 csc_chan_ctl = 0; u32 csc_ctl = VC5_MT_CP_CSC_CTL_ENABLE | VC4_SET_FIELD(VC4_HD_CSC_CTL_MODE_CUSTOM, VC5_MT_CP_CSC_CTL_MODE);

• HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, 0x354021);

• if (drm_hdmi_bus_fmt_is_yuv422(bridge_state->output_bus_cfg.format)) {

• csc_ctl |= VC4_SET_FIELD(VC5_MT_CP_CSC_CTL_FILTER_MODE_444_TO_422_STANDARD,
  

• 			 VC5_MT_CP_CSC_CTL_FILTER_MODE_444_TO_422) |
  

• 	VC5_MT_CP_CSC_CTL_USE_444_TO_422 |
  

• 	VC5_MT_CP_CSC_CTL_USE_RNG_SUPPRESSION;
  

Indention appears to be wrong.

• if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode))

• vc5_hdmi_set_csc_coeffs(vc4_hdmi, &vc5_hdmi_csc_full_rgb_unity);
  

• else

• vc5_hdmi_set_csc_coeffs(vc4_hdmi, &vc5_hdmi_csc_full_rgb_to_limited_rgb);
  

• csc_chan_ctl |= VC4_SET_FIELD(VC5_MT_CP_CHANNEL_CTL_OUTPUT_REMAP_LEGACY_STYLE,
  

• 			      VC5_MT_CP_CHANNEL_CTL_OUTPUT_REMAP);
  
  

• if_cfg |= VC4_SET_FIELD(VC5_DVP_HT_VEC_INTERFACE_CFG_SEL_422_FORMAT_422_LEGACY,
  

• 			VC5_DVP_HT_VEC_INTERFACE_CFG_SEL_422);
  

• vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_full_yuv422_bt709);
  

• } else if (drm_hdmi_bus_fmt_is_yuv444(bridge_state->output_bus_cfg.format)) {

• vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_full_yuv444_bt709);
  

• } else if (drm_hdmi_bus_fmt_is_rgb(bridge_state->output_bus_cfg.format)) {

• if_xbar = 0x354021;
  

• if (vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode))
  

• 	vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_unity);
  

• else
  

• 	vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_limited_rgb);
  

• }

• HDMI_WRITE(HDMI_VEC_INTERFACE_CFG, if_cfg);

• HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, if_xbar);

• HDMI_WRITE(HDMI_CSC_CHANNEL_CTL, csc_chan_ctl); HDMI_WRITE(HDMI_CSC_CTL, csc_ctl); }

@@ -1012,6 +1074,7 @@ static const struct drm_bridge_funcs vc4_hdmi_bridge_funcs = { .atomic_check = vc4_hdmi_bridge_atomic_check, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,

• .atomic_get_output_bus_fmts = drm_atomic_helper_bridge_hdmi_get_output_bus_fmts, .atomic_reset = drm_atomic_helper_bridge_reset, .mode_valid = vc4_hdmi_bridge_mode_valid, };

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index e1b58eac766f..d03b9ad72412 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -52,6 +52,7 @@ enum vc4_hdmi_field { HDMI_CSC_24_23, HDMI_CSC_32_31, HDMI_CSC_34_33,

• HDMI_CSC_CHANNEL_CTL, HDMI_CSC_CTL,

  /*

@@ -116,6 +117,7 @@ enum vc4_hdmi_field { HDMI_TX_PHY_POWERDOWN_CTL, HDMI_TX_PHY_RESET_CTL, HDMI_TX_PHY_TMDS_CLK_WORD_SEL,

• HDMI_VEC_INTERFACE_CFG, HDMI_VEC_INTERFACE_XBAR, HDMI_VERTA0, HDMI_VERTA1,

@@ -240,6 +242,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = { VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8),

VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc),

• VC5_DVP_REG(HDMI_VEC_INTERFACE_CFG, 0x0ec), VC5_DVP_REG(HDMI_VEC_INTERFACE_XBAR, 0x0f0),

  VC5_PHY_REG(HDMI_TX_PHY_RESET_CTL, 0x000),

@@ -285,6 +288,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = { VC5_CSC_REG(HDMI_CSC_24_23, 0x010), VC5_CSC_REG(HDMI_CSC_32_31, 0x014), VC5_CSC_REG(HDMI_CSC_34_33, 0x018),

• VC5_CSC_REG(HDMI_CSC_CHANNEL_CTL, 0x02c), };

  static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = {

@@ -319,6 +323,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = { VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8),

VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc),

• VC5_DVP_REG(HDMI_VEC_INTERFACE_CFG, 0x0ec), VC5_DVP_REG(HDMI_VEC_INTERFACE_XBAR, 0x0f0),

  VC5_PHY_REG(HDMI_TX_PHY_RESET_CTL, 0x000),

@@ -364,6 +369,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = { VC5_CSC_REG(HDMI_CSC_24_23, 0x010), VC5_CSC_REG(HDMI_CSC_32_31, 0x014), VC5_CSC_REG(HDMI_CSC_34_33, 0x018),

• VC5_CSC_REG(HDMI_CSC_CHANNEL_CTL, 0x02c), };

  static inline

diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h index 9d7c034c8b4f..ff5c5faa019e 100644 --- a/drivers/gpu/drm/vc4/vc4_regs.h +++ b/drivers/gpu/drm/vc4/vc4_regs.h @@ -744,11 +744,27 @@ # define VC4_HD_CSC_CTL_RGB2YCC BIT(1) # define VC4_HD_CSC_CTL_ENABLE BIT(0)

+# define VC5_MT_CP_CSC_CTL_USE_444_TO_422 BIT(6) +# define VC5_MT_CP_CSC_CTL_FILTER_MODE_444_TO_422_MASK \

• 				VC4_MASK(5, 4)
  

+# define VC5_MT_CP_CSC_CTL_FILTER_MODE_444_TO_422_STANDARD \

• 				3
  

These defines should fit onto a single 100-character line. (?) Here and below.

Acked-by: Thomas Zimmermann tzimmermann@suse.de

+# define VC5_MT_CP_CSC_CTL_USE_RNG_SUPPRESSION BIT(3) # define VC5_MT_CP_CSC_CTL_ENABLE BIT(2) # define VC5_MT_CP_CSC_CTL_MODE_MASK VC4_MASK(1, 0)

+# define VC5_MT_CP_CHANNEL_CTL_OUTPUT_REMAP_MASK \

• 				VC4_MASK(7, 6)
  

+# define VC5_MT_CP_CHANNEL_CTL_OUTPUT_REMAP_LEGACY_STYLE \

• 				2
  

• # define VC4_DVP_HT_CLOCK_STOP_PIXEL BIT(1)

+# define VC5_DVP_HT_VEC_INTERFACE_CFG_SEL_422_MASK \

• 				VC4_MASK(3, 2)
  

+# define VC5_DVP_HT_VEC_INTERFACE_CFG_SEL_422_FORMAT_422_LEGACY \

• 				2
  

• /* HVS display list information. */ #define HVS_BOOTLOADER_DLIST_END 32

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

In order to implement a fallback mechanism to YUV422 when the pixel rate is too high, let's move the pixel rate computation to a function of its own that will be shared across two functions.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 36 +++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 407b468dab67..c4f91d39d91c 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -991,22 +991,16 @@ static void vc4_hdmi_bridge_post_crtc_enable(struct drm_bridge *bridge, #define WIFI_2_4GHz_CH1_MIN_FREQ 2400000000ULL #define WIFI_2_4GHz_CH1_MAX_FREQ 2422000000ULL

-static int vc4_hdmi_bridge_atomic_check(struct drm_bridge *bridge,

• 			struct drm_bridge_state *bridge_state,
  

• 			struct drm_crtc_state *crtc_state,
  

• 			struct drm_connector_state *conn_state)
  

+static unsigned long vc4_hdmi_calc_pixel_rate(struct drm_bridge *bridge,

• 			      struct drm_bridge_state *bridge_state,
  

• 			      struct drm_crtc_state *crtc_state,
  

• 			      struct drm_connector_state *conn_state)
  

  {

• struct vc4_hdmi_connector_state *vc4_state = conn_state_to_vc4_hdmi_conn_state(conn_state); struct drm_display_mode *mode = &crtc_state->adjusted_mode;
• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge); unsigned long long pixel_rate = mode->clock * 1000;

• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge); unsigned long long tmds_rate;

• if (vc4_hdmi->variant->unsupported_odd_h_timings &&

•    ((mode->hdisplay % 2) || (mode->hsync_start % 2) ||
  

•     (mode->hsync_end % 2) || (mode->htotal % 2)))
  

• return -EINVAL;
  

• /*
  • The 1440p@60 pixel rate is in the same range than the first
  • WiFi channel (between 2.4GHz and 2.422GHz with 22MHz

@@ -1032,6 +1026,26 @@ static int vc4_hdmi_bridge_atomic_check(struct drm_bridge *bridge, if (mode->flags & DRM_MODE_FLAG_DBLCLK) pixel_rate = pixel_rate * 2;

• return pixel_rate;

+}

+static int vc4_hdmi_bridge_atomic_check(struct drm_bridge *bridge,

• 			struct drm_bridge_state *bridge_state,
  

• 			struct drm_crtc_state *crtc_state,
  

• 			struct drm_connector_state *conn_state)
  

+{

• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge);
• struct vc4_hdmi_connector_state *vc4_state =

• conn_state_to_vc4_hdmi_conn_state(conn_state);
  

• struct drm_display_mode *mode = &crtc_state->adjusted_mode;
• unsigned long long pixel_rate;
• if (vc4_hdmi->variant->unsupported_odd_h_timings &&

•    ((mode->hdisplay % 2) || (mode->hsync_start % 2) ||
  

•     (mode->hsync_end % 2) || (mode->htotal % 2)))
  

• return -EINVAL;
  

• pixel_rate = vc4_hdmi_calc_pixel_rate(bridge, bridge_state, crtc_state, conn_state); if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) return -EINVAL;

Am 17.03.21 um 16:43 schrieb Maxime Ripard:

When using the modes that need the highest pixel rate we support (such as 4k at 60Hz), using a 10 or 12 bpc output will put us over the limit of what we can achieve.

In such a case, let's force our output to be YUV422 so that we can go back down under the required clock rate.

Signed-off-by: Maxime Ripard maxime@cerno.tech

Acked-by: Thomas Zimmermann tzimmermann@suse.de

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 37 +++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index c4f91d39d91c..12eda1e76338 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1029,6 +1029,41 @@ static unsigned long vc4_hdmi_calc_pixel_rate(struct drm_bridge *bridge, return pixel_rate; }

+static u32 *vc4_hdmi_bridge_atomic_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)
  

+{

• struct vc4_hdmi *vc4_hdmi = bridge_to_vc4_hdmi(bridge);
• unsigned long long pixel_rate = vc4_hdmi_calc_pixel_rate(bridge,

• 						 bridge_state,
  

• 						 crtc_state,
  

• 						 conn_state);
  

• /*

• * If our pixel rate is too fast, force YUV422 and hope it works
  

• */
  

• if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) {

• u32 *output_fmts;
  

• output_fmts = kzalloc(sizeof(*output_fmts), GFP_KERNEL);
  

• if (!output_fmts)
  

• 	return NULL;
  

• *output_fmts = MEDIA_BUS_FMT_UYVY8_1X16;
  

• *num_output_fmts = 1;
  

• return output_fmts;
  

• }
• return drm_atomic_helper_bridge_hdmi_get_output_bus_fmts(bridge,

• 						 bridge_state,
  

• 						 crtc_state,
  

• 						 conn_state,
  

• 						 num_output_fmts);
  

+}

• static int vc4_hdmi_bridge_atomic_check(struct drm_bridge *bridge, struct drm_bridge_state *bridge_state, struct drm_crtc_state *crtc_state,

@@ -1088,7 +1123,7 @@ static const struct drm_bridge_funcs vc4_hdmi_bridge_funcs = { .atomic_check = vc4_hdmi_bridge_atomic_check, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,

• .atomic_get_output_bus_fmts = drm_atomic_helper_bridge_hdmi_get_output_bus_fmts,

• .atomic_get_output_bus_fmts = vc4_hdmi_bridge_atomic_get_output_bus_fmts, .atomic_reset = drm_atomic_helper_bridge_reset, .mode_valid = vc4_hdmi_bridge_mode_valid, };