[PATCH v5 0/6] drm/vc4: hdmi: Yet Another Approach to HDMI YUV output

[PATCH v5 0/6] drm/vc4: hdmi: Yet Another Approach to HDMI YUV output

[Maxime Ripard]

Hi,

This is another attempt at supporting the HDMI YUV output in the vc4 HDMI
driver.

This is a follow-up of
https://lore.kernel.org/dri-devel/20210317154352.732095-1-maxime@cerno.tech/

And the discussions that occured recently on the mailing lists and IRC about
this.

The series mentioned above had multiple issues, the main one being that it was
a bit too much complicated for what we wanted to achieve. This series is taking
a much simpler approach with an ad-hoc solution.

I think some parts of it could still be moved to KMS helpers (notably, the
output format enum, and the helper to set the infoframe for it) and structures
(the output format stored in drm_connector_state). This would also interact
nicely with the work done here:

https://lore.kernel.org/dri-devel/20211118103814.524670-1-maxime@cerno.tech/

This can come as a second step though.

The other issues with the first attempt was that nothing was reported to
userspace about the decision we made about the format, and that this decision
was essentially policy, without any way for the userspace to influence it.

Those two points however are being worked on by Werner in a cross-driver
effort:

https://lore.kernel.org/dri-devel/e452775c-5b95-bbfd-e818-f1480f556336@tuxedocomputers.com/

Since it's a KMS decision, I don't think we should hold off any driver as long
as it's consistent with what the other drivers are doing.

Let me know what you think,
Maxime

---

Changes from v4:
  - Fix a clock calculation
  - Rebased on latest drm-misc-next tag

Changes from v3:
  - Rebased on latest next
  - Fixed build error

Changes from v2:
  - Rename the output format enum
  - Split the edid_hdmi_dc_modes in two for RGB444 and YUV444
  - Remove color_formats modifications from _parse_deep_color entirely
  - Fixed comment formatting
  - Fixed mode_valid that would always return true
  - Fixed max_tmds_clock handling

Changes from v1:
  - Fixed an EDID parsing error for YUV422
  - Fixed the scrambling setup when using a bpc > 8
  - Added some logging
  - Fixed some build-bot warnings
  - Fixed a number of HDMI specifications and EDID issues
  - Try to max out the bpc every time

Maxime Ripard (6):
  drm/vc4: hdmi: Move clock validation to its own function
  drm/vc4: hdmi: Move clock calculation into its own function
  drm/vc4: hdmi: Take the sink maximum TMDS clock into account
  drm/vc4: hdmi: Take bpp into account for the scrambler
  drm/vc4: hdmi: Always try to have the highest bpc
  drm/vc4: hdmi: Support HDMI YUV output

 drivers/gpu/drm/vc4/vc4_hdmi.c      | 401 +++++++++++++++++++++++++---
 drivers/gpu/drm/vc4/vc4_hdmi.h      |  21 ++
 drivers/gpu/drm/vc4/vc4_hdmi_regs.h |   6 +
 drivers/gpu/drm/vc4/vc4_regs.h      |  16 ++
 4 files changed, 409 insertions(+), 35 deletions(-)

-- 
2.34.1

[PATCH v5 1/6] drm/vc4: hdmi: Move clock validation to its own function

[Maxime Ripard]

Our code is doing the same clock rate validation in multiple instances.
Let's create a helper to share the rate validation.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index e3121eb5f605..105911644b02 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1261,6 +1261,19 @@ static void vc4_hdmi_encoder_atomic_mode_set(struct drm_encoder *encoder,
 	mutex_unlock(&vc4_hdmi->mutex);
 }
 
+static enum drm_mode_status
+vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
+			     unsigned long long clock)
+{
+	if (clock > vc4_hdmi->variant->max_pixel_clock)
+		return MODE_CLOCK_HIGH;
+
+	if (vc4_hdmi->disable_4kp60 && clock > HDMI_14_MAX_TMDS_CLK)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
 #define WIFI_2_4GHz_CH1_MIN_FREQ	2400000000ULL
 #define WIFI_2_4GHz_CH1_MAX_FREQ	2422000000ULL
 
@@ -1304,10 +1317,7 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
 	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
 		pixel_rate = pixel_rate * 2;
 
-	if (pixel_rate > vc4_hdmi->variant->max_pixel_clock)
-		return -EINVAL;
-
-	if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK))
+	if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, pixel_rate) != MODE_OK)
 		return -EINVAL;
 
 	vc4_state->pixel_rate = pixel_rate;
@@ -1326,13 +1336,7 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder,
 	     (mode->hsync_end % 2) || (mode->htotal % 2)))
 		return MODE_H_ILLEGAL;
 
-	if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock)
-		return MODE_CLOCK_HIGH;
-
-	if (vc4_hdmi->disable_4kp60 && vc4_hdmi_mode_needs_scrambling(mode))
-		return MODE_CLOCK_HIGH;
-
-	return MODE_OK;
+	return vc4_hdmi_encoder_clock_valid(vc4_hdmi, mode->clock * 1000);
 }
 
 static const struct drm_encoder_helper_funcs vc4_hdmi_encoder_helper_funcs = {
-- 
2.34.1

[PATCH v5 2/6] drm/vc4: hdmi: Move clock calculation into its own function

[Maxime Ripard]

The code to compute our clock rate for a given setup will be called in
multiple places in the next patches, so let's create a separate function
for it.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 49 +++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 105911644b02..a1fa37ad350d 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1274,6 +1274,35 @@ vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
 	return MODE_OK;
 }
 
