[PATCH v7 00/22] Add new general DRM property "color format"

[PATCH v7 00/22] Add new general DRM property "color format"

[Nicolas Frattaroli]

Hello,

this is a follow-up to
https://lore.kernel.org/all/20250911130739.4936-1-marius.vlad@collabora.com/
which in of itself is a follow-up to
https://lore.kernel.org/dri-devel/20240115160554.720247-1-andri@yngvason.is/ where
a new DRM connector property has been added allowing users to
force a particular color format.

That in turn was actually also a follow-up from Werner Sembach's posted at
https://lore.kernel.org/dri-devel/20210630151018.330354-1-wse@tuxedocomputers.com/

As the number of cooks have reached critical mass, I'm hoping I'll be
the last person to touch this particular series.

We have an implementation in Weston at
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1825 that
adds support for this property. This patch series has been tested
against that MR on i915 (HDMI, DP), amdgpu (HDMI, DP) and on rockchip
(HDMI).

You can also manually test this with modetest like so, but beware that
this is a non-atomic invocation, so testing YUV420 like this will result
in weird outcomes if only some of the modes support YUV420:

  $ modetest -s 115:1920x1080-60@NV12 -w 115:'color format':4

where 115 is the connector ID and '4' is the enum value for a particular
color format.

General notes on the approach taken by me: instead of silently switching
to a different format than was explicitly requested, or even worse,
outputting something to the sink the sink doesn't support, bubble up an
error to userspace instead. "color format" is a "I want this" type
property, not a "force this" type property, i.e. the kernel will respect
the limits imposed by the hardware.

I'm not sure if my drm_bridge change actually achieves what I want in a
more complex bridge setup. I'd need to either come up with a virtual
bridge to test these scenarios, or spend some time making a flat flex
cable adapter for the DSI-HDMI bridge board I have here. Before I invest
too much time into either of those, I'd like to get some feedback on
this approach however.

Things I've tested:
- HDMI (YCbCr 4:4:4 + YCbCr 4:2:2 (8-bit) + RGB + Auto) on RK3588
- HDMI (YCbCr 4:4:4 + YCbCr 4:2:2 (8-bit) + RGB + Auto) on RK3576
- HDMI + DP (YCbCr 4:4:4, YCbCr 4:2:0, RGB, Auto) on Intel N97 (i915)
  DP-MST is untested, but I expect it to work the same.
- HDMI (YCbCr 4:4:4, YCbCr 4:2:2, YCbCr 4:2:0, RGB, Auto) + DP (YCbCr
  4:4:4, RGB, Auto) on an AMD Radeon RX 550 (amdgpu). DP-MST is
  untested.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v7:
- Fix drm_bridge kunit test build failure caused by rebasing across an
  API change.
- Make compilers shut up about unused EDID definitions in the test
  suites.
- Empty line checkpatch fixes that b4 prep --check didn't catch.
- Link to v6: https://lore.kernel.org/r/20260121-color-format-v6-0-7b81a771cd0b@collabora.com

Changes in v6:
- Checkpatch fixes
- Add drm_bridge.c kerneldoc fix patch to b4 deps so the kernel docs
  required for every contribution to the subsystem can be built
- dw-hdmi-qp core has gained the atomic_get_output_bus_fmts bridge func,
  which allows it to participate in the drm_bridge chain recursive format
  selection code properly.
- The Rockchip dw-hdmi-qp integration now no longer reimplements the
  color format logic (improperly), but reads the bus format of the first
  bridge as set by the recursive bridge format selection. If the input
  format is FIXED, it'll use the output format. Otherwise, the input
  format is used.
- In the synopsys drivers, YUV422 uses the same bus format as the non-qp
  hdmi encoder driver. Probably correcter this way. The Rockchip vop2
  is_yuv function has been extended to recognise this format as well.
- KUnit tests for drm_bridge chains are now included, which exercise the
  chain's recursive bus format selection.
- On HDMI connectors, the drm_bridge bus format selection will try to target
  the color format that the HDMI layer came up with. This means the AUTO
  logic is not duplicated for HDMI connectors.
- The enum conversion function commit gained a function for converting
  from hdmi_colorspace to drm_color_format, and its author changed as no
  original code remains anyway. Marius is still included as a
  Co-developer.
- Some tests for the HDMI state helper's mode_valid have been written.
  They are incomplete as we lack a test EDID for a 420-also mode that
  would violate the clock constraints on RGB. I hacked one together with
  a hex editor, but it reports a too high of a clock rate, and there's
  no EDID editor I could find which supports these extension blocks.
- The color_format KUnit tests have been more heavily parameterised, the
  auto case absorbed into other tests, and the comments around them
  rewritten.
- Add a few paragraphs of documentation that explain the bridge format
  selection, and how to make use of it in a display driver.
- Link to v5: https://lore.kernel.org/r/20251128-color-format-v5-0-63e82f1db1e1@collabora.com

Changes in v5:
- Rebase onto drm-tip
- Drop DRM_MODE_COLOR_FORMAT_* as an enum
- Unify DRM_COLOR_FORMAT_NONE and DRM_COLOR_FORMAT_AUTO, with AUTO being
  0. This makes conversion and general logic much easier.
- Adjust the drm_color_format enum to not needlessly renumber the
  existing defines, as it doesn't need to correspond to how HDMI numbers
  them.
- Make the DRM-to-HDMI conversion function static inline __pure, because
  the assembly it generates is tiny, and the function is pure.
- Don't accept nothing as the list of supported color formats for
  registration of the property.
- Drop the per-connector variants of the color format registration
  function, as it's not needed.
- drm_hdmi_state_helper: Fix mode_valid rejecting 420-only modes.
- drm_hdmi_state_helper: Only fall back to YUV420 with
  DRM_COLOR_FORMAT_AUTO.
- drm_hdmi_state_helper: Remove redundant AUTO->RGB condition, as the
  conversion already does this.
- Add KUnit tests for hdmi_compute_config.
- drm/bridge: Refactor bus_format_is_color_fmt and add a few more YUV422
  formats.
- Register the color format property in drmm_connector_hdmi_init based
  on the supported HDMI formats passed to it. This means rockchip
  dw_hdmi_qp no longer needs to register it.
- amdgpu: Simplify YUV420 logic
- amdgpu: Don't try to pick YUV444 on YUV420-only modes
- i915: Try to make behaviour more or less the same as that of the drm
  hdmi state helper.
- rockchip dw_hdmi_qp: Set supported HDMI formats
- rockchip dw_hdmi_qp: Set the right VO GRF values depending on color
  format.
- rockchip dw_hdmi_qp: Act on the color format property in this driver,
  rather than in VOP2, by setting the bus_format appropriately.
- rockchip VOP2: Can the BCSH-based implementation. BCSH isn't available
  on all video ports of the hardware, and the code was extremely
  suspect. Instead, plug into the existing YUV-to-RGB/RGB-to-YUV code,
  which can be done now that the HDMI driver sets the bus format.
- A whole bunch of Rockchip VOP2 fixes.
- Link to v4: https://lore.kernel.org/r/20251117-color-format-v4-0-0ded72bd1b00@collabora.com

Changes in v4:
- Rebase onto next-20251117
- Get rid of HDMI_COLORSPACE_AUTO
- Split hdmi_compute_config change into separate patch
- Add missing symbol export for color_format_to_hdmi_colorspace to fix
  builds in certain configurations
- Drop "drm: Pass supported color formats straight onto drm_bridge"
- Make dw-hdmi-qp set the platform data's supported color formats as
  the bridge's supported HDMI color formats
- drm_hdmi_state_helper: pass requested color format to
  hdmi_compute_format_bpc if set.
- drm_bridge: limit the bus formats to those explicitly requested with
  the color format property during the atomic bridge check call,
  specifically in drm_atomic_bridge_chain_select_bus_fmts.
- i915: Remove INTEL_OUTPUT_FORMAT_AUTO, as automatic format selection
  does not need to involve the hardware state
- i915: Deduplicate ntel_output_format_to_drm_color_format code by
  moving it as a static inline __pure function into a shared header
- i915: rework logic in HDMI, DP and DP-MST output config functions to
  remove redundant locals, simplify execution flow, and return an error
  to userspace if an explicit color_format request can't be satisfied.
- i915: assign myself as the author and make the others Co-developers,
  so that they don't get the blame for any of my bugs.
- amdgpu: refactor fill_stream_properties_from_drm_display_mode to
  improve readability and ensure that impossible color format requests
  get bubbled up to userspace as errors
- amdgpu: don't pick YUV444 over RGB.
- amdgpu: assign authorship to myself, with others as Co-developers, as
  logic was modified and the blame should fall on me
- dw_hdmi_qp-rockchip: set the supported color formats platform data
  member
- rockchip: remove drm property registration for rk3066_hdmi and
  inno_hdmi. None of the platforms that use these use vop2 as the
  video output processor.
- Link to v3: https://lore.kernel.org/all/20250911130739.4936-1-marius.vlad@collabora.com/

Changes in v3 by mvlad compared to Andri's v2 series:
- renamed the property to just 'color format'
- the property is added dynamically similar to the Colorspace property
- a key point from previous comments was that drivers should advertise
  the color formats they support and userspace would query EDID and
  perform an intersection from those color formats which users can
  further use. With this patch set each driver that adds this property
  has such list of hard-coded color formats, but fundamentally the idea
  is that driver can query the HW and do that on its own. The
  infrastructure is now in place to allow to do that
- by default the 'AUTO' color format is set. With this patch series that
  has been introduced as a fallback to RGB. Drivers could further
  customize this behavour and could perform additional checks on the sink
  to pick another suitable color format they'd like for AUTO
- drm_bridge bridge code has been improved to allow initialization with
  the same color formats list as the DRM connector property. Similarly, bpc
  pick-up now takes the color format into consideration when deciding
  which bpc to choose from
- The new DRM color format re-uses HDMI_COLORPSACE enum and provides an
  enum translations between the two to avoid touching all other drivers that
  use HDMI_COLORPSACE enum. I believe at this point that this allows the
  least amount of disruption and avoids a massive bike shedding around
  that part
- a rockchip implementation has been by my colleague Derek Foreman
- YUV444 color format has been added in i915
- address comment about "Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A
  check" where aconnector might be invalid
- Link to v2: https://lore.kernel.org/dri-devel/20240115160554.720247-1-andri@yngvason.is/

---
Andri Yngvason (1):
      drm: Add new general DRM property "color format"

Nicolas Frattaroli (20):
      drm: Add enum conversions between DRM_COLOR_FORMAT and HDMI_COLORSPACE
      drm/bridge: Act on the DRM color format property
      drm/display: hdmi-state-helper: Act on color format DRM property
      drm/display: hdmi-state-helper: Try subsampling in mode_valid
      drm/i915: Implement the "color format" DRM property
      drm/amdgpu: Implement "color format" DRM property
      drm/rockchip: Add YUV422 output mode constants for VOP2
      drm/rockchip: vop2: Fix YUV444 output
      drm/rockchip: vop2: Add RK3576 to the RG swap special case
      drm/rockchip: vop2: Recognise 10/12-bit YUV422 as YUV formats
      drm/rockchip: vop2: Set correct output format for RK3576 YUV422
      drm/bridge: dw-hdmi-qp: Implement atomic_get_output_bus_fmts
      drm/rockchip: dw_hdmi_qp: Implement "color format" DRM property
      drm/rockchip: dw_hdmi_qp: Set supported_formats platdata
      drm/connector: Register color format property on HDMI connectors
      drm/tests: hdmi: Add tests for the color_format property
      drm/tests: hdmi: Add tests for HDMI helper's mode_valid
      drm/tests: edid: Add __maybe_unused attribute to EDID definitions
      drm/tests: bridge: Add KUnit tests for bridge chain format selection
      drm/bridge: Document bridge chain format selection

Werner Sembach (1):
      drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check

 Documentation/gpu/drm-kms-helpers.rst              |   6 +
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  88 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    |  13 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c       |  64 ++
 drivers/gpu/drm/display/drm_hdmi_state_helper.c    |  27 +-
 drivers/gpu/drm/drm_atomic_helper.c                |   5 +
 drivers/gpu/drm/drm_atomic_uapi.c                  |  11 +
 drivers/gpu/drm/drm_bridge.c                       | 118 ++-
 drivers/gpu/drm/drm_connector.c                    | 155 ++++
 drivers/gpu/drm/i915/display/intel_connector.c     |  11 +
 drivers/gpu/drm/i915/display/intel_connector.h     |   1 +
 drivers/gpu/drm/i915/display/intel_display_types.h |  15 +
 drivers/gpu/drm/i915/display/intel_dp.c            |  55 +-
 drivers/gpu/drm/i915/display/intel_dp.h            |   4 +
 drivers/gpu/drm/i915/display/intel_dp_mst.c        |  36 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c          |  54 +-
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c     | 111 ++-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h        |   4 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c       |  41 +-
 drivers/gpu/drm/tests/drm_bridge_test.c            | 812 +++++++++++++++++++++
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 300 ++++++++
 drivers/gpu/drm/tests/drm_kunit_edid.h             |  27 +-
 include/drm/drm_connector.h                        |  89 ++-
 23 files changed, 1980 insertions(+), 67 deletions(-)
---
base-commit: dc0b90be8d513256a6594095731a702287f29602
change-id: 20251028-color-format-49fd202b7183
prerequisite-change-id: 20251231-drm-bridge-alloc-getput-drm_of_find_bridge-kdoc-fix-fb00c84c525a:v1
prerequisite-patch-id: bbdc03403e29a985540228dfa789dbfa47861e4e

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v7 01/22] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check

[Nicolas Frattaroli]

From: Werner Sembach <wse@tuxedocomputers.com>

Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check that was performed in the
drm_mode_is_420_only() case, but not in the drm_mode_is_420_also() &&
force_yuv420_output case.

Without further knowledge if YCbCr 4:2:0 is supported outside of HDMI,
there is no reason to use RGB when the display
reports drm_mode_is_420_only() even on a non HDMI connection.

This patch also moves both checks in the same if-case. This  eliminates an
extra else-if-case.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Andri Yngvason <andri@yngvason.is>
Tested-by: Andri Yngvason <andri@yngvason.is>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index cb13a2b0de62..4a2491a66fdd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6699,12 +6699,9 @@ static void fill_stream_properties_from_drm_display_mode(
 	timing_out->v_border_top = 0;
 	timing_out->v_border_bottom = 0;
 	/* TODO: un-hardcode */
-	if (drm_mode_is_420_only(info, mode_in)
-			&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
-		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
-	else if (drm_mode_is_420_also(info, mode_in)
-			&& aconnector
-			&& aconnector->force_yuv420_output)
+	if (drm_mode_is_420_only(info, mode_in) ||
+	    (aconnector && aconnector->force_yuv420_output &&
+	     drm_mode_is_420_also(info, mode_in)))
 		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
 	else if ((connector->display_info.color_formats & DRM_COLOR_FORMAT_YCBCR422)
 			&& aconnector

-- 
2.52.0

[PATCH v7 02/22] drm: Add new general DRM property "color format"

[Nicolas Frattaroli]

From: Andri Yngvason <andri@yngvason.is>

Add a new general DRM property named "color format" which can be used by
userspace to request the display driver to output a particular color
format.

Possible options are:
    - auto (setup by default, driver internally picks the color format)
    - rgb
    - ycbcr444
    - ycbcr422
    - ycbcr420

Drivers should advertise from this list which formats they support.
Together with this list and EDID data from the sink we should be able
to relay a list of usable color formats to users to pick from.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Andri Yngvason <andri@yngvason.is>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_atomic_helper.c |   5 ++
 drivers/gpu/drm/drm_atomic_uapi.c   |  11 +++
 drivers/gpu/drm/drm_connector.c     | 142 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h         |  35 +++++++--
 4 files changed, 188 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index cc1f0c102414..1c2fa2e5f9b6 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -737,6 +737,11 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 			if (old_connector_state->max_requested_bpc !=
 			    new_connector_state->max_requested_bpc)
 				new_crtc_state->connectors_changed = true;
+
+			if (old_connector_state->color_format !=
+			    new_connector_state->color_format)
+				new_crtc_state->connectors_changed = true;
+
 		}
 
 		if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index dc013a22bf26..6f87b7bf567d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -931,6 +931,15 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 		state->privacy_screen_sw_state = val;
 	} else if (property == connector->broadcast_rgb_property) {
 		state->hdmi.broadcast_rgb = val;
+	} else if (property == connector->color_format_property) {
+		if (val >= DRM_COLOR_FORMAT_COUNT) {
+			drm_dbg_atomic(connector->dev,
+				       "[CONNECTOR:%d:%s] unknown color format %llu\n",
+				       connector->base.id, connector->name, val);
+			return -EINVAL;
+		}
+
+		state->color_format = val ? BIT(val - 1) : DRM_COLOR_FORMAT_AUTO;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -1016,6 +1025,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->privacy_screen_sw_state;
 	} else if (property == connector->broadcast_rgb_property) {
 		*val = state->hdmi.broadcast_rgb;
+	} else if (property == connector->color_format_property) {
+		*val = ffs(state->color_format);
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4f5b27fab475..507787619b80 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1354,6 +1354,32 @@ static const char * const colorspace_names[] = {
 	[DRM_MODE_COLORIMETRY_BT601_YCC] = "BT601_YCC",
 };
 
+/**
+ * drm_get_color_format_name - return a string for color format
+ * @color_fmt: color format to return the name of
+ *
+ * Returns a string constant matching the format's name, or NULL if no match
+ * is found.
+ */
+const char *drm_get_color_format_name(enum drm_color_format color_fmt)
+{
+	switch (color_fmt) {
+	case DRM_COLOR_FORMAT_AUTO:
+		return "AUTO";
+	case DRM_COLOR_FORMAT_RGB444:
+		return "RGB";
+	case DRM_COLOR_FORMAT_YCBCR444:
+		return "YUV 4:4:4";
+	case DRM_COLOR_FORMAT_YCBCR422:
+		return "YUV 4:2:2";
+	case DRM_COLOR_FORMAT_YCBCR420:
+		return "YUV 4:2:0";
+	default:
+		return NULL;
+	}
+}
+EXPORT_SYMBOL(drm_get_color_format_name);
+
 /**
  * drm_get_colorspace_name - return a string for color encoding
  * @colorspace: color space to compute name of
@@ -1383,6 +1409,20 @@ static const u32 hdmi_colorspaces =
 	BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65) |
 	BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER);
 
+/* already bit-shifted */
+static const u32 hdmi_colorformats =
+	DRM_COLOR_FORMAT_RGB444 |
+	DRM_COLOR_FORMAT_YCBCR444 |
+	DRM_COLOR_FORMAT_YCBCR422 |
+	DRM_COLOR_FORMAT_YCBCR420;
+
+/* already bit-shifted */
+static const u32 dp_colorformats =
+	DRM_COLOR_FORMAT_RGB444 |
+	DRM_COLOR_FORMAT_YCBCR444 |
+	DRM_COLOR_FORMAT_YCBCR422 |
+	DRM_COLOR_FORMAT_YCBCR420;
+
 /*
  * As per DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel Encoding/Colorimetry
  * Format Table 2-120
@@ -2634,6 +2674,89 @@ int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_mode_create_hdmi_colorspace_property);
 
+/**
+ * drm_mode_create_color_format_property - create color format property
+ * @connector: connector to create the color format property on
+ * @supported_color_formats: bitmask of &enum drm_color_format values the
+ *                           connector supports
+ *
+ * Called by a driver to create a color format property. Must be attached to
+ * the desired connector afterwards.
+ *
+ * @supported_color_formats should only include color formats the connector
+ * type can actually support.
+ *
+ * Returns:
+ * 0 on success, negative errno on error
+ */
+int drm_mode_create_color_format_property(struct drm_connector *connector,
+					  u32 supported_color_formats)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_prop_enum_list enum_list[DRM_COLOR_FORMAT_COUNT];
+	unsigned int len = 1;
+	unsigned int i = 1;
+	u32 fmt;
+
+	if (connector->color_format_property)
+		return 0;
+
+	if (!supported_color_formats) {
+		drm_err(dev, "No supported color formats provided on [CONNECTOR:%d:%s]\n",
+			connector->base.id, connector->name);
+		return -EINVAL;
+	}
+
+	if (supported_color_formats & ~GENMASK_U32(DRM_COLOR_FORMAT_COUNT - 1, 0)) {
+		drm_err(dev, "Unknown color formats provided on [CONNECTOR:%d:%s]\n",
+			connector->base.id, connector->name);
+		return -EINVAL;
+	}
+
+	switch (connector->connector_type) {
+	case DRM_MODE_CONNECTOR_HDMIA:
+	case DRM_MODE_CONNECTOR_HDMIB:
+		if (supported_color_formats & ~hdmi_colorformats) {
+			drm_err(dev, "Color formats not allowed for HDMI on [CONNECTOR:%d:%s]\n",
+				connector->base.id, connector->name);
+			return -EINVAL;
+		}
+		break;
+	case DRM_MODE_CONNECTOR_DisplayPort:
+	case DRM_MODE_CONNECTOR_eDP:
+		if (supported_color_formats & ~dp_colorformats) {
+			drm_err(dev, "Color formats not allowed for DP on [CONNECTOR:%d:%s]\n",
+				connector->base.id, connector->name);
+			return -EINVAL;
+		}
+		break;
+	}
+
+	enum_list[0].name = drm_get_color_format_name(DRM_COLOR_FORMAT_AUTO);
+	enum_list[0].type = 0;
+
+	while (supported_color_formats) {
+		fmt = BIT(i - 1);
+		if (supported_color_formats & fmt) {
+			supported_color_formats ^= fmt;
+			enum_list[len].name = drm_get_color_format_name(fmt);
+			enum_list[len].type = i;
+			len++;
+		}
+		i++;
+	}
+
+	connector->color_format_property =
+		drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, "color format",
+					 enum_list, len);
+
+	if (!connector->color_format_property)
+		return -ENOMEM;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_color_format_property);
+
 /**
  * drm_mode_create_dp_colorspace_property - create dp colorspace property
  * @connector: connector to create the Colorspace property on.
@@ -2851,6 +2974,25 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
 
+/**
+ * drm_connector_attach_color_format_property - attach "force color format" property
+ * @connector: connector to attach force color format property on.
+ *
+ * This is used to add support for selecting a color format on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_color_format_property(struct drm_connector *connector)
+{
+	struct drm_property *prop = connector->color_format_property;
+
+	drm_object_attach_property(&connector->base, prop, DRM_COLOR_FORMAT_AUTO);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_color_format_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 7eaec37ae1c7..b5604dca728a 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -556,6 +556,16 @@ enum drm_colorspace {
 	DRM_MODE_COLORIMETRY_COUNT
 };
 
+enum drm_color_format {
+	DRM_COLOR_FORMAT_AUTO			= 0,
+	DRM_COLOR_FORMAT_RGB444			= BIT(0),
+	DRM_COLOR_FORMAT_YCBCR444		= BIT(1),
+	DRM_COLOR_FORMAT_YCBCR422		= BIT(2),
+	DRM_COLOR_FORMAT_YCBCR420		= BIT(3),
+};
+
+#define DRM_COLOR_FORMAT_COUNT 5
+
 /**
  * enum drm_bus_flags - bus_flags info for &drm_display_info
  *
@@ -699,11 +709,6 @@ struct drm_display_info {
 	 */
 	enum subpixel_order subpixel_order;
 