+static unsigned long long
+vc4_hdmi_encoder_compute_mode_clock(const struct drm_display_mode *mode,
+				    unsigned int bpc)
+{
+	unsigned long long clock = mode->clock * 1000;
+
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		clock = clock * 2;
+
+	return clock * bpc / 8;
+}
+
+static int
+vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
+			       struct vc4_hdmi_connector_state *vc4_state,
+			       const struct drm_display_mode *mode,
+			       unsigned int bpc)
+{
+	unsigned long long clock;
+
+	clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc);
+	if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, clock) != MODE_OK)
+		return -EINVAL;
+
+	vc4_state->pixel_rate = clock;
+
+	return 0;
+}
+
 #define WIFI_2_4GHz_CH1_MIN_FREQ	2400000000ULL
 #define WIFI_2_4GHz_CH1_MAX_FREQ	2422000000ULL
 
@@ -1286,6 +1315,7 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
 	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 	unsigned long long pixel_rate = mode->clock * 1000;
 	unsigned long long tmds_rate;
+	int ret;
 
 	if (vc4_hdmi->variant->unsupported_odd_h_timings &&
 	    ((mode->hdisplay % 2) || (mode->hsync_start % 2) ||
@@ -1306,21 +1336,10 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
 		pixel_rate = mode->clock * 1000;
 	}
 
-	if (conn_state->max_bpc == 12) {
-		pixel_rate = pixel_rate * 150;
-		do_div(pixel_rate, 100);
-	} else if (conn_state->max_bpc == 10) {
-		pixel_rate = pixel_rate * 125;
-		do_div(pixel_rate, 100);
-	}
-
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
-		pixel_rate = pixel_rate * 2;
-
-	if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, pixel_rate) != MODE_OK)
-		return -EINVAL;
-
-	vc4_state->pixel_rate = pixel_rate;
+	ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state, mode,
+					     conn_state->max_bpc);
+	if (ret)
+		return ret;
 
 	return 0;
 }
-- 
2.34.1

[PATCH v5 3/6] drm/vc4: hdmi: Take the sink maximum TMDS clock into account

[Maxime Ripard]

In the function that validates that the clock isn't too high, we've only
taken our controller limitations into account so far.

However, the sink can have a limit on the maximum TMDS clock it can deal
with too which is exposed through the EDID and the drm_display_info.

Make sure we check it.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a1fa37ad350d..4c3a5959c7f5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1265,12 +1265,18 @@ static enum drm_mode_status
 vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
 			     unsigned long long clock)
 {
+	const struct drm_connector *connector = &vc4_hdmi->connector;
+	const struct drm_display_info *info = &connector->display_info;
+
 	if (clock > vc4_hdmi->variant->max_pixel_clock)
 		return MODE_CLOCK_HIGH;
 
 	if (vc4_hdmi->disable_4kp60 && clock > HDMI_14_MAX_TMDS_CLK)
 		return MODE_CLOCK_HIGH;
 
+	if (info->max_tmds_clock && clock > (info->max_tmds_clock * 1000))
+		return MODE_CLOCK_HIGH;
+
 	return MODE_OK;
 }
 
-- 
2.34.1

[PATCH v5 4/6] drm/vc4: hdmi: Take bpp into account for the scrambler

[Maxime Ripard]

The current code only base its decision for whether the scrambler must be
enabled or not on the pixel clock of the mode, but doesn't take the bits
per color into account.

Let's leverage the new function to compute the clock rate in the
scrambler setup code.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +++++++++++++----
 drivers/gpu/drm/vc4/vc4_hdmi.h |  5 +++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 4c3a5959c7f5..03d0813992a7 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -99,9 +99,17 @@
 
 #define HDMI_14_MAX_TMDS_CLK   (340 * 1000 * 1000)
 
-static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode)
+
+static unsigned long long
+vc4_hdmi_encoder_compute_mode_clock(const struct drm_display_mode *mode,
+				    unsigned int bpc);
+
+static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode,
+					   unsigned int bpc)
 {
-	return (mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK;
+	unsigned long long clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc);
+
+	return clock > HDMI_14_MAX_TMDS_CLK;
 }
 
 static bool vc4_hdmi_is_full_range_rgb(struct vc4_hdmi *vc4_hdmi,
@@ -272,7 +280,7 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
 		struct drm_display_mode *mode;
 
 		list_for_each_entry(mode, &connector->probed_modes, head) {
-			if (vc4_hdmi_mode_needs_scrambling(mode)) {
+			if (vc4_hdmi_mode_needs_scrambling(mode, 8)) {
 				drm_warn_once(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz.");
 				drm_warn_once(drm, "Please change your config.txt file to add hdmi_enable_4kp60.");
 			}
@@ -613,7 +621,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
 	if (!vc4_hdmi_supports_scrambling(encoder, mode))
 		return;
 
-	if (!vc4_hdmi_mode_needs_scrambling(mode))
+	if (!vc4_hdmi_mode_needs_scrambling(mode, vc4_hdmi->output_bpc))
 		return;
 
 	drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
@@ -1255,6 +1263,7 @@ static void vc4_hdmi_encoder_atomic_mode_set(struct drm_encoder *encoder,
 	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 
 	mutex_lock(&vc4_hdmi->mutex);
+	vc4_hdmi->output_bpc = conn_state->max_bpc;
 	memcpy(&vc4_hdmi->saved_adjusted_mode,
 	       &crtc_state->adjusted_mode,
 	       sizeof(vc4_hdmi->saved_adjusted_mode));
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 2b6aaafc020a..31b7d27deb8e 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -216,6 +216,11 @@ struct vc4_hdmi {
 	 * the scrambler on? Protected by @mutex.
 	 */
 	bool scdc_enabled;
+
+	/**
+	 * @output_bpc: BPC currently being used. Protected by @mutex.
+	 */
+	unsigned int output_bpc;
 };
 
 static inline struct vc4_hdmi *
-- 
2.34.1

[PATCH v5 5/6] drm/vc4: hdmi: Always try to have the highest bpc

[Maxime Ripard]

Currently we take the max_bpc property as the bpc value and do not try
anything else.

However, what the other drivers seem to be doing is that they would try
with the highest bpc allowed by the max_bpc property and the hardware
capabilities, test if it results in an acceptable configuration, and if
not decrease the bpc and try again.

Let's use the same logic.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 44 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/vc4/vc4_hdmi.h |  4 +++-
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 03d0813992a7..3ce002e485ce 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -352,6 +352,7 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector *connector)
 		return NULL;
 
 	new_state->pixel_rate = vc4_state->pixel_rate;
+	new_state->output_bpc = vc4_state->output_bpc;
 	__drm_atomic_helper_connector_duplicate_state(connector, &new_state->base);
 
 	return &new_state->base;
@@ -906,6 +907,8 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi,
 				 struct drm_connector_state *state,
 				 struct drm_display_mode *mode)
 {
+	const struct vc4_hdmi_connector_state *vc4_state =
+		conn_state_to_vc4_hdmi_conn_state(state);
 	bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
 	bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
 	bool interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE;
@@ -953,7 +956,7 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi,
 	HDMI_WRITE(HDMI_VERTB0, vertb_even);
 	HDMI_WRITE(HDMI_VERTB1, vertb);
 
-	switch (state->max_bpc) {
+	switch (vc4_state->output_bpc) {
 	case 12:
 		gcp = 6;
 		gcp_en = true;
@@ -1261,9 +1264,11 @@ static void vc4_hdmi_encoder_atomic_mode_set(struct drm_encoder *encoder,
 					     struct drm_connector_state *conn_state)
 {
 	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+	struct vc4_hdmi_connector_state *vc4_state =
+		conn_state_to_vc4_hdmi_conn_state(conn_state);
 
 	mutex_lock(&vc4_hdmi->mutex);
-	vc4_hdmi->output_bpc = conn_state->max_bpc;
+	vc4_hdmi->output_bpc = vc4_state->output_bpc;
 	memcpy(&vc4_hdmi->saved_adjusted_mode,
 	       &crtc_state->adjusted_mode,
 	       sizeof(vc4_hdmi->saved_adjusted_mode));
@@ -1318,6 +1323,38 @@ vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
 	return 0;
 }
 
+static int
+vc4_hdmi_encoder_compute_config(const struct vc4_hdmi *vc4_hdmi,
+				struct vc4_hdmi_connector_state *vc4_state,
+				const struct drm_display_mode *mode)
+{
+	struct drm_connector_state *conn_state = &vc4_state->base;
+	unsigned int max_bpc = clamp_t(unsigned int, conn_state->max_bpc, 8, 12);
+	unsigned int bpc;
+	int ret;
+
+	for (bpc = max_bpc; bpc >= 8; bpc -= 2) {
+		drm_dbg(dev, "Trying with a %d bpc output\n", bpc);
+
+		ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
+						     mode, bpc);
+		if (ret)
+			continue;
+
+		vc4_state->output_bpc = bpc;
+
+		drm_dbg(dev,
+			"Mode %ux%u @ %uHz: Found configuration: bpc: %u, clock: %llu\n",
+			mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode),
+			vc4_state->output_bpc,
+			vc4_state->pixel_rate);
+
+		break;
+	}
+
+	return ret;
+}
+
 #define WIFI_2_4GHz_CH1_MIN_FREQ	2400000000ULL
 #define WIFI_2_4GHz_CH1_MAX_FREQ	2422000000ULL
 
@@ -1351,8 +1388,7 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
 		pixel_rate = mode->clock * 1000;
 	}
 
-	ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state, mode,
-					     conn_state->max_bpc);
+	ret = vc4_hdmi_encoder_compute_config(vc4_hdmi, vc4_state, mode);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 31b7d27deb8e..05d2b0eeaa9b 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -218,7 +218,8 @@ struct vc4_hdmi {
 	bool scdc_enabled;
 
 	/**
-	 * @output_bpc: BPC currently being used. Protected by @mutex.
+	 * @output_bpc: Copy of @vc4_connector_state.output_bpc for use
+	 * outside of KMS hooks. Protected by @mutex.
 	 */
 	unsigned int output_bpc;
 };
@@ -240,6 +241,7 @@ encoder_to_vc4_hdmi(struct drm_encoder *encoder)
 struct vc4_hdmi_connector_state {
 	struct drm_connector_state	base;
 	unsigned long long		pixel_rate;
+	unsigned int 			output_bpc;
 };
 
 static inline struct vc4_hdmi_connector_state *
-- 
2.34.1

[PATCH v5 6/6] drm/vc4: hdmi: Support HDMI YUV output

[Maxime Ripard]

In addition to the RGB444 output, the BCM2711 HDMI controller supports
the YUV444 and YUV422 output formats.

Let's add support for them in the driver, but still use RGB as the
preferred format.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c      | 289 ++++++++++++++++++++++++++--
 drivers/gpu/drm/vc4/vc4_hdmi.h      |  14 ++
 drivers/gpu/drm/vc4/vc4_hdmi_regs.h |   6 +
 drivers/gpu/drm/vc4/vc4_regs.h      |  16 ++
 4 files changed, 309 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 3ce002e485ce..787ddc5ba7d7 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -99,15 +99,30 @@
 
 #define HDMI_14_MAX_TMDS_CLK   (340 * 1000 * 1000)
 
+static const char * const output_format_str[] = {
+	[VC4_HDMI_OUTPUT_RGB]		= "RGB",
+	[VC4_HDMI_OUTPUT_YUV420]	= "YUV 4:2:0",
+	[VC4_HDMI_OUTPUT_YUV422]	= "YUV 4:2:2",
+	[VC4_HDMI_OUTPUT_YUV444]	= "YUV 4:4:4",
+};
+
+static const char *vc4_hdmi_output_fmt_str(enum vc4_hdmi_output_format fmt)
+{
+	if (fmt >= ARRAY_SIZE(output_format_str))
+		return "invalid";
+
+	return output_format_str[fmt];
+}
 
 static unsigned long long
 vc4_hdmi_encoder_compute_mode_clock(const struct drm_display_mode *mode,
-				    unsigned int bpc);
+				    unsigned int bpc, enum vc4_hdmi_output_format fmt);
 
 static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode,
-					   unsigned int bpc)
+					   unsigned int bpc,
+					   enum vc4_hdmi_output_format fmt)
 {
-	unsigned long long clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc);
+	unsigned long long clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc, fmt);
 
 	return clock > HDMI_14_MAX_TMDS_CLK;
 }