-#define DRM_COLOR_FORMAT_RGB444		(1<<0)
-#define DRM_COLOR_FORMAT_YCBCR444	(1<<1)
-#define DRM_COLOR_FORMAT_YCBCR422	(1<<2)
-#define DRM_COLOR_FORMAT_YCBCR420	(1<<3)
-
 	/**
 	 * @panel_orientation: Read only connector property for built-in panels,
 	 * indicating the orientation of the panel vs the device's casing.
@@ -1107,6 +1112,13 @@ struct drm_connector_state {
 	 */
 	enum drm_colorspace colorspace;
 
+	/**
+	 * @color_format: State variable for Connector property to request
+	 * color format change on Sink. This is most commonly used to switch
+	 * between RGB to YUV and vice-versa.
+	 */
+	enum drm_color_format color_format;
+
 	/**
 	 * @writeback_job: Writeback job for writeback connectors
 	 *
@@ -2105,6 +2117,12 @@ struct drm_connector {
 	 */
 	struct drm_property *colorspace_property;
 
+	/**
+	 * @color_format_property: Connector property to set the suitable
+	 * color format supported by the sink.
+	 */
+	struct drm_property *color_format_property;
+
 	/**
 	 * @path_blob_ptr:
 	 *
@@ -2506,6 +2524,9 @@ int drm_mode_create_dp_colorspace_property(struct drm_connector *connector,
 int drm_mode_create_content_type_property(struct drm_device *dev);
 int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
 
+int drm_mode_create_color_format_property(struct drm_connector *connector,
+					  u32 supported_color_formats);
+
 int drm_connector_set_path_property(struct drm_connector *connector,
 				    const char *path);
 int drm_connector_set_tile_property(struct drm_connector *connector);
@@ -2587,6 +2608,10 @@ bool drm_connector_has_possible_encoder(struct drm_connector *connector,
 					struct drm_encoder *encoder);
 const char *drm_get_colorspace_name(enum drm_colorspace colorspace);
 
+int drm_connector_attach_color_format_property(struct drm_connector *connector);
+
+const char *drm_get_color_format_name(enum drm_color_format color_fmt);
+
 /**
  * drm_for_each_connector_iter - connector_list iterator macro
  * @connector: &struct drm_connector pointer used as cursor

-- 
2.52.0

[PATCH v7 03/22] drm: Add enum conversions between DRM_COLOR_FORMAT and HDMI_COLORSPACE

[Nicolas Frattaroli]

While the two enums have similar values, they're not identical, and
HDMI's enum is defined as per the HDMI standard.

Add a simple conversion function from DRM to HDMI. Unexpected inputs
aren't handled in any clever way, DRM_COLOR_FORMAT_AUTO and any other
value that doesn't cleanly map to HDMI just gets returned as
HDMI_COLORSPACE_RGB.

Add a second conversion function that gets a DRM_COLOR_FORMAT from an
HDMI_COLORSPACE as well. In this case, reserved HDMI values that can't
be converted will result in an -EINVAL return value.

Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 include/drm/drm_connector.h | 54 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index b5604dca728a..ffeb42f3b4a3 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2612,6 +2612,60 @@ int drm_connector_attach_color_format_property(struct drm_connector *connector);
 
 const char *drm_get_color_format_name(enum drm_color_format color_fmt);
 
+/**
+ * drm_color_format_to_hdmi_colorspace - convert DRM color format to HDMI
+ * @fmt: the &enum drm_color_format to convert
+ *
+ * Convert a given &enum drm_color_format to an equivalent
+ * &enum hdmi_colorspace. For non-representable values and
+ * %DRM_COLOR_FORMAT_AUTO, the value %HDMI_COLORSPACE_RGB is returned.
+ *
+ * Returns: the corresponding &enum hdmi_colorspace value
+ */
+static inline enum hdmi_colorspace __pure
+drm_color_format_to_hdmi_colorspace(enum drm_color_format fmt)
+{
+	switch (fmt) {
+	default:
+	case DRM_COLOR_FORMAT_AUTO:
+	case DRM_COLOR_FORMAT_RGB444:
+		return HDMI_COLORSPACE_RGB;
+	case DRM_COLOR_FORMAT_YCBCR444:
+		return HDMI_COLORSPACE_YUV444;
+	case DRM_COLOR_FORMAT_YCBCR422:
+		return HDMI_COLORSPACE_YUV422;
+	case DRM_COLOR_FORMAT_YCBCR420:
+		return HDMI_COLORSPACE_YUV420;
+	}
+}
+
+/**
+ * drm_color_format_from_hdmi_colorspace - convert HDMI color format to DRM
+ * @fmt: the &enum hdmi_colorspace to convert
+ *
+ * Convert a given &enum hdmi_colorspace to an equivalent
+ * &enum drm_color_format. For non-representable values,
+ * %-EINVAL is returned.
+ *
+ * Returns: the corresponding &enum drm_color_format value, or %-EINVAL
+ */
+static inline enum drm_color_format __pure
+drm_color_format_from_hdmi_colorspace(enum hdmi_colorspace fmt)
+{
+	switch (fmt) {
+	default:
+		return -EINVAL;
+	case HDMI_COLORSPACE_RGB:
+		return DRM_COLOR_FORMAT_RGB444;
+	case HDMI_COLORSPACE_YUV444:
+		return DRM_COLOR_FORMAT_YCBCR444;
+	case HDMI_COLORSPACE_YUV422:
+		return DRM_COLOR_FORMAT_YCBCR422;
+	case HDMI_COLORSPACE_YUV420:
+		return DRM_COLOR_FORMAT_YCBCR420;
+	}
+}
+
 /**
  * drm_for_each_connector_iter - connector_list iterator macro
  * @connector: &struct drm_connector pointer used as cursor

-- 
2.52.0

[PATCH v7 04/22] drm/bridge: Act on the DRM color format property

[Nicolas Frattaroli]

The new DRM color format property allows userspace to request a specific
color format on a connector. In turn, this fills the connector state's
color_format member to switch color formats.

Make drm_bridges consider the color_format set in the connector state
during the atomic bridge check. For bridges that represent HDMI bridges,
rely on whatever format the HDMI logic set. Reject any output bus
formats that do not correspond to the requested color format.

Non-HDMI last bridges with DRM_COLOR_FORMAT_AUTO set will end up
choosing the first output format that functions to make a whole
recursive bridge chain format selection succeed.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_bridge.c | 79 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 6dcf8f6d3ecf..b0dfa03dbb81 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1116,6 +1116,47 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge,
 	return ret;
 }
 
+static bool __pure bus_format_is_color_fmt(u32 bus_fmt, enum drm_color_format fmt)
+{
+	if (fmt == DRM_COLOR_FORMAT_AUTO)
+		return true;
+
+	switch (bus_fmt) {
+	case MEDIA_BUS_FMT_FIXED:
+		return true;
+	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 fmt == DRM_COLOR_FORMAT_RGB444;
+	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 fmt == DRM_COLOR_FORMAT_YCBCR444;
+	case MEDIA_BUS_FMT_UYVY8_1X16:
+	case MEDIA_BUS_FMT_VYUY8_1X16:
+	case MEDIA_BUS_FMT_YUYV8_1X16:
+	case MEDIA_BUS_FMT_YVYU8_1X16:
+	case MEDIA_BUS_FMT_UYVY10_1X20:
+	case MEDIA_BUS_FMT_YUYV10_1X20:
+	case MEDIA_BUS_FMT_VYUY10_1X20:
+	case MEDIA_BUS_FMT_YVYU10_1X20:
+	case MEDIA_BUS_FMT_UYVY12_1X24:
+	case MEDIA_BUS_FMT_VYUY12_1X24:
+	case MEDIA_BUS_FMT_YUYV12_1X24:
+	case MEDIA_BUS_FMT_YVYU12_1X24:
+		return fmt == DRM_COLOR_FORMAT_YCBCR422;
+	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 fmt == DRM_COLOR_FORMAT_YCBCR420;
+	default:
+		return false;
+	}
+}
+
 /*
  * This function is called by &drm_atomic_bridge_chain_check() just before
  * calling &drm_bridge_funcs.atomic_check() on all elements of the chain.
@@ -1159,6 +1200,7 @@ drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge *bridge,
 	struct drm_encoder *encoder = bridge->encoder;
 	struct drm_bridge_state *last_bridge_state;
 	unsigned int i, num_out_bus_fmts = 0;
+	enum drm_color_format fmt;
 	u32 *out_bus_fmts;
 	int ret = 0;
 
@@ -1200,13 +1242,48 @@ drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge *bridge,
 			out_bus_fmts[0] = MEDIA_BUS_FMT_FIXED;
 	}
 
+	/*
+	 * On HDMI connectors, use the output format chosen by whatever does the
+	 * HDMI logic. For everyone else, just trust that the bridge out_bus_fmts
+	 * are sorted by preference for %DRM_COLOR_FORMAT_AUTO, as
+	 * bus_format_is_color_fmt() always returns true for AUTO.
+	 */
+	if (last_bridge->ops & DRM_BRIDGE_OP_HDMI) {
+		fmt = drm_color_format_from_hdmi_colorspace(conn_state->hdmi.output_format);
+		if (fmt < 0) {
+			ret = fmt;
+			drm_dbg_kms(last_bridge->dev,
+				    "Couldn't convert HDMI format to DRM format: %pe\n",
+				    ERR_PTR(ret));
+			goto out_free;
+		}
+		drm_dbg_kms(last_bridge->dev, "HDMI bridge requests format %s\n",
+			    drm_get_color_format_name(fmt));
+	} else {
+		fmt = conn_state->color_format;
+		drm_dbg_kms(last_bridge->dev, "Non-HDMI bridge requests format %s\n",
+			    drm_get_color_format_name(fmt));
+	}
+
 	for (i = 0; i < num_out_bus_fmts; i++) {
+		if (!bus_format_is_color_fmt(out_bus_fmts[i], fmt)) {
+			drm_dbg_kms(last_bridge->dev,
+				    "Skipping bus format 0x%04x as it doesn't match %s\n",
+				    out_bus_fmts[i], drm_get_color_format_name(fmt));
+			ret = -ENOTSUPP;
+			continue;
+		}
 		ret = select_bus_fmt_recursive(bridge, last_bridge, crtc_state,
 					       conn_state, out_bus_fmts[i]);
-		if (ret != -ENOTSUPP)
+		if (ret != -ENOTSUPP) {
+			drm_dbg_kms(last_bridge->dev,
+				    "Found bridge chain ending with bus format 0x%04x\n",
+				    out_bus_fmts[i]);
 			break;
+		}
 	}
 
+out_free:
 	kfree(out_bus_fmts);
 
 	return ret;

-- 
2.52.0

[PATCH v7 05/22] drm/display: hdmi-state-helper: Act on color format DRM property

[Nicolas Frattaroli]

With the introduction of the "color format" DRM property, which allows
userspace to request a specific color format, the HDMI state helper
should implement this.

Implement it by checking whether the property is set and set to
something other than auto. If so, pass the requested color format, and
otherwise set RGB.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index a1d16762ac7a..1ea3b9c93aa5 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -649,11 +649,21 @@ hdmi_compute_config(const struct drm_connector *connector,
 	unsigned int max_bpc = clamp_t(unsigned int,
 				       conn_state->max_bpc,
 				       8, connector->max_bpc);
+	enum hdmi_colorspace hdmi_colorspace =
+		drm_color_format_to_hdmi_colorspace(conn_state->color_format);
 	int ret;
 
 	ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
-				      HDMI_COLORSPACE_RGB);
+				      hdmi_colorspace);
 	if (ret) {
+		/* If a color format was explicitly requested, don't fall back */
+		if (conn_state->color_format) {
+			drm_dbg_kms(connector->dev,
+				    "Explicitly set color format '%s' doesn't work.\n",
+				    drm_get_color_format_name(conn_state->color_format));
+			return ret;
+		}
+
 		if (connector->ycbcr_420_allowed) {
 			ret = hdmi_compute_format_bpc(connector, conn_state,
 						      mode, max_bpc,

-- 
2.52.0

[PATCH v7 06/22] drm/display: hdmi-state-helper: Try subsampling in mode_valid

[Nicolas Frattaroli]

drm_hdmi_connector_mode_valid assumes modes are only valid if they work
with RGB. The reality is more complex however: YCbCr 4:2:0
chroma-subsampled modes only require half the pixel clock that the same
mode would require in RGB.

This leads to drm_hdmi_connector_mode_valid rejecting perfectly valid
420-only or 420-also modes.

Fix this by checking whether the mode is 420-capable first. If so, then
proceed by checking it with HDMI_COLORSPACE_YUV420 so long as the
connector has legalized 420, otherwise error out. If the mode is not
420-capable, check with RGB as was previously always the case.

Fixes: 47368ab437fd ("drm/display: hdmi: add generic mode_valid helper")
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index 1ea3b9c93aa5..ceabfdb30f7a 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -898,8 +898,21 @@ drm_hdmi_connector_mode_valid(struct drm_connector *connector,
 			      const struct drm_display_mode *mode)
 {
 	unsigned long long clock;
+	enum hdmi_colorspace fmt;
+
+	if (drm_mode_is_420_only(&connector->display_info, mode)) {
+		if (connector->ycbcr_420_allowed)
+			fmt = HDMI_COLORSPACE_YUV420;
+		else
+			return MODE_NO_420;
+	} else if (drm_mode_is_420_also(&connector->display_info, mode) &&
+		   connector->ycbcr_420_allowed) {
+		fmt = HDMI_COLORSPACE_YUV420;
+	} else {
+		fmt = HDMI_COLORSPACE_RGB;
+	}
 
-	clock = drm_hdmi_compute_mode_clock(mode, 8, HDMI_COLORSPACE_RGB);
+	clock = drm_hdmi_compute_mode_clock(mode, 8, fmt);
 	if (!clock)
 		return MODE_ERROR;
 

-- 
2.52.0

[PATCH v7 07/22] drm/i915: Implement the "color format" DRM property

[Nicolas Frattaroli]

This includes RGB, YUV420, YUV444 and Auto. Auto will pick RGB, unless
the mode being asked for is YUV420-only, in which case it picks YUV420.

Should the explicitly requested color format not be supported by the
sink, then an error is returned to userspace, so that it can make a
better choice.

Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Co-developed-by: Andri Yngvason <andri@yngvason.is>
Signed-off-by: Andri Yngvason <andri@yngvason.is>
Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/i915/display/intel_connector.c     | 11 +++++
 drivers/gpu/drm/i915/display/intel_connector.h     |  1 +
 drivers/gpu/drm/i915/display/intel_display_types.h | 15 ++++++
 drivers/gpu/drm/i915/display/intel_dp.c            | 55 +++++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_dp.h            |  4 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c        | 36 +++++++++++++-
 drivers/gpu/drm/i915/display/intel_hdmi.c          | 54 +++++++++++++++------
 7 files changed, 148 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
index 682bf1be350d..2d690e0400b5 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -38,6 +38,10 @@
 #include "intel_hdcp.h"
 #include "intel_panel.h"
 
+static const u32 supported_colorformats = DRM_COLOR_FORMAT_RGB444 |
+					  DRM_COLOR_FORMAT_YCBCR444 |
+					  DRM_COLOR_FORMAT_YCBCR420;
+
 static void intel_connector_modeset_retry_work_fn(struct work_struct *work)
 {
 	struct intel_connector *connector = container_of(work, typeof(*connector),
@@ -321,6 +325,13 @@ intel_attach_dp_colorspace_property(struct drm_connector *connector)
 		drm_connector_attach_colorspace_property(connector);
 }
 
+void
+intel_attach_colorformat_property(struct drm_connector *connector)
+{
+	if (!drm_mode_create_color_format_property(connector, supported_colorformats))
+		drm_connector_attach_color_format_property(connector);
+}
+
 void
 intel_attach_scaling_mode_property(struct drm_connector *connector)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h
index 0aa86626e646..fe6149d1d559 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.h
+++ b/drivers/gpu/drm/i915/display/intel_connector.h
@@ -31,6 +31,7 @@ void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
 void intel_attach_aspect_ratio_property(struct drm_connector *connector);
 void intel_attach_hdmi_colorspace_property(struct drm_connector *connector);
 void intel_attach_dp_colorspace_property(struct drm_connector *connector);
+void intel_attach_colorformat_property(struct drm_connector *connector);
 void intel_attach_scaling_mode_property(struct drm_connector *connector);
 void intel_connector_queue_modeset_retry_work(struct intel_connector *connector);
 void intel_connector_cancel_modeset_retry_work(struct intel_connector *connector);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index e6298279dc89..520d5670f8da 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -2251,6 +2251,21 @@ to_intel_frontbuffer(struct drm_framebuffer *fb)
 	return fb ? to_intel_framebuffer(fb)->frontbuffer : NULL;
 }
 
+static inline __pure enum drm_color_format
+intel_output_format_to_drm_color_format(enum intel_output_format input)
+{
+	switch (input) {
+	case INTEL_OUTPUT_FORMAT_RGB:
+		return DRM_COLOR_FORMAT_RGB444;
+	case INTEL_OUTPUT_FORMAT_YCBCR444:
+		return DRM_COLOR_FORMAT_YCBCR444;
+	case INTEL_OUTPUT_FORMAT_YCBCR420:
+		return DRM_COLOR_FORMAT_YCBCR420;
+	}
+
+	return DRM_COLOR_FORMAT_AUTO;
+}
+
 /*
  * Conversion functions/macros from various pointer types to struct
  * intel_display pointer.
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 79fd3b8d8b25..063c8514fd41 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1178,7 +1178,7 @@ dfp_can_convert(struct intel_dp *intel_dp,
 	return false;
 }
 
-static enum intel_output_format
+enum intel_output_format
 intel_dp_output_format(struct intel_connector *connector,
 		       enum intel_output_format sink_format)
 {
@@ -3187,17 +3187,24 @@ intel_dp_compute_output_format(struct intel_encoder *encoder,
 	struct intel_connector *connector = intel_dp->attached_connector;
 	const struct drm_display_info *info = &connector->base.display_info;
 	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
-	bool ycbcr_420_only;
+	enum drm_color_format sink_format_drm;
 	int ret;
 
-	ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
+	if ((conn_state->color_format == DRM_COLOR_FORMAT_YCBCR420 &&
+	     drm_mode_is_420(info, adjusted_mode)) ||
+	    (conn_state->color_format == DRM_COLOR_FORMAT_AUTO &&
+	     drm_mode_is_420_only(info, adjusted_mode)))
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	else if (conn_state->color_format == DRM_COLOR_FORMAT_YCBCR444)
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR444;
+	else
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 
-	if (ycbcr_420_only && !connector->base.ycbcr_420_allowed) {
+	if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+	    !connector->base.ycbcr_420_allowed) {
 		drm_dbg_kms(display->drm,
-			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
-		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
-	} else {
-		crtc_state->sink_format = intel_dp_sink_format(connector, adjusted_mode);
+			"YCbCr 4:2:0 mode requested but unsupported by connector.\n");
+		return -EINVAL;
 	}
 
 	crtc_state->output_format = intel_dp_output_format(connector, crtc_state->sink_format);
@@ -3205,9 +3212,20 @@ intel_dp_compute_output_format(struct intel_encoder *encoder,
 	ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
 					   respect_downstream_limits);
 	if (ret) {
-		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
-		    !connector->base.ycbcr_420_allowed ||
-		    !drm_mode_is_420_also(info, adjusted_mode))
+		/*
+		 * If no valid link config can be found due to bandwidth constraints,
+		 * degrade from RGB/YCbCr 4:4:4 to YCbCr 4:2:0 if permitted by
+		 * the source and sink.
+		 */
+		if (!connector->base.ycbcr_420_allowed)
+			return ret;
+		/* No point in trying YCbCr420 a second time. */
+		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+			return ret;
+		if (!drm_mode_is_420(info, adjusted_mode))
+			return ret;
+		/* If a non-AUTO color format is chosen, don't fall back. */
+		if (conn_state->color_format)
 			return ret;
 
 		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
@@ -3215,9 +3233,20 @@ intel_dp_compute_output_format(struct intel_encoder *encoder,
 								   crtc_state->sink_format);
 		ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state,
 						   respect_downstream_limits);
+		if (ret)
+			return ret;
 	}
 
-	return ret;
+	sink_format_drm = intel_output_format_to_drm_color_format(crtc_state->sink_format);
+	if (conn_state->color_format && conn_state->color_format != sink_format_drm) {
+		drm_dbg_kms(display->drm,
+			    "Explicitly asked for color format %s, got sink format %s\n",
+			    drm_get_color_format_name(conn_state->color_format),
+			    drm_get_color_format_name(sink_format_drm));
+		return -EINVAL;
+	}
+
+	return 0;
 }
 
 void
@@ -6684,6 +6713,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *_connec
 		intel_attach_dp_colorspace_property(&connector->base);
 	}
 