@@ -280,7 +295,7 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
 		struct drm_display_mode *mode;
 
 		list_for_each_entry(mode, &connector->probed_modes, head) {
-			if (vc4_hdmi_mode_needs_scrambling(mode, 8)) {
+			if (vc4_hdmi_mode_needs_scrambling(mode, 8, VC4_HDMI_OUTPUT_RGB)) {
 				drm_warn_once(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz.");
 				drm_warn_once(drm, "Please change your config.txt file to add hdmi_enable_4kp60.");
 			}
@@ -337,6 +352,7 @@ static void vc4_hdmi_connector_reset(struct drm_connector *connector)
 
 	new_state->base.max_bpc = 8;
 	new_state->base.max_requested_bpc = 8;
+	new_state->output_format = VC4_HDMI_OUTPUT_RGB;
 	drm_atomic_helper_connector_tv_reset(connector);
 }
 
@@ -353,6 +369,7 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector *connector)
 
 	new_state->pixel_rate = vc4_state->pixel_rate;
 	new_state->output_bpc = vc4_state->output_bpc;
+	new_state->output_format = vc4_state->output_format;
 	__drm_atomic_helper_connector_duplicate_state(connector, &new_state->base);
 
 	return &new_state->base;
@@ -496,11 +513,38 @@ 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_avi_infoframe_colorspace(struct hdmi_avi_infoframe *frame,
+					      enum vc4_hdmi_output_format fmt)
+{
+	switch (fmt) {
+	case VC4_HDMI_OUTPUT_RGB:
+		frame->colorspace = HDMI_COLORSPACE_RGB;
+		break;
+
+	case VC4_HDMI_OUTPUT_YUV420:
+		frame->colorspace = HDMI_COLORSPACE_YUV420;
+		break;
+
+	case VC4_HDMI_OUTPUT_YUV422:
+		frame->colorspace = HDMI_COLORSPACE_YUV422;
+		break;
+
+	case VC4_HDMI_OUTPUT_YUV444:
+		frame->colorspace = HDMI_COLORSPACE_YUV444;
+		break;
+
+	default:
+		break;
+	}
+}
+
 static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 {
 	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 	struct drm_connector *connector = &vc4_hdmi->connector;
 	struct drm_connector_state *cstate = connector->state;
+	struct vc4_hdmi_connector_state *vc4_state =
+		conn_state_to_vc4_hdmi_conn_state(cstate);
 	const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
 	union hdmi_infoframe frame;
 	int ret;
@@ -520,6 +564,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 					   HDMI_QUANTIZATION_RANGE_FULL :
 					   HDMI_QUANTIZATION_RANGE_LIMITED);
 	drm_hdmi_avi_infoframe_colorimetry(&frame.avi, cstate);
+	vc4_hdmi_avi_infoframe_colorspace(&frame.avi, vc4_state->output_format);
 	drm_hdmi_avi_infoframe_bars(&frame.avi, cstate);
 
 	vc4_hdmi_write_infoframe(encoder, &frame);
@@ -622,7 +667,9 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
 	if (!vc4_hdmi_supports_scrambling(encoder, mode))
 		return;
 
-	if (!vc4_hdmi_mode_needs_scrambling(mode, vc4_hdmi->output_bpc))
+	if (!vc4_hdmi_mode_needs_scrambling(mode,
+					    vc4_hdmi->output_bpc,
+					    vc4_hdmi->output_format))
 		return;
 
 	drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
@@ -817,6 +864,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])
 {
@@ -834,19 +913,53 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,
 			       struct drm_connector_state *state,
 			       const struct drm_display_mode *mode)
 {
+	struct vc4_hdmi_connector_state *vc4_state =
+		conn_state_to_vc4_hdmi_conn_state(state);
 	unsigned long flags;
+	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);
 
 	spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 
-	HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, 0x354021);
+	switch (vc4_state->output_format) {
+	case VC4_HDMI_OUTPUT_YUV444:
+		vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_full_yuv444_bt709);
+		break;
 
-	if (!vc4_hdmi_is_full_range_rgb(vc4_hdmi, mode))
-		vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_limited_rgb);
-	else
-		vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_unity);
+	case VC4_HDMI_OUTPUT_YUV422:
+		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;
 
+		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);
+		break;
+
+	case VC4_HDMI_OUTPUT_RGB:
+		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_to_limited_rgb);
+		else
+			vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_unity);
+		break;
+
+	default:
+		break;
+	}
+
+	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);
 
 	spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
@@ -972,6 +1085,15 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi,
 		break;
 	}
 
+	/*
+	 * YCC422 is always 36-bit and not considered deep colour so
+	 * doesn't signal in GCP.
+	 */
+	if (vc4_state->output_format == VC4_HDMI_OUTPUT_YUV422) {
+		gcp = 4;
+		gcp_en = false;
+	}
+
 	reg = HDMI_READ(HDMI_DEEP_COLOR_CONFIG_1);
 	reg &= ~(VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK |
 		 VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH_MASK);
@@ -1269,12 +1391,97 @@ static void vc4_hdmi_encoder_atomic_mode_set(struct drm_encoder *encoder,
 
 	mutex_lock(&vc4_hdmi->mutex);
 	vc4_hdmi->output_bpc = vc4_state->output_bpc;
+	vc4_hdmi->output_format = vc4_state->output_format;
 	memcpy(&vc4_hdmi->saved_adjusted_mode,
 	       &crtc_state->adjusted_mode,
 	       sizeof(vc4_hdmi->saved_adjusted_mode));
 	mutex_unlock(&vc4_hdmi->mutex);
 }
 
+static bool
+vc4_hdmi_sink_supports_format_bpc(const struct vc4_hdmi *vc4_hdmi,
+				  const struct drm_display_info *info,
+				  const struct drm_display_mode *mode,
+				  unsigned int format, unsigned int bpc)
+{
+	struct drm_device *dev = vc4_hdmi->connector.dev;
+	u8 vic = drm_match_cea_mode(mode);
+
+	if (vic == 1 && bpc != 8) {
+		drm_dbg(dev, "VIC1 requires a bpc of 8, got %u\n", bpc);
+		return false;
+	}
+
+	if (!info->is_hdmi &&
+	    (format != VC4_HDMI_OUTPUT_RGB || bpc != 8)) {
+		drm_dbg(dev, "DVI Monitors require an RGB output at 8 bpc\n");
+		return false;
+	}
+
+	switch (format) {
+	case VC4_HDMI_OUTPUT_RGB:
+		drm_dbg(dev, "RGB Format, checking the constraints.\n");
+
+		if (!(info->color_formats & DRM_COLOR_FORMAT_RGB444))
+			return false;
+
+		if (bpc == 10 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30)) {
+			drm_dbg(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
+			return false;
+		}
+
+		if (bpc == 12 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36)) {
+			drm_dbg(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
+			return false;
+		}
+
+		drm_dbg(dev, "RGB format supported in that configuration.\n");
+
+		return true;
+
+	case VC4_HDMI_OUTPUT_YUV422:
+		drm_dbg(dev, "YUV422 format, checking the constraints.\n");
+
+		if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR422)) {
+			drm_dbg(dev, "Sink doesn't support YUV422.\n");
+			return false;
+		}
+
+		if (bpc != 12) {
+			drm_dbg(dev, "YUV422 only supports 12 bpc.\n");
+			return false;
+		}
+
+		drm_dbg(dev, "YUV422 format supported in that configuration.\n");
+
+		return true;
+
+	case VC4_HDMI_OUTPUT_YUV444:
+		drm_dbg(dev, "YUV444 format, checking the constraints.\n");
+
+		if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR444)) {
+			drm_dbg(dev, "Sink doesn't support YUV444.\n");
+			return false;
+		}
+
+		if (bpc == 10 && !(info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_30)) {
+			drm_dbg(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
+			return false;
+		}
+
+		if (bpc == 12 && !(info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_36)) {
+			drm_dbg(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
+			return false;
+		}
+
+		drm_dbg(dev, "YUV444 format supported in that configuration.\n");
+
+		return true;
+	}
+
+	return false;
+}
+
 static enum drm_mode_status
 vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
 			     unsigned long long clock)
@@ -1296,13 +1503,17 @@ vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
 
 static unsigned long long
 vc4_hdmi_encoder_compute_mode_clock(const struct drm_display_mode *mode,
-				    unsigned int bpc)
+				    unsigned int bpc,
+				    enum vc4_hdmi_output_format fmt)
 {
 	unsigned long long clock = mode->clock * 1000;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
 		clock = clock * 2;
 
+	if (fmt == VC4_HDMI_OUTPUT_YUV422)
+		bpc = 8;
+
 	return clock * bpc / 8;
 }
 
@@ -1310,11 +1521,11 @@ static int
 vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
 			       struct vc4_hdmi_connector_state *vc4_state,
 			       const struct drm_display_mode *mode,
-			       unsigned int bpc)
+			       unsigned int bpc, unsigned int fmt)
 {
 	unsigned long long clock;
 
-	clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc);
+	clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc, fmt);
 	if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, clock) != MODE_OK)
 		return -EINVAL;
 
@@ -1323,11 +1534,56 @@ vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
 	return 0;
 }
 