+	intel_attach_colorformat_property(&connector->base);
+
 	if (intel_dp_has_gamut_metadata_dip(&dp_to_dig_port(intel_dp)->base))
 		drm_connector_attach_hdr_output_metadata_property(&connector->base);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 25bfbfd291b0..fd47a4bcba6e 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -205,6 +205,10 @@ bool intel_dp_compute_config_limits(struct intel_dp *intel_dp,
 void intel_dp_get_dsc_sink_cap(u8 dpcd_rev,
 			       const struct drm_dp_desc *desc, bool is_branch,
 			       struct intel_connector *connector);
+enum intel_output_format
+intel_dp_output_format(struct intel_connector *connector,
+		       enum intel_output_format sink_format);
+
 bool intel_dp_has_gamut_metadata_dip(struct intel_encoder *encoder);
 
 bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 24f8e60df9ac..5118decfbc97 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -607,6 +607,8 @@ static int mst_stream_compute_config(struct intel_encoder *encoder,
 		to_intel_connector(conn_state->connector);
 	const struct drm_display_mode *adjusted_mode =
 		&pipe_config->hw.adjusted_mode;
+	const struct drm_display_info *info = &connector->base.display_info;
+	enum drm_color_format sink_format_drm;
 	struct link_config_limits limits;
 	bool dsc_needed, joiner_needs_dsc;
 	int num_joined_pipes;
@@ -625,10 +627,35 @@ static int mst_stream_compute_config(struct intel_encoder *encoder,
 	if (num_joined_pipes > 1)
 		pipe_config->joiner_pipes = GENMASK(crtc->pipe + num_joined_pipes - 1, crtc->pipe);
 
-	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
-	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
+	if ((conn_state->color_format == DRM_COLOR_FORMAT_YCBCR420 &&
+	     drm_mode_is_420(info, adjusted_mode)) ||
+	    (conn_state->color_format == DRM_COLOR_FORMAT_AUTO &&
+	     drm_mode_is_420_only(info, adjusted_mode)))
+		pipe_config->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	else if (conn_state->color_format == DRM_COLOR_FORMAT_YCBCR444)
+		pipe_config->sink_format = INTEL_OUTPUT_FORMAT_YCBCR444;
+	else
+		pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
+
+	if (pipe_config->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+	    !connector->base.ycbcr_420_allowed) {
+		drm_dbg_kms(display->drm,
+			    "YCbCr 4:2:0 mode requested but unsupported by connector.\n");
+		return -EINVAL;
+	}
+
+	pipe_config->output_format = intel_dp_output_format(connector, pipe_config->sink_format);
 	pipe_config->has_pch_encoder = false;
 
+	sink_format_drm = intel_output_format_to_drm_color_format(pipe_config->sink_format);
+	if (conn_state->color_format && conn_state->color_format != sink_format_drm) {
+		drm_dbg_kms(display->drm,
+			    "Unsupported color format %s (0x%x), sink has 0x%x\n",
+			    drm_get_color_format_name(conn_state->color_format),
+			    conn_state->color_format, sink_format_drm);
+		return -EINVAL;
+	}
+
 	joiner_needs_dsc = intel_dp_joiner_needs_dsc(display, num_joined_pipes);
 
 	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
@@ -1608,6 +1635,11 @@ static int mst_topology_add_connector_properties(struct intel_dp *intel_dp,
 	if (connector->base.max_bpc_property)
 		drm_connector_attach_max_bpc_property(&connector->base, 6, 12);
 
+	connector->base.color_format_property =
+		intel_dp->attached_connector->base.color_format_property;
+	if (connector->base.color_format_property)
+		intel_attach_colorformat_property(&connector->base);
+
 	return drm_connector_set_path_property(&connector->base, pathprop);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 05e898d10a2b..8c402a0794d0 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2270,30 +2270,54 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
 	struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 	const struct drm_display_info *info = &connector->base.display_info;
-	bool ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
+	enum drm_color_format req_fmt = conn_state->color_format;
+	enum drm_color_format sink_format_drm;
 	int ret;
 
-	crtc_state->sink_format =
-		intel_hdmi_sink_format(crtc_state, connector, ycbcr_420_only);
-
-	if (ycbcr_420_only && crtc_state->sink_format != INTEL_OUTPUT_FORMAT_YCBCR420) {
-		drm_dbg_kms(display->drm,
-			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible. Falling back to RGB.\n");
-		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_RGB;
-	}
+	if (!req_fmt)
+		crtc_state->sink_format =
+			intel_hdmi_sink_format(crtc_state, connector,
+					       drm_mode_is_420_only(info, adjusted_mode));
+	else if (req_fmt == DRM_COLOR_FORMAT_YCBCR444)
+		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR444;
+	else
+		crtc_state->sink_format =
+			intel_hdmi_sink_format(crtc_state, connector,
+					       req_fmt == DRM_COLOR_FORMAT_YCBCR420);
 
 	crtc_state->output_format = intel_hdmi_output_format(crtc_state);
 	ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
 	if (ret) {
-		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
-		    !crtc_state->has_hdmi_sink ||
-		    !connector->base.ycbcr_420_allowed ||
-		    !drm_mode_is_420_also(info, adjusted_mode))
+		/*
+		 * If no valid link config can be found due to bandwidth constraints,
+		 * degrade from RGB/YCbCr 4:4:4 to YCbCr 4:2:0 if permitted by
+		 * the source and sink.
+		 */
+		if (!connector->base.ycbcr_420_allowed)
+			return ret;
+		/* No point in trying YCbCr420 a second time. */
+		if (crtc_state->sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+			return ret;
+		if (!drm_mode_is_420(info, adjusted_mode))
+			return ret;
+		/* If a non-AUTO color format is chosen, don't fall back. */
+		if (req_fmt)
 			return ret;
 
 		crtc_state->sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 		crtc_state->output_format = intel_hdmi_output_format(crtc_state);
 		ret = intel_hdmi_compute_clock(encoder, crtc_state, respect_downstream_limits);
+		if (ret)
+			return ret;
+	}
+
+	sink_format_drm = intel_output_format_to_drm_color_format(crtc_state->sink_format);
+	if (req_fmt && req_fmt != sink_format_drm) {
+		drm_dbg_kms(display->drm,
+			    "Explicitly asked for color format %s, got sink format %s\n",
+			    drm_get_color_format_name(req_fmt),
+			    drm_get_color_format_name(sink_format_drm));
+		ret = -EINVAL;
 	}
 
 	return ret;
@@ -2689,8 +2713,10 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *_
 	if (DISPLAY_VER(display) >= 10)
 		drm_connector_attach_hdr_output_metadata_property(&connector->base);
 
-	if (!HAS_GMCH(display))
+	if (!HAS_GMCH(display)) {
 		drm_connector_attach_max_bpc_property(&connector->base, 8, 12);
+		intel_attach_colorformat_property(&connector->base);
+	}
 }
 
 /*

-- 
2.52.0

[PATCH v7 08/22] drm/amdgpu: Implement "color format" DRM property

[Nicolas Frattaroli]

The "color format" DRM property allows userspace to explicitly pick a
color format to use. If an unsupported color format is requested,
userspace will be given an error instead of silently having its request
disobeyed.

The default case, which is AUTO, picks YCbCr 4:2:0 if it's a 4:2:0-only
mode, and RGB in all other cases.

Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Co-developed-by: Andri Yngvason <andri@yngvason.is>
Signed-off-by: Andri Yngvason <andri@yngvason.is>
Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 87 +++++++++++++++++++---
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    | 13 ++++
 2 files changed, 90 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4a2491a66fdd..a3edf6ce5c5a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6681,11 +6681,14 @@ static void fill_stream_properties_from_drm_display_mode(
 	const struct dc_stream_state *old_stream,
 	int requested_bpc)
 {
+	bool is_dp_or_hdmi = dc_is_hdmi_signal(stream->signal) || dc_is_dp_signal(stream->signal);
 	struct dc_crtc_timing *timing_out = &stream->timing;
 	const struct drm_display_info *info = &connector->display_info;
 	struct amdgpu_dm_connector *aconnector = NULL;
 	struct hdmi_vendor_infoframe hv_frame;
 	struct hdmi_avi_infoframe avi_frame;
+	bool want_420;
+	bool want_422;
 	ssize_t err;
 
 	if (connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK)
@@ -6698,20 +6701,39 @@ static void fill_stream_properties_from_drm_display_mode(
 	timing_out->h_border_right = 0;
 	timing_out->v_border_top = 0;
 	timing_out->v_border_bottom = 0;
-	/* TODO: un-hardcode */
-	if (drm_mode_is_420_only(info, mode_in) ||
-	    (aconnector && aconnector->force_yuv420_output &&
-	     drm_mode_is_420_also(info, mode_in)))
+
+	want_420 = (aconnector && aconnector->force_yuv420_output) ||
+		   (connector_state->color_format == DRM_COLOR_FORMAT_YCBCR420);
+	want_422 = (aconnector && aconnector->force_yuv422_output) ||
+		   (connector_state->color_format == DRM_COLOR_FORMAT_YCBCR422);
+
+	if (drm_mode_is_420_only(info, mode_in) && (want_420 || !connector_state->color_format)) {
 		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
-	else if ((connector->display_info.color_formats & DRM_COLOR_FORMAT_YCBCR422)
-			&& aconnector
-			&& aconnector->force_yuv422_output)
+	} else if (drm_mode_is_420_also(info, mode_in) && want_420) {
+		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
+	} else if ((info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
+		   && want_422 && is_dp_or_hdmi) {
 		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR422;
-	else if ((connector->display_info.color_formats & DRM_COLOR_FORMAT_YCBCR444)
-			&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
+	} else if (connector_state->color_format == DRM_COLOR_FORMAT_YCBCR444 &&
+		   (info->color_formats & DRM_COLOR_FORMAT_YCBCR444) && is_dp_or_hdmi) {
 		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
-	else
+	} else if ((connector_state->color_format == DRM_COLOR_FORMAT_RGB444 ||
+		   !connector_state->color_format)) {
 		timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
+	} else {
+		/*
+		 * If a format was explicitly requested but the requested format
+		 * can't be satisfied, set it to an invalid value so that an
+		 * error bubbles up to userspace. This way, userspace knows it
+		 * needs to make a better choice.
+		 */
+		if (connector_state->color_format)
+			timing_out->pixel_encoding = PIXEL_ENCODING_UNDEFINED;
+		else if (drm_mode_is_420_only(info, mode_in))
+			timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
+		else
+			timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
+	}
 
 	timing_out->timing_3d_format = TIMING_3D_FORMAT_NONE;
 	timing_out->display_color_depth = convert_color_depth_from_display_info(
@@ -8059,6 +8081,38 @@ static enum dc_status dm_validate_stream_and_context(struct dc *dc,
 	return dc_result;
 }
 
+static enum dc_status
+dm_validate_stream_color_format(const struct drm_connector_state *drm_state,
+				const struct dc_stream_state *stream)
+{
+	enum dc_pixel_encoding encoding;
+
+	if (!drm_state->color_format)
+		return DC_OK;
+
+	switch (drm_state->color_format) {
+	case DRM_COLOR_FORMAT_AUTO:
+	case DRM_COLOR_FORMAT_RGB444:
+		encoding = PIXEL_ENCODING_RGB;
+		break;
+	case DRM_COLOR_FORMAT_YCBCR444:
+		encoding = PIXEL_ENCODING_YCBCR444;
+		break;
+	case DRM_COLOR_FORMAT_YCBCR422:
+		encoding = PIXEL_ENCODING_YCBCR422;
+		break;
+	case DRM_COLOR_FORMAT_YCBCR420:
+		encoding = PIXEL_ENCODING_YCBCR420;
+		break;
+	default:
+		encoding = PIXEL_ENCODING_UNDEFINED;
+		break;
+	}
+
+	return encoding == stream->timing.pixel_encoding ?
+		DC_OK : DC_UNSUPPORTED_VALUE;
+}
+
 struct dc_stream_state *
 create_validate_stream_for_sink(struct drm_connector *connector,
 				const struct drm_display_mode *drm_mode,
@@ -8105,6 +8159,9 @@ create_validate_stream_for_sink(struct drm_connector *connector,
 		if (dc_result == DC_OK)
 			dc_result = dm_validate_stream_and_context(adev->dm.dc, stream);
 
+		if (dc_result == DC_OK)
+			dc_result = dm_validate_stream_color_format(drm_state, stream);
+
 		if (dc_result != DC_OK) {
 			drm_dbg_kms(connector->dev, "Pruned mode %d x %d (clk %d) %s %s -- %s\n",
 				      drm_mode->hdisplay,
@@ -8928,6 +8985,12 @@ static const u32 supported_colorspaces =
 	BIT(DRM_MODE_COLORIMETRY_BT2020_RGB) |
 	BIT(DRM_MODE_COLORIMETRY_BT2020_YCC);
 
+static const u32 supported_colorformats =
+	DRM_COLOR_FORMAT_RGB444 |
+	DRM_COLOR_FORMAT_YCBCR444 |
+	DRM_COLOR_FORMAT_YCBCR422 |
+	DRM_COLOR_FORMAT_YCBCR420;
+
 void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 				     struct amdgpu_dm_connector *aconnector,
 				     int connector_type,
@@ -9049,6 +9112,10 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 
 		if (adev->dm.hdcp_workqueue)
 			drm_connector_attach_content_protection_property(&aconnector->base, true);
+
+		if (!drm_mode_create_color_format_property(&aconnector->base,
+							   supported_colorformats))
+			drm_connector_attach_color_format_property(&aconnector->base);
 	}
 
 	if (connector_type == DRM_MODE_CONNECTOR_eDP) {
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 5e92eaa67aa3..a7b1274ead8d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -52,6 +52,12 @@
 
 #define PEAK_FACTOR_X1000 1006
 
+static const u32 supported_colorformats =
+	DRM_COLOR_FORMAT_RGB444 |
+	DRM_COLOR_FORMAT_YCBCR444 |
+	DRM_COLOR_FORMAT_YCBCR422 |
+	DRM_COLOR_FORMAT_YCBCR420;
+
 /*
  * This function handles both native AUX and I2C-Over-AUX transactions.
  */
@@ -679,6 +685,13 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	if (connector->max_bpc_property)
 		drm_connector_attach_max_bpc_property(connector, 8, 16);
 
+	connector->color_format_property = master->base.color_format_property;
+	if (connector->color_format_property) {
+		if (!drm_mode_create_color_format_property(&aconnector->base,
+							   supported_colorformats))
+			drm_connector_attach_color_format_property(&aconnector->base);
+	}
+
 	connector->vrr_capable_property = master->base.vrr_capable_property;
 	if (connector->vrr_capable_property)
 		drm_connector_attach_vrr_capable_property(connector);

-- 
2.52.0

[PATCH v7 09/22] drm/rockchip: Add YUV422 output mode constants for VOP2

[Nicolas Frattaroli]

The Rockchip display controller has a general YUV422 output mode, and
some SoC-specific connector-specific output modes for RK3576.

Add them, based on the values in downstream and the TRM (dsp_out_mode in
RK3576 TRM Part 2, register POST*_CTRL_POST_DSP_CTRL).

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 2e86ad00979c..4705dc6b8bd7 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -30,10 +30,14 @@
 #define ROCKCHIP_OUT_MODE_P565		2
 #define ROCKCHIP_OUT_MODE_BT656		5
 #define ROCKCHIP_OUT_MODE_S888		8
+#define ROCKCHIP_OUT_MODE_YUV422	9
 #define ROCKCHIP_OUT_MODE_S888_DUMMY	12
 #define ROCKCHIP_OUT_MODE_YUV420	14
 /* for use special outface */
 #define ROCKCHIP_OUT_MODE_AAAA		15
+/* SoC specific output modes */
+#define ROCKCHIP_OUT_MODE_YUV422_RK3576_DP	12
+#define ROCKCHIP_OUT_MODE_YUV422_RK3576_HDMI	13
 
 /* output flags */
 #define ROCKCHIP_OUTPUT_DSI_DUAL	BIT(0)

-- 
2.52.0

[PATCH v7 10/22] drm/rockchip: vop2: Fix YUV444 output

[Nicolas Frattaroli]

YUV444 (aka YCbCr444) output isn't working quite right on RK3588. The
resulting image on the display, while identifying itself as YUV444, has
some components swapped, even after adding the necessary DRM formats to
the conversion functions.

Judging by downstream, this is because YUV444 also needs an rb swap
performed in the AFBC case.

Add the DRM formats to the appropriate switch statements, and add a
function for checking whether an rb swap needs to be performed in the
AFBC case.

Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index ec3b4fde10db..469c63dd97d5 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -176,6 +176,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
 	case DRM_FORMAT_ARGB2101010:
 	case DRM_FORMAT_XBGR2101010:
 	case DRM_FORMAT_ABGR2101010:
+	case DRM_FORMAT_VUY101010:
 		return VOP2_FMT_XRGB101010;
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_ARGB8888:
@@ -184,6 +185,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
 		return VOP2_FMT_ARGB8888;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_BGR888:
+	case DRM_FORMAT_VUY888:
 		return VOP2_FMT_RGB888;
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_BGR565:
@@ -225,6 +227,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
 	case DRM_FORMAT_ARGB2101010:
 	case DRM_FORMAT_XBGR2101010:
 	case DRM_FORMAT_ABGR2101010:
+	case DRM_FORMAT_VUY101010:
 		return VOP2_AFBC_FMT_ARGB2101010;
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_ARGB8888:
@@ -233,6 +236,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
 		return VOP2_AFBC_FMT_ARGB8888;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_BGR888:
+	case DRM_FORMAT_VUY888:
 		return VOP2_AFBC_FMT_RGB888;
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_BGR565:
@@ -270,6 +274,19 @@ static bool vop2_win_rb_swap(u32 format)
 	}
 }
 
+static bool vop2_afbc_rb_swap(u32 format)
+{
+	switch (format) {
+	case DRM_FORMAT_NV24:
+	case DRM_FORMAT_NV30:
+	case DRM_FORMAT_VUY888:
+	case DRM_FORMAT_VUY101010:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static bool vop2_afbc_uv_swap(u32 format)
 {
 	switch (format) {
@@ -1291,6 +1308,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
 		 /* It's for head stride, each head size is 16 byte */
 		stride = ALIGN(stride, block_w) / block_w * 16;
 
+		rb_swap = vop2_afbc_rb_swap(fb->format->format);
 		uv_swap = vop2_afbc_uv_swap(fb->format->format);
 		/*
 		 * This is a workaround for crazy IC design, Cluster
@@ -1308,6 +1326,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
 			vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1);
 		vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format);
 		vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap);
+		vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap);
 		/*
 		 * On rk3566/8, this bit is auto gating enable,
 		 * but this function is not work well so we need

-- 
2.52.0

[PATCH v7 11/22] drm/rockchip: vop2: Add RK3576 to the RG swap special case

[Nicolas Frattaroli]

Much like RK3588, RK3576 requires an RG swap to be performed for YUV444
8-bit and YUV444 10-bit bus formats.

Add its version to the already existing check for RK3588, so that YUV444
output is correct on this platform.

Fixes: 944757a4cba6 ("drm/rockchip: vop2: Add support for rk3576")
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 469c63dd97d5..f21efbca42e6 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -354,7 +354,8 @@ static bool vop2_output_uv_swap(u32 bus_format, u32 output_mode)
 
 static bool vop2_output_rg_swap(struct vop2 *vop2, u32 bus_format)
 {
-	if (vop2->version == VOP_VERSION_RK3588) {
+	if (vop2->version == VOP_VERSION_RK3588 ||
+	    vop2->version == VOP_VERSION_RK3576) {
 		if (bus_format == MEDIA_BUS_FMT_YUV8_1X24 ||
 		    bus_format == MEDIA_BUS_FMT_YUV10_1X30)
 			return true;

-- 
2.52.0

[PATCH v7 12/22] drm/rockchip: vop2: Recognise 10/12-bit YUV422 as YUV formats

[Nicolas Frattaroli]

The Rockchip VOP2 video output driver has a "is_yuv_output" function,
which returns true when a given bus format is a YUV format, and false
otherwise.

This switch statement is lacking the bus format used for YUV422 10-bit,
as well as the bus format used for YUV422 12-bit.

Add MEDIA_BUS_FMT_YUYV10_1X20 and MEDIA_BUS_FMT_YUYV12_1X24 to
is_yuv_output's switch cases to resolve this.

Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index f21efbca42e6..3dd7b7571487 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -369,6 +369,9 @@ static bool is_yuv_output(u32 bus_format)
 	switch (bus_format) {
 	case MEDIA_BUS_FMT_YUV8_1X24:
 	case MEDIA_BUS_FMT_YUV10_1X30:
+	case MEDIA_BUS_FMT_YUYV10_1X20:
+	case MEDIA_BUS_FMT_UYVY10_1X20:
+	case MEDIA_BUS_FMT_YUYV12_1X24:
 	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
 	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
 	case MEDIA_BUS_FMT_YUYV8_2X8:

-- 
2.52.0

[PATCH v7 13/22] drm/rockchip: vop2: Set correct output format for RK3576 YUV422

[Nicolas Frattaroli]

For RK3576 to be able to output YUV422 signals, it first needs to be
able to pick the right output mode in the display controller to do so.

The RK3576 hardware specifies different output formats depending on the
used display protocol.

Adjust the written register value based on the SoC and connector, so
other users of vcstate->output_mode don't have to care about this.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 3dd7b7571487..d93c6779286e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1706,6 +1706,22 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
 	if (vcstate->output_mode == ROCKCHIP_OUT_MODE_AAAA &&
 	    !(vp_data->feature & VOP2_VP_FEATURE_OUTPUT_10BIT))
 		out_mode = ROCKCHIP_OUT_MODE_P888;
+	else if (vcstate->output_mode == ROCKCHIP_OUT_MODE_YUV422 &&
+		 vop2->version == VOP_VERSION_RK3576)
+		switch (vcstate->output_type) {
+		case DRM_MODE_CONNECTOR_DisplayPort:
+		case DRM_MODE_CONNECTOR_eDP:
+			out_mode = ROCKCHIP_OUT_MODE_YUV422_RK3576_DP;
+			break;
+		case DRM_MODE_CONNECTOR_HDMIA:
+			out_mode = ROCKCHIP_OUT_MODE_YUV422_RK3576_HDMI;
+			break;
+		default:
+			drm_err(vop2->drm, "Unknown DRM_MODE_CONNECTOR %d\n",
+				vcstate->output_type);
+			vop2_unlock(vop2);
+			return;
+		}
 	else
 		out_mode = vcstate->output_mode;
 

-- 
2.52.0

[PATCH v7 14/22] drm/bridge: dw-hdmi-qp: Implement atomic_get_output_bus_fmts

[Nicolas Frattaroli]

The atomic_get_output_bus_fmts callback is used by the DRM bridge layer
to recursively select a suitable output format in a bridge chain.

As a bridge that outputs to HDMI, dw-hdmi-qp will have its output
formats determined by which formats the platform-specific integration of
the hardware supports, and the chosen HDMI output bit depth.

Implement this callback. The returned u32* buffer is supposed to be
freed by the caller of this callback, as specified by the callback's
documentation.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 64 ++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index ab7fed6214e0..82a63964f7f7 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -11,6 +11,7 @@
 #include <linux/export.h>
 #include <linux/i2c.h>
 #include <linux/irq.h>
+#include <linux/media-bus-format.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
@@ -862,6 +863,68 @@ static int dw_hdmi_qp_config_audio_infoframe(struct dw_hdmi_qp *hdmi,
 	return 0;
 }
 
+static u32*
+dw_hdmi_qp_bridge_get_output_bus_fmts(struct drm_bridge *bridge,
+				      struct drm_bridge_state *bridge_state,
+				      struct drm_crtc_state *crtc_state,
+				      struct drm_connector_state *conn_state,
+				      unsigned int *num_output_fmts)
+{
+	unsigned int num_fmts = 0;
+	u32 *out_fmts;
+
+	/*
+	 * bridge->supported_formats is a bit field of the HDMI_COLORSPACE_* enums.
+	 * These enums are defined by the HDMI standard, and currently top out at
+	 * 7. Consequently, BIT(7) is the highest bit that will be set here, unless
+	 * the standard runs out of reserved pixel formats. Therefore, hweight8()
+	 * will give an accurate count of how many bus formats we'll output.
+	 */
+	out_fmts = kmalloc_array(hweight8(bridge->supported_formats), sizeof(u32),
+				 GFP_KERNEL);
+	if (!out_fmts) {
+		*num_output_fmts = 0;
+		return NULL;
+	}
+
+	switch (conn_state->hdmi.output_bpc) {
+	case 12:
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_RGB))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_RGB121212_1X36;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV444))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_YUV12_1X36;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV422))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYVY12_1X24;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV420))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYYVYY12_0_5X36;
+		break;
+	case 10:
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_RGB))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_RGB101010_1X30;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV444))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_YUV10_1X30;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV422))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYVY10_1X20;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV420))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYYVYY10_0_5X30;
+		break;
+	default:
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_RGB))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_RGB888_1X24;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV444))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_YUV8_1X24;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV422))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYVY8_1X16;
+		if (bridge->supported_formats & BIT(HDMI_COLORSPACE_YUV420))
+			out_fmts[num_fmts++] = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
+		break;
+	}
+
+	*num_output_fmts = num_fmts;
+
+	return out_fmts;
+}
+
 static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 					    struct drm_atomic_state *state)
 {
@@ -1213,6 +1276,7 @@ static int dw_hdmi_qp_cec_transmit(struct drm_bridge *bridge, u8 attempts,
 #endif /* CONFIG_DRM_DW_HDMI_QP_CEC */
 
 static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
+	.atomic_get_output_bus_fmts = dw_hdmi_qp_bridge_get_output_bus_fmts,
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_reset = drm_atomic_helper_bridge_reset,

-- 
2.52.0

[PATCH v7 15/22] drm/rockchip: dw_hdmi_qp: Implement "color format" DRM property

[Nicolas Frattaroli]

Switch between requested color formats by setting the right bus formats,
configuring the VO GRF registers, and setting the right output mode.

To do this, the encoder's atomic_check queries the bus format of the
first bridge, which was determined by the bridge chain recursive format
selection. Pick the input format if it's !FIXED, otherwise, pick the
output format.

The previously unused GRF register color format defines are redone as
well. Both RK3588 and RK3576 use the same defines; it didn't look like
this as there was a typo in the previously (unused) definition.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 107 ++++++++++++++++++++++---
 1 file changed, 98 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 1a09bcc96c3e..d39a4284b92c 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -11,6 +11,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/hw_bitfield.h>
 #include <linux/mfd/syscon.h>
+#include <linux/media-bus-format.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
@@ -43,10 +44,6 @@
 #define RK3576_8BPC			0x0
 #define RK3576_10BPC			0x6
 #define RK3576_COLOR_FORMAT_MASK	GENMASK(7, 4)
-#define RK3576_RGB			0x9
-#define RK3576_YUV422			0x1
-#define RK3576_YUV444			0x2
-#define RK3576_YUV420			0x3
 #define RK3576_CECIN_MASK		BIT(3)
 
 #define RK3576_VO0_GRF_SOC_CON14	0x0038
@@ -74,8 +71,6 @@
 #define RK3588_8BPC			0x0
 #define RK3588_10BPC			0x6
 #define RK3588_COLOR_FORMAT_MASK	GENMASK(3, 0)
-#define RK3588_RGB			0x0
-#define RK3588_YUV420			0x3
 #define RK3588_SCLIN_MASK		BIT(9)
 #define RK3588_SDAIN_MASK		BIT(10)
 #define RK3588_MODE_MASK		BIT(11)
@@ -87,6 +82,11 @@
 #define HOTPLUG_DEBOUNCE_MS		150
 #define MAX_HDMI_PORT_NUM		2
 
+#define RK_COLOR_FMT_RGB		0x0
+#define RK_COLOR_FMT_YUV422		0x1
+#define RK_COLOR_FMT_YUV444		0x2
+#define RK_COLOR_FMT_YUV420		0x3
+
 struct rockchip_hdmi_qp {
 	struct device *dev;
 	struct regmap *regmap;
@@ -115,6 +115,33 @@ static struct rockchip_hdmi_qp *to_rockchip_hdmi_qp(struct drm_encoder *encoder)
 	return container_of(rkencoder, struct rockchip_hdmi_qp, encoder);
 }
 
+/**
+ * dw_hdmi_qp_rockchip_bus_fmt_to_reg - converts a bus format to a GRF reg value
+ * @bus_fmt: One of the MEDIA_BUS_FMT_s allowed by this driver's atomic_check
+ *
+ * Returns: an unshifted value to be written to the COLOR_FORMAT GRF register
+ * on success, or %-EINVAL if the bus format is not supported.
+ */
+static int __pure dw_hdmi_qp_rockchip_bus_fmt_to_reg(u32 bus_fmt)
+{
+	switch (bus_fmt) {
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB101010_1X30:
+		return RK_COLOR_FMT_RGB;
+	case MEDIA_BUS_FMT_UYVY8_1X16:
+	case MEDIA_BUS_FMT_UYVY10_1X20:
+		return RK_COLOR_FMT_YUV422;
+	case MEDIA_BUS_FMT_YUV8_1X24:
+	case MEDIA_BUS_FMT_YUV10_1X30:
+		return RK_COLOR_FMT_YUV444;
+	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
+		return RK_COLOR_FMT_YUV420;
+	}
+
+	return -EINVAL;
+}
+
 static void dw_hdmi_qp_rockchip_encoder_enable(struct drm_encoder *encoder)
 {
 	struct rockchip_hdmi_qp *hdmi = to_rockchip_hdmi_qp(encoder);
@@ -130,29 +157,83 @@ static void dw_hdmi_qp_rockchip_encoder_enable(struct drm_encoder *encoder)
 		hdmi->ctrl_ops->enc_init(hdmi, to_rockchip_crtc_state(crtc->state));
 }
 
+/**
+ * dw_hdmi_qp_rockchip_get_vop_format - get the bus format VOP should output
+ * @encoder: pointer to a &struct drm_encoder
+ * @conn_state: pointer to the current atomic &struct drm_connector_state
+ *
+ * Determines which bus format the Rockchip video processor should output as
+ * to feed into the bridge chain.
+ *
+ * Returns a MEDIA_BUS_FMT_* on success, or %0 on error.
+ */
+static u32 dw_hdmi_qp_rockchip_get_vop_format(struct drm_encoder *encoder,
+					      struct drm_connector_state *conn_state)
+{
+	struct drm_bridge *bridge __free(drm_bridge_put) = NULL;
+	struct drm_bridge_state *bstate;
+
+	bridge = drm_bridge_chain_get_first_bridge(encoder);
+	if (!bridge)
+		return 0;
+
+	bstate = drm_atomic_get_bridge_state(conn_state->state, bridge);
+	if (!bstate)
+		return 0;
+
+	if (bstate->input_bus_cfg.format != MEDIA_BUS_FMT_FIXED)
+		return bstate->input_bus_cfg.format;
+
+	return bstate->output_bus_cfg.format;
+}
+
 static int
 dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
 					 struct drm_crtc_state *crtc_state,
 					 struct drm_connector_state *conn_state)
 {
-	struct rockchip_hdmi_qp *hdmi = to_rockchip_hdmi_qp(encoder);
 	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+	struct rockchip_hdmi_qp *hdmi = to_rockchip_hdmi_qp(encoder);
 	union phy_configure_opts phy_cfg = {};
+	u32 ingest_fmt;
 	int ret;
 
+	ingest_fmt = dw_hdmi_qp_rockchip_get_vop_format(encoder, conn_state);
+	if (!ingest_fmt)
+		return -EINVAL;
+
 	if (hdmi->tmds_char_rate == conn_state->hdmi.tmds_char_rate &&
-	    s->output_bpc == conn_state->hdmi.output_bpc)
+	    s->output_bpc == conn_state->hdmi.output_bpc &&
+	    s->bus_format == ingest_fmt)
 		return 0;
 
+	switch (ingest_fmt) {
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB101010_1X30:
+	case MEDIA_BUS_FMT_YUV8_1X24:
+	case MEDIA_BUS_FMT_YUV10_1X30:
+		s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
+		break;
+	case MEDIA_BUS_FMT_UYVY8_1X16:
+		s->output_mode = ROCKCHIP_OUT_MODE_YUV422;
+		break;
+	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
+		s->output_mode = ROCKCHIP_OUT_MODE_YUV420;
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	phy_cfg.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
 	phy_cfg.hdmi.bpc = conn_state->hdmi.output_bpc;
 
 	ret = phy_configure(hdmi->phy, &phy_cfg);
 	if (!ret) {
 		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
-		s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
 		s->output_type = DRM_MODE_CONNECTOR_HDMIA;
 		s->output_bpc = conn_state->hdmi.output_bpc;
+		s->bus_format = ingest_fmt;
 	} else {
 		dev_err(hdmi->dev, "Failed to configure phy: %d\n", ret);
 	}
@@ -382,6 +463,7 @@ static void dw_hdmi_qp_rk3588_io_init(struct rockchip_hdmi_qp *hdmi)
 static void dw_hdmi_qp_rk3576_enc_init(struct rockchip_hdmi_qp *hdmi,
 				       struct rockchip_crtc_state *state)
 {
+	int color = dw_hdmi_qp_rockchip_bus_fmt_to_reg(state->bus_format);
 	u32 val;
 
 	if (state->output_bpc == 10)
@@ -389,12 +471,16 @@ static void dw_hdmi_qp_rk3576_enc_init(struct rockchip_hdmi_qp *hdmi,
 	else
 		val = FIELD_PREP_WM16(RK3576_COLOR_DEPTH_MASK, RK3576_8BPC);
 
+	if (likely(color > 0))
+		val |= FIELD_PREP_WM16(RK3576_COLOR_FORMAT_MASK, color);
+
 	regmap_write(hdmi->vo_regmap, RK3576_VO0_GRF_SOC_CON8, val);
 }
 
 static void dw_hdmi_qp_rk3588_enc_init(struct rockchip_hdmi_qp *hdmi,
 				       struct rockchip_crtc_state *state)
 {
+	int color = dw_hdmi_qp_rockchip_bus_fmt_to_reg(state->bus_format);
 	u32 val;
 
 	if (state->output_bpc == 10)
@@ -402,6 +488,9 @@ static void dw_hdmi_qp_rk3588_enc_init(struct rockchip_hdmi_qp *hdmi,
 	else
 		val = FIELD_PREP_WM16(RK3588_COLOR_DEPTH_MASK, RK3588_8BPC);
 
+	if (likely(color > 0))
+		val |= FIELD_PREP_WM16(RK3588_COLOR_FORMAT_MASK, color);
+
 	regmap_write(hdmi->vo_regmap,
 		     hdmi->port_id ? RK3588_GRF_VO1_CON6 : RK3588_GRF_VO1_CON3,
 		     val);

-- 
2.52.0

[PATCH v7 16/22] drm/rockchip: dw_hdmi_qp: Set supported_formats platdata

[Nicolas Frattaroli]

With the introduction of the supported_formats member in the
dw-hdmi-qp platform data struct, drivers that have access to this
information should now set it.

Set it in the rockchip dw_hdmi_qp glue driver.

This allows this information to be passed down to the dw-hdmi-qp core,
which sets it in the bridge it creates, and consequently will allow the
common HDMI bridge code to act on it.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index d39a4284b92c..1608b494b757 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -601,6 +601,10 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	plat_data.phy_data = hdmi;
 	plat_data.max_bpc = 10;
 
+	plat_data.supported_formats = BIT(HDMI_COLORSPACE_RGB) |
+				      BIT(HDMI_COLORSPACE_YUV444) |
+				      BIT(HDMI_COLORSPACE_YUV422);
+
 	encoder = &hdmi->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
 

-- 
2.52.0

[PATCH v7 17/22] drm/connector: Register color format property on HDMI connectors

[Nicolas Frattaroli]

The drmm_connector_hdmi_init function can figure out what DRM color
formats are supported by a particular connector based on the supported
HDMI format bitmask that's passed in.

Use it to register the drm color format property.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_connector.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 507787619b80..1bec7e5c2215 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -578,6 +578,7 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
 			     unsigned long supported_formats,
 			     unsigned int max_bpc)
 {
+	u32 supported_drm_formats = 0;
 	int ret;
 
 	if (!vendor || !product)
@@ -627,6 +628,18 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
 	if (max_bpc > 8)
 		drm_connector_attach_hdr_output_metadata_property(connector);
 
+	if (supported_formats & BIT(HDMI_COLORSPACE_RGB))
+		supported_drm_formats |= DRM_COLOR_FORMAT_RGB444;
+	if (supported_formats & BIT(HDMI_COLORSPACE_YUV444))
+		supported_drm_formats |= DRM_COLOR_FORMAT_YCBCR444;
+	if (supported_formats & BIT(HDMI_COLORSPACE_YUV422))
+		supported_drm_formats |= DRM_COLOR_FORMAT_YCBCR422;
+	if (supported_formats & BIT(HDMI_COLORSPACE_YUV420))
+		supported_drm_formats |= DRM_COLOR_FORMAT_YCBCR420;
+
+	if (!drm_mode_create_color_format_property(connector, supported_drm_formats))
+		drm_connector_attach_color_format_property(connector);
+
 	connector->hdmi.funcs = hdmi_funcs;
 
 	return 0;

-- 
2.52.0

[PATCH v7 18/22] drm/tests: hdmi: Add tests for the color_format property

[Nicolas Frattaroli]

Add some KUnit tests to check the color_format property is working as
expected with the HDMI state helper.

Existing tests are extended to also test the DRM_COLOR_FORMAT_AUTO case,
in order to avoid duplicating test cases. For the explicitly selected
color format cases, parameterized tests are added.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 191 +++++++++++++++++++++
 1 file changed, 191 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index 4bdcea3c7435..f9648f9de46b 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -60,6 +60,23 @@ static struct drm_display_mode *find_preferred_mode(struct drm_connector *connec
 	return preferred;
 }
 
+static struct drm_display_mode *find_420_only_mode(struct drm_connector *connector)
+{
+	struct drm_device *drm = connector->dev;
+	struct drm_display_mode *mode;
+
+	mutex_lock(&drm->mode_config.mutex);
+	list_for_each_entry(mode, &connector->modes, head) {
+		if (drm_mode_is_420_only(&connector->display_info, mode)) {
+			mutex_unlock(&drm->mode_config.mutex);
+			return mode;
+		}
+	}
+	mutex_unlock(&drm->mode_config.mutex);
+
+	return NULL;
+}
+
 static int set_connector_edid(struct kunit *test, struct drm_connector *connector,
 			      const void *edid, size_t edid_len)
 {
@@ -1547,6 +1564,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback_yuv420(struct kunit *test)
  *   RGB/10bpc
  * - The chosen mode has a TMDS character rate lower than the display
  *   supports in YUV422/12bpc.
+ * - The HDMI connector state's color format property is unset (i.e. AUTO)
  *
  * Then we will prefer to keep the RGB format with a lower bpc over
  * picking YUV422.
@@ -1609,6 +1627,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback_ignore_yuv422(struct kunit
 
 	conn_state = conn->state;
 	KUNIT_ASSERT_NOT_NULL(test, conn_state);
+	KUNIT_ASSERT_EQ(test, conn_state->color_format, DRM_COLOR_FORMAT_AUTO);
 
 	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 10);
 	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
@@ -1626,6 +1645,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback_ignore_yuv422(struct kunit
  *   RGB/8bpc
  * - The chosen mode has a TMDS character rate lower than the display
  *   supports in YUV420/12bpc.
+ * - The HDMI connector state's color format property is unset (i.e. AUTO)
  *
  * Then we will prefer to keep the RGB format with a lower bpc over
  * picking YUV420.
@@ -1687,6 +1707,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback_ignore_yuv420(struct kunit
 
 	conn_state = conn->state;
 	KUNIT_ASSERT_NOT_NULL(test, conn_state);
+	KUNIT_ASSERT_EQ(test, conn_state->color_format, DRM_COLOR_FORMAT_AUTO);
 
 	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
 	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
@@ -2198,6 +2219,172 @@ static void drm_test_check_disable_connector(struct kunit *test)
 	drm_modeset_acquire_fini(&ctx);
 }
 
+struct color_format_test_param {
+	enum drm_color_format fmt;
+	enum hdmi_colorspace expected;
+	int expected_ret;
+	const char *desc;
+};
+
+/* Test that if:
+ * - an HDMI connector supports RGB, YUV444, YUV422, and YUV420
+ * - the display supports RGB, YUV444, YUV422, and YUV420
+ * - the "color format" property is set
+ * then, for the preferred mode, for a given "color format" option:
+ * - DRM_COLOR_FORMAT_AUTO results in an HDMI output format of RGB
+ * - DRM_COLOR_FORMAT_YCBCR422 results in an HDMI output format of YUV422
+ * - DRM_COLOR_FORMAT_YCBCR420 results in an HDMI output format of YUV420
+ * - DRM_COLOR_FORMAT_YCBCR444 results in an HDMI output format of YUV444
+ * - DRM_COLOR_FORMAT_RGB results in an HDMI output format of RGB
+ */
+static void drm_test_check_hdmi_color_format(struct kunit *test)
+{
+	const struct color_format_test_param *param = test->param_value;
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	struct drm_connector_state *conn_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_display_info *info;
+	struct drm_display_mode *preferred;
+	int ret;
+
+	priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV422) |
+				BIT(HDMI_COLORSPACE_YUV420) |
+				BIT(HDMI_COLORSPACE_YUV444),
+				12,
+				&dummy_connector_hdmi_funcs,
+				test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	KUNIT_ASSERT_TRUE(test, priv->connector.ycbcr_420_allowed);
+
+	info = &priv->connector.display_info;
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, info);
+	preferred = find_preferred_mode(&priv->connector);
+	KUNIT_ASSERT_TRUE(test, drm_mode_is_420(info, preferred));
+
+	state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+	conn_state = drm_atomic_get_connector_state(state, &priv->connector);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	conn_state->color_format = param->fmt;
+
+	ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+	ret = drm_atomic_set_mode_for_crtc(crtc_state, preferred);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state->enable = true;
+	crtc_state->active = true;
+
+	ret = drm_atomic_check_only(state);
+	KUNIT_EXPECT_EQ(test, ret, param->expected_ret);
+	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, param->expected);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
+static const struct color_format_test_param hdmi_color_format_params[] = {
+	{ DRM_COLOR_FORMAT_AUTO, HDMI_COLORSPACE_RGB, 0, "AUTO -> RGB" },
+	{ DRM_COLOR_FORMAT_YCBCR422, HDMI_COLORSPACE_YUV422, 0, "YCBCR422 -> YUV422" },
+	{ DRM_COLOR_FORMAT_YCBCR420, HDMI_COLORSPACE_YUV420, 0, "YCBCR420 -> YUV420" },
+	{ DRM_COLOR_FORMAT_YCBCR444, HDMI_COLORSPACE_YUV444, 0, "YCBCR444 -> YUV444" },
+	{ DRM_COLOR_FORMAT_RGB444, HDMI_COLORSPACE_RGB, 0, "RGB -> RGB" },
+};
+
+KUNIT_ARRAY_PARAM_DESC(check_hdmi_color_format, hdmi_color_format_params, desc);
+
+/* Test that if:
+ * - the HDMI connector supports RGB, YUV422, YUV420, and YUV444
+ * - the display has a YUV420-only mode
+ * - the "color format" property is explicitly set (i.e. !AUTO)
+ * then:
+ * - color format DRM_COLOR_FORMAT_RGB will fail drm_atomic_check_only for the
+ *   YUV420-only mode with -EINVAL
+ * - color format DRM_COLOR_FORMAT_YCBCR444 will fail drm_atomic_check_only for
+ *   the YUV420-only mode with -EINVAL
+ * - color format DRM_COLOR_FORMAT_YCBCR422 will fail drm_atomic_check_only for
+ *   the YUV420-only mode with -EINVAL
+ * - color format DRM_COLOR_FORMAT_YCBCR420 passes drm_atomic_check_only for
+ *   the YUV420-only mode
+ */
+static void drm_test_check_hdmi_color_format_420_only(struct kunit *test)
+{
+	const struct color_format_test_param *param = test->param_value;
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	struct drm_connector_state *conn_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_display_mode *dank;
+	int ret;
+
+	priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV422) |
+				BIT(HDMI_COLORSPACE_YUV420) |
+				BIT(HDMI_COLORSPACE_YUV444),
+				12,
+				&dummy_connector_hdmi_funcs,
+				test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	dank = find_420_only_mode(&priv->connector);
+	KUNIT_ASSERT_NOT_NULL(test, dank);
+
+	state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+	conn_state = drm_atomic_get_connector_state(state, &priv->connector);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	conn_state->color_format = param->fmt;
+
+	ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+	ret = drm_atomic_set_mode_for_crtc(crtc_state, dank);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state->enable = true;
+	crtc_state->active = true;
+
+	ret = drm_atomic_check_only(state);
+	KUNIT_EXPECT_EQ(test, ret, param->expected_ret);
+	if (!param->expected_ret)
+		KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, param->expected);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+};
+
+static const struct color_format_test_param hdmi_color_format_420_only_params[] = {
+	{ DRM_COLOR_FORMAT_RGB444, HDMI_COLORSPACE_RGB, -EINVAL, "RGB should fail" },
+	{ DRM_COLOR_FORMAT_YCBCR444, HDMI_COLORSPACE_YUV444, -EINVAL, "YUV444 should fail" },
+	{ DRM_COLOR_FORMAT_YCBCR422, HDMI_COLORSPACE_YUV422, -EINVAL, "YUV422 should fail" },
+	{ DRM_COLOR_FORMAT_YCBCR420, HDMI_COLORSPACE_YUV420, 0, "YUV420 should work" },
+};
+
+KUNIT_ARRAY_PARAM_DESC(check_hdmi_color_format_420_only,
+		       hdmi_color_format_420_only_params, desc);
+
 static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
 	KUNIT_CASE(drm_test_check_broadcast_rgb_auto_cea_mode),
 	KUNIT_CASE(drm_test_check_broadcast_rgb_auto_cea_mode_vic_1),
@@ -2227,6 +2414,10 @@ static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
 	KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_8bpc),
 	KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_10bpc),
 	KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_12bpc),