+static int
+vc4_hdmi_encoder_compute_format(const struct vc4_hdmi *vc4_hdmi,
+				struct vc4_hdmi_connector_state *vc4_state,
+				const struct drm_display_mode *mode,
+				unsigned int bpc)
+{
+	struct drm_device *dev = vc4_hdmi->connector.dev;
+	const struct drm_connector *connector = &vc4_hdmi->connector;
+	const struct drm_display_info *info = &connector->display_info;
+	unsigned int format;
+
+	drm_dbg(dev, "Trying with an RGB output\n");
+
+	format = VC4_HDMI_OUTPUT_RGB;
+	if (vc4_hdmi_sink_supports_format_bpc(vc4_hdmi, info, mode, format, bpc)) {
+		int ret;
+
+		ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
+						     mode, bpc, format);
+		if (!ret) {
+			vc4_state->output_format = format;
+			return 0;
+		}
+	}
+
+	drm_dbg(dev, "Failed, Trying with an YUV422 output\n");
+
+	format = VC4_HDMI_OUTPUT_YUV422;
+	if (vc4_hdmi_sink_supports_format_bpc(vc4_hdmi, info, mode, format, bpc)) {
+		int ret;
+
+		ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
+						     mode, bpc, format);
+		if (!ret) {
+			vc4_state->output_format = format;
+			return 0;
+		}
+	}
+
+	drm_dbg(dev, "Failed. No Format Supported for that bpc count.\n");
+
+	return -EINVAL;
+}
+
 static int
 vc4_hdmi_encoder_compute_config(const struct vc4_hdmi *vc4_hdmi,
 				struct vc4_hdmi_connector_state *vc4_state,
 				const struct drm_display_mode *mode)
 {
+	struct drm_device *dev = vc4_hdmi->connector.dev;
 	struct drm_connector_state *conn_state = &vc4_state->base;
 	unsigned int max_bpc = clamp_t(unsigned int, conn_state->max_bpc, 8, 12);
 	unsigned int bpc;
@@ -1336,17 +1592,18 @@ vc4_hdmi_encoder_compute_config(const struct vc4_hdmi *vc4_hdmi,
 	for (bpc = max_bpc; bpc >= 8; bpc -= 2) {
 		drm_dbg(dev, "Trying with a %d bpc output\n", bpc);
 
-		ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
-						     mode, bpc);
+		ret = vc4_hdmi_encoder_compute_format(vc4_hdmi, vc4_state,
+						      mode, bpc);
 		if (ret)
 			continue;
 
 		vc4_state->output_bpc = bpc;
 
 		drm_dbg(dev,
-			"Mode %ux%u @ %uHz: Found configuration: bpc: %u, clock: %llu\n",
+			"Mode %ux%u @ %uHz: Found configuration: bpc: %u, fmt: %s, clock: %llu\n",
 			mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode),
 			vc4_state->output_bpc,
+			vc4_hdmi_output_fmt_str(vc4_state->output_format),
 			vc4_state->pixel_rate);
 
 		break;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 05d2b0eeaa9b..f015ae80c5d4 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -117,6 +117,13 @@ struct vc4_hdmi_audio {
 	bool streaming;
 };
 
+enum vc4_hdmi_output_format {
+	VC4_HDMI_OUTPUT_RGB,
+	VC4_HDMI_OUTPUT_YUV422,
+	VC4_HDMI_OUTPUT_YUV444,
+	VC4_HDMI_OUTPUT_YUV420,
+};
+
 /* General HDMI hardware state. */
 struct vc4_hdmi {
 	struct vc4_hdmi_audio audio;
@@ -222,6 +229,12 @@ struct vc4_hdmi {
 	 * outside of KMS hooks. Protected by @mutex.
 	 */
 	unsigned int output_bpc;
+
+	/**
+	 * @output_format: Copy of @vc4_connector_state.output_format
+	 * for use outside of KMS hooks. Protected by @mutex.
+	 */
+	enum vc4_hdmi_output_format output_format;
 };
 
 static inline struct vc4_hdmi *
@@ -242,6 +255,7 @@ struct vc4_hdmi_connector_state {
 	struct drm_connector_state	base;
 	unsigned long long		pixel_rate;
 	unsigned int 			output_bpc;
+	enum vc4_hdmi_output_format	output_format;
 };
 
 static inline struct vc4_hdmi_connector_state *
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
index fc971506bd4f..a040356b6bdc 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
@@ -54,6 +54,7 @@ enum vc4_hdmi_field {
 	HDMI_CSC_24_23,
 	HDMI_CSC_32_31,
 	HDMI_CSC_34_33,
+	HDMI_CSC_CHANNEL_CTL,
 	HDMI_CSC_CTL,
 
 	/*
@@ -119,6 +120,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,
@@ -244,6 +246,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = {
 	VC4_HDMI_REG(HDMI_SCRAMBLER_CTL, 0x1c4),
 
 	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),
@@ -289,6 +292,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[] = {
@@ -324,6 +328,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = {
 	VC4_HDMI_REG(HDMI_SCRAMBLER_CTL, 0x1c4),
 
 	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),
@@ -369,6 +374,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 33410718089e..c8210247cf24 100644
--- a/drivers/gpu/drm/vc4/vc4_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_regs.h
@@ -774,11 +774,27 @@ enum {
 # 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
+# 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
 
-- 
2.34.1

Re: [PATCH v5 2/6] drm/vc4: hdmi: Move clock calculation into its own function

[Ville Syrjälä]

On Thu, Jan 27, 2022 at 03:10:17PM +0100, Maxime Ripard wrote:
> The code to compute our clock rate for a given setup will be called in
> multiple places in the next patches, so let's create a separate function
> for it.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 49 +++++++++++++++++++++++-----------
>  1 file changed, 34 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 105911644b02..a1fa37ad350d 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1274,6 +1274,35 @@ vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
>  	return MODE_OK;
>  }
>  
> +static unsigned long long
> +vc4_hdmi_encoder_compute_mode_clock(const struct drm_display_mode *mode,
> +				    unsigned int bpc)
> +{
> +	unsigned long long clock = mode->clock * 1000;
> +
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		clock = clock * 2;
> +
> +	return clock * bpc / 8;

div_u64()/etc.?

> +}
> +
> +static int
> +vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
> +			       struct vc4_hdmi_connector_state *vc4_state,
> +			       const struct drm_display_mode *mode,
> +			       unsigned int bpc)
> +{
> +	unsigned long long clock;
> +
> +	clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc);
> +	if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, clock) != MODE_OK)
> +		return -EINVAL;
> +
> +	vc4_state->pixel_rate = clock;

This thing seems a bit confused between pixels vs. TMDS characters.
Either that or some/all of the pixel_clock/rate things are just
misnamed?

> +
> +	return 0;
> +}
> +
>  #define WIFI_2_4GHz_CH1_MIN_FREQ	2400000000ULL
>  #define WIFI_2_4GHz_CH1_MAX_FREQ	2422000000ULL
>  
> @@ -1286,6 +1315,7 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
>  	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
>  	unsigned long long pixel_rate = mode->clock * 1000;
>  	unsigned long long tmds_rate;
> +	int ret;
>  
>  	if (vc4_hdmi->variant->unsupported_odd_h_timings &&
>  	    ((mode->hdisplay % 2) || (mode->hsync_start % 2) ||
> @@ -1306,21 +1336,10 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
>  		pixel_rate = mode->clock * 1000;
>  	}
>  
> -	if (conn_state->max_bpc == 12) {
> -		pixel_rate = pixel_rate * 150;
> -		do_div(pixel_rate, 100);
> -	} else if (conn_state->max_bpc == 10) {
> -		pixel_rate = pixel_rate * 125;
> -		do_div(pixel_rate, 100);
> -	}
> -
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		pixel_rate = pixel_rate * 2;
> -
> -	if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, pixel_rate) != MODE_OK)
> -		return -EINVAL;
> -
> -	vc4_state->pixel_rate = pixel_rate;
> +	ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state, mode,
> +					     conn_state->max_bpc);
> +	if (ret)
> +		return ret;
>  
>  	return 0;
>  }
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

Re: [PATCH v5 6/6] drm/vc4: hdmi: Support HDMI YUV output

[Ville Syrjälä]

On Thu, Jan 27, 2022 at 03:10:21PM +0100, Maxime Ripard wrote:
> +/*
> + * 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] = {

I think YCbCr output is supposed to be limited range. Or at least 
that was the case with DP. For HDMI/CTA IIRC the spec is super
unclear/confusing when it talks about the YCC quantizaton range
stuff. 

Currently i915 will only do limited range BT.709 YCbCr output.

> +	{ 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])
>  {
<snip>
> @@ -1323,11 +1534,56 @@ vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
>  	return 0;
>  }
>  
> +static int
> +vc4_hdmi_encoder_compute_format(const struct vc4_hdmi *vc4_hdmi,
> +				struct vc4_hdmi_connector_state *vc4_state,
> +				const struct drm_display_mode *mode,
> +				unsigned int bpc)
> +{
> +	struct drm_device *dev = vc4_hdmi->connector.dev;
> +	const struct drm_connector *connector = &vc4_hdmi->connector;
> +	const struct drm_display_info *info = &connector->display_info;
> +	unsigned int format;
> +
> +	drm_dbg(dev, "Trying with an RGB output\n");
> +
> +	format = VC4_HDMI_OUTPUT_RGB;
> +	if (vc4_hdmi_sink_supports_format_bpc(vc4_hdmi, info, mode, format, bpc)) {
> +		int ret;
> +
> +		ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
> +						     mode, bpc, format);
> +		if (!ret) {
> +			vc4_state->output_format = format;
> +			return 0;
> +		}
> +	}

You seem to have the bpc vs. format selection in the opposite order to
i915. We try to exhaust all RGB color depths first, and only then fall
back to YCbCr 4:2:0. The automagic YCbCr 4:2:0 fallback is only
really there because without it you may not be able to light up the
display at all (at least if you want the higher resolutions).

With the current i915 logic 4:2:2 doesn't make any sense since it has 
the same requirements as 8bpc RGB. Hence we don't even implement
YCbCr 4:2:2 (also hw didn't have it until recently). There has
occasionally been some talk about introducing a new property
to give the user explicit control over this stuff. If/when that
happens I guess we might want to revisit the 4:2:2 situation for
i915.

> +
> +	drm_dbg(dev, "Failed, Trying with an YUV422 output\n");
> +
> +	format = VC4_HDMI_OUTPUT_YUV422;
> +	if (vc4_hdmi_sink_supports_format_bpc(vc4_hdmi, info, mode, format, bpc)) {
> +		int ret;
> +
> +		ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
> +						     mode, bpc, format);
> +		if (!ret) {
> +			vc4_state->output_format = format;
> +			return 0;
> +		}
> +	}
> +
> +	drm_dbg(dev, "Failed. No Format Supported for that bpc count.\n");
> +
> +	return -EINVAL;
> +}
> +

-- 
Ville Syrjälä
Intel

Re: [PATCH v5 2/6] drm/vc4: hdmi: Move clock calculation into its own function

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

Hi Ville,

Thanks for your review

On Thu, Feb 03, 2022 at 09:59:15PM +0200, Ville Syrjälä wrote:
> > +static int
> > +vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
> > +			       struct vc4_hdmi_connector_state *vc4_state,
> > +			       const struct drm_display_mode *mode,
> > +			       unsigned int bpc)
> > +{
> > +	unsigned long long clock;
> > +
> > +	clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc);
> > +	if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, clock) != MODE_OK)
> > +		return -EINVAL;
> > +
> > +	vc4_state->pixel_rate = clock;
> 
> This thing seems a bit confused between pixels vs. TMDS characters.
> Either that or some/all of the pixel_clock/rate things are just
> misnamed?

Yeah, this is the TMDS characters rate, I'll rename it.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 6/6] drm/vc4: hdmi: Support HDMI YUV output

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3577 bytes --]

On Thu, Feb 03, 2022 at 10:07:22PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 27, 2022 at 03:10:21PM +0100, Maxime Ripard wrote:
> > +/*
> > + * 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] = {
> 
> I think YCbCr output is supposed to be limited range. Or at least 
> that was the case with DP. For HDMI/CTA IIRC the spec is super
> unclear/confusing when it talks about the YCC quantizaton range
> stuff. 
> 
> Currently i915 will only do limited range BT.709 YCbCr output.

Indeed I haven't been able to find anything in the spec, so I'll just
drop it, if only to remain consistent across drivers.

> > +	{ 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])
> >  {
> <snip>
> > @@ -1323,11 +1534,56 @@ vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
> >  	return 0;
> >  }
> >  
> > +static int
> > +vc4_hdmi_encoder_compute_format(const struct vc4_hdmi *vc4_hdmi,
> > +				struct vc4_hdmi_connector_state *vc4_state,
> > +				const struct drm_display_mode *mode,
> > +				unsigned int bpc)
> > +{
> > +	struct drm_device *dev = vc4_hdmi->connector.dev;
> > +	const struct drm_connector *connector = &vc4_hdmi->connector;
> > +	const struct drm_display_info *info = &connector->display_info;
> > +	unsigned int format;
> > +
> > +	drm_dbg(dev, "Trying with an RGB output\n");
> > +
> > +	format = VC4_HDMI_OUTPUT_RGB;
> > +	if (vc4_hdmi_sink_supports_format_bpc(vc4_hdmi, info, mode, format, bpc)) {
> > +		int ret;
> > +
> > +		ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
> > +						     mode, bpc, format);
> > +		if (!ret) {
> > +			vc4_state->output_format = format;
> > +			return 0;
> > +		}
> > +	}
> 
> You seem to have the bpc vs. format selection in the opposite order to
> i915. We try to exhaust all RGB color depths first, and only then fall
> back to YCbCr 4:2:0. The automagic YCbCr 4:2:0 fallback is only
> really there because without it you may not be able to light up the
> display at all (at least if you want the higher resolutions).
> 
> With the current i915 logic 4:2:2 doesn't make any sense since it has 
> the same requirements as 8bpc RGB. Hence we don't even implement
> YCbCr 4:2:2 (also hw didn't have it until recently).

Our use-case is slightly different, but the basic idea is the same:
since we support from 8 to 12 bpc, an output to YUV422 output can prove
useful if we are reading a 12 bpc content and we don't have the
bandwidth to use RGB.

Thus, when we have a max_bpc of 12, we try RGB and YUV422, if none of
them work we try RGB in 10 and 8 bpc.

This is indeed a bit different than i915, but it doesn't seem
fundamentally different to me.

> There has occasionally been some talk about introducing a new property
> to give the user explicit control over this stuff. If/when that
> happens I guess we might want to revisit the 4:2:2 situation for i915.

This seems to be stalled entirely though, and this approach isn't
incompatible with whatever that would involve.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 6/6] drm/vc4: hdmi: Support HDMI YUV output

[Ville Syrjälä]

On Thu, Feb 10, 2022 at 11:03:43AM +0100, Maxime Ripard wrote:
> On Thu, Feb 03, 2022 at 10:07:22PM +0200, Ville Syrjälä wrote:
> > On Thu, Jan 27, 2022 at 03:10:21PM +0100, Maxime Ripard wrote:
> > > +/*
> > > + * 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] = {
> > 
> > I think YCbCr output is supposed to be limited range. Or at least 
> > that was the case with DP. For HDMI/CTA IIRC the spec is super
> > unclear/confusing when it talks about the YCC quantizaton range
> > stuff. 
> > 
> > Currently i915 will only do limited range BT.709 YCbCr output.
> 
> Indeed I haven't been able to find anything in the spec, so I'll just
> drop it, if only to remain consistent across drivers.
> 
> > > +	{ 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])
> > >  {
> > <snip>
> > > @@ -1323,11 +1534,56 @@ vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
> > >  	return 0;
> > >  }
> > >  
> > > +static int
> > > +vc4_hdmi_encoder_compute_format(const struct vc4_hdmi *vc4_hdmi,
> > > +				struct vc4_hdmi_connector_state *vc4_state,
> > > +				const struct drm_display_mode *mode,
> > > +				unsigned int bpc)
> > > +{
> > > +	struct drm_device *dev = vc4_hdmi->connector.dev;
> > > +	const struct drm_connector *connector = &vc4_hdmi->connector;
> > > +	const struct drm_display_info *info = &connector->display_info;
> > > +	unsigned int format;
> > > +
> > > +	drm_dbg(dev, "Trying with an RGB output\n");
> > > +
> > > +	format = VC4_HDMI_OUTPUT_RGB;
> > > +	if (vc4_hdmi_sink_supports_format_bpc(vc4_hdmi, info, mode, format, bpc)) {
> > > +		int ret;
> > > +
> > > +		ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
> > > +						     mode, bpc, format);
> > > +		if (!ret) {
> > > +			vc4_state->output_format = format;
> > > +			return 0;
> > > +		}
> > > +	}
> > 
> > You seem to have the bpc vs. format selection in the opposite order to
> > i915. We try to exhaust all RGB color depths first, and only then fall
> > back to YCbCr 4:2:0. The automagic YCbCr 4:2:0 fallback is only
> > really there because without it you may not be able to light up the
> > display at all (at least if you want the higher resolutions).
> > 
> > With the current i915 logic 4:2:2 doesn't make any sense since it has 
> > the same requirements as 8bpc RGB. Hence we don't even implement
> > YCbCr 4:2:2 (also hw didn't have it until recently).
> 
> Our use-case is slightly different, but the basic idea is the same:
> since we support from 8 to 12 bpc, an output to YUV422 output can prove
> useful if we are reading a 12 bpc content and we don't have the
> bandwidth to use RGB.
> 
> Thus, when we have a max_bpc of 12, we try RGB and YUV422, if none of
> them work we try RGB in 10 and 8 bpc.
> 
> This is indeed a bit different than i915, but it doesn't seem
> fundamentally different to me.

I guess as long as the user gets a semi-correct picture it's
mostly irrelevant.

But at least on i915 gamma correction becomes harder on certain
platforms (GLK) if we do YCbCr output since the LUT is after
the CSC matrix :( Pretty sure we don't handle that case in any
kind of sane way atm. So that's another reason why we prefer
RGB over YCbCr.

-- 
Ville Syrjälä
Intel