+	KUNIT_CASE_PARAM(drm_test_check_hdmi_color_format,
+			 check_hdmi_color_format_gen_params),
+	KUNIT_CASE_PARAM(drm_test_check_hdmi_color_format_420_only,
+			 check_hdmi_color_format_420_only_gen_params),
 	/*
 	 * TODO: We should have tests to check that a change in the
 	 * format triggers a CRTC mode change just like we do for the

-- 
2.52.0

[PATCH v7 19/22] drm/tests: hdmi: Add tests for HDMI helper's mode_valid

[Nicolas Frattaroli]

Add some KUnit tests to verify that the HDMI state helper's mode_valid
implementation does not improperly reject chroma subsampled modes on the
basis of their clock rate not being satisfiable in RGB.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 109 +++++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index f9648f9de46b..b2c421d7e986 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -77,6 +77,23 @@ static struct drm_display_mode *find_420_only_mode(struct drm_connector *connect
 	return NULL;
 }
 
+static struct drm_display_mode *find_420_also_mode(struct drm_connector *connector)
+{
+	struct drm_device *drm = connector->dev;
+	struct drm_display_mode *mode;
+
+	mutex_lock(&drm->mode_config.mutex);
+	list_for_each_entry(mode, &connector->modes, head) {
+		if (drm_mode_is_420_also(&connector->display_info, mode)) {
+			mutex_unlock(&drm->mode_config.mutex);
+			return mode;
+		}
+	}
+	mutex_unlock(&drm->mode_config.mutex);
+
+	return NULL;
+}
+
 static int set_connector_edid(struct kunit *test, struct drm_connector *connector,
 			      const void *edid, size_t edid_len)
 {
@@ -2700,11 +2717,103 @@ static void drm_test_check_mode_valid_reject_max_clock(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, preferred->clock, 25200);
 }
 
+/*
+ * Test that drm_hdmi_connector_mode_valid() will accept modes that require a
+ * 4:2:0 chroma subsampling, even if said mode would violate maximum clock
+ * constraints if it used RGB 4:4:4.
+ */
+static void drm_test_check_mode_valid_yuv420_only_max_clock(struct kunit *test)
+{
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	struct drm_display_mode *dank;
+	struct drm_connector *conn;
+
+	priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+				BIT(HDMI_COLORSPACE_RGB) |
+				BIT(HDMI_COLORSPACE_YUV420),
+				8,
+				&dummy_connector_hdmi_funcs,
+				test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	conn = &priv->connector;
+	KUNIT_ASSERT_EQ(test, conn->display_info.max_tmds_clock, 200 * 1000);
+
+	dank = find_420_only_mode(conn);
+	KUNIT_ASSERT_NOT_NULL(test, dank);
+	KUNIT_EXPECT_EQ(test, dank->hdisplay, 3840);
+	KUNIT_EXPECT_EQ(test, dank->vdisplay, 2160);
+
+	/*
+	 * Note: The mode's "clock" here is not accurate to the actual TMDS
+	 * clock that HDMI will use for a subsampled mode. Hence, why the mode's
+	 * clock is above the .max_tmds_clock of 200MHz.
+	 */
+	KUNIT_EXPECT_EQ(test, dank->clock, 297000);
+}
+
+/*
+ * Test that drm_hdmi_connector_mode_valid() will reject modes that require
+ * 4:2:0 chroma subsampling, if the connector does not support 4:2:0.
+ */
+static void
+drm_test_check_mode_valid_reject_yuv420_only_connector(struct kunit *test)
+{
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	struct drm_display_mode *dank;
+	struct drm_connector *conn;
+
+	priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				8,
+				&dummy_connector_hdmi_funcs,
+				test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	conn = &priv->connector;
+	KUNIT_ASSERT_EQ(test, conn->display_info.max_tmds_clock, 200 * 1000);
+
+	dank = find_420_only_mode(conn);
+	KUNIT_EXPECT_NULL(test, dank);
+}
+
+/*
+ * Test that drm_hdmi_connector_mode_valid() will accept modes that allow (among
+ * other color formats) 4:2:0 chroma subsampling, even if the connector does not
+ * support 4:2:0, but the mode's clock works for RGB 4:4:4.
+ */
+static void
+drm_test_check_mode_valid_accept_yuv420_also_connector_rgb(struct kunit *test)
+{
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	struct drm_display_mode *mode;
+	struct drm_connector *conn;
+
+	priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+				BIT(HDMI_COLORSPACE_RGB),
+				8,
+				&dummy_connector_hdmi_funcs,
+				test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	conn = &priv->connector;
+	KUNIT_ASSERT_EQ(test, conn->display_info.max_tmds_clock, 340 * 1000);
+
+	mode = find_420_also_mode(conn);
+	KUNIT_ASSERT_NOT_NULL(test, mode);
+	KUNIT_EXPECT_EQ(test, mode->hdisplay, 3840);
+	KUNIT_EXPECT_EQ(test, mode->vdisplay, 2160);
+	KUNIT_EXPECT_EQ(test, mode->clock, 297000);
+}
+
 static struct kunit_case drm_atomic_helper_connector_hdmi_mode_valid_tests[] = {
 	KUNIT_CASE(drm_test_check_mode_valid),
 	KUNIT_CASE(drm_test_check_mode_valid_reject),
 	KUNIT_CASE(drm_test_check_mode_valid_reject_rate),
 	KUNIT_CASE(drm_test_check_mode_valid_reject_max_clock),
+	KUNIT_CASE(drm_test_check_mode_valid_yuv420_only_max_clock),
+	KUNIT_CASE(drm_test_check_mode_valid_reject_yuv420_only_connector),
+	KUNIT_CASE(drm_test_check_mode_valid_accept_yuv420_also_connector_rgb),
 	{ }
 };
 

-- 
2.52.0

[PATCH v7 20/22] drm/tests: edid: Add __maybe_unused attribute to EDID definitions

[Nicolas Frattaroli]

It's normal for a test suite to need an EDID, but not all EDIDs across
every test. Silence compiler warnings caused by unused definitions by
using the appropriate attribute.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/tests/drm_kunit_edid.h | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_kunit_edid.h b/drivers/gpu/drm/tests/drm_kunit_edid.h
index f4923157f5bf..18bb7043d7e8 100644
--- a/drivers/gpu/drm/tests/drm_kunit_edid.h
+++ b/drivers/gpu/drm/tests/drm_kunit_edid.h
@@ -54,7 +54,8 @@
  *
  * EDID conformity: PASS
  */
-static const unsigned char test_edid_dvi_1080p[] = {
+static const unsigned char __maybe_unused
+test_edid_dvi_1080p[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
 	0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -160,7 +161,8 @@ static const unsigned char test_edid_dvi_1080p[] = {
  *
  * EDID conformity: FAIL
  */
-static const unsigned char test_edid_hdmi_1080p_rgb_max_100mhz[] = {
+static const unsigned char __maybe_unused
+test_edid_hdmi_1080p_rgb_max_100mhz[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
 	0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
@@ -268,7 +270,8 @@ static const unsigned char test_edid_hdmi_1080p_rgb_max_100mhz[] = {
  *
  * EDID conformity: PASS
  */
-static const unsigned char test_edid_hdmi_1080p_rgb_max_200mhz[] = {
+static const unsigned char __maybe_unused
+test_edid_hdmi_1080p_rgb_max_200mhz[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
 	0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
@@ -387,7 +390,8 @@ static const unsigned char test_edid_hdmi_1080p_rgb_max_200mhz[] = {
  *
  * EDID conformity: PASS
  */
-static const unsigned char test_edid_hdmi_1080p_rgb_max_200mhz_hdr[] = {
+static const unsigned char __maybe_unused
+test_edid_hdmi_1080p_rgb_max_200mhz_hdr[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
 	0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
@@ -495,7 +499,8 @@ static const unsigned char test_edid_hdmi_1080p_rgb_max_200mhz_hdr[] = {
  *
  * EDID conformity: PASS
  */
-static const unsigned char test_edid_hdmi_1080p_rgb_max_340mhz[] = {
+static const unsigned char __maybe_unused
+test_edid_hdmi_1080p_rgb_max_340mhz[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
 	0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
@@ -609,7 +614,8 @@ static const unsigned char test_edid_hdmi_1080p_rgb_max_340mhz[] = {
  *
  * EDID conformity: PASS
  */
-static const unsigned char test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz[] = {
+static const unsigned char __maybe_unused
+test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
 	0x1a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
@@ -723,7 +729,8 @@ static const unsigned char test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz[] = {
  *
  * EDID conformity: PASS
  */
-static const unsigned char test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz[] = {
+static const unsigned char __maybe_unused
+test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
 	0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
@@ -841,7 +848,8 @@ static const unsigned char test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz[] = {
  *     VIC  95:  3840x2160   30.000000 Hz  16:9     67.500 kHz    297.000000 MHz
  * Checksum: 0xca
  */
-static const unsigned char test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz[] = {
+static const unsigned char __maybe_unused
+test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x34, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0xff, 0x23, 0x01, 0x03, 0x80, 0x60, 0x36, 0x78,
 	0x0f, 0xee, 0x91, 0xa3, 0x54, 0x4c, 0x99, 0x26, 0x0f, 0x50, 0x54, 0x20,
@@ -955,7 +963,8 @@ static const unsigned char test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz[
  *   YCbCr 4:2:0 Video Data Block:
  * Checksum: 0x84
  */
-static const unsigned char test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz[] = {
+static const unsigned char __maybe_unused
+test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x34, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0xff, 0x23, 0x01, 0x03, 0x80, 0x60, 0x36, 0x78,
 	0x0f, 0xee, 0x91, 0xa3, 0x54, 0x4c, 0x99, 0x26, 0x0f, 0x50, 0x54, 0x20,

-- 
2.52.0

[PATCH v7 21/22] drm/tests: bridge: Add KUnit tests for bridge chain format selection

[Nicolas Frattaroli]

With the "color format" property, the bridge chain format selection has
gained increased complexity. Instead of simply finding any sequence of
bus formats that works, the bridge chain format selection needs to pick
a sequence that results in the requested color format.

Add KUnit tests for this new logic. These take the form of some pleasant
preprocessor macros to make it less cumbersome to define test bridges
with a set of possible input and output formats.

The input and output formats are defined for bridges in the form of
tuples, where the first member defines the input format, and the second
member defines the output format that can be produced from this input
format. This means the tests can construct scenarios in which not all
inputs can be converted to all outputs.

Some tests are added to test interesting scenarios to exercise the bus
format selection in the presence of a specific color format request.

Furthermore, tests are added to verify that bridge chains that end in an
HDMI connector will always prefer RGB when the color format is
DRM_COLOR_FORMAT_AUTO, as is the behaviour in the HDMI state helpers.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/tests/drm_bridge_test.c | 812 ++++++++++++++++++++++++++++++++
 1 file changed, 812 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index 887020141c7f..ac86f3dfe518 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -2,15 +2,23 @@
 /*
  * Kunit test for drm_bridge functions
  */
+#include <linux/cleanup.h>
+#include <linux/media-bus-format.h>
+
 #include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_atomic_uapi.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_bridge_connector.h>
 #include <drm/drm_bridge_helper.h>
+#include <drm/drm_edid.h>
 #include <drm/drm_kunit_helpers.h>
+#include <drm/drm_managed.h>
 
 #include <kunit/device.h>
 #include <kunit/test.h>
 
+#include "drm_kunit_edid.h"
+
 /*
  * Mimick the typical "private" struct defined by a bridge driver, which
  * embeds a bridge plus other fields.
@@ -37,6 +45,27 @@ struct drm_bridge_init_priv {
 	bool destroyed;
 };
 
+struct drm_bridge_chain_priv {
+	struct drm_device drm;
+	struct drm_encoder encoder;
+	struct drm_plane *plane;
+	struct drm_crtc *crtc;
+	struct drm_connector *connector;
+	unsigned int num_bridges;
+
+	/**
+	 * @test_bridges: array of pointers to &struct drm_bridge_priv entries
+	 *                of which the first @num_bridges entries are valid.
+	 */
+	struct drm_bridge_priv **test_bridges;
+	/**
+	 * @destroyed: bool array of size @num_bridges serving the same function
+	 *             as &struct drm_bridge_init_priv::destroyed does for a
+	 *             single bridge test.
+	 */
+	bool *destroyed;
+};
+
 static struct drm_bridge_priv *bridge_to_priv(struct drm_bridge *bridge)
 {
 	return container_of(bridge, struct drm_bridge_priv, bridge);
@@ -50,6 +79,21 @@ static void drm_test_bridge_priv_destroy(struct drm_bridge *bridge)
 	priv->destroyed = true;
 }
 
+static void drm_test_bridge_chain_priv_destroy(struct drm_bridge *bridge)
+{
+	struct drm_bridge_priv *bridge_priv = bridge_to_priv(bridge);
+	struct drm_bridge_chain_priv *priv = (struct drm_bridge_chain_priv *)bridge_priv->data;
+	unsigned int i;
+
+	for (i = 0; i < priv->num_bridges; i++) {
+		if (priv->test_bridges[i] != bridge_priv)
+			continue;
+
+		priv->destroyed[i] = true;
+		break;
+	}
+}
+
 static void drm_test_bridge_enable(struct drm_bridge *bridge)
 {
 	struct drm_bridge_priv *priv = bridge_to_priv(bridge);
@@ -95,6 +139,230 @@ static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
 	.atomic_reset		= drm_atomic_helper_bridge_reset,
 };
 
+/**
+ * struct fmt_tuple - a tuple of input/output MEDIA_BUS_FMT_*
+ */
+struct fmt_tuple {
+	u32 in_fmt;
+	u32 out_fmt;
+};
+
+/*
+ * Format mapping that only accepts RGB888, and outputs only RGB888
+ */
+static const struct fmt_tuple rgb8_passthrough[] = {
+	{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+};
+
+/*
+ * Format mapping that only accepts YUV444, and outputs only YUV444
+ */
+static const struct fmt_tuple yuv8_passthrough[] = {
+	{ MEDIA_BUS_FMT_YUV8_1X24,   MEDIA_BUS_FMT_YUV8_1X24 },
+};
+
+/*
+ * Format mapping where 8bpc RGB -> 8bpc YUV444, or ID(RGB) or ID(YUV444)
+ */
+static const struct fmt_tuple rgb8_to_yuv8_or_id[] = {
+	{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+	{ MEDIA_BUS_FMT_YUV8_1X24,   MEDIA_BUS_FMT_YUV8_1X24 },
+	{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+};
+
+static const struct fmt_tuple rgb8_to_id_yuv8_or_yuv8_to_yuv422_yuv420[] = {
+	{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+	{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+	{ MEDIA_BUS_FMT_YUV8_1X24,   MEDIA_BUS_FMT_UYVY8_1X16 },
+	{ MEDIA_BUS_FMT_YUV8_1X24,   MEDIA_BUS_FMT_UYYVYY8_0_5X24 },
+};
+
+/*
+ * Format mapping where 8bpc YUV444 -> 8bpc RGB, or ID(YUV444)
+ */
+static const struct fmt_tuple yuv8_to_rgb8_or_id[] = {
+	{ MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+	{ MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+};
+
+/*
+ * A format mapping that acts like a video processor that generates an RGB signal
+ */
+static const struct fmt_tuple rgb_producer[] = {
+	{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+	{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB101010_1X30 },
+	{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB121212_1X36 },
+};
+
+/*
+ * A format mapping that acts like a video processor that generates an 8-bit RGB,
+ * YUV444 or YUV420 signal
+ */
+static const struct fmt_tuple rgb_yuv444_yuv420_producer[] = {
+	{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+	{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_YUV8_1X24 },
+	{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_UYYVYY8_0_5X24 },
+};
+
+static const struct fmt_tuple rgb8_yuv444_yuv422_passthrough[] = {
+	{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+	{ MEDIA_BUS_FMT_YUV8_1X24,   MEDIA_BUS_FMT_YUV8_1X24 },
+	{ MEDIA_BUS_FMT_UYVY8_1X16,  MEDIA_BUS_FMT_UYVY8_1X16 },
+};
+
+static const struct fmt_tuple yuv444_yuv422_rgb8_passthrough[] = {
+	{ MEDIA_BUS_FMT_YUV8_1X24,   MEDIA_BUS_FMT_YUV8_1X24 },
+	{ MEDIA_BUS_FMT_UYVY8_1X16,  MEDIA_BUS_FMT_UYVY8_1X16 },
+	{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+};
+
+static bool fmt_in_list(const u32 fmt, const u32 *out_fmts, const size_t num_fmts)
+{
+	size_t i;
+
+	for (i = 0; i < num_fmts; i++)
+		if (out_fmts[i] == fmt)
+			return true;
+
+	return false;
+}
+
+/**
+ * get_tuples_out_fmts - Get unique output formats of a &struct fmt_tuple list
+ * @fmt_tuples: array of &struct fmt_tuple
+ * @num_fmt_tuples: number of entries in @fmt_tuples
+ * @out_fmts: target array to store the unique output bus formats
+ *
+ * Returns the number of unique output formats, i.e. the number of entries in
+ * @out_fmts that were populated with sensible values.
+ */
+static size_t get_tuples_out_fmts(const struct fmt_tuple *fmt_tuples,
+				  const size_t num_fmt_tuples, u32 *out_fmts)
+{
+	size_t num_unique = 0;
+	size_t i;
+
+	for (i = 0; i < num_fmt_tuples; i++)
+		if (!fmt_in_list(fmt_tuples[i].out_fmt, out_fmts, num_unique))
+			out_fmts[num_unique++] = fmt_tuples[i].out_fmt;
+
+	return num_unique;
+}
+
+#define DEFINE_FMT_FUNCS_FROM_TUPLES(name) \
+static u32 *drm_test_bridge_ ## name ## _out_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)		\
+{												\
+	u32 *out_fmts = kcalloc(ARRAY_SIZE((name)), sizeof(u32), GFP_KERNEL);			\
+												\
+	if (out_fmts)										\
+		*num_output_fmts = get_tuples_out_fmts((name), ARRAY_SIZE((name)), out_fmts);	\
+	else											\
+		*num_output_fmts = 0;								\
+												\
+	return out_fmts;									\
+}												\
+												\
+static u32 *drm_test_bridge_ ## name ## _in_fmts(struct drm_bridge *bridge,			\
+						 struct drm_bridge_state *bridge_state,		\
+						 struct drm_crtc_state *crtc_state,		\
+						 struct drm_connector_state *conn_state,	\
+						 u32 output_fmt,				\
+						 unsigned int *num_input_fmts)			\
+{												\
+	u32 *in_fmts = kcalloc(ARRAY_SIZE((name)), sizeof(u32), GFP_KERNEL);			\
+	unsigned int num_fmts = 0;								\
+	size_t i;										\
+												\
+	if (!in_fmts) {										\
+		*num_input_fmts = 0;								\
+		return NULL;									\
+	}											\
+												\
+	for (i = 0; i < ARRAY_SIZE((name)); i++)						\
+		if ((name)[i].out_fmt == output_fmt)						\
+			in_fmts[num_fmts++] = (name)[i].in_fmt;					\
+												\
+	*num_input_fmts = num_fmts;								\
+												\
+	return in_fmts;										\
+}
+
+#define DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI_FUNC(ident, input_fmts_func, output_fmts_func,	\
+						 hdmi_write_infoframe_func,			\
+						 hdmi_clear_infoframe_func)			\
+static const struct drm_bridge_funcs (ident) = {						\
+	.destroy			= drm_test_bridge_chain_priv_destroy,			\
+	.atomic_enable			= drm_test_bridge_atomic_enable,			\
+	.atomic_disable			= drm_test_bridge_atomic_disable,			\
+	.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,			\
+	.atomic_get_input_bus_fmts	= (input_fmts_func),					\
+	.atomic_get_output_bus_fmts	= (output_fmts_func),					\
+	.hdmi_write_avi_infoframe	= (hdmi_write_infoframe_func),				\
+	.hdmi_clear_avi_infoframe	= (hdmi_clear_infoframe_func),				\
+	.hdmi_write_hdmi_infoframe	= (hdmi_write_infoframe_func),				\
+	.hdmi_clear_hdmi_infoframe	= (hdmi_clear_infoframe_func),				\
+}
+
+#define DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_FUNC(ident, input_fmts_func, output_fmts_func)		\
+	DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI_FUNC(ident, input_fmts_func, output_fmts_func,	\
+						 NULL, NULL)
+
+#define DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(_name)						\
+	DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_FUNC(_name ## _funcs,				\
+					    drm_test_bridge_ ## _name ## _in_fmts,	\
+					    drm_test_bridge_ ## _name ## _out_fmts)
+
+static int drm_test_bridge_write_infoframe_stub(struct drm_bridge *bridge,
+						const u8 *buffer, size_t len)
+{
+	return 0;
+}
+
+static int drm_test_bridge_clear_infoframe_stub(struct drm_bridge *bridge)
+{
+	return 0;
+}
+
+#define DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI(_name)						\
+	DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI_FUNC(_name ## _hdmi ## _funcs,			\
+						 drm_test_bridge_ ## _name ## _in_fmts,		\
+						 drm_test_bridge_ ## _name ## _out_fmts,	\
+						 drm_test_bridge_write_infoframe_stub,		\
+						 drm_test_bridge_clear_infoframe_stub)
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb8_passthrough)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb8_passthrough);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(yuv8_passthrough)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(yuv8_passthrough);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb8_to_yuv8_or_id)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb8_to_yuv8_or_id);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(yuv8_to_rgb8_or_id)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(yuv8_to_rgb8_or_id);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb_producer)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb_producer);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb_yuv444_yuv420_producer)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb_yuv444_yuv420_producer);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb8_to_id_yuv8_or_yuv8_to_yuv422_yuv420)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb8_to_id_yuv8_or_yuv8_to_yuv422_yuv420);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb8_yuv444_yuv422_passthrough)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb8_yuv444_yuv422_passthrough);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(yuv444_yuv422_rgb8_passthrough)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(yuv444_yuv422_rgb8_passthrough);
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI(yuv444_yuv422_rgb8_passthrough);
+
 KUNIT_DEFINE_ACTION_WRAPPER(drm_bridge_remove_wrapper,
 			    drm_bridge_remove,
 			    struct drm_bridge *);
@@ -180,6 +448,123 @@ drm_test_bridge_init(struct kunit *test, const struct drm_bridge_funcs *funcs)
 	return priv;
 }
 
+static struct drm_bridge_chain_priv *
+drm_test_bridge_chain_init(struct kunit *test, unsigned int num_bridges,
+			   const struct drm_bridge_funcs **funcs)
+{
+	struct drm_bridge_chain_priv *priv;
+	const struct drm_edid *edid;
+	struct drm_bridge *prev;
+	struct drm_encoder *enc;
+	struct drm_bridge *bridge;
+	struct drm_device *drm;
+	bool has_hdmi = false;
+	struct device *dev;
+	unsigned int i;
+	int ret;
+
+	dev = drm_kunit_helper_alloc_device(test);
+	if (IS_ERR(dev))
+		return ERR_CAST(dev);
+
+	priv = drm_kunit_helper_alloc_drm_device(test, dev, struct drm_bridge_chain_priv,
+						 drm, DRIVER_MODESET | DRIVER_ATOMIC);
+	if (IS_ERR(priv))
+		return ERR_CAST(priv);
+
+	drm = &priv->drm;
+
+	priv->test_bridges = drmm_kmalloc_array(drm, num_bridges, sizeof(*priv->test_bridges),
+						GFP_KERNEL);
+	if (!priv->test_bridges)
+		return ERR_PTR(-ENOMEM);
+
+	priv->destroyed = drmm_kcalloc(drm, num_bridges, sizeof(*priv->destroyed), GFP_KERNEL);
+	if (!priv->destroyed)
+		return ERR_PTR(-ENOMEM);
+
+	priv->num_bridges = num_bridges;
+
+	for (i = 0; i < num_bridges; i++) {
+		priv->test_bridges[i] = devm_drm_bridge_alloc(dev, struct drm_bridge_priv,
+							      bridge, funcs[i]);
+		if (IS_ERR(priv->test_bridges[i]))
+			return ERR_CAST(priv->test_bridges[i]);
+
+		priv->test_bridges[i]->data = priv;
+	}
+
+	priv->plane = drm_kunit_helper_create_primary_plane(test, drm, NULL, NULL,
+							    NULL, 0, NULL);
+	if (IS_ERR(priv->plane))
+		return ERR_CAST(priv->plane);
+
+	priv->crtc = drm_kunit_helper_create_crtc(test, drm, priv->plane, NULL,
+						  NULL, NULL);
+	if (IS_ERR(priv->crtc))
+		return ERR_CAST(priv->crtc);
+
+	enc = &priv->encoder;
+	ret = drmm_encoder_init(drm, enc, NULL, DRM_MODE_ENCODER_TMDS, NULL);
+	if (ret)
+		return ERR_PTR(ret);
+
+	enc->possible_crtcs = drm_crtc_mask(priv->crtc);
+
+	prev = NULL;
+	for (i = 0; i < num_bridges; i++) {
+		bridge = &priv->test_bridges[i]->bridge;
+		bridge->type = DRM_MODE_CONNECTOR_VIRTUAL;
+
+		if (bridge->funcs->hdmi_write_hdmi_infoframe) {
+			has_hdmi = true;
+			bridge->ops |= DRM_BRIDGE_OP_HDMI;
+			bridge->type = DRM_MODE_CONNECTOR_HDMIA;
+			bridge->vendor = "LNX";
+			bridge->product = "KUnit";
+			bridge->supported_formats = (BIT(HDMI_COLORSPACE_RGB) |
+						     BIT(HDMI_COLORSPACE_YUV444) |
+						     BIT(HDMI_COLORSPACE_YUV422) |
+						     BIT(HDMI_COLORSPACE_YUV420));
+		}
+
+		ret = drm_kunit_bridge_add(test, bridge);
+		if (ret)
+			return ERR_PTR(ret);
+
+		ret = drm_bridge_attach(enc, bridge, prev, 0);
+		if (ret)
+			return ERR_PTR(ret);
+
+		prev = bridge;
+	}
+
+	priv->connector = drm_bridge_connector_init(drm, enc);
+	if (IS_ERR(priv->connector))
+		return ERR_CAST(priv->connector);
+
+	drm_connector_attach_encoder(priv->connector, enc);
+
+	drm_mode_config_reset(drm);
+
+	if (!has_hdmi)
+		return priv;
+
+	scoped_guard(mutex, &drm->mode_config.mutex) {
+		edid = drm_edid_alloc(test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz,
+				ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz));
+		if (!edid)
+			return ERR_PTR(-EINVAL);
+
+		drm_edid_connector_update(priv->connector, edid);
+		KUNIT_ASSERT_GT(test, drm_edid_connector_add_modes(priv->connector), 0);
+
+		ret = priv->connector->funcs->fill_modes(priv->connector, 4096, 4096);
+	}
+
+	return priv;
+}
+
 /*
  * Test that drm_bridge_get_current_state() returns the last committed
  * state for an atomic bridge.
@@ -508,14 +893,441 @@ static struct kunit_suite drm_bridge_alloc_test_suite = {
 	.test_cases = drm_bridge_alloc_tests,
 };
 
+/**
+ * drm_test_bridge_chain_verify_fmt - Verify bridge chain format selection
+ * @test: pointer to KUnit test object
+ * @priv: pointer to a &struct drm_bridge_chain_priv for this chain
+ * @expected: constant array of &struct fmt_tuple describing the expected
+ *            input and output bus formats
+ * @num_expected: number of entries in @expected
+ *
+ * Runs the KUNIT_EXPECT clauses to verify the bridge chain format selection
+ * resulted in the expected formats. If %0 is given as a format in a
+ * &struct fmt_tuple, then it is understood to mean "any".
+ *
+ * Must be called with the modeset lock held.
+ */
+static void drm_test_bridge_chain_verify_fmt(struct kunit *test,
+					     struct drm_bridge_chain_priv *priv,
+					     const struct fmt_tuple *const expected,
+					     const unsigned int num_expected)
+{
+	struct drm_bridge_state *bstate;
+	unsigned int i = 0;
+
+	drm_for_each_bridge_in_chain_scoped(&priv->encoder, bridge) {
+		KUNIT_ASSERT_LT(test, i, num_expected);
+
+		bstate = drm_bridge_get_current_state(bridge);
+		if (expected[i].in_fmt)
+			KUNIT_EXPECT_EQ(test, bstate->input_bus_cfg.format,
+					expected[i].in_fmt);
+		if (expected[i].out_fmt)
+			KUNIT_EXPECT_EQ(test, bstate->output_bus_cfg.format,
+					expected[i].out_fmt);
+
+		i++;
+	}
+
+	KUNIT_ASSERT_EQ_MSG(test, i, num_expected,
+			    "Fewer bridges (%u) than expected (%u)\n", i, num_expected);
+}
+
+/*
+ * Test that constructs a bridge chain in which an RGB888 producer is chained to
+ * two bridges that will convert from RGB to YUV and from YUV to RGB respectively.
+ *
+ * The test requests an output color_format of RGB using the color_format property,
+ * so to satisfy this request, the bridge chain must take a detour over YUV.
+ */
+static void drm_test_bridge_rgb_yuv_rgb(struct kunit *test)
+{
+	static const struct drm_bridge_funcs *funcs[] = {
+		&rgb_producer_funcs,
+		&rgb8_to_yuv8_or_id_funcs,
+		&yuv8_to_rgb8_or_id_funcs,
+	};
+	static const struct fmt_tuple expected[] = {
+		{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+		{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+		{ MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+	};
+	struct drm_connector_state *conn_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_chain_priv *priv;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_display_mode *mode;
+	int ret;
+
+	priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+	conn_state = drm_atomic_get_connector_state(state, priv->connector);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+	conn_state->color_format = DRM_COLOR_FORMAT_RGB444;
+
+	ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+	ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state->enable = true;
+	crtc_state->active = true;
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	drm_test_bridge_chain_verify_fmt(test, priv, expected, ARRAY_SIZE(expected));
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
+/*
+ * Test in which a bridge capable of producing RGB, YUV444 and YUV420 has to
+ * produce RGB and convert with a downstream bridge in the chain to reach the
+ * requested YUV444 color format, as no direct path exists between its YUV444
+ * and the last bridge.
+ *
+ * The rationale behind this test is to devise a scenario in which naively
+ * assuming any format the video processor can output, and the connector
+ * requests, is the right format to pick, does not work.
+ */
+static void drm_test_bridge_must_convert_to_yuv444(struct kunit *test)
+{
+	static const struct drm_bridge_funcs *funcs[] = {
+		&rgb_yuv444_yuv420_producer_funcs,
+		&rgb8_passthrough_funcs,
+		&rgb8_to_id_yuv8_or_yuv8_to_yuv422_yuv420_funcs,
+		&rgb8_yuv444_yuv422_passthrough_funcs,
+	};
+	static const struct fmt_tuple expected[] = {
+		{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+		{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+		{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+		{ MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+	};
+	struct drm_connector_state *conn_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_chain_priv *priv;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_display_mode *mode;
+	int ret;
+
+	priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+	conn_state = drm_atomic_get_connector_state(state, priv->connector);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+	conn_state->color_format = DRM_COLOR_FORMAT_YCBCR444;
+
+	ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+	ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state->enable = true;
+	crtc_state->active = true;
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	drm_test_bridge_chain_verify_fmt(test, priv, expected, ARRAY_SIZE(expected));
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
+/*
+ * Test which checks that no matter the order of bus formats returned by an
+ * HDMI bridge, RGB is preferred on DRM_COLOR_FORMAT_AUTO if it's available.
+ */
+static void drm_test_bridge_hdmi_auto_rgb(struct kunit *test)
+{
+	static const struct drm_bridge_funcs *funcs[] = {
+		&rgb_yuv444_yuv420_producer_funcs,
+		&yuv444_yuv422_rgb8_passthrough_hdmi_funcs,
+	};
+	static const struct fmt_tuple expected[] = {
+		{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+		{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+	};
+	struct drm_connector_state *conn_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_chain_priv *priv;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_display_mode *mode;
+	int ret;
+
+	priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+	conn_state = drm_atomic_get_connector_state(state, priv->connector);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+	KUNIT_ASSERT_EQ(test, conn_state->color_format, DRM_COLOR_FORMAT_AUTO);
+
+	ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+	ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state->enable = true;
+	crtc_state->active = true;
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+
+	drm_test_bridge_chain_verify_fmt(test, priv, expected, ARRAY_SIZE(expected));
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
+/*
+ * Test which checks that DRM_COLOR_FORMAT_AUTO on non-HDMI connectors will
+ * result in the first bus format on the output to be picked.
+ */
+static void drm_test_bridge_auto_first(struct kunit *test)
+{
+	static const struct drm_bridge_funcs *funcs[] = {
+		&rgb_yuv444_yuv420_producer_funcs,
+		&yuv444_yuv422_rgb8_passthrough_funcs,
+	};
+	static const struct fmt_tuple expected[] = {
+		{ MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_YUV8_1X24 },
+		{ MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+	};
+	struct drm_connector_state *conn_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_chain_priv *priv;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_display_mode *mode;
+	int ret;
+
+	priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+	conn_state = drm_atomic_get_connector_state(state, priv->connector);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+	KUNIT_ASSERT_EQ(test, conn_state->color_format, DRM_COLOR_FORMAT_AUTO);
+
+	ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+	ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state->enable = true;
+	crtc_state->active = true;
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	drm_test_bridge_chain_verify_fmt(test, priv, expected, ARRAY_SIZE(expected));
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
+/*
+ * Test which checks that in a configuration of bridge chains where an RGB
+ * producer is hooked to a YUV444-only pass-through, the atomic commit fails as
+ * the bridge format selection cannot find a valid sequence of bus formats.
+ */
+static void drm_test_bridge_rgb_yuv_no_path(struct kunit *test)
+{
+	static const struct drm_bridge_funcs *funcs[] = {
+		&rgb_producer_funcs,
+		&yuv8_passthrough_funcs,
+		&rgb8_yuv444_yuv422_passthrough_funcs,
+	};
+	struct drm_connector_state *conn_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_chain_priv *priv;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_display_mode *mode;
+	int ret;
+
+	priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+	conn_state = drm_atomic_get_connector_state(state, priv->connector);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+	ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+	ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	crtc_state->enable = true;
+	crtc_state->active = true;
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_EXPECT_EQ(test, ret, -ENOTSUPP);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
+static struct kunit_case drm_bridge_bus_fmt_tests[] = {
+	KUNIT_CASE(drm_test_bridge_rgb_yuv_rgb),
+	KUNIT_CASE(drm_test_bridge_must_convert_to_yuv444),
+	KUNIT_CASE(drm_test_bridge_hdmi_auto_rgb),
+	KUNIT_CASE(drm_test_bridge_auto_first),
+	KUNIT_CASE(drm_test_bridge_rgb_yuv_no_path),
+	{ }
+};
+
+static struct kunit_suite drm_bridge_bus_fmt_test_suite = {
+	.name = "drm_bridge_bus_fmt",
+	.test_cases = drm_bridge_bus_fmt_tests,
+};
+
 kunit_test_suites(
 	&drm_bridge_get_current_state_test_suite,
 	&drm_bridge_helper_reset_crtc_test_suite,
 	&drm_bridge_alloc_test_suite,
+	&drm_bridge_bus_fmt_test_suite,
 );
 
 MODULE_AUTHOR("Maxime Ripard <mripard@kernel.org>");
 MODULE_AUTHOR("Luca Ceresoli <luca.ceresoli@bootlin.com>");
+MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
 
 MODULE_DESCRIPTION("Kunit test for drm_bridge functions");
 MODULE_LICENSE("GPL");

-- 
2.52.0

[PATCH v7 22/22] drm/bridge: Document bridge chain format selection

[Nicolas Frattaroli]

The bridge chain format selection behaviour was, until now,
undocumented. With the addition of the "color format" DRM property, it's
not sufficiently complex enough that documentation is warranted,
especially for driver authors trying to do the right thing.

Add a high-level overview of how the process is supposed to work, and
mention what the display driver is supposed to do if it wants to make
use of this functionality.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 Documentation/gpu/drm-kms-helpers.rst |  6 ++++++
 drivers/gpu/drm/drm_bridge.c          | 39 +++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 781129f78b06..47c4f593cf9d 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -181,6 +181,12 @@ Bridge Operations
 .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
    :doc: bridge operations
 
+Bridge Chain Format Selection
+-----------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_bridge.c
+   :doc: bridge chain format selection
+
 Bridge Connector Helper
 -----------------------
 
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index b0dfa03dbb81..a155b8aa4a3d 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -198,6 +198,45 @@
  * driver.
  */
 
+/**
+ * DOC: bridge chain format selection
+ *
+ * A bridge chain, from display output processor to connector, may contain
+ * bridges capable of converting between bus formats on their inputs, and
+ * output formats on their outputs. For example, a bridge may be able to convert
+ * from RGB to YCbCr 4:4:4, and pass through YCbCr 4:2:0 as-is, but not convert
+ * from RGB to YCbCr 4:2:0. This means not all input formats map to all output
+ * formats.
+ *
+ * Further adding to this, a desired output color format, as specified with the
+ * "color format" DRM property, might not correspond to what the display driver
+ * should set at its output 1:1. The bridge chain it feeds into may only be able
+ * to reach the desired output format, if a conversion from a different starting
+ * format is performed.
+ *
+ * To deal with this complexity, the recursive bridge chain bus format selection
+ * logic starts with the last bridge in the chain, usually the connector, and
+ * then recursively walks the chain of bridges backwards to the first bridge,
+ * trying to find a path.
+ *
+ * For a display driver to work in such a scenario, it should read the first
+ * bridge's bridge state to figure out which bus format the chain resolved to.
+ * If the first bridge's input format resolved to %MEDIA_BUS_FMT_FIXED, then its
+ * output format should be used.
+ *
+ * Special handling is done for HDMI as it relates to format selection. Instead
+ * of directly using the "color format" DRM property for bridge chains that end
+ * in HDMI bridges, the bridge chain format selection logic will trust the logic
+ * that set the HDMI output format. For the common HDMI state helper
+ * functionality, this means that %DRM_COLOR_FORMAT_AUTO will allow fallbacks to
+ * YCBCr 4:2:0 if the bandwidth requirements would otherwise be too high but the
+ * mode and connector allow it.
+ *
+ * For bridge chains that do not end in an HDMI bridge, %DRM_COLOR_FORMAT_AUTO
+ * will be satisfied with the first output format on the last bridge for which
+ * it can find a path back to the first bridge.
+ */
+
 /* Protect bridge_list and bridge_lingering_list */
 static DEFINE_MUTEX(bridge_lock);
 static LIST_HEAD(bridge_list);

-- 
2.52.0

Re:[PATCH v7 09/22] drm/rockchip: Add YUV422 output mode constants for VOP2

[Andy Yan]

Hello,

At 2026-01-21 22:45:16, "Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> wrote:
>The Rockchip display controller has a general YUV422 output mode, and
>some SoC-specific connector-specific output modes for RK3576.
>
>Add them, based on the values in downstream and the TRM (dsp_out_mode in
>RK3576 TRM Part 2, register POST*_CTRL_POST_DSP_CTRL).
>
>Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

   Reviewed-by: Andy Yan <andyshrk@163.com>


>---
> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>index 2e86ad00979c..4705dc6b8bd7 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>@@ -30,10 +30,14 @@
> #define ROCKCHIP_OUT_MODE_P565		2
> #define ROCKCHIP_OUT_MODE_BT656		5
> #define ROCKCHIP_OUT_MODE_S888		8
>+#define ROCKCHIP_OUT_MODE_YUV422	9
> #define ROCKCHIP_OUT_MODE_S888_DUMMY	12
> #define ROCKCHIP_OUT_MODE_YUV420	14
> /* for use special outface */
> #define ROCKCHIP_OUT_MODE_AAAA		15
>+/* SoC specific output modes */
>+#define ROCKCHIP_OUT_MODE_YUV422_RK3576_DP	12
>+#define ROCKCHIP_OUT_MODE_YUV422_RK3576_HDMI	13
> 
> /* output flags */
> #define ROCKCHIP_OUTPUT_DSI_DUAL	BIT(0)
>
>-- 
>2.52.0

Re:[PATCH v7 10/22] drm/rockchip: vop2: Fix YUV444 output

[Andy Yan]

Hello Nicolas，

At 2026-01-21 22:45:17, "Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> wrote:
>YUV444 (aka YCbCr444) output isn't working quite right on RK3588. The
>resulting image on the display, while identifying itself as YUV444, has
>some components swapped, even after adding the necessary DRM formats to
>the conversion functions.
>
>Judging by downstream, this is because YUV444 also needs an rb swap
>performed in the AFBC case.
>
>Add the DRM formats to the appropriate switch statements, and add a
>function for checking whether an rb swap needs to be performed in the
>AFBC case.
>
>Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
>Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>index ec3b4fde10db..469c63dd97d5 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>@@ -176,6 +176,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
> 	case DRM_FORMAT_ARGB2101010:
> 	case DRM_FORMAT_XBGR2101010:
> 	case DRM_FORMAT_ABGR2101010:
>+	case DRM_FORMAT_VUY101010:
> 		return VOP2_FMT_XRGB101010;
> 	case DRM_FORMAT_XRGB8888:
> 	case DRM_FORMAT_ARGB8888:
>@@ -184,6 +185,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
> 		return VOP2_FMT_ARGB8888;
> 	case DRM_FORMAT_RGB888:
> 	case DRM_FORMAT_BGR888:
>+	case DRM_FORMAT_VUY888:
> 		return VOP2_FMT_RGB888;
> 	case DRM_FORMAT_RGB565:
> 	case DRM_FORMAT_BGR565:
>@@ -225,6 +227,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
> 	case DRM_FORMAT_ARGB2101010:
> 	case DRM_FORMAT_XBGR2101010:
> 	case DRM_FORMAT_ABGR2101010:
>+	case DRM_FORMAT_VUY101010:
> 		return VOP2_AFBC_FMT_ARGB2101010;
> 	case DRM_FORMAT_XRGB8888:
> 	case DRM_FORMAT_ARGB8888:
>@@ -233,6 +236,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
> 		return VOP2_AFBC_FMT_ARGB8888;
> 	case DRM_FORMAT_RGB888:
> 	case DRM_FORMAT_BGR888:
>+	case DRM_FORMAT_VUY888:

How did you test this format? It seems tools like modetest don’t support testing this pattern.



> 		return VOP2_AFBC_FMT_RGB888;
> 	case DRM_FORMAT_RGB565:
> 	case DRM_FORMAT_BGR565:
>@@ -270,6 +274,19 @@ static bool vop2_win_rb_swap(u32 format)
> 	}
> }
> 
>+static bool vop2_afbc_rb_swap(u32 format)
>+{
>+	switch (format) {
>+	case DRM_FORMAT_NV24:
>+	case DRM_FORMAT_NV30:
>+	case DRM_FORMAT_VUY888:
>+	case DRM_FORMAT_VUY101010:
>+		return true;
>+	default:
>+		return false;
>+	}
>+}
>+
> static bool vop2_afbc_uv_swap(u32 format)
> {
> 	switch (format) {
>@@ -1291,6 +1308,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
> 		 /* It's for head stride, each head size is 16 byte */
> 		stride = ALIGN(stride, block_w) / block_w * 16;
> 
>+		rb_swap = vop2_afbc_rb_swap(fb->format->format);
> 		uv_swap = vop2_afbc_uv_swap(fb->format->format);
> 		/*
> 		 * This is a workaround for crazy IC design, Cluster
>@@ -1308,6 +1326,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
> 			vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1);
> 		vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format);
> 		vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap);
>+		vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap);
> 		/*
> 		 * On rk3566/8, this bit is auto gating enable,
> 		 * but this function is not work well so we need
>
>-- 
>2.52.0
>

Re:[PATCH v7 11/22] drm/rockchip: vop2: Add RK3576 to the RG swap special case

[Andy Yan]

Hello Nicolas，

At 2026-01-21 22:45:18, "Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> wrote:
>Much like RK3588, RK3576 requires an RG swap to be performed for YUV444
>8-bit and YUV444 10-bit bus formats.
>
>Add its version to the already existing check for RK3588, so that YUV444
>output is correct on this platform.
>
>Fixes: 944757a4cba6 ("drm/rockchip: vop2: Add support for rk3576")
>Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>


   Reviewed-by: Andy Yan <andyshrk@163.com>

>---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>index 469c63dd97d5..f21efbca42e6 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>@@ -354,7 +354,8 @@ static bool vop2_output_uv_swap(u32 bus_format, u32 output_mode)
> 
> static bool vop2_output_rg_swap(struct vop2 *vop2, u32 bus_format)
> {
>-	if (vop2->version == VOP_VERSION_RK3588) {
>+	if (vop2->version == VOP_VERSION_RK3588 ||
>+	    vop2->version == VOP_VERSION_RK3576) {
> 		if (bus_format == MEDIA_BUS_FMT_YUV8_1X24 ||
> 		    bus_format == MEDIA_BUS_FMT_YUV10_1X30)
> 			return true;
>
>-- 
>2.52.0
>

Re:[PATCH v7 12/22] drm/rockchip: vop2: Recognise 10/12-bit YUV422 as YUV formats

[Andy Yan]

Hello Nicolas，
At 2026-01-21 22:45:19, "Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> wrote:
>The Rockchip VOP2 video output driver has a "is_yuv_output" function,
>which returns true when a given bus format is a YUV format, and false
>otherwise.
>
>This switch statement is lacking the bus format used for YUV422 10-bit,
>as well as the bus format used for YUV422 12-bit.
>
>Add MEDIA_BUS_FMT_YUYV10_1X20 and MEDIA_BUS_FMT_YUYV12_1X24 to
>is_yuv_output's switch cases to resolve this.
>
>Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
>Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>index f21efbca42e6..3dd7b7571487 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>@@ -369,6 +369,9 @@ static bool is_yuv_output(u32 bus_format)
> 	switch (bus_format) {
> 	case MEDIA_BUS_FMT_YUV8_1X24:
> 	case MEDIA_BUS_FMT_YUV10_1X30:
>+	case MEDIA_BUS_FMT_YUYV10_1X20:
>+	case MEDIA_BUS_FMT_UYVY10_1X20:

>+	case MEDIA_BUS_FMT_YUYV12_1X24:

         MEDIA_BUS_FMT_YUYV12_1X24 is not supported by VOP,  the max output depth of VOP is 10




> 	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
> 	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
> 	case MEDIA_BUS_FMT_YUYV8_2X8:
>
>-- 
>2.52.0
>

Re:[PATCH v7 13/22] drm/rockchip: vop2: Set correct output format for RK3576 YUV422

[Andy Yan]

Hello Nicolas,

At 2026-01-21 22:45:20, "Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> wrote:
>For RK3576 to be able to output YUV422 signals, it first needs to be
>able to pick the right output mode in the display controller to do so.
>
>The RK3576 hardware specifies different output formats depending on the
>used display protocol.
>
>Adjust the written register value based on the SoC and connector, so
>other users of vcstate->output_mode don't have to care about this.
>
>Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>


    Reviewed-by: Andy Yan <andyshrk@163.com>
>---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>index 3dd7b7571487..d93c6779286e 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>@@ -1706,6 +1706,22 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
> 	if (vcstate->output_mode == ROCKCHIP_OUT_MODE_AAAA &&
> 	    !(vp_data->feature & VOP2_VP_FEATURE_OUTPUT_10BIT))
> 		out_mode = ROCKCHIP_OUT_MODE_P888;
>+	else if (vcstate->output_mode == ROCKCHIP_OUT_MODE_YUV422 &&
>+		 vop2->version == VOP_VERSION_RK3576)
>+		switch (vcstate->output_type) {
>+		case DRM_MODE_CONNECTOR_DisplayPort:
>+		case DRM_MODE_CONNECTOR_eDP:
>+			out_mode = ROCKCHIP_OUT_MODE_YUV422_RK3576_DP;
>+			break;
>+		case DRM_MODE_CONNECTOR_HDMIA:
>+			out_mode = ROCKCHIP_OUT_MODE_YUV422_RK3576_HDMI;
>+			break;
>+		default:
>+			drm_err(vop2->drm, "Unknown DRM_MODE_CONNECTOR %d\n",
>+				vcstate->output_type);
>+			vop2_unlock(vop2);
>+			return;
>+		}
> 	else
> 		out_mode = vcstate->output_mode;
> 
>
>-- 
>2.52.0
>

Re: [PATCH v7 10/22] drm/rockchip: vop2: Fix YUV444 output

[Nicolas Frattaroli]

On Thursday, 22 January 2026 09:28:54 Central European Standard Time Andy Yan wrote:
> 
> Hello Nicolas，
> 
> At 2026-01-21 22:45:17, "Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> wrote:
> >YUV444 (aka YCbCr444) output isn't working quite right on RK3588. The
> >resulting image on the display, while identifying itself as YUV444, has
> >some components swapped, even after adding the necessary DRM formats to
> >the conversion functions.
> >
> >Judging by downstream, this is because YUV444 also needs an rb swap
> >performed in the AFBC case.
> >
> >Add the DRM formats to the appropriate switch statements, and add a
> >function for checking whether an rb swap needs to be performed in the
> >AFBC case.
> >
> >Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> >Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> >---
> > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >index ec3b4fde10db..469c63dd97d5 100644
> >--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >@@ -176,6 +176,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
> > 	case DRM_FORMAT_ARGB2101010:
> > 	case DRM_FORMAT_XBGR2101010:
> > 	case DRM_FORMAT_ABGR2101010:
> >+	case DRM_FORMAT_VUY101010:
> > 		return VOP2_FMT_XRGB101010;
> > 	case DRM_FORMAT_XRGB8888:
> > 	case DRM_FORMAT_ARGB8888:
> >@@ -184,6 +185,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
> > 		return VOP2_FMT_ARGB8888;
> > 	case DRM_FORMAT_RGB888:
> > 	case DRM_FORMAT_BGR888:
> >+	case DRM_FORMAT_VUY888:
> > 		return VOP2_FMT_RGB888;
> > 	case DRM_FORMAT_RGB565:
> > 	case DRM_FORMAT_BGR565:
> >@@ -225,6 +227,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
> > 	case DRM_FORMAT_ARGB2101010:
> > 	case DRM_FORMAT_XBGR2101010:
> > 	case DRM_FORMAT_ABGR2101010:
> >+	case DRM_FORMAT_VUY101010:
> > 		return VOP2_AFBC_FMT_ARGB2101010;
> > 	case DRM_FORMAT_XRGB8888:
> > 	case DRM_FORMAT_ARGB8888:
> >@@ -233,6 +236,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
> > 		return VOP2_AFBC_FMT_ARGB8888;
> > 	case DRM_FORMAT_RGB888:
> > 	case DRM_FORMAT_BGR888:
> >+	case DRM_FORMAT_VUY888:
> 
> How did you test this format? It seems tools like modetest don’t support testing this pattern.
> 

Hi Andy,

using the rest of this series, which implements the "color format"
DRM property, and the corresponding weston MR that makes use of it[1].

I create a ~/.config/weston.ini with the following contents:

    [output]
    name=HDMI-A-1
    color-format=yuv444

This will make Weston try to set the output format to 10-bit YUV444. To
limit it to 8-bit, you can add `max-bpc=8`. The monitor's EDID needs to
report YUV444 support, otherwise that Weston version won't let you set
this property.

Link: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1859 [1]

Kind regards,
Nicolas Frattaroli

> 
> 
> > 		return VOP2_AFBC_FMT_RGB888;
> > 	case DRM_FORMAT_RGB565:
> > 	case DRM_FORMAT_BGR565:
> >@@ -270,6 +274,19 @@ static bool vop2_win_rb_swap(u32 format)
> > 	}
> > }
> > 
> >+static bool vop2_afbc_rb_swap(u32 format)
> >+{
> >+	switch (format) {
> >+	case DRM_FORMAT_NV24:
> >+	case DRM_FORMAT_NV30:
> >+	case DRM_FORMAT_VUY888:
> >+	case DRM_FORMAT_VUY101010:
> >+		return true;
> >+	default:
> >+		return false;
> >+	}
> >+}
> >+
> > static bool vop2_afbc_uv_swap(u32 format)
> > {
> > 	switch (format) {
> >@@ -1291,6 +1308,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
> > 		 /* It's for head stride, each head size is 16 byte */
> > 		stride = ALIGN(stride, block_w) / block_w * 16;
> > 
> >+		rb_swap = vop2_afbc_rb_swap(fb->format->format);
> > 		uv_swap = vop2_afbc_uv_swap(fb->format->format);
> > 		/*
> > 		 * This is a workaround for crazy IC design, Cluster
> >@@ -1308,6 +1326,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
> > 			vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1);
> > 		vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format);
> > 		vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap);
> >+		vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap);
> > 		/*
> > 		 * On rk3566/8, this bit is auto gating enable,
> > 		 * but this function is not work well so we need
> >
> 

Re:Re: [PATCH v7 10/22] drm/rockchip: vop2: Fix YUV444 output

[Andy Yan]

Hello Nicolas,

在 2026-01-22 20:59:41，"Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> 写道：
>On Thursday, 22 January 2026 09:28:54 Central European Standard Time Andy Yan wrote:
>> 
>> Hello Nicolas，
>> 
>> At 2026-01-21 22:45:17, "Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> wrote:
>> >YUV444 (aka YCbCr444) output isn't working quite right on RK3588. The
>> >resulting image on the display, while identifying itself as YUV444, has
>> >some components swapped, even after adding the necessary DRM formats to
>> >the conversion functions.
>> >
>> >Judging by downstream, this is because YUV444 also needs an rb swap
>> >performed in the AFBC case.
>> >
>> >Add the DRM formats to the appropriate switch statements, and add a
>> >function for checking whether an rb swap needs to be performed in the
>> >AFBC case.
>> >
>> >Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
>> >Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>> >---
>> > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 19 +++++++++++++++++++
>> > 1 file changed, 19 insertions(+)
>> >
>> >diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> >index ec3b4fde10db..469c63dd97d5 100644
>> >--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> >+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> >@@ -176,6 +176,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
>> > 	case DRM_FORMAT_ARGB2101010:
>> > 	case DRM_FORMAT_XBGR2101010:
>> > 	case DRM_FORMAT_ABGR2101010:
>> >+	case DRM_FORMAT_VUY101010:
>> > 		return VOP2_FMT_XRGB101010;
>> > 	case DRM_FORMAT_XRGB8888:
>> > 	case DRM_FORMAT_ARGB8888:
>> >@@ -184,6 +185,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
>> > 		return VOP2_FMT_ARGB8888;
>> > 	case DRM_FORMAT_RGB888:
>> > 	case DRM_FORMAT_BGR888:
>> >+	case DRM_FORMAT_VUY888:
>> > 		return VOP2_FMT_RGB888;
>> > 	case DRM_FORMAT_RGB565:
>> > 	case DRM_FORMAT_BGR565:
>> >@@ -225,6 +227,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
>> > 	case DRM_FORMAT_ARGB2101010:
>> > 	case DRM_FORMAT_XBGR2101010:
>> > 	case DRM_FORMAT_ABGR2101010:
>> >+	case DRM_FORMAT_VUY101010:
>> > 		return VOP2_AFBC_FMT_ARGB2101010;
>> > 	case DRM_FORMAT_XRGB8888:
>> > 	case DRM_FORMAT_ARGB8888:
>> >@@ -233,6 +236,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
>> > 		return VOP2_AFBC_FMT_ARGB8888;
>> > 	case DRM_FORMAT_RGB888:
>> > 	case DRM_FORMAT_BGR888:
>> >+	case DRM_FORMAT_VUY888:
>> 
>> How did you test this format? It seems tools like modetest don’t support testing this pattern.
>> 
>
>Hi Andy,
>
>using the rest of this series, which implements the "color format"
>DRM property, and the corresponding weston MR that makes use of it[1].
>
>I create a ~/.config/weston.ini with the following contents:
>
>    [output]
>    name=HDMI-A-1
>    color-format=yuv444
>
>This will make Weston try to set the output format to 10-bit YUV444. To
>limit it to 8-bit, you can add `max-bpc=8`. The monitor's EDID needs to
>report YUV444 support, otherwise that Weston version won't let you set
>this property.
>


This looks a bit strange. Your commit message and the Weston configuration here both target the output format, 
but the patch modifies the functions vop2_convert_format and vop2_convert_afbc_format, which are responsible for
converting the data formats of planes/framebuffers (fb)—these have nothing to do with the output format.


>Link: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1859 [1]
>
>Kind regards,
>Nicolas Frattaroli
>
>> 
>> 
>> > 		return VOP2_AFBC_FMT_RGB888;
>> > 	case DRM_FORMAT_RGB565:
>> > 	case DRM_FORMAT_BGR565:
>> >@@ -270,6 +274,19 @@ static bool vop2_win_rb_swap(u32 format)
>> > 	}
>> > }
>> > 
>> >+static bool vop2_afbc_rb_swap(u32 format)
>> >+{
>> >+	switch (format) {
>> >+	case DRM_FORMAT_NV24:
>> >+	case DRM_FORMAT_NV30:
>> >+	case DRM_FORMAT_VUY888:
>> >+	case DRM_FORMAT_VUY101010:
>> >+		return true;
>> >+	default:
>> >+		return false;
>> >+	}
>> >+}
>> >+
>> > static bool vop2_afbc_uv_swap(u32 format)
>> > {
>> > 	switch (format) {
>> >@@ -1291,6 +1308,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
>> > 		 /* It's for head stride, each head size is 16 byte */
>> > 		stride = ALIGN(stride, block_w) / block_w * 16;
>> > 
>> >+		rb_swap = vop2_afbc_rb_swap(fb->format->format);
>> > 		uv_swap = vop2_afbc_uv_swap(fb->format->format);
>> > 		/*
>> > 		 * This is a workaround for crazy IC design, Cluster
>> >@@ -1308,6 +1326,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
>> > 			vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1);
>> > 		vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format);
>> > 		vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap);
>> >+		vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap);
>> > 		/*
>> > 		 * On rk3566/8, this bit is auto gating enable,
>> > 		 * but this function is not work well so we need
>> >
>> 
>
>
>
>

Re: [PATCH v7 02/22] drm: Add new general DRM property "color format"

[Maxime Ripard]

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

Hi,

On Wed, Jan 21, 2026 at 03:45:09PM +0100, Nicolas Frattaroli wrote:
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 7eaec37ae1c7..b5604dca728a 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -556,6 +556,16 @@ enum drm_colorspace {
>  	DRM_MODE_COLORIMETRY_COUNT
>  };
>  
> +enum drm_color_format {
> +	DRM_COLOR_FORMAT_AUTO			= 0,
> +	DRM_COLOR_FORMAT_RGB444			= BIT(0),
> +	DRM_COLOR_FORMAT_YCBCR444		= BIT(1),
> +	DRM_COLOR_FORMAT_YCBCR422		= BIT(2),
> +	DRM_COLOR_FORMAT_YCBCR420		= BIT(3),
> +};
> +
> +#define DRM_COLOR_FORMAT_COUNT 5
> +

I don't really see a reason to expose an enum, with a bunch of values
that are all mutually exclusive, as a bitmask. It's pretty inconsistent
with most (all?) the other similar properties we have.

I appreciate you did that to avoid fixing up every driver using those
values, but then maybe we don't have to? We could create a userspace
facing enum, and convert to DRM_COLOR_FORMAT internally.

Maxime

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

Re: [PATCH v7 03/22] drm: Add enum conversions between DRM_COLOR_FORMAT and HDMI_COLORSPACE

[Maxime Ripard]

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

On Wed, Jan 21, 2026 at 03:45:10PM +0100, Nicolas Frattaroli wrote:
> While the two enums have similar values, they're not identical, and
> HDMI's enum is defined as per the HDMI standard.
> 
> Add a simple conversion function from DRM to HDMI. Unexpected inputs
> aren't handled in any clever way, DRM_COLOR_FORMAT_AUTO and any other
> value that doesn't cleanly map to HDMI just gets returned as
> HDMI_COLORSPACE_RGB.
> 
> Add a second conversion function that gets a DRM_COLOR_FORMAT from an
> HDMI_COLORSPACE as well. In this case, reserved HDMI values that can't
> be converted will result in an -EINVAL return value.
> 
> Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  include/drm/drm_connector.h | 54 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index b5604dca728a..ffeb42f3b4a3 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2612,6 +2612,60 @@ int drm_connector_attach_color_format_property(struct drm_connector *connector);
>  
>  const char *drm_get_color_format_name(enum drm_color_format color_fmt);
>  
> +/**
> + * drm_color_format_to_hdmi_colorspace - convert DRM color format to HDMI
> + * @fmt: the &enum drm_color_format to convert
> + *
> + * Convert a given &enum drm_color_format to an equivalent
> + * &enum hdmi_colorspace. For non-representable values and
> + * %DRM_COLOR_FORMAT_AUTO, the value %HDMI_COLORSPACE_RGB is returned.
> + *
> + * Returns: the corresponding &enum hdmi_colorspace value
> + */
> +static inline enum hdmi_colorspace __pure
> +drm_color_format_to_hdmi_colorspace(enum drm_color_format fmt)
> +{
> +	switch (fmt) {
> +	default:
> +	case DRM_COLOR_FORMAT_AUTO:
> +	case DRM_COLOR_FORMAT_RGB444:
> +		return HDMI_COLORSPACE_RGB;

I don't think that's correct. What auto ends up as totally depends on
the atomic state it comes with.

At the very least, you should output a warning there, because that case
should never happen.

> +	case DRM_COLOR_FORMAT_YCBCR444:
> +		return HDMI_COLORSPACE_YUV444;
> +	case DRM_COLOR_FORMAT_YCBCR422:
> +		return HDMI_COLORSPACE_YUV422;
> +	case DRM_COLOR_FORMAT_YCBCR420:
> +		return HDMI_COLORSPACE_YUV420;
> +	}
> +}
> +
> +/**
> + * drm_color_format_from_hdmi_colorspace - convert HDMI color format to DRM
> + * @fmt: the &enum hdmi_colorspace to convert
> + *
> + * Convert a given &enum hdmi_colorspace to an equivalent
> + * &enum drm_color_format. For non-representable values,
> + * %-EINVAL is returned.
> + *
> + * Returns: the corresponding &enum drm_color_format value, or %-EINVAL
> + */
> +static inline enum drm_color_format __pure
> +drm_color_format_from_hdmi_colorspace(enum hdmi_colorspace fmt)
> +{
> +	switch (fmt) {
> +	default:
> +		return -EINVAL;

Wait, what?

-EINVAL is not a valid value for your enum.

Maxime

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

Re: [PATCH v7 05/22] drm/display: hdmi-state-helper: Act on color format DRM property

[Maxime Ripard]

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

On Wed, Jan 21, 2026 at 03:45:12PM +0100, Nicolas Frattaroli wrote:
> With the introduction of the "color format" DRM property, which allows
> userspace to request a specific color format, the HDMI state helper
> should implement this.
> 
> Implement it by checking whether the property is set and set to
> something other than auto. If so, pass the requested color format, and
> otherwise set RGB.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index a1d16762ac7a..1ea3b9c93aa5 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -649,11 +649,21 @@ hdmi_compute_config(const struct drm_connector *connector,
>  	unsigned int max_bpc = clamp_t(unsigned int,
>  				       conn_state->max_bpc,
>  				       8, connector->max_bpc);
> +	enum hdmi_colorspace hdmi_colorspace =
> +		drm_color_format_to_hdmi_colorspace(conn_state->color_format);
>  	int ret;
>  
>  	ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
> -				      HDMI_COLORSPACE_RGB);
> +				      hdmi_colorspace);

I still think we shoud be more explicit there, with something like

if (conn_state->color_format != DRM_COLOR_FORMAT_AUTO) {
	return hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc, drm_color_format_to_hdmi_colorspace(conn_state->color_format))
}

ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
    			      HDMI_COLORSPACE_RGB);

Otherwise, it's pretty easy to get confused between the behaviour
drm_color_format_to_hdmi_colorspace() to return RGB when it get AUTO...

>  	if (ret) {
> +		/* If a color format was explicitly requested, don't fall back */
> +		if (conn_state->color_format) {

... or that auto is actually 0 in that enum.

Or between the auto and non-auto code path.

Maxime

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

Re: [PATCH v7 02/22] drm: Add new general DRM property "color format"

[Nicolas Frattaroli]

Hello,

On Friday, 6 February 2026 15:05:08 Central European Standard Time Maxime Ripard wrote:
> Hi,
> 
> On Wed, Jan 21, 2026 at 03:45:09PM +0100, Nicolas Frattaroli wrote:
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 7eaec37ae1c7..b5604dca728a 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -556,6 +556,16 @@ enum drm_colorspace {
> >  	DRM_MODE_COLORIMETRY_COUNT
> >  };
> >  
> > +enum drm_color_format {
> > +	DRM_COLOR_FORMAT_AUTO			= 0,
> > +	DRM_COLOR_FORMAT_RGB444			= BIT(0),
> > +	DRM_COLOR_FORMAT_YCBCR444		= BIT(1),
> > +	DRM_COLOR_FORMAT_YCBCR422		= BIT(2),
> > +	DRM_COLOR_FORMAT_YCBCR420		= BIT(3),
> > +};
> > +
> > +#define DRM_COLOR_FORMAT_COUNT 5
> > +
> 
> I don't really see a reason to expose an enum, with a bunch of values
> that are all mutually exclusive, as a bitmask. It's pretty inconsistent
> with most (all?) the other similar properties we have.
> 
> I appreciate you did that to avoid fixing up every driver using those
> values, but then maybe we don't have to? We could create a userspace
> facing enum, and convert to DRM_COLOR_FORMAT internally.

This is what the series did at v5 and earlier. IMHO it was kind of
counter-productive, because we then had two different things for the
same purpose, and some conversion logic between them. I think it's more
error prone to do it that way (think: mixing up the two), and doesn't
have a clear benefit. Just to give a picture of how bad things get:

1. we have the HDMI color format (aka "HDMI_COLORSPACE")
2. we have driver specific output color formats, e.g. the intel ones
3. we have DRM_COLOR_FORMAT
4. we have the bus formats (multiple per color format)
5. we have the DRM plane formats (again, multiple per color format)

Adding a sixth into the mix feels a bit bad because we'll then need to
justify why we should have another layer of switch-case statements.

Kind regards,
Nicolas Frattaroli

> 
> Maxime
> 

Re: [PATCH v7 10/22] drm/rockchip: vop2: Fix YUV444 output

[Nicolas Frattaroli]

Hi Andy,

On Friday, 23 January 2026 02:29:02 Central European Standard Time Andy Yan wrote:
> 
> Hello Nicolas,
> 
> 在 2026-01-22 20:59:41，"Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> 写道：
> >On Thursday, 22 January 2026 09:28:54 Central European Standard Time Andy Yan wrote:
> >> 
> >> Hello Nicolas，
> >> 
> >> At 2026-01-21 22:45:17, "Nicolas Frattaroli" <nicolas.frattaroli@collabora.com> wrote:
> >> >YUV444 (aka YCbCr444) output isn't working quite right on RK3588. The
> >> >resulting image on the display, while identifying itself as YUV444, has
> >> >some components swapped, even after adding the necessary DRM formats to
> >> >the conversion functions.
> >> >
> >> >Judging by downstream, this is because YUV444 also needs an rb swap
> >> >performed in the AFBC case.
> >> >
> >> >Add the DRM formats to the appropriate switch statements, and add a
> >> >function for checking whether an rb swap needs to be performed in the
> >> >AFBC case.
> >> >
> >> >Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> >> >Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> >> >---
> >> > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 19 +++++++++++++++++++
> >> > 1 file changed, 19 insertions(+)
> >> >
> >> >diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> >index ec3b4fde10db..469c63dd97d5 100644
> >> >--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> >+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> >@@ -176,6 +176,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
> >> > 	case DRM_FORMAT_ARGB2101010:
> >> > 	case DRM_FORMAT_XBGR2101010:
> >> > 	case DRM_FORMAT_ABGR2101010:
> >> >+	case DRM_FORMAT_VUY101010:
> >> > 		return VOP2_FMT_XRGB101010;
> >> > 	case DRM_FORMAT_XRGB8888:
> >> > 	case DRM_FORMAT_ARGB8888:
> >> >@@ -184,6 +185,7 @@ static enum vop2_data_format vop2_convert_format(u32 format)
> >> > 		return VOP2_FMT_ARGB8888;
> >> > 	case DRM_FORMAT_RGB888:
> >> > 	case DRM_FORMAT_BGR888:
> >> >+	case DRM_FORMAT_VUY888:
> >> > 		return VOP2_FMT_RGB888;
> >> > 	case DRM_FORMAT_RGB565:
> >> > 	case DRM_FORMAT_BGR565:
> >> >@@ -225,6 +227,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
> >> > 	case DRM_FORMAT_ARGB2101010:
> >> > 	case DRM_FORMAT_XBGR2101010:
> >> > 	case DRM_FORMAT_ABGR2101010:
> >> >+	case DRM_FORMAT_VUY101010:
> >> > 		return VOP2_AFBC_FMT_ARGB2101010;
> >> > 	case DRM_FORMAT_XRGB8888:
> >> > 	case DRM_FORMAT_ARGB8888:
> >> >@@ -233,6 +236,7 @@ static enum vop2_afbc_format vop2_convert_afbc_format(u32 format)
> >> > 		return VOP2_AFBC_FMT_ARGB8888;
> >> > 	case DRM_FORMAT_RGB888:
> >> > 	case DRM_FORMAT_BGR888:
> >> >+	case DRM_FORMAT_VUY888:
> >> 
> >> How did you test this format? It seems tools like modetest don’t support testing this pattern.
> >> 
> >
> >Hi Andy,
> >
> >using the rest of this series, which implements the "color format"
> >DRM property, and the corresponding weston MR that makes use of it[1].
> >
> >I create a ~/.config/weston.ini with the following contents:
> >
> >    [output]
> >    name=HDMI-A-1
> >    color-format=yuv444
> >
> >This will make Weston try to set the output format to 10-bit YUV444. To
> >limit it to 8-bit, you can add `max-bpc=8`. The monitor's EDID needs to
> >report YUV444 support, otherwise that Weston version won't let you set
> >this property.
> >
> 
> 
> This looks a bit strange. Your commit message and the Weston configuration here both target the output format, 
> but the patch modifies the functions vop2_convert_format and vop2_convert_afbc_format, which are responsible for
> converting the data formats of planes/framebuffers (fb)—these have nothing to do with the output format.

Yep, I've now re-tested this in various ways and this commit doesn't do
what I thought it did. I think when I authored it, this was still doing
BCSH based conversion and may have depended on this at some stage. Also
possible that I didn't do a clean test run of solely these changes to
come to my conclusions.

YUV444 primary planes aren't supported by RK3588 at all from what I gather,
so I have no clue where I ran into this and how this fixed it.

Testing on RK3576 without this, and also playing around with gbm-format,
I also don't ever get into the situation where this is needed for correct
output; it seems like EGLConfig always only exposes RGBA formats anyway,
so Panfrost may still lack YUV format support for the buffers.

I'll drop this patch on the next revision, but I'll keep the changes in
mind if an atomic modesetting test workload that sets YUV plane formats
ever comes to be.

Thanks for the reviews!

Kind regards,
Nicolas Frattaroli

> 
> 
> >Link: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1859 [1]
> >
> >Kind regards,
> >Nicolas Frattaroli
> >
> >> 
> >> 
> >> > 		return VOP2_AFBC_FMT_RGB888;
> >> > 	case DRM_FORMAT_RGB565:
> >> > 	case DRM_FORMAT_BGR565:
> >> >@@ -270,6 +274,19 @@ static bool vop2_win_rb_swap(u32 format)
> >> > 	}
> >> > }
> >> > 
> >> >+static bool vop2_afbc_rb_swap(u32 format)
> >> >+{
> >> >+	switch (format) {
> >> >+	case DRM_FORMAT_NV24:
> >> >+	case DRM_FORMAT_NV30:
> >> >+	case DRM_FORMAT_VUY888:
> >> >+	case DRM_FORMAT_VUY101010:
> >> >+		return true;
> >> >+	default:
> >> >+		return false;
> >> >+	}
> >> >+}
> >> >+
> >> > static bool vop2_afbc_uv_swap(u32 format)
> >> > {
> >> > 	switch (format) {
> >> >@@ -1291,6 +1308,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
> >> > 		 /* It's for head stride, each head size is 16 byte */
> >> > 		stride = ALIGN(stride, block_w) / block_w * 16;
> >> > 
> >> >+		rb_swap = vop2_afbc_rb_swap(fb->format->format);
> >> > 		uv_swap = vop2_afbc_uv_swap(fb->format->format);
> >> > 		/*
> >> > 		 * This is a workaround for crazy IC design, Cluster
> >> >@@ -1308,6 +1326,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
> >> > 			vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1);
> >> > 		vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format);
> >> > 		vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap);
> >> >+		vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap);
> >> > 		/*
> >> > 		 * On rk3566/8, this bit is auto gating enable,
> >> > 		 * but this function is not work well so we need
> >> >
> >> 
> >
> >
> >
> >
> 

Re: [PATCH v7 03/22] drm: Add enum conversions between DRM_COLOR_FORMAT and HDMI_COLORSPACE

[Nicolas Frattaroli]

On Friday, 6 February 2026 15:08:46 Central European Standard Time Maxime Ripard wrote:
> On Wed, Jan 21, 2026 at 03:45:10PM +0100, Nicolas Frattaroli wrote:
> > While the two enums have similar values, they're not identical, and
> > HDMI's enum is defined as per the HDMI standard.
> > 
> > Add a simple conversion function from DRM to HDMI. Unexpected inputs
> > aren't handled in any clever way, DRM_COLOR_FORMAT_AUTO and any other
> > value that doesn't cleanly map to HDMI just gets returned as
> > HDMI_COLORSPACE_RGB.
> > 
> > Add a second conversion function that gets a DRM_COLOR_FORMAT from an
> > HDMI_COLORSPACE as well. In this case, reserved HDMI values that can't
> > be converted will result in an -EINVAL return value.
> > 
> > Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
> > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  include/drm/drm_connector.h | 54 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> > 
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index b5604dca728a..ffeb42f3b4a3 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -2612,6 +2612,60 @@ int drm_connector_attach_color_format_property(struct drm_connector *connector);
> >  
> >  const char *drm_get_color_format_name(enum drm_color_format color_fmt);
> >  
> > +/**
> > + * drm_color_format_to_hdmi_colorspace - convert DRM color format to HDMI
> > + * @fmt: the &enum drm_color_format to convert
> > + *
> > + * Convert a given &enum drm_color_format to an equivalent
> > + * &enum hdmi_colorspace. For non-representable values and
> > + * %DRM_COLOR_FORMAT_AUTO, the value %HDMI_COLORSPACE_RGB is returned.
> > + *
> > + * Returns: the corresponding &enum hdmi_colorspace value
> > + */
> > +static inline enum hdmi_colorspace __pure
> > +drm_color_format_to_hdmi_colorspace(enum drm_color_format fmt)
> > +{
> > +	switch (fmt) {
> > +	default:
> > +	case DRM_COLOR_FORMAT_AUTO:
> > +	case DRM_COLOR_FORMAT_RGB444:
> > +		return HDMI_COLORSPACE_RGB;
> 
> I don't think that's correct. What auto ends up as totally depends on
> the atomic state it comes with.
> 
> At the very least, you should output a warning there, because that case
> should never happen.

Yeah, my hope was to keep this function __pure so that the compiler
has maximum freedom to do whatever. With a WARN, it's got side-effects
now, and we're no longer pure. With a status return value and an output
parameter, it's no longer pure either, because the output parameter is
not local memory.

The limiting factor here is that as I understand correctly, I can't
really extend the hdmi_colorspace enum, as it's basically 1:1 from
the standard. Doing this would be the ideal solution, because we'd
keep the function pure and without surprise conversions happening.

Looking at hdmi_colorspace_get_name in drivers/video/hdmi.c, it returns
"Invalid" for any value not in the enum itself. Would it be allowable
to tack an HDMI_COLORSPACE_INVALID at the end of the enum with perhaps
a negative value, or is there a different approach you'd prefer?

I agree that the AUTO-to-RGB conversion shouldn't happen here, that's
a recipe for things implicitly relying on this behaviour, which isn't
great. (And I think I even do this in "hdmi-state-helper: Act on color
format DRM property", where thinking about it again I agree this isn't
super obvious and should be done explicitly instead.)

> > +	case DRM_COLOR_FORMAT_YCBCR444:
> > +		return HDMI_COLORSPACE_YUV444;
> > +	case DRM_COLOR_FORMAT_YCBCR422:
> > +		return HDMI_COLORSPACE_YUV422;
> > +	case DRM_COLOR_FORMAT_YCBCR420:
> > +		return HDMI_COLORSPACE_YUV420;
> > +	}
> > +}
> > +
> > +/**
> > + * drm_color_format_from_hdmi_colorspace - convert HDMI color format to DRM
> > + * @fmt: the &enum hdmi_colorspace to convert
> > + *
> > + * Convert a given &enum hdmi_colorspace to an equivalent
> > + * &enum drm_color_format. For non-representable values,
> > + * %-EINVAL is returned.
> > + *
> > + * Returns: the corresponding &enum drm_color_format value, or %-EINVAL
> > + */
> > +static inline enum drm_color_format __pure
> > +drm_color_format_from_hdmi_colorspace(enum hdmi_colorspace fmt)
> > +{
> > +	switch (fmt) {
> > +	default:
> > +		return -EINVAL;
> 
> Wait, what?
> 
> -EINVAL is not a valid value for your enum.

Not the only part of the kernel where we rely on the int-ness of
enums, but your complaint has been noted :) I guess this means
this approach won't fly for the opposite direction. Thankfully,
in this direction, we can extend the drm_color_format enum to
have an error value.

Kind regards,
Nicolas Frattaroli

> 
> Maxime
> 

Re: [PATCH v7 18/22] drm/tests: hdmi: Add tests for the color_format property

[Maxime Ripard]

On Wed, 21 Jan 2026 15:45:25 +0100, Nicolas Frattaroli wrote:
> Add some KUnit tests to check the color_format property is working as
> expected with the HDMI state helper.
> 
> Existing tests are extended to also test the DRM_COLOR_FORMAT_AUTO case,
> in order to avoid duplicating test cases. For the explicitly selected
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v7 19/22] drm/tests: hdmi: Add tests for HDMI helper's mode_valid

[Maxime Ripard]

On Wed, 21 Jan 2026 15:45:26 +0100, Nicolas Frattaroli wrote:
> Add some KUnit tests to verify that the HDMI state helper's mode_valid
> implementation does not improperly reject chroma subsampled modes on the
> basis of their clock rate not being satisfiable in RGB.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v7 20/22] drm/tests: edid: Add __maybe_unused attribute to EDID definitions

[Maxime Ripard]

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

Hi,

On Wed, Jan 21, 2026 at 03:45:27PM +0100, Nicolas Frattaroli wrote:
> It's normal for a test suite to need an EDID, but not all EDIDs across
> every test. Silence compiler warnings caused by unused definitions by
> using the appropriate attribute.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

It's not clear to me when the EDIDs could be compiled but the tests
wouldn't, and as far as I know it's all part of the same module.
Expanding a bit on how you can trigger that warning would be great.

With that fixed,
Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime

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

Re: [PATCH v7 21/22] drm/tests: bridge: Add KUnit tests for bridge chain format selection

[Maxime Ripard]

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

Hi,

On Wed, Jan 21, 2026 at 03:45:28PM +0100, Nicolas Frattaroli wrote:
> With the "color format" property, the bridge chain format selection has
> gained increased complexity. Instead of simply finding any sequence of
> bus formats that works, the bridge chain format selection needs to pick
> a sequence that results in the requested color format.
> 
> Add KUnit tests for this new logic. These take the form of some pleasant
> preprocessor macros to make it less cumbersome to define test bridges
> with a set of possible input and output formats.
> 
> The input and output formats are defined for bridges in the form of
> tuples, where the first member defines the input format, and the second
> member defines the output format that can be produced from this input
> format. This means the tests can construct scenarios in which not all
> inputs can be converted to all outputs.
> 
> Some tests are added to test interesting scenarios to exercise the bus
> format selection in the presence of a specific color format request.
> 
> Furthermore, tests are added to verify that bridge chains that end in an
> HDMI connector will always prefer RGB when the color format is
> DRM_COLOR_FORMAT_AUTO, as is the behaviour in the HDMI state helpers.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/tests/drm_bridge_test.c | 812 ++++++++++++++++++++++++++++++++
>  1 file changed, 812 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
> index 887020141c7f..ac86f3dfe518 100644
> --- a/drivers/gpu/drm/tests/drm_bridge_test.c
> +++ b/drivers/gpu/drm/tests/drm_bridge_test.c
> @@ -2,15 +2,23 @@
>  /*
>   * Kunit test for drm_bridge functions
>   */
> +#include <linux/cleanup.h>
> +#include <linux/media-bus-format.h>
> +
>  #include <drm/drm_atomic_state_helper.h>
> +#include <drm/drm_atomic_uapi.h>
>  #include <drm/drm_bridge.h>
>  #include <drm/drm_bridge_connector.h>
>  #include <drm/drm_bridge_helper.h>
> +#include <drm/drm_edid.h>
>  #include <drm/drm_kunit_helpers.h>
> +#include <drm/drm_managed.h>
>  
>  #include <kunit/device.h>
>  #include <kunit/test.h>
>  
> +#include "drm_kunit_edid.h"
> +
>  /*
>   * Mimick the typical "private" struct defined by a bridge driver, which
>   * embeds a bridge plus other fields.
> @@ -37,6 +45,27 @@ struct drm_bridge_init_priv {
>  	bool destroyed;
>  };
>  
> +struct drm_bridge_chain_priv {
> +	struct drm_device drm;
> +	struct drm_encoder encoder;
> +	struct drm_plane *plane;
> +	struct drm_crtc *crtc;
> +	struct drm_connector *connector;
> +	unsigned int num_bridges;
> +
> +	/**
> +	 * @test_bridges: array of pointers to &struct drm_bridge_priv entries
> +	 *                of which the first @num_bridges entries are valid.
> +	 */
> +	struct drm_bridge_priv **test_bridges;
> +	/**
> +	 * @destroyed: bool array of size @num_bridges serving the same function
> +	 *             as &struct drm_bridge_init_priv::destroyed does for a
> +	 *             single bridge test.
> +	 */
> +	bool *destroyed;

AFAIK, the destroyed field was added to test the refcounting work. We
don't really need it here, so there's no point in adding it I.

> +};
> +
>  static struct drm_bridge_priv *bridge_to_priv(struct drm_bridge *bridge)
>  {
>  	return container_of(bridge, struct drm_bridge_priv, bridge);
> @@ -50,6 +79,21 @@ static void drm_test_bridge_priv_destroy(struct drm_bridge *bridge)
>  	priv->destroyed = true;
>  }
>  
> +static void drm_test_bridge_chain_priv_destroy(struct drm_bridge *bridge)
> +{
> +	struct drm_bridge_priv *bridge_priv = bridge_to_priv(bridge);
> +	struct drm_bridge_chain_priv *priv = (struct drm_bridge_chain_priv *)bridge_priv->data;
> +	unsigned int i;
> +
> +	for (i = 0; i < priv->num_bridges; i++) {
> +		if (priv->test_bridges[i] != bridge_priv)
> +			continue;
> +
> +		priv->destroyed[i] = true;
> +		break;
> +	}
> +}
> +

And similarly, we probably can drop that hook.

With this fixed,
Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime

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

Re: [PATCH v7 22/22] drm/bridge: Document bridge chain format selection

[Maxime Ripard]

On Wed, 21 Jan 2026 15:45:29 +0100, Nicolas Frattaroli wrote:
> The bridge chain format selection behaviour was, until now,
> undocumented. With the addition of the "color format" DRM property, it's
> not sufficiently complex enough that documentation is warranted,
> especially for driver authors trying to do the right thing.
> 
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v7 02/22] drm: Add new general DRM property "color format"

[Maxime Ripard]

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

Hi,

On Fri, Feb 06, 2026 at 04:26:56PM +0100, Nicolas Frattaroli wrote:
> On Friday, 6 February 2026 15:05:08 Central European Standard Time Maxime Ripard wrote:
> > On Wed, Jan 21, 2026 at 03:45:09PM +0100, Nicolas Frattaroli wrote:
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index 7eaec37ae1c7..b5604dca728a 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -556,6 +556,16 @@ enum drm_colorspace {
> > >  	DRM_MODE_COLORIMETRY_COUNT
> > >  };
> > >  
> > > +enum drm_color_format {
> > > +	DRM_COLOR_FORMAT_AUTO			= 0,
> > > +	DRM_COLOR_FORMAT_RGB444			= BIT(0),
> > > +	DRM_COLOR_FORMAT_YCBCR444		= BIT(1),
> > > +	DRM_COLOR_FORMAT_YCBCR422		= BIT(2),
> > > +	DRM_COLOR_FORMAT_YCBCR420		= BIT(3),
> > > +};
> > > +
> > > +#define DRM_COLOR_FORMAT_COUNT 5
> > > +
> > 
> > I don't really see a reason to expose an enum, with a bunch of values
> > that are all mutually exclusive, as a bitmask. It's pretty inconsistent
> > with most (all?) the other similar properties we have.
> > 
> > I appreciate you did that to avoid fixing up every driver using those
> > values, but then maybe we don't have to? We could create a userspace
> > facing enum, and convert to DRM_COLOR_FORMAT internally.
> 
> This is what the series did at v5 and earlier. IMHO it was kind of
> counter-productive, because we then had two different things for the
> same purpose, and some conversion logic between them. I think it's more
> error prone to do it that way (think: mixing up the two), and doesn't
> have a clear benefit. Just to give a picture of how bad things get:
> 
> 1. we have the HDMI color format (aka "HDMI_COLORSPACE")
> 2. we have driver specific output color formats, e.g. the intel ones
> 3. we have DRM_COLOR_FORMAT
> 4. we have the bus formats (multiple per color format)
> 5. we have the DRM plane formats (again, multiple per color format)
> 
> Adding a sixth into the mix feels a bit bad because we'll then need to
> justify why we should have another layer of switch-case statements.

Yeah, but they are all semantically different:

* The userspace one you want to introduce is going to be a superset of
  all the valid output format for all the output busses we support (so,
  HDMI + DP + etc.)

* plane formats are the input format, we have much more variation there,
  and we will never output these. We can ignore these.

* bus formats are somewhat similar, they are more about the wiring
  between bridges than anything else, and they are not exposed to
  userspace. We can ignore these too.

* DRM_COLOR_FORMAT are definitely redundant.

* The intel color formats are also redundant, but also internal. I would
  expect them to converge to whatever we come up here eventually (but
  really don't expect you to do that work).

* And HDMI_COLORSPACE is really mandated by the HDMI spec, and is only
  about HDMI connectors. It will never fully overlap with what we come
  up with here, if only because HDMI cares about things we don't.

So we really have two formats in my opinion: the one exposed through the
uapi, and the internal one exposed to driver.

In my view, the internal -> uapi conversion is trivial because the uapi
one is a superset of the internal one (if only for auto). The uapi ->
internal one needs to deal and resolve what auto means, but your code
already does that.

I don't really care about the internal format, as long as drivers don't
have to be smart about it, so auto shouldn't be exposed to drivers. As
far as I'm concerned, DRM_COLOR_FORMAT would fit that bill if it wasn't
for the fact that it's both a bitmask and an enum depending on the
context, which makes it pretty weird and error prone to deal with.

Maxime

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

Re: [PATCH v7 03/22] drm: Add enum conversions between DRM_COLOR_FORMAT and HDMI_COLORSPACE

[Maxime Ripard]

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

Hi,

On Sat, Feb 07, 2026 at 08:55:16PM +0100, Nicolas Frattaroli wrote:
> On Friday, 6 February 2026 15:08:46 Central European Standard Time Maxime Ripard wrote:
> > On Wed, Jan 21, 2026 at 03:45:10PM +0100, Nicolas Frattaroli wrote:
> > > While the two enums have similar values, they're not identical, and
> > > HDMI's enum is defined as per the HDMI standard.
> > > 
> > > Add a simple conversion function from DRM to HDMI. Unexpected inputs
> > > aren't handled in any clever way, DRM_COLOR_FORMAT_AUTO and any other
> > > value that doesn't cleanly map to HDMI just gets returned as
> > > HDMI_COLORSPACE_RGB.
> > > 
> > > Add a second conversion function that gets a DRM_COLOR_FORMAT from an
> > > HDMI_COLORSPACE as well. In this case, reserved HDMI values that can't
> > > be converted will result in an -EINVAL return value.
> > > 
> > > Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
> > > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > ---
> > >  include/drm/drm_connector.h | 54 +++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 54 insertions(+)
> > > 
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index b5604dca728a..ffeb42f3b4a3 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -2612,6 +2612,60 @@ int drm_connector_attach_color_format_property(struct drm_connector *connector);
> > >  
> > >  const char *drm_get_color_format_name(enum drm_color_format color_fmt);
> > >  
> > > +/**
> > > + * drm_color_format_to_hdmi_colorspace - convert DRM color format to HDMI
> > > + * @fmt: the &enum drm_color_format to convert
> > > + *
> > > + * Convert a given &enum drm_color_format to an equivalent
> > > + * &enum hdmi_colorspace. For non-representable values and
> > > + * %DRM_COLOR_FORMAT_AUTO, the value %HDMI_COLORSPACE_RGB is returned.
> > > + *
> > > + * Returns: the corresponding &enum hdmi_colorspace value
> > > + */
> > > +static inline enum hdmi_colorspace __pure
> > > +drm_color_format_to_hdmi_colorspace(enum drm_color_format fmt)
> > > +{
> > > +	switch (fmt) {
> > > +	default:
> > > +	case DRM_COLOR_FORMAT_AUTO:
> > > +	case DRM_COLOR_FORMAT_RGB444:
> > > +		return HDMI_COLORSPACE_RGB;
> > 
> > I don't think that's correct. What auto ends up as totally depends on
> > the atomic state it comes with.
> > 
> > At the very least, you should output a warning there, because that case
> > should never happen.
> 
> Yeah, my hope was to keep this function __pure so that the compiler
> has maximum freedom to do whatever. With a WARN, it's got side-effects
> now, and we're no longer pure. With a status return value and an output
> parameter, it's no longer pure either, because the output parameter is
> not local memory.
> 
> The limiting factor here is that as I understand correctly, I can't
> really extend the hdmi_colorspace enum, as it's basically 1:1 from
> the standard. Doing this would be the ideal solution, because we'd
> keep the function pure and without surprise conversions happening.

I feel like this kind of loops back into the other two reviews I did:
you paint yourself into a corner by having auto in the enum, and by
passing it directly to that function.

If, instead, you don't allow auto in the drm_color_format enum, and
resolve auto in the hdmi_compute_config function instead of passing it
directly, then we don't have to deal with it here.

> Looking at hdmi_colorspace_get_name in drivers/video/hdmi.c, it returns
> "Invalid" for any value not in the enum itself. Would it be allowable
> to tack an HDMI_COLORSPACE_INVALID at the end of the enum with perhaps
> a negative value, or is there a different approach you'd prefer?

And again, if we only ever have to deal with RGB, YUV420, 444 or 422,
then we always have valid values for HDMI_COLORSPACE.

Plus, the hdmi_colorspace enum matches what the hdmi spec defines, so we
can't really extend it, and most importantly, hdmi_colorspace_get_name()
is only ever used for debugging / logging purposes, it's never in the
"functional" path.

Maxime

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

Re: [PATCH v7 03/22] drm: Add enum conversions between DRM_COLOR_FORMAT and HDMI_COLORSPACE

[Nicolas Frattaroli]

On Tuesday, 10 February 2026 18:24:46 Central European Standard Time Maxime Ripard wrote:
> Hi,
> 
> On Sat, Feb 07, 2026 at 08:55:16PM +0100, Nicolas Frattaroli wrote:
> > On Friday, 6 February 2026 15:08:46 Central European Standard Time Maxime Ripard wrote:
> > > On Wed, Jan 21, 2026 at 03:45:10PM +0100, Nicolas Frattaroli wrote:
> > > > While the two enums have similar values, they're not identical, and
> > > > HDMI's enum is defined as per the HDMI standard.
> > > > 
> > > > Add a simple conversion function from DRM to HDMI. Unexpected inputs
> > > > aren't handled in any clever way, DRM_COLOR_FORMAT_AUTO and any other
> > > > value that doesn't cleanly map to HDMI just gets returned as
> > > > HDMI_COLORSPACE_RGB.
> > > > 
> > > > Add a second conversion function that gets a DRM_COLOR_FORMAT from an
> > > > HDMI_COLORSPACE as well. In this case, reserved HDMI values that can't
> > > > be converted will result in an -EINVAL return value.
> > > > 
> > > > Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
> > > > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > > ---
> > > >  include/drm/drm_connector.h | 54 +++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 54 insertions(+)
> > > > 
> > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > > index b5604dca728a..ffeb42f3b4a3 100644
> > > > --- a/include/drm/drm_connector.h
> > > > +++ b/include/drm/drm_connector.h
> > > > @@ -2612,6 +2612,60 @@ int drm_connector_attach_color_format_property(struct drm_connector *connector);
> > > >  
> > > >  const char *drm_get_color_format_name(enum drm_color_format color_fmt);
> > > >  
> > > > +/**
> > > > + * drm_color_format_to_hdmi_colorspace - convert DRM color format to HDMI
> > > > + * @fmt: the &enum drm_color_format to convert
> > > > + *
> > > > + * Convert a given &enum drm_color_format to an equivalent
> > > > + * &enum hdmi_colorspace. For non-representable values and
> > > > + * %DRM_COLOR_FORMAT_AUTO, the value %HDMI_COLORSPACE_RGB is returned.
> > > > + *
> > > > + * Returns: the corresponding &enum hdmi_colorspace value
> > > > + */
> > > > +static inline enum hdmi_colorspace __pure
> > > > +drm_color_format_to_hdmi_colorspace(enum drm_color_format fmt)
> > > > +{
> > > > +	switch (fmt) {
> > > > +	default:
> > > > +	case DRM_COLOR_FORMAT_AUTO:
> > > > +	case DRM_COLOR_FORMAT_RGB444:
> > > > +		return HDMI_COLORSPACE_RGB;
> > > 
> > > I don't think that's correct. What auto ends up as totally depends on
> > > the atomic state it comes with.
> > > 
> > > At the very least, you should output a warning there, because that case
> > > should never happen.
> > 
> > Yeah, my hope was to keep this function __pure so that the compiler
> > has maximum freedom to do whatever. With a WARN, it's got side-effects
> > now, and we're no longer pure. With a status return value and an output
> > parameter, it's no longer pure either, because the output parameter is
> > not local memory.
> > 
> > The limiting factor here is that as I understand correctly, I can't
> > really extend the hdmi_colorspace enum, as it's basically 1:1 from
> > the standard. Doing this would be the ideal solution, because we'd
> > keep the function pure and without surprise conversions happening.
> 
> I feel like this kind of loops back into the other two reviews I did:
> you paint yourself into a corner by having auto in the enum, and by
> passing it directly to that function.
> 
> If, instead, you don't allow auto in the drm_color_format enum, and
> resolve auto in the hdmi_compute_config function instead of passing it
> directly, then we don't have to deal with it here.

You're right. Though that means I need to figure out how I'll handle
the conversion from enum to bitfield for !HDMI cases. I guess for DP,
I can do this in drm_dp_helper or something. I don't know if i915 or
amdgpu call into that at all though, but it's as good of a time as any
to start doing so now if not. Just also learned about the existence of
`enum dp_pixelformat`, which imho has the best naming out of all of
these enums. (I hope "HDMI Colorspace" to describe pixel formats is
straight from the standard because if it's something we in the kernel
came up with then I will put renaming it on my list of long-term
kernel tasks to get around to some day.)

And yes I will add the necessary KUnit tests for the DP helper side of
things as well. :)

Nothing registers the property for DSI so we should be good on that front
for now.

Thanks for the reviews.

Kind regards,
Nicolas Frattaroli

> > Looking at hdmi_colorspace_get_name in drivers/video/hdmi.c, it returns
> > "Invalid" for any value not in the enum itself. Would it be allowable
> > to tack an HDMI_COLORSPACE_INVALID at the end of the enum with perhaps
> > a negative value, or is there a different approach you'd prefer?
> 
> And again, if we only ever have to deal with RGB, YUV420, 444 or 422,
> then we always have valid values for HDMI_COLORSPACE.
> 
> Plus, the hdmi_colorspace enum matches what the hdmi spec defines, so we
> can't really extend it, and most importantly, hdmi_colorspace_get_name()
> is only ever used for debugging / logging purposes, it's never in the
> "functional" path.
> 
> Maxime
>