Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v14 17/28] drm/rockchip: vop2: Recognise 10-bit YUV422 as YUV format
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli, Cristian Ciocaltea
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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.

Add the two component orderings of the YUV422 10-bit bus formats to the
switch statement.

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

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index d3b76e7e785e..59d15bacf023 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -352,6 +352,8 @@ 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_UYYVYY8_0_5X24:
 	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
 	case MEDIA_BUS_FMT_YUYV8_2X8:

-- 
2.53.0



^ permalink raw reply related

* [PATCH v14 19/28] drm/bridge: dw-hdmi-qp: Use common HDMI output bus fmts helper
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli, Cristian Ciocaltea
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

Make use of the common drm_bridge_funcs.atomic_get_output_bus_fmts
helper for HDMI bridge connectors.

This allows dw-hdmi-qp HDMI bridges to participate in recursive bus
format selection in a meaningful way.

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

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index d649a1cf07f5..5380aa7d82a8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -1192,6 +1192,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 = drm_atomic_helper_bridge_get_hdmi_output_bus_fmts,
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_reset = drm_atomic_helper_bridge_reset,

-- 
2.53.0



^ permalink raw reply related

* [PATCH v14 15/28] drm/rockchip: Add YUV422 output mode constants for VOP2
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli, Andy Yan
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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).

Reviewed-by: Andy Yan <andyshrk@163.com>
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.53.0



^ permalink raw reply related

* [PATCH v14 22/28] drm/connector: Register color format property on HDMI connectors
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli, Dmitry Baryshkov
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_connector.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 41a5ab1e563e..b91c1f76355e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -627,6 +627,10 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
 	if (max_bpc > 8)
 		drm_connector_attach_hdr_output_metadata_property(connector);
 
+	ret = drm_connector_attach_color_format_property(connector, supported_formats);
+	if (ret)
+		return ret;
+
 	connector->hdmi.funcs = hdmi_funcs;
 
 	return 0;

-- 
2.53.0



^ permalink raw reply related

* [PATCH v14 24/28] drm/tests: hdmi: Add tests for HDMI helper's mode_valid
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
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 3444c93c615f..74c9933eabfc 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)
 {
@@ -2745,11 +2762,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.53.0



^ permalink raw reply related

* [PATCH v14 23/28] drm/tests: hdmi: Add tests for the color_format property
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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_CONNECTOR_COLOR_FORMAT_AUTO case, in order to avoid duplicating test
cases. For the explicitly selected color format cases, parameterized
tests are added.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 236 +++++++++++++++++++++
 1 file changed, 236 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 a4357efaa983..3444c93c615f 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_CONNECTOR_COLOR_FORMAT_AUTO);
 
 	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 10);
 	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, DRM_OUTPUT_COLOR_FORMAT_RGB444);
@@ -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_CONNECTOR_COLOR_FORMAT_AUTO);
 
 	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
 	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, DRM_OUTPUT_COLOR_FORMAT_RGB444);
@@ -2198,6 +2219,217 @@ static void drm_test_check_disable_connector(struct kunit *test)
 	drm_modeset_acquire_fini(&ctx);
 }
 
+struct color_format_test_param {
+	enum drm_connector_color_format fmt;
+	enum drm_output_color_format 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_CONNECTOR_COLOR_FORMAT_AUTO results in an output format of RGB
+ * - DRM_CONNECTOR_COLOR_FORMAT_YCBCR422 results in an output format of YUV422
+ * - DRM_CONNECTOR_COLOR_FORMAT_YCBCR420 results in an output format of YUV420
+ * - DRM_CONNECTOR_COLOR_FORMAT_YCBCR444 results in an output format of YUV444
+ * - DRM_CONNECTOR_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(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+				BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
+				BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420) |
+				BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444),
+				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[] = {
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_AUTO,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_RGB444,
+		.expected_ret = 0,
+		.desc = "AUTO -> RGB"
+	},
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR422,
+		.expected_ret = 0,
+		.desc = "YCBCR422 -> YUV422"
+	},
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR420,
+		.expected_ret = 0,
+		.desc = "YCBCR420 -> YUV420"
+	},
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR444,
+		.expected_ret = 0,
+		.desc = "YCBCR444 -> YUV444"
+	},
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_RGB444,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_RGB444,
+		.expected_ret = 0,
+		.desc = "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_CONNECTOR_COLOR_FORMAT_RGB444 will fail
+ *   drm_atomic_check_only for the YUV420-only mode with -EINVAL
+ * - color format DRM_CONNECTOR_COLOR_FORMAT_YCBCR444 will fail
+ *   drm_atomic_check_only for the YUV420-only mode with -EINVAL
+ * - color format DRM_CONNECTOR_COLOR_FORMAT_YCBCR422 will fail
+ *   drm_atomic_check_only for the YUV420-only mode with -EINVAL
+ * - color format DRM_CONNECTOR_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(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+				BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
+				BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420) |
+				BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444),
+				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[] = {
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_RGB444,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_RGB444,
+		.expected_ret = -EINVAL,
+		.desc = "RGB should fail"
+	},
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR444,
+		.expected_ret = -EINVAL,
+		.desc = "YUV444 should fail"
+	},
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR422,
+		.expected_ret = -EINVAL,
+		.desc = "YUV422 should fail"
+	},
+	{
+		.fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
+		.expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR420,
+		.expected_ret = 0,
+		.desc = "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 +2459,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.53.0



^ permalink raw reply related

* [PATCH v14 18/28] drm/rockchip: vop2: Set correct output format for RK3576 YUV422
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli, Andy Yan
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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.

Reviewed-by: Andy Yan <andyshrk@163.com>
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 59d15bacf023..e76e00f0a530 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1702,6 +1702,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.53.0



^ permalink raw reply related

* [PATCH v14 25/28] drm/tests: bridge: Add KUnit tests for bridge chain format selection
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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_CONNECTOR_COLOR_FORMAT_AUTO, as is the behaviour in the HDMI state
helpers.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/tests/drm_bridge_test.c | 787 ++++++++++++++++++++++++++++++++
 1 file changed, 787 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index 887020141c7f..cb821c606070 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,21 @@ 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;
+};
+
 static struct drm_bridge_priv *bridge_to_priv(struct drm_bridge *bridge)
 {
 	return container_of(bridge, struct drm_bridge_priv, bridge);
@@ -95,6 +118,229 @@ 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) = {						\
+	.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 +426,119 @@ 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->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(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+						     BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
+						     BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
+						     BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420));
+		}
+
+		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 +867,442 @@ 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_CONNECTOR_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_CONNECTOR_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_CONNECTOR_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_CONNECTOR_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, DRM_OUTPUT_COLOR_FORMAT_RGB444);
+
+	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_CONNECTOR_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_CONNECTOR_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.53.0



^ permalink raw reply related

* [PATCH v14 26/28] drm/tests: bridge: Add test for HDMI output bus formats helper
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

The common atomic_get_output_bus_fmts helper for HDMI bridge connectors,
called drm_atomic_helper_bridge_get_hdmi_output_bus_fmts, should return
an array of output bus formats depending on the supported formats of the
connector, and the current output BPC.

Add a test to exercise some of this helper.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/tests/drm_bridge_test.c | 184 ++++++++++++++++++++++++++++++++
 1 file changed, 184 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index cb821c606070..d9bd930b1197 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -5,6 +5,7 @@
 #include <linux/cleanup.h>
 #include <linux/media-bus-format.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_atomic_uapi.h>
 #include <drm/drm_bridge.h>
@@ -118,6 +119,28 @@ static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
 	.atomic_reset		= drm_atomic_helper_bridge_reset,
 };
 
+static int dummy_clear_infoframe(struct drm_bridge *bridge)
+{
+	return 0;
+}
+
+static int dummy_write_infoframe(struct drm_bridge *bridge, const u8 *buffer,
+				 size_t len)
+{
+	return 0;
+}
+
+static const struct drm_bridge_funcs drm_test_bridge_bus_fmts_funcs = {
+	.atomic_get_output_bus_fmts	= drm_atomic_helper_bridge_get_hdmi_output_bus_fmts,
+	.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,
+	.hdmi_write_avi_infoframe	= dummy_write_infoframe,
+	.hdmi_write_hdmi_infoframe	= dummy_write_infoframe,
+	.hdmi_clear_avi_infoframe	= dummy_clear_infoframe,
+	.hdmi_clear_hdmi_infoframe	= dummy_clear_infoframe,
+};
+
 /**
  * struct fmt_tuple - a tuple of input/output MEDIA_BUS_FMT_*
  */
@@ -539,6 +562,83 @@ drm_test_bridge_chain_init(struct kunit *test, unsigned int num_bridges,
 	return priv;
 }
 
+static struct drm_bridge_init_priv *
+drm_test_bridge_hdmi_init(struct kunit *test, const struct drm_bridge_funcs *funcs,
+			  unsigned int supported_formats, int max_bpc)
+{
+	struct drm_bridge_init_priv *priv;
+	struct drm_encoder *enc;
+	struct drm_bridge *bridge;
+	struct drm_device *drm;
+	struct device *dev;
+	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_init_priv, drm,
+						 DRIVER_MODESET | DRIVER_ATOMIC);
+	if (IS_ERR(priv))
+		return ERR_CAST(priv);
+
+	priv->test_bridge = devm_drm_bridge_alloc(dev, struct drm_bridge_priv, bridge, funcs);
+	if (IS_ERR(priv->test_bridge))
+		return ERR_CAST(priv->test_bridge);
+
+	priv->test_bridge->data = priv;
+
+	drm = &priv->drm;
+	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);
+
+	bridge = &priv->test_bridge->bridge;
+	bridge->type = DRM_MODE_CONNECTOR_HDMIA;
+	bridge->supported_formats = supported_formats;
+	bridge->max_bpc = max_bpc;
+	bridge->ops |= DRM_BRIDGE_OP_HDMI;
+	bridge->vendor = "LNX";
+	bridge->product = "KUnit";
+
+	ret = drm_kunit_bridge_add(test, bridge);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = drm_bridge_attach(enc, bridge, NULL, 0);
+	if (ret)
+		return ERR_PTR(ret);
+
+	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);
+
+	return priv;
+}
+
 /*
  * Test that drm_bridge_get_current_state() returns the last committed
  * state for an atomic bridge.
@@ -786,10 +886,94 @@ static void drm_test_drm_bridge_helper_reset_crtc_legacy(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, bridge_priv->disable_count, 1);
 }
 
+/*
+ * Test that a bridge using the drm_atomic_helper_bridge_get_hdmi_output_bus_fmts()
+ * function for &drm_bridge_funcs.atomic_get_output_bus_fmts behaves as expected
+ * for an HDMI connector bridge. Does so by creating an HDMI bridge connector
+ * with RGB444, YCBCR444, and YCBCR420 (but not YCBCR422) as supported formats,
+ * sets the output depth to 8 bits per component, and then validates the returned
+ * list of bus formats.
+ */
+static void drm_test_drm_bridge_helper_hdmi_output_bus_fmts(struct kunit *test)
+{
+	struct drm_connector_state *conn_state;
+	struct drm_bridge_state *bridge_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_init_priv *priv;
+	struct drm_crtc_state *crtc_state;
+	struct drm_atomic_state *state;
+	struct drm_display_mode *mode;
+	unsigned int num_output_fmts;
+	struct drm_bridge *bridge;
+	u32 *out_bus_fmts;
+	int ret;
+
+	priv = drm_test_bridge_hdmi_init(test, &drm_test_bridge_bus_fmts_funcs,
+					 BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+					 BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
+					 BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420),
+					 12);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	bridge = &priv->test_bridge->bridge;
+
+	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);
+
+	conn_state->hdmi.output_bpc = 8;
+
+	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;
+
+	bridge_state = drm_atomic_get_bridge_state(state, bridge);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bridge_state);
+
+	out_bus_fmts = bridge->funcs->atomic_get_output_bus_fmts(
+		bridge, bridge_state, crtc_state, conn_state, &num_output_fmts);
+	KUNIT_EXPECT_NOT_NULL(test, out_bus_fmts);
+	KUNIT_EXPECT_EQ(test, num_output_fmts, 3);
+
+	KUNIT_EXPECT_EQ(test, out_bus_fmts[0], MEDIA_BUS_FMT_RGB888_1X24);
+	KUNIT_EXPECT_EQ(test, out_bus_fmts[1], MEDIA_BUS_FMT_YUV8_1X24);
+	KUNIT_EXPECT_EQ(test, out_bus_fmts[2], MEDIA_BUS_FMT_UYYVYY8_0_5X24);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	kfree(out_bus_fmts);
+}
+
 static struct kunit_case drm_bridge_helper_reset_crtc_tests[] = {
 	KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_atomic),
 	KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_atomic_disabled),
 	KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_legacy),
+	KUNIT_CASE(drm_test_drm_bridge_helper_hdmi_output_bus_fmts),
 	{ }
 };
 

-- 
2.53.0



^ permalink raw reply related

* [PATCH v14 27/28] drm/bridge: Document bridge chain format selection
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 Documentation/gpu/drm-kms-helpers.rst |  6 ++++++
 drivers/gpu/drm/drm_bridge.c          | 40 +++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index b4a9e5ae81f6..bf5a9d909cf3 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -169,6 +169,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 3843d45a82cb..8ddac8424163 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -198,6 +198,46 @@
  * 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 1:1 to what the display
+ * driver should set at its output. 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_CONNECTOR_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_CONNECTOR_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.53.0



^ permalink raw reply related

* [PATCH v14 28/28] drm/connector: Update docs of "colorspace" for color format prop
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

The colorspace property's documentation states that BT2020_RGB and
BT2020_YCC are equivalent, and the output format depends on the driver.

Now that there is a "color format" property that userspace can use to
explicitly set a format, update the colorspace docs to mention this.

The behaviour here is not changed for userspace that doesn't know about
the color format property yet, as the color format property defaults to
"AUTO", where the choice of output format is left up to drivers.

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

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b91c1f76355e..52f9a6a8daf7 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2573,7 +2573,8 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
  *		conversion matrix and convert to the appropriate quantization
  *		range.
  *		The variants BT2020_RGB and BT2020_YCC are equivalent and the
- *		driver chooses between RGB and YCbCr on its own.
+ *		driver chooses between RGB and YCbCr based on the color format
+ *		property.
  *
  *	SMPTE_170M_YCC:
  *	BT709_YCC:

-- 
2.53.0



^ permalink raw reply related

* [PATCH v14 21/28] drm/rockchip: dw_hdmi_qp: Set supported_formats platdata
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli, Cristian Ciocaltea
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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.

Reviewed-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
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 9bf727c1dd7d..a7a8bdb76ce6 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -602,6 +602,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(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
+				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422);
+
 	encoder = &hdmi->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
 

-- 
2.53.0



^ permalink raw reply related

* Re: [PATCH v13 0/4] Enable Remote GPIO over RPMSG on i.MX Platform
From: Shenwei Wang @ 2026-04-23 19:08 UTC (permalink / raw)
  To: Mathieu Poirier, Andrew Lunn
  Cc: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Frank Li,
	Sascha Hauer, Shuah Khan, linux-gpio@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
	devicetree@vger.kernel.org, linux-remoteproc@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	dl-linux-imx
In-Reply-To: <CANLsYkypRaFTTP7MLLLR+=AB5JnRTA4i130qvWzB1qoAuM9FWQ@mail.gmail.com>



> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: Thursday, April 23, 2026 7:53 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Andrew Lunn <andrew@lunn.ch>
> Cc: Linus Walleij <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>;
> Jonathan Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn
> Andersson <andersson@kernel.org>; Frank Li <frank.li@nxp.com>; Sascha Hauer
> <s.hauer@pengutronix.de>; Shuah Khan <skhan@linuxfoundation.org>; linux-
> 
> Once again Andrew Lunn was left out.
> 

Seems an issue of get_maintainer.pl, but he is included in the thread for the gpio‑rpmsg driver patch..
Cc: Andrew Lunn <andrew@lunn.ch>

> On Wed, 22 Apr 2026 at 15:29, Shenwei Wang <shenwei.wang@nxp.com>
> wrote:
> >
> > Support the remote devices on the remote processor via the RPMSG bus
> > on i.MX platform.
> >
> > Changes in v13:
> >  - drop the support for legacy NXP firmware.
> >  - remove the fixed_up hooks from the rpmsg gpio driver.
> >  - code cleanup.
> >
> > Changes in v12:
> >  - Fixed the "underline" warning reported by Randy.
> >
> > Changes in v11:
> >  - Expand RPMSG for the first time per Shuah's review comment.
> >
> > Changes in v10:
> >  - Update gpio-rpmsg.rst according to Daniel Baluta's review comments.
> >  - Add a kernel CONFIG for fixed up handlers and only enable it on
> >    i.MX products.
> >  - Fixed bugs reported by kernel test robot.
> >
> > Changes in v9:
> >  - Reuse the gpio-virtio design for command and IRQ type definitions.
> >  - Remove msg_id, version, and vendor fields from the generic protocol.
> >  - Add fixed-up handlers to support legacy firmware.
> >
> > Changes in v8:
> >  - Add "depends on REMOTEPROC" in Kconfig to fix the build error reported
> >    by the kernel test robot.
> >  - Move the .rst patch before the .yaml patch.
> >  - Handle the "ngpios" DT property based on Andrew's feedback.
> >
> > Changes in v7:
> >  - Reworked the driver to use the rpmsg_driver framework instead of
> >    platform_driver, based on feedback from Bjorn and Arnaud.
> >  - Updated gpio-rpmsg.yaml and imx_rproc.yaml according to comments from
> >    Rob and Arnaud.
> >  - Further refinements to gpio-rpmsg.yaml per Arnaud's feedback.
> >
> > Changes in v6:
> >  - make the driver more generic with the actions below:
> >      rename the driver file to gpio-rpmsg.c
> >      remove the imx related info in the function and variable names
> >      rename the imx_rpmsg.h to rpdev_info.h
> >      create a gpio-rpmsg.yaml and refer it in imx_rproc.yaml
> >  - update the gpio-rpmsg.rst according to the feedback from Andrew and
> >    move the source file to driver-api/gpio
> >  - fix the bug reported by Zhongqiu Han
> >  - remove the I2C related info
> >
> > Changes in v5:
> >  - move the gpio-rpmsg.rst from admin-guide to staging directory after
> >    discussion with Randy Dunlap.
> >  - add include files with some code improvements per Bartosz's comments.
> >
> > Changes in v4:
> >  - add a documentation to describe the transport protocol per Andrew's
> >    comments.
> >  - add a new handler to get the gpio direction.
> >
> > Changes in v3:
> >  - fix various format issue and return value check per Peng 's review
> >    comments.
> >  - add the logic to also populate the subnodes which are not in the
> >    device map per Arnaud's request. (in imx_rproc.c)
> >  - update the yaml per Frank's review comments.
> >
> > Changes in v2:
> >  - re-implemented the gpio driver per Linus Walleij's feedback by using
> >    GPIOLIB_IRQCHIP helper library.
> >  - fix various format issue per Mathieu/Peng 's review comments.
> >  - update the yaml doc per Rob's feedback
> >
> > Shenwei Wang (4):
> >   docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
> >   dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
> >   gpio: rpmsg: add generic rpmsg GPIO driver
> >   arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
> >
> >  .../devicetree/bindings/gpio/gpio-rpmsg.yaml  |  55 ++
> >  .../bindings/remoteproc/fsl,imx-rproc.yaml    |  53 ++
> >  Documentation/driver-api/gpio/gpio-rpmsg.rst  | 266 ++++++++
> >  Documentation/driver-api/gpio/index.rst       |   1 +
> >  arch/arm64/boot/dts/freescale/imx8ulp.dtsi    |  25 +
> >  drivers/gpio/Kconfig                          |  17 +
> >  drivers/gpio/Makefile                         |   1 +
> >  drivers/gpio/gpio-rpmsg.c                     | 573 ++++++++++++++++++
> >  8 files changed, 991 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> >  create mode 100644 Documentation/driver-api/gpio/gpio-rpmsg.rst
> >  create mode 100644 drivers/gpio/gpio-rpmsg.c
> >
> > --
> > 2.43.0
> >

^ permalink raw reply

* [PATCH v14 20/28] drm/rockchip: dw_hdmi_qp: Implement "color format" DRM property
From: Nicolas Frattaroli @ 2026-04-23 19:03 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan
  Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
	linux-rockchip, intel-gfx, intel-xe, linux-doc, wayland-devel,
	Nicolas Frattaroli, Cristian Ciocaltea
In-Reply-To: <20260423-color-format-v14-0-449a419ccbd4@collabora.com>

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.

Reviewed-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
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 c78db7f8ab6c..9bf727c1dd7d 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>
@@ -44,10 +45,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
@@ -75,8 +72,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)
@@ -88,6 +83,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;
@@ -116,6 +116,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);
@@ -131,29 +158,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);
 	}
@@ -383,6 +464,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)
@@ -390,12 +472,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)
@@ -403,6 +489,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.53.0



^ permalink raw reply related

* Re: [PATCH v13 0/4] Enable Remote GPIO over RPMSG on i.MX Platform
From: Shenwei Wang @ 2026-04-23 19:11 UTC (permalink / raw)
  To: Andrew Lunn, Mathieu Poirier
  Cc: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Frank Li,
	Sascha Hauer, Shuah Khan, linux-gpio@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
	devicetree@vger.kernel.org, linux-remoteproc@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	dl-linux-imx
In-Reply-To: <cb6b8ec9-296e-40fe-848d-ae87463ff1db@lunn.ch>



> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, April 23, 2026 8:54 AM
> To: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Shenwei Wang <shenwei.wang@nxp.com>; Linus Walleij
> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Jonathan Corbet
> <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn Andersson
> <andersson@kernel.org>; Frank Li <frank.li@nxp.com>; Sascha Hauer
> <s.hauer@pengutronix.de>; Shuah Khan <skhan@linuxfoundation.org>; linux-
> gpio@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
> devicetree@vger.kernel.org; linux-remoteproc@vger.kernel.org;
> imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-
> imx@nxp.com>
> Subject: [EXT] Re: [PATCH v13 0/4] Enable Remote GPIO over RPMSG on i.MX
> Platform
> 
> Caution: This is an external email. Please take care when clicking links or opening
> attachments. When in doubt, report the message using the 'Report this email'
> button
> 
> 
> On Thu, Apr 23, 2026 at 06:53:12AM -0600, Mathieu Poirier wrote:
> > Once again Andrew Lunn was left out.
> >
> > On Wed, 22 Apr 2026 at 15:29, Shenwei Wang <shenwei.wang@nxp.com>
> wrote:
> > >
> > > Support the remote devices on the remote processor via the RPMSG bus
> > > on i.MX platform.
> > >
> > > Changes in v13:
> > >  - drop the support for legacy NXP firmware.
> > >  - remove the fixed_up hooks from the rpmsg gpio driver.
> > >  - code cleanup.
> 
> That looks like a step forward. Now we don't care about legacy NXP firmware, it
> makes it easier to make bigger changes, like use the messages format from gpio-
> virtio.
> 

The implementation adopts the gpio‑virtio design, including its commands, parameters, and error codes.

Shenwei

>          Andrew

^ permalink raw reply

* [PATCH V1 0/3] ACPI/IORT: Honor Root Complex PASID descriptors on SMMUv3
From: Vidya Sagar @ 2026-04-23 19:14 UTC (permalink / raw)
  To: rafael, lenb, saket.dumbre, lpieralisi, guohanjun, sudeep.holla,
	will, catalin.marinas, joro, robin.murphy, jgg, nicolinc, praan
  Cc: vsethi, sdonthineni, kthota, sagar.tv, linux-acpi,
	linux-arm-kernel, acpica-devel, iommu, linux-kernel, Vidya Sagar

Hi,
This series enhances Linux to read and honor the Root Complex (RC) PASID
descriptors introduced by IORT Issue E.c (ARM DEN 0049E.c, January 2022,
RC node revision 4):

  - PASID Capabilities at byte offset 33, bits[4:0] = Max PASID Width.
  - Flags at byte offset 36, bit 0 = "Root Complex supports PASID".

Both fields have been spec'd for over four years but are not consumed
anywhere in the tree. Worse, include/acpi/actbl2.h only covers the
PASID Capabilities field - the trailing "u8 reserved[]" flexible array
stops just before the Flags field at offset 36, so even firmware that
populates it cannot be read today. The existing ACPI_IORT_PRI_SUPPORTED,
ACPI_IORT_PASID_FWD_SUPPORTED and ACPI_IORT_PASID_MAX_WIDTH macros are
dead code, and ACPICA upstream has the same gap.

Today arm_smmu_enable_pasid() calls pci_enable_pasid() for any PCI
master with a PASID capability, regardless of whether the upstream RC
actually supports PASID and regardless of the RC's declared Max PASID
Width. On a platform whose RC cannot forward PASID to the SMMU, this
enables the PCIe PASID capability unnecessarily on the endpoint (the
traffic then gets dropped upstream) and sets a non-zero master->ssid_bits,
which complicates teardown ordering with ATS (see the comment block in
arm_smmu_probe_device() for that ordering constraint).

The series fixes this in three bite-sized patches, modeled on the
CANWBS support from commit 807404d66fcf ("ACPI/IORT: Support CANWBS
memory access flag"):

  Patch 1 (ACPICA): extend struct acpi_iort_root_complex to cover the
    offset-36 Flags field and add the ACPI_IORT_RC_PASID_SUPPORTED
    mask. With #pragma pack(1) already in effect for actbl2.h, this
    lands the new field at the spec-mandated absolute offset 36.

  Patch 2 (ACPI/IORT): add three static helpers that read the new RC
    PASID fields (guarded by node->revision >= 4 where appropriate),
    set two new iommu_fwspec flags from iort_iommu_configure_id(), and
    expose two device-facing wrappers so IOMMU drivers can clamp
    endpoint PASID widths without walking the IORT themselves. Stubs
    for !CONFIG_ACPI_IORT live in include/linux/acpi_iort.h.
      IOMMU_FWSPEC_PCI_RC_PASID      (bit 2): RC declares PASID support
      IOMMU_FWSPEC_PCI_RC_PASID_FWD  (bit 3): RC forwards PASID on
                                              translated transactions
  Patch 3 (arm-smmu-v3): consume the new fwspec flag and the new
    max-width helper in arm_smmu_enable_pasid(). If the RC explicitly
    declares no PASID support, skip pci_enable_pasid() silently.
    Otherwise clamp num_pasids by min(endpoint cap, 1 << RC width) and
    only proceed when the effective count is > 1.
    
Policy: all new gating is conditional on
iort_pci_rc_pasid_max_width_known(), i.e. the presence of an IORT RC
node at revision >= 4. Platforms booting with older IORT firmware (RC
node revision 0..3) see no behavioural change and continue to enable
PASID purely on the basis of the endpoint capability. This keeps the
series safe for platforms that ship pre-E.c tables.

Out of scope intentionally:
  - Wiring the new fwspec flags into Intel/AMD IOMMU drivers - IORT
    is ARM-only.
  - Consuming IOMMU_FWSPEC_PCI_RC_PASID_FWD in arm-smmu-v3 beyond the
    existing CD/STE programming. The fwspec flag is plumbed now so
    future work (e.g. gating PASID forwarding hints in STE.S1DSS) has
    the information available.
  - Exposing a "pasid-num-bits" software node property for PCI devices,
    parallel to what iort_named_component_init() already does for named
    components. The direct helper is simpler and avoids creating a
    software node per RC.

Thanks,
Vidya Sagar

Vidya Sagar (3):
  ACPICA: IORT: Add Root Complex PASID Flags field
  ACPI/IORT: Plumb Root Complex PASID descriptors into iommu_fwspec
  iommu/arm-smmu-v3: Honor IORT Root Complex PASID descriptors

 drivers/acpi/arm64/iort.c                   | 80 +++++++++++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 26 ++++++-
 include/acpi/actbl2.h                       |  6 +-
 include/linux/acpi_iort.h                   |  6 ++
 include/linux/iommu.h                       |  4 ++
 5 files changed, 118 insertions(+), 4 deletions(-)

-- 
2.25.1



^ permalink raw reply

* [PATCH V1 1/3] ACPICA: IORT: Add Root Complex PASID Flags field
From: Vidya Sagar @ 2026-04-23 19:14 UTC (permalink / raw)
  To: rafael, lenb, saket.dumbre, lpieralisi, guohanjun, sudeep.holla,
	will, catalin.marinas, joro, robin.murphy, jgg, nicolinc, praan
  Cc: vsethi, sdonthineni, kthota, sagar.tv, linux-acpi,
	linux-arm-kernel, acpica-devel, iommu, linux-kernel, Vidya Sagar
In-Reply-To: <20260423191417.2031652-1-vidyas@nvidia.com>

The IORT spec, Issue E.c (ARM DEN 0049E.c, January 2022), bumps the
Root Complex Node to revision 4 and adds two PASID descriptors:

  - PASID Capabilities at byte offset 33 (2 bytes), bits[4:0] of which
    report the Max PASID Width supported by the Root Complex.
  - Flags at byte offset 36 (4 bytes), bit 0 of which reports whether
    the Root Complex itself supports PASID. This is distinct from the
    existing ATS Attribute bit 2 (at offset 24) that only reports
    whether the RC forwards PASID information on translated
    transactions.

The ACPICA struct in include/acpi/actbl2.h was updated for the E.c
PASID Capabilities descriptor (offset 33) but stops short with a
trailing 'u8 reserved[]' flexible array, so the new Flags field at
offset 36 is unreachable and the existing ACPI_IORT_PASID_*
definitions have no consumer.

Replace the trailing flexible array with a fixed 'u8 reserved[1]'
followed by 'u32 flags' so the struct fully covers RC node revision 4,
and add the ACPI_IORT_RC_PASID_SUPPORTED mask for bit 0 of the new
field. With #pragma pack(1) in effect for actbl2.h, this lands the new
field at the spec-mandated absolute offset 36.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 include/acpi/actbl2.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 5c0b55e7b3e4..5a4450e66358 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -748,7 +748,8 @@ struct acpi_iort_root_complex {
 	u32 pci_segment_number;
 	u8 memory_address_limit;	/* Memory address size limit */
 	u16 pasid_capabilities;	/* PASID Capabilities */
-	u8 reserved[];		/* Reserved, must be zero */
+	u8 reserved[1];		/* Reserved, must be zero */
+	u32 flags;		/* Flags (IORT E.c, RC node revision >= 4) */
 };
 
 /* Masks for ats_attribute field above */
@@ -760,6 +761,9 @@ struct acpi_iort_root_complex {
 /* Masks for pasid_capabilities field above */
 #define ACPI_IORT_PASID_MAX_WIDTH       (0x1F)	/* Bits 0-4 */
 
+/* Masks for flags field above */
+#define ACPI_IORT_RC_PASID_SUPPORTED    (1)	/* The root complex PASID support */
+
 struct acpi_iort_smmu {
 	u64 base_address;	/* SMMU base address */
 	u64 span;		/* Length of memory range */
-- 
2.25.1



^ permalink raw reply related

* [PATCH V1 2/3] ACPI/IORT: Plumb Root Complex PASID descriptors into iommu_fwspec
From: Vidya Sagar @ 2026-04-23 19:14 UTC (permalink / raw)
  To: rafael, lenb, saket.dumbre, lpieralisi, guohanjun, sudeep.holla,
	will, catalin.marinas, joro, robin.murphy, jgg, nicolinc, praan
  Cc: vsethi, sdonthineni, kthota, sagar.tv, linux-acpi,
	linux-arm-kernel, acpica-devel, iommu, linux-kernel, Vidya Sagar
In-Reply-To: <20260423191417.2031652-1-vidyas@nvidia.com>

The IORT spec, Issue E.c (ARM DEN 0049E.c, January 2022), gives
software two ways to learn about Root Complex PASID capability:

  - the Flags field at byte offset 36 (RC node revision >= 4), bit 0
    of which says whether the Root Complex itself supports PASID, and
  - bit 2 of the long-standing ATS Attribute field at offset 24, which
    says whether the Root Complex forwards PASID information on
    translated transactions to the SMMU.

Neither is consumed by Linux today. Modeled after the CANWBS support
in commit 807404d66fcf ("ACPI/IORT: Support CANWBS memory access
flag"), expose both bits via the existing iommu_fwspec mechanism so
IOMMU drivers can consult them at device probe time:

  - IOMMU_FWSPEC_PCI_RC_PASID      (bit 2): RC declares PASID support
  - IOMMU_FWSPEC_PCI_RC_PASID_FWD  (bit 3): RC forwards PASID

Add three new static helpers in iort.c that read the corresponding
fields, with iort_pci_rc_supports_pasid() and the new
iort_pci_rc_pasid_max_width() helper guarded by node->revision >= 4
because their backing storage was only added in E.c. The PASID
forwarding bit lives in the older ats_attribute field and needs no
guard. Set the new fwspec flags from iort_iommu_configure_id().

In addition, expose two device-facing wrappers,
iort_pci_rc_pasid_max_width_known() and _for_dev(), so IOMMU drivers
can clamp endpoint PASID widths by what the Root Complex can actually
carry without having to walk the IORT themselves. Provide stubs in
include/linux/acpi_iort.h for !CONFIG_ACPI_IORT.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/acpi/arm64/iort.c | 80 +++++++++++++++++++++++++++++++++++++++
 include/linux/acpi_iort.h |  6 +++
 include/linux/iommu.h     |  4 ++
 3 files changed, 90 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index af7a9b2fd5bc..40486de6bd79 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1352,6 +1352,36 @@ static bool iort_pci_rc_supports_canwbs(struct acpi_iort_node *node)
 	return memory_access->memory_flags & ACPI_IORT_MF_CANWBS;
 }
 
+static bool iort_pci_rc_supports_pasid(struct acpi_iort_node *node)
+{
+	struct acpi_iort_root_complex *pci_rc;
+
+	if (node->revision < 4)
+		return false;
+
+	pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+	return pci_rc->flags & ACPI_IORT_RC_PASID_SUPPORTED;
+}
+
+static bool iort_pci_rc_supports_pasid_fwd(struct acpi_iort_node *node)
+{
+	struct acpi_iort_root_complex *pci_rc;
+
+	pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+	return pci_rc->ats_attribute & ACPI_IORT_PASID_FWD_SUPPORTED;
+}
+
+static int iort_pci_rc_pasid_max_width(struct acpi_iort_node *node)
+{
+	struct acpi_iort_root_complex *pci_rc;
+
+	if (node->revision < 4)
+		return -ENODEV;
+
+	pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+	return FIELD_GET(ACPI_IORT_PASID_MAX_WIDTH, pci_rc->pasid_capabilities);
+}
+
 static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
 			    u32 streamid)
 {
@@ -1471,6 +1501,10 @@ int iort_iommu_configure_id(struct device *dev, const u32 *id_in)
 			fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
 		if (fwspec && iort_pci_rc_supports_canwbs(node))
 			fwspec->flags |= IOMMU_FWSPEC_PCI_RC_CANWBS;
+		if (fwspec && iort_pci_rc_supports_pasid(node))
+			fwspec->flags |= IOMMU_FWSPEC_PCI_RC_PASID;
+		if (fwspec && iort_pci_rc_supports_pasid_fwd(node))
+			fwspec->flags |= IOMMU_FWSPEC_PCI_RC_PASID_FWD;
 	} else {
 		node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
 				      iort_match_node_callback, dev);
@@ -1556,6 +1590,52 @@ int iort_dma_get_ranges(struct device *dev, u64 *limit)
 		return nc_dma_get_range(dev, limit);
 }
 
+static struct acpi_iort_node *iort_pci_rc_node_for_dev(struct device *dev)
+{
+	struct pci_bus *pbus;
+
+	if (!dev_is_pci(dev))
+		return NULL;
+
+	pbus = to_pci_dev(dev)->bus;
+	return iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+			      iort_match_node_callback, &pbus->dev);
+}
+
+/**
+ * iort_pci_rc_pasid_max_width_known() - Whether IORT firmware describes the
+ * Root Complex PASID width for the given PCI device.
+ * @dev: PCI device to lookup
+ *
+ * Returns true iff a matching IORT Root Complex node exists and has revision
+ * >= 4 (IORT spec E.c), i.e. the PASID Capabilities descriptor is present.
+ */
+bool iort_pci_rc_pasid_max_width_known(struct device *dev)
+{
+	struct acpi_iort_node *node = iort_pci_rc_node_for_dev(dev);
+
+	return node && node->revision >= 4;
+}
+
+/**
+ * iort_pci_rc_pasid_max_width_for_dev() - Look up the Root Complex Max PASID
+ * Width for the given PCI device.
+ * @dev: PCI device to lookup
+ *
+ * Returns the Max PASID Width (bits[4:0] of PASID Capabilities) declared by
+ * the Root Complex node in IORT firmware, or a negative errno when the field
+ * is not present (RC node revision < 4) or the device is not PCI.
+ */
+int iort_pci_rc_pasid_max_width_for_dev(struct device *dev)
+{
+	struct acpi_iort_node *node = iort_pci_rc_node_for_dev(dev);
+
+	if (!node)
+		return -ENODEV;
+
+	return iort_pci_rc_pasid_max_width(node);
+}
+
 static void __init acpi_iort_register_irq(int hwirq, const char *name,
 					  int trigger,
 					  struct resource *res)
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 17bb3374f4ca..befe19d87c2c 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -45,6 +45,8 @@ void iort_put_rmr_sids(struct fwnode_handle *iommu_fwnode,
 int iort_dma_get_ranges(struct device *dev, u64 *limit);
 int iort_iommu_configure_id(struct device *dev, const u32 *id_in);
 void iort_iommu_get_resv_regions(struct device *dev, struct list_head *head);
+bool iort_pci_rc_pasid_max_width_known(struct device *dev);
+int iort_pci_rc_pasid_max_width_for_dev(struct device *dev);
 phys_addr_t acpi_iort_dma_get_max_cpu_address(void);
 #else
 static inline u32 iort_msi_map_id(struct device *dev, u32 id)
@@ -71,6 +73,10 @@ static inline int iort_iommu_configure_id(struct device *dev, const u32 *id_in)
 static inline
 void iort_iommu_get_resv_regions(struct device *dev, struct list_head *head)
 { }
+static inline bool iort_pci_rc_pasid_max_width_known(struct device *dev)
+{ return false; }
+static inline int iort_pci_rc_pasid_max_width_for_dev(struct device *dev)
+{ return -ENODEV; }
 
 static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void)
 { return PHYS_ADDR_MAX; }
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e587d4ac4d33..e78d7f56d603 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1118,6 +1118,10 @@ struct iommu_fwspec {
 #define IOMMU_FWSPEC_PCI_RC_ATS			(1 << 0)
 /* CANWBS is supported */
 #define IOMMU_FWSPEC_PCI_RC_CANWBS		(1 << 1)
+/* Root complex declares PASID support (IORT E.c Flags bit 0) */
+#define IOMMU_FWSPEC_PCI_RC_PASID		(1 << 2)
+/* Root complex forwards PASID on translated transactions (IORT ATS bit 2) */
+#define IOMMU_FWSPEC_PCI_RC_PASID_FWD		(1 << 3)
 
 /*
  * An iommu attach handle represents a relationship between an iommu domain
-- 
2.25.1



^ permalink raw reply related

* [PATCH V1 3/3] iommu/arm-smmu-v3: Honor IORT Root Complex PASID descriptors
From: Vidya Sagar @ 2026-04-23 19:14 UTC (permalink / raw)
  To: rafael, lenb, saket.dumbre, lpieralisi, guohanjun, sudeep.holla,
	will, catalin.marinas, joro, robin.murphy, jgg, nicolinc, praan
  Cc: vsethi, sdonthineni, kthota, sagar.tv, linux-acpi,
	linux-arm-kernel, acpica-devel, iommu, linux-kernel, Vidya Sagar
In-Reply-To: <20260423191417.2031652-1-vidyas@nvidia.com>

The SMMUv3 driver currently calls pci_enable_pasid() for any PCI
master that exposes a PASID capability, regardless of whether the
upstream Root Complex actually supports PASID and regardless of the
RC's declared Max PASID Width. With IORT spec E.c (RC node revision
>= 4) firmware reports both, so we can do better:

  - If the IORT Root Complex node says PASID is not supported
    (Flags bit 0 == 0 at byte offset 36), enabling PASID on the
    endpoint is futile - the RC will not forward the PASID prefix to
    the SMMU - so skip pci_enable_pasid() silently.

  - If the IORT Root Complex node reports a Max PASID Width (bits[4:0]
    of PASID Capabilities at offset 33), clamp the endpoint's
    pci_max_pasids() result by 1 << width before computing the SMMU
    SSID width. This prevents master->ssid_bits from exceeding what
    the RC can actually carry.

Both behaviours are gated on iort_pci_rc_pasid_max_width_known(), i.e.
RC node revision >= 4, so platforms with older IORT firmware see no
behavioural change and continue to enable PASID purely on the basis
of the endpoint capability.

Use the new IOMMU_FWSPEC_PCI_RC_PASID fwspec flag (set by IORT) for
the support check, and call iort_pci_rc_pasid_max_width_for_dev() for
the width clamp; both pieces are wired up in
iort_iommu_configure_id() by the previous patch.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 26 ++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index e8d7dbe495f0..2b269307fd33 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3071,16 +3071,28 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master)
 
 static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
 {
-	int ret;
-	int features;
-	int num_pasids;
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev);
 	struct pci_dev *pdev;
+	int features, num_pasids, ret, rc_width;
 
 	if (!dev_is_pci(master->dev))
 		return -ENODEV;
 
 	pdev = to_pci_dev(master->dev);
 
+	/*
+	 * IORT E.c (RC node revision >= 4) reports whether the root
+	 * complex actually supports PASID. If it does not, enabling
+	 * PASID on the endpoint is futile - the RC will not forward
+	 * the PASID prefix - so skip silently. Older firmware is
+	 * treated as "unknown / assume supported" to preserve the
+	 * pre-E.c behaviour.
+	 */
+	if (fwspec &&
+	    !(fwspec->flags & IOMMU_FWSPEC_PCI_RC_PASID) &&
+	    iort_pci_rc_pasid_max_width_known(master->dev))
+		return 0;
+
 	features = pci_pasid_features(pdev);
 	if (features < 0)
 		return features;
@@ -3089,6 +3101,14 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
 	if (num_pasids <= 0)
 		return num_pasids;
 
+	/* Clamp by what the root complex can carry, when known. */
+	rc_width = iort_pci_rc_pasid_max_width_for_dev(master->dev);
+	if (rc_width >= 0)
+		num_pasids = min_t(int, num_pasids, 1 << rc_width);
+
+	if (num_pasids <= 1)
+		return 0;
+
 	ret = pci_enable_pasid(pdev, features);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to enable PASID\n");
-- 
2.25.1



^ permalink raw reply related

* Re: [PATCH v11 00/14] barrier: Add smp_cond_load_{relaxed,acquire}_timeout()
From: Ankur Arora @ 2026-04-23 19:29 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Ankur Arora, linux-kernel, linux-arch, linux-arm-kernel, linux-pm,
	bpf, arnd, catalin.marinas, will, peterz, mark.rutland, harisokn,
	cl, ast, rafael, daniel.lezcano, memxor, zhenglifeng1, xueshuai,
	rdunlap, david.laight.linux, joao.m.martins, boris.ostrovsky,
	konrad.wilk, ashok.bhat
In-Reply-To: <20260423101631.84f26592d629eee46fbde6f8@linux-foundation.org>


Andrew Morton <akpm@linux-foundation.org> writes:

> On Wed,  8 Apr 2026 17:55:24 +0530 Ankur Arora <ankur.a.arora@oracle.com> wrote:
>
>> The core kernel often uses smp_cond_load_{relaxed,acquire}() to spin
>> on condition variables with architectural primitives used to avoid
>> hammering the relevant cachelines.
>>
>> ...
>>
>> Accordingly add two interfaces (with their generic and arm64 specific
>> implementations):
>>
>>    smp_cond_load_relaxed_timeout(ptr, cond_expr, time_expr, timeout)
>>    smp_cond_load_acquire_timeout(ptr, cond_expr, time_expr, timeout)
>>
>> Also add tif_need_resched_relaxed_wait() which wraps the polling
>> pattern and its scheduler specific details in poll_idle().
>> In addition add atomic_cond_read_*_timeout(),
>
> Thanks, I'll add this to mm.git's mm-new branch today.

Great. Thanks!

> This isn't am MM patchset, but mm-new isn't included in linux-next, and
> linux-next isn't presently open for 7.1 material.
>
> After -rc1 I'll move the series into mm.git's mm-nonmm-unstable branch,
> where it will get linux-next exposure.

Ack that.

> I see that further review/comment has been requested - hopefully this
> will happen over the next couple of months, but please do continue to
> chase this down if you feel the need.

Will do.

>> Haris Okanovic also saw improvement in real workloads due to the
>> cpuidle changes: "observed 4-6% improvements in memcahed, cassandra,
>> mysql, and postgresql under certain loads. Other applications likely
>> benefit too." [12]
>
> Those are significant improvements.   Three years :(

Part of the reason was that the barrier interface was part of a series
focused on virtualization via cpuidle-haltpoll. As that was reviewed,
the tale changed in the telling, and it made more sense to separate
the two.

Will send out the cpuidle changes for review which Haris is running
with.

--
ankur


^ permalink raw reply

* Re: [REGRESSION] rseq: refactoring in v6.19 broke everyone on arm64 and tcmalloc everywhere
From: Thomas Gleixner @ 2026-04-23 19:31 UTC (permalink / raw)
  To: Mathias Stearn
  Cc: Dmitry Vyukov, Jinjie Ruan, linux-man, Mark Rutland,
	Mathieu Desnoyers, Catalin Marinas, Will Deacon, Boqun Feng,
	Paul E. McKenney, Chris Kennelly, regressions, linux-kernel,
	linux-arm-kernel, Peter Zijlstra, Ingo Molnar, Blake Oler
In-Reply-To: <CAHnCjA0UBNXfjHw=Y34OrAyGRNUtVF+zWd3ugyX6pd_mCk8K9w@mail.gmail.com>

On Thu, Apr 23 2026 at 12:51, Mathias Stearn wrote:
> On Thu, Apr 23, 2026 at 12:39 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> The kernel clears rseq_cs reliably when user space was interrupted and:
>>
>>     the task was preempted
>> or
>>     the return from interrupt delivers a signal
>>
>> If the task invoked a syscall then there is absolutely no reason to do
>> either of this because syscalls from within a critical section are a
>> bug and catched when enabling rseq debugging.
>>
>> The original code did this along with unconditionally updating CPU/MMCID
>> which resulted in ~15% performance regression on a syscall heavy
>> database benchmark once glibc started to register rseq.
>
> Just to be clear TCMalloc does not need either rseq_cs to be cleared
> or cpu_id_start to be written to on syscalls because it doesn't do
> syscalls from critical sections. It will actually benefit (slightly)
> from not updating cpu_id_start on syscalls.

I know that it does not do syscalls from within critical sections, but
it relies on cpu_id_start being unconditionally updated in one way or
the other.

> It is specifically in the cases where an rseq would need to be aborted
> (preemption, signals, migration, and membarrier IPI with the rseq
> flag) that TCMalloc relies on cpu_id_start being written. It does rely
> on that write even when not inside the critical section, because it
> effectively uses that to detect if there were any would-cause-abort
> events in between two critical sections. But since it leaves the
> rseq_cs pointer non-null between critical sections, so you dont need
> to add _any_ overhead for programs that never make use of rseq after
> registration, or add any overhead to syscalls even for those who do.

Well. According to the comment in the tcmalloc code:

// Calculation of the address of the current CPU slabs region is needed for
// allocation/deallocation fast paths, but is quite expensive. Due to variable
// shift and experimental support for "virtual CPUs", the calculation involves
// several additional loads and dependent calculations. Pseudo-code for the
// address calculation is as follows:
//
//   cpu_offset = TcmallocSlab.virtual_cpu_id_offset_;
//   cpu = *(&__rseq_abi + virtual_cpu_id_offset_);
//   slabs_and_shift = TcmallocSlab.slabs_and_shift_;
//   shift = slabs_and_shift & kShiftMask;
//   shifted_cpu = cpu << shift;
//   slabs = slabs_and_shift & kSlabsMask;
//   slabs += shifted_cpu;
//
// To remove this calculation from fast paths, we cache the slabs address
// for the current CPU in thread local storage. However, when a thread is
// rescheduled to another CPU, we somehow need to understand that the cached

                  ^^^^^^^^^^^

// address is not valid anymore. To achieve this, we overlap the top 4 bytes
// of the cached address with __rseq_abi.cpu_id_start. When a thread is
// rescheduled the kernel overwrites cpu_id_start with the current CPU number,
// which gives us the signal that the cached address is not valid anymore.

The kernel still as of today (the arm64 bug aside) updates the
cpu_id_start and cpu_id fields in rseq when a task is rescheduled to
another CPU.

So if the code only requires to know when it got rescheduled to another
CPU then it still should work, no?

But it does not, which makes it clear that it relies on this
undocumented behaviour of the kernel to rewrite rseq::cpu_id_start
unconditionally. I'm not yet convinced that it relies on it only when
interrupted between two subsequent critical sections. We'll see.

....

Now we come to the best part of this comment:

// Note: this makes __rseq_abi.cpu_id_start unusable for its original purpose.

So any code sequence which ends up in:

   x = tcmalloc();
   dostuff(x)
     evaluate(rseq::cpu_id_start, rseq::cpu_id)

is doomed. This might be acceptable for Google internal usage where they
control the full stack and can prevent anyone else to utilize rseq, but
in an open ecosystem that's obviously a non-starter.

And they definitely forgot to add this to the comment:

// Never enable CONFIG_RSEQ_DEBUG in the kernel when you use tcmalloc as
// it will expose the blatant ABI abuse and therefore will kill your
// application.

If your assumption that the rewrite is only required when rseq::rseq_cs
is non NULL and user space was interrupted is correct, then the obvious
no-brainer would have been to add:

        __u64	rseq_usr_data;

to struct rseq and clear that unconditionally when rseq::rseq_cs is
cleared.

But that would have been too simple, would work independent of endianess
and not in the way of anybody else.

But I know that's incompatible with the features first, correctness
later and we own the world anyway mindset.

Just for giggles I asked Google Gemini about the implications of
tmalloc's rseq abuse. The answer is pretty clear:

   "In short, TCMalloc treats RSEQ as a private optimization rather than
    a shared system resource, which compromises the stability and
    extensibility of any application that needs RSEQ for anything other
    than memory allocation."

It's also very clear about the wilful ignorance of the tcmalloc people:

   "In summary, the developers have known for at least 6 years that the
    implementation was non-standard and conflicting with other rseq
    usage. The github issue which requested glibc compatibility was
    opened in 2022 and has been unresolved since then."

Thanks,

        tglx


^ permalink raw reply

* Re: [REGRESSION] rseq: refactoring in v6.19 broke everyone on arm64 and tcmalloc everywhere
From: Thomas Gleixner @ 2026-04-23 19:39 UTC (permalink / raw)
  To: Chris Kennelly
  Cc: Mathias Stearn, Peter Zijlstra, Mathieu Desnoyers,
	Catalin Marinas, Will Deacon, Boqun Feng, Paul E. McKenney,
	Dmitry Vyukov, regressions, linux-kernel, linux-arm-kernel,
	Ingo Molnar, Mark Rutland, Jinjie Ruan, Blake Oler,
	Linus Torvalds
In-Reply-To: <CAEE+yb=rZFxxOhCw+6wsUt5PE6=ebbKjEggQ_j_G3qSkMQZsfg@mail.gmail.com>

On Thu, Apr 23 2026 at 13:38, Chris Kennelly wrote:
> On Thu, Apr 23, 2026 at 1:19 PM Thomas Gleixner <tglx@kernel.org> wrote:
>>   3) The RO for userspace property has been enforced by RSEQ debugging
>>      mode since day one. If such a debug enabled kernel detects user
>>      space changing the field it kills the task/application.
>
> The optimization in TCMalloc that you're describing has been available
> since September 2023:
> https://github.com/google/tcmalloc/commit/aaa4fbf6fcdce1b7f86fcadd659874645c75ddb9

And the github issue which requested glibc compatibility was opened in
Sept. 2022:

      https://github.com/google/tcmalloc/issues/144

> I thought the RSEQ debug checks were added in December 2024:
> https://github.com/torvalds/linux/commit/7d5265ffcd8b41da5e09066360540d6e0716e9cd,
> but perhaps I misidentified the ones in question.

I might have misread the git log. But that still does not justify the
violation of a documented ABI for the price that nobody else can use it
once tcmalloc is in play:

   x = tcmalloc();
   dostuff(x)
     evaluate(rseq::cpu_id_start, rseq::cpu_id) <- FAIL

>>   7) tcmalloc violates the ABI from day one and has since refused to
>>      address the problem despite being offered a kernel side rseq
>>      extension to solve it many years ago.
>
> I know there was some discussion around a preemption notification
> scheme, rseq_sched_state; but I thought the discussion moved in favor
> of the timeslice extension interface that recently landed. Timeslice
> extension solves some use cases, but I'm not sure it addresses this
> one.

No it does not. That's an orthogonal optimization.

Thanks,

        tglx


^ permalink raw reply

* [PATCH v4 28/58] perf syscall-counts-by-pid: Port syscall-counts-by-pid to use python module
From: Ian Rogers @ 2026-04-23 19:43 UTC (permalink / raw)
  To: acme, adrian.hunter, james.clark, leo.yan, namhyung, tmricht
  Cc: alice.mei.rogers, dapeng1.mi, linux-arm-kernel, linux-kernel,
	linux-perf-users, mingo, peterz, Ian Rogers
In-Reply-To: <20260423161006.1762700-1-irogers@google.com>

Rewrite tools/perf/scripts/python/syscall-counts-by-pid.py to use the
python module and various style changes. By avoiding the overheads in
the `perf script` execution the performance improves by more than 3.8x
as shown in the following (with PYTHON_PATH and PERF_EXEC_PATH set as
necessary):

```
$ perf record -e raw_syscalls:sys_enter -a sleep 1
...
$ time perf script tools/perf/scripts/python/syscall-counts-by-pid.py perf
Install the python-audit package to get syscall names.
For example:
  # apt-get install python3-audit (Ubuntu)
  # yum install python3-audit (Fedora)
  etc.

Press control+C to stop and show the summary
Warning:
1 out of order events recorded.

syscall events for perf:

comm [pid]/syscalls                            count
 ---------------------------------------  ----------

perf [3886080]
  1                                           538989
  16                                              32
  203                                             17
  3                                                2
  257                                              1
  204                                              1
  15                                               1
  0                                                1

perf [3886082]
  7                                                1

real    0m3.852s
user    0m3.512s
sys     0m0.336s
$ time python3 tools/perf/python/syscall-counts-by-pid.py perf
Warning:
1 out of order events recorded.

syscall events for perf:

comm [pid]/syscalls                           count
 --------------------------------------- -----------

perf [3886080]
  write                                      538989
  ioctl                                          32
  sched_setaffinity                              17
  close                                           2
  openat                                          1
  sched_getaffinity                               1
  rt_sigreturn                                    1
  read                                            1

perf [3886082]
  poll                                            1

real    0m1.011s
user    0m0.963s
sys     0m0.048s
```

Assisted-by: Gemini:gemini-3.1-pro-preview
Signed-off-by: Ian Rogers <irogers@google.com>
---
v2:

1. Removed Unused Variable: Removed id_keys which was assigned but
   never read.

2. Fallback for Unknown Syscalls: If perf.syscall_name() returns None
   for an unmapped ID, it now falls back to the numeric ID string to
   prevent TypeError crashes during string formatting.

3. Fallback for Syscall Number Attribute: It now checks for
   __syscall_nr first, and if missing, falls back to checking for nr .

4. Robust Process Resolution: Added a try-except block around
   session.process(sample.pid).comm() to handle untracked PIDs
   gracefully instead of crashing on a TypeError .

5. Restored PID Filtering: The script now attempts to parse the
   positional argument as an integer to filter by Process ID. If that
   fails, it treats it as a command name (COMM) string to filter by,
   restoring behavior from the original legacy script.

6. Support for Custom Input Files: Added a -i / --input command-line
   argument to support arbitrarily named trace files, removing the
   hardcoded "perf.data" restriction.
---
 tools/perf/python/syscall-counts-by-pid.py | 88 ++++++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100755 tools/perf/python/syscall-counts-by-pid.py

diff --git a/tools/perf/python/syscall-counts-by-pid.py b/tools/perf/python/syscall-counts-by-pid.py
new file mode 100755
index 000000000000..45a98e6e8e01
--- /dev/null
+++ b/tools/perf/python/syscall-counts-by-pid.py
@@ -0,0 +1,88 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+"""
+Displays system-wide system call totals, broken down by syscall.
+If a [comm] arg is specified, only syscalls called by [comm] are displayed.
+"""
+
+import argparse
+from collections import defaultdict
+import perf
+
+syscalls: dict[tuple[str, int, int], int] = defaultdict(int)
+for_comm = None
+for_pid = None
+session = None
+
+
+def print_syscall_totals():
+    """Print aggregated statistics."""
+    if for_comm is not None:
+        print(f"\nsyscall events for {for_comm}:\n")
+    elif for_pid is not None:
+        print(f"\nsyscall events for PID {for_pid}:\n")
+    else:
+        print("\nsyscall events:\n")
+
+    print(f"{'comm [pid]/syscalls':<40} {'count':>10}")
+    print("---------------------------------------- -----------")
+
+    sorted_keys = sorted(syscalls.keys(), key=lambda k: (k[0], k[1], k[2]))
+    current_comm_pid = None
+    for comm, pid, sc_id in sorted_keys:
+        if current_comm_pid != (comm, pid):
+            print(f"\n{comm} [{pid}]")
+            current_comm_pid = (comm, pid)
+        name = perf.syscall_name(sc_id) or str(sc_id)
+        print(f"  {name:<38} {syscalls[(comm, pid, sc_id)]:>10}")
+
+
+def process_event(sample):
+    """Process a single sample event."""
+    event_name = str(sample.evsel)
+    if event_name == "evsel(raw_syscalls:sys_enter)":
+        sc_id = getattr(sample, "id", -1)
+    elif event_name.startswith("evsel(syscalls:sys_enter_"):
+        sc_id = getattr(sample, "__syscall_nr", None)
+        if sc_id is None:
+            sc_id = getattr(sample, "nr", -1)
+    else:
+        return
+
+    if sc_id == -1:
+        return
+
+    pid = sample.sample_pid
+
+    if for_pid and pid != for_pid:
+        return
+
+    comm = "unknown"
+    try:
+        if session:
+            proc = session.process(pid)
+            if proc:
+                comm = proc.comm()
+    except (TypeError, AttributeError):
+        pass
+
+    if for_comm and comm != for_comm:
+        return
+    syscalls[(comm, pid, sc_id)] += 1
+
+
+if __name__ == "__main__":
+    ap = argparse.ArgumentParser()
+    ap.add_argument("filter", nargs="?", help="COMM or PID to filter by")
+    ap.add_argument("-i", "--input", default="perf.data", help="Input file name")
+    args = ap.parse_args()
+
+    if args.filter:
+        try:
+            for_pid = int(args.filter)
+        except ValueError:
+            for_comm = args.filter
+
+    session = perf.session(perf.data(args.input), sample=process_event)
+    session.process_events()
+    print_syscall_totals()
-- 
2.54.0.rc2.533.g4f5dca5207-goog



^ permalink raw reply related

* [PATCH v4 29/58] perf futex-contention: Port futex-contention to use python module
From: Ian Rogers @ 2026-04-23 19:43 UTC (permalink / raw)
  To: acme, adrian.hunter, james.clark, leo.yan, namhyung, tmricht
  Cc: alice.mei.rogers, dapeng1.mi, linux-arm-kernel, linux-kernel,
	linux-perf-users, mingo, peterz, Ian Rogers
In-Reply-To: <20260423194428.1846255-1-irogers@google.com>

Rewrite tools/perf/scripts/python/futex-contention.py to use the
python module and various style changes. By avoiding the overheads in
the `perf script` execution the performance improves by more than 3.2x
as shown in the following (with PYTHON_PATH and PERF_EXEC_PATH set as
necessary):

```
$ perf record -e syscalls:sys_*_futex -a sleep 1
...
$ time perf script tools/perf/scripts/python/futex-contention.py
Install the python-audit package to get syscall names.
For example:
  # apt-get install python3-audit (Ubuntu)
  # yum install python3-audit (Fedora)
  etc.

Press control+C to stop and show the summary
aaa/4[2435653] lock 7f76b380c878 contended 1 times, 1099 avg ns [max: 1099 ns, min 1099 ns]
...
real    0m1.007s
user    0m0.935s
sys     0m0.072s
$ time python3 tools/perf/python/futex-contention.py
...
real    0m0.314s
user    0m0.259s
sys     0m0.056s
```

Assisted-by: Gemini:gemini-3.1-pro-preview
Signed-off-by: Ian Rogers <irogers@google.com>
---
v2:

1. Fixed Module Import Failure: Corrected the type annotations from
   [int, int] to Tuple[int, int] .  The previous code would raise a
   TypeError at module import time because lists cannot be used as
   types in dictionary annotations.

2. Prevented Out-Of-Memory Crashes: Replaced the approach of storing
   every single duration in a list with a LockStats class that
   maintains running aggregates (count, total time, min, max). This
   ensures O(1) memory usage per lock/thread pair rather than
   unbounded memory growth.

3. Support for Custom Input Files: Added a -i / --input command-line
   argument to support processing arbitrarily named trace files,
   removing the hardcoded "perf.data" restriction.

4. Robust Process Lookup: Added a check to ensure session is
   initialized before calling session.  process() , preventing
   potential NoneType attribute errors if events are processed during
   initialization.
---
 tools/perf/python/futex-contention.py | 87 +++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100755 tools/perf/python/futex-contention.py

diff --git a/tools/perf/python/futex-contention.py b/tools/perf/python/futex-contention.py
new file mode 100755
index 000000000000..7c5c3d0ca60a
--- /dev/null
+++ b/tools/perf/python/futex-contention.py
@@ -0,0 +1,87 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+"""Measures futex contention."""
+
+import argparse
+from collections import defaultdict
+from typing import Dict, Tuple
+import perf
+
+class LockStats:
+    """Aggregate lock contention information."""
+    def __init__(self) -> None:
+        self.count = 0
+        self.total_time = 0
+        self.min_time = 0
+        self.max_time = 0
+
+    def add(self, duration: int) -> None:
+        """Add a new duration measurement."""
+        self.count += 1
+        self.total_time += duration
+        if self.count == 1:
+            self.min_time = duration
+            self.max_time = duration
+        else:
+            self.min_time = min(self.min_time, duration)
+            self.max_time = max(self.max_time, duration)
+
+    def avg(self) -> float:
+        """Return average duration."""
+        return self.total_time / self.count if self.count > 0 else 0.0
+
+process_names: Dict[int, str] = {}
+start_times: Dict[int, Tuple[int, int]] = {}
+session = None
+durations: Dict[Tuple[int, int], LockStats] = defaultdict(LockStats)
+
+FUTEX_WAIT = 0
+FUTEX_WAKE = 1
+FUTEX_PRIVATE_FLAG = 128
+FUTEX_CLOCK_REALTIME = 256
+FUTEX_CMD_MASK = ~(FUTEX_PRIVATE_FLAG | FUTEX_CLOCK_REALTIME)
+
+
+def process_event(sample: perf.sample_event) -> None:
+    """Process a single sample event."""
+    def handle_start(tid: int, uaddr: int, op: int, start_time: int) -> None:
+        if (op & FUTEX_CMD_MASK) != FUTEX_WAIT:
+            return
+        if tid not in process_names:
+            try:
+                if session:
+                    process = session.process(tid)
+                    if process:
+                        process_names[tid] = process.comm()
+            except (TypeError, AttributeError):
+                return
+        start_times[tid] = (uaddr, start_time)
+
+    def handle_end(tid: int, end_time: int) -> None:
+        if tid not in start_times:
+            return
+        (uaddr, start_time) = start_times[tid]
+        del start_times[tid]
+        durations[(tid, uaddr)].add(end_time - start_time)
+
+    event_name = str(sample.evsel)
+    if event_name == "evsel(syscalls:sys_enter_futex)":
+        uaddr = getattr(sample, "uaddr", 0)
+        op = getattr(sample, "op", 0)
+        handle_start(sample.sample_tid, uaddr, op, sample.sample_time)
+    elif event_name == "evsel(syscalls:sys_exit_futex)":
+        handle_end(sample.sample_tid, sample.sample_time)
+
+
+if __name__ == "__main__":
+    ap = argparse.ArgumentParser(description="Measure futex contention")
+    ap.add_argument("-i", "--input", default="perf.data", help="Input file name")
+    args = ap.parse_args()
+
+    session = perf.session(perf.data(args.input), sample=process_event)
+    session.process_events()
+
+    for ((t, u), stats) in sorted(durations.items()):
+        avg_ns = stats.avg()
+        print(f"{process_names.get(t, 'unknown')}[{t}] lock {u:x} contended {stats.count} times, "
+              f"{avg_ns:.0f} avg ns [max: {stats.max_time} ns, min {stats.min_time} ns]")
-- 
2.54.0.rc2.533.g4f5dca5207-goog



^ permalink raw reply related

* [PATCH v4 30/58] perf flamegraph: Port flamegraph to use python module
From: Ian Rogers @ 2026-04-23 19:43 UTC (permalink / raw)
  To: acme, adrian.hunter, james.clark, leo.yan, namhyung, tmricht
  Cc: alice.mei.rogers, dapeng1.mi, linux-arm-kernel, linux-kernel,
	linux-perf-users, mingo, peterz, Ian Rogers
In-Reply-To: <20260423194428.1846255-1-irogers@google.com>

Add a port of the flamegraph script that uses the perf python module
directly. This approach is significantly faster than using perf script
callbacks as it avoids creating intermediate dictionaries for all
event fields.

Assisted-by: Gemini:gemini-3.1-pro-preview
Signed-off-by: Ian Rogers <irogers@google.com>
---
v2:

1. Performance Optimization: Changed Node.children from a list to a
   dictionary, reducing the lookup time in find_or_create_node from
   O(N) to O(1) and avoiding performance bottlenecks on wide call
   graphs.

2. Callchain Fallback: Added a fallback to use the sample's top-level
   symbol or instruction pointer if no callchain is present, ensuring
   the script still generates meaningful output rather than just
   process names.

3. Template Downloading Fix: Corrected the logic handling the
   --allow-download flag and custom HTTP URLs. It no longer warns
   about missing local files when a URL is provided, and won't
   silently overwrite custom URLs with the default one.

4. Output Stream Separation: Moved informational warnings to
   sys.stderr to prevent them from corrupting the resulting HTML/JSON
   file when the user streams the output to stdout (e.g., using -o -
   ).

5. XSS Protection: Added basic HTML entity escaping for < , > , and &
   within the embedded JSON data blocks. This mitigates the risk of
   cross-site scripting if trace data contains maliciously formed
   process or symbol names.
---
 tools/perf/python/flamegraph.py | 250 ++++++++++++++++++++++++++++++++
 1 file changed, 250 insertions(+)
 create mode 100755 tools/perf/python/flamegraph.py

diff --git a/tools/perf/python/flamegraph.py b/tools/perf/python/flamegraph.py
new file mode 100755
index 000000000000..f3f69e5a88c2
--- /dev/null
+++ b/tools/perf/python/flamegraph.py
@@ -0,0 +1,250 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+"""
+flamegraph.py - create flame graphs from perf samples using perf python module
+"""
+
+import argparse
+import hashlib
+import json
+import os
+import subprocess
+import sys
+import urllib.request
+from typing import Dict, Optional, Union
+import perf
+
+MINIMAL_HTML = """<head>
+  <link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/d3-flamegraph.css">
+</head>
+<body>
+  <div id="chart"></div>
+  <script type="text/javascript" src="https://d3js.org/d3.v7.js"></script>
+  <script type="text/javascript" src="https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/d3-flamegraph.min.js"></script>
+  <script type="text/javascript">
+  const stacks = [/** @flamegraph_json **/];
+  // Note, options is unused.
+  const options = [/** @options_json **/];
+
+  var chart = flamegraph();
+  d3.select("#chart")
+        .datum(stacks[0])
+        .call(chart);
+  </script>
+</body>
+"""
+
+class Node:
+    """A node in the flame graph tree."""
+    def __init__(self, name: str, libtype: str):
+        self.name = name
+        self.libtype = libtype
+        self.value: int = 0
+        self.children: dict[str, Node] = {}
+
+    def to_json(self) -> Dict[str, Union[str, int, list[Dict]]]:
+        """Convert the node to a JSON-serializable dictionary."""
+        return {
+            "n": self.name,
+            "l": self.libtype,
+            "v": self.value,
+            "c": [x.to_json() for x in self.children.values()]
+        }
+
+
+class FlameGraphCLI:
+    """Command-line interface for generating flame graphs."""
+    def __init__(self, args):
+        self.args = args
+        self.stack = Node("all", "root")
+        self.session = None
+
+    @staticmethod
+    def get_libtype_from_dso(dso: Optional[str]) -> str:
+        """Determine the library type from the DSO name."""
+        if dso and (dso == "[kernel.kallsyms]" or dso.endswith("/vmlinux") or dso == "[kernel]"):
+            return "kernel"
+        return ""
+
+    @staticmethod
+    def find_or_create_node(node: Node, name: str, libtype: str) -> Node:
+        """Find a child node with the given name or create a new one."""
+        if name in node.children:
+            return node.children[name]
+        child = Node(name, libtype)
+        node.children[name] = child
+        return child
+
+    def process_event(self, sample) -> None:
+        """Process a single perf sample event."""
+        if self.args.event_name and str(sample.evsel) != self.args.event_name:
+            return
+
+        pid = sample.sample_pid
+        dso_type = ""
+        try:
+            thread = self.session.process(sample.sample_tid)
+            comm = thread.comm()
+        except Exception:
+            comm = "[unknown]"
+
+        if pid == 0:
+            comm = "swapper"
+            dso_type = "kernel"
+        else:
+            comm = f"{comm} ({pid})"
+
+        node = self.find_or_create_node(self.stack, comm, dso_type)
+
+        callchain = sample.callchain
+        if callchain:
+            # We want to traverse from root to leaf.
+            # perf callchain iterator gives leaf to root.
+            # We collect them and reverse.
+            frames = list(callchain)
+            for entry in reversed(frames):
+                name = entry.symbol or "[unknown]"
+                libtype = self.get_libtype_from_dso(entry.dso)
+                node = self.find_or_create_node(node, name, libtype)
+        else:
+            # Fallback if no callchain
+            name = getattr(sample, "symbol", "[unknown]")
+            libtype = self.get_libtype_from_dso(getattr(sample, "dso", "[unknown]"))
+            node = self.find_or_create_node(node, name, libtype)
+
+        node.value += 1
+
+    def get_report_header(self) -> str:
+        """Get the header from the perf report."""
+        try:
+            input_file = self.args.input or "perf.data"
+            output = subprocess.check_output(["perf", "report", "--header-only", "-i", input_file])
+            result = output.decode("utf-8")
+            if self.args.event_name:
+                result += "\nFocused event: " + self.args.event_name
+            return result
+        except Exception:
+            return ""
+
+    def run(self) -> None:
+        """Run the flame graph generation."""
+        input_file = self.args.input or "perf.data"
+        if not os.path.exists(input_file):
+            print(f"Error: {input_file} not found. (try 'perf record' first)", file=sys.stderr)
+            sys.exit(1)
+
+        try:
+            self.session = perf.session(perf.data(input_file),
+                                        sample=self.process_event)
+        except Exception as e:
+            print(f"Error opening session: {e}", file=sys.stderr)
+            sys.exit(1)
+
+        self.session.process_events()
+
+        stacks_json = json.dumps(self.stack, default=lambda x: x.to_json())
+        # Escape HTML special characters to prevent XSS
+        stacks_json = stacks_json.replace("<", "\\u003c") \
+            .replace(">", "\\u003e").replace("&", "\\u0026")
+
+        if self.args.format == "html":
+            report_header = self.get_report_header()
+            options = {
+                "colorscheme": self.args.colorscheme,
+                "context": report_header
+            }
+            options_json = json.dumps(options)
+            options_json = options_json.replace("<", "\\u003c") \
+                .replace(">", "\\u003e").replace("&", "\\u0026")
+
+            template = self.args.template
+            template_md5sum = None
+            output_str = None
+
+            if not os.path.isfile(template):
+                if template.startswith("http://") or template.startswith("https://"):
+                    if not self.args.allow_download:
+                        print("Warning: Downloading templates is disabled. "
+                              "Use --allow-download.", file=sys.stderr)
+                        template = None
+                else:
+                    print(f"Warning: Template file '{template}' not found.", file=sys.stderr)
+                    if self.args.allow_download:
+                        print("Using default CDN template.", file=sys.stderr)
+                        template = (
+                            "https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/templates/"
+                            "d3-flamegraph-base.html"
+                        )
+                        template_md5sum = "143e0d06ba69b8370b9848dcd6ae3f36"
+                    else:
+                        template = None
+
+            use_minimal = False
+            try:
+                if not template:
+                    use_minimal = True
+                elif template.startswith("http"):
+                    with urllib.request.urlopen(template) as url_template:
+                        output_str = "".join([l.decode("utf-8") for l in url_template.readlines()])
+                else:
+                    with open(template, "r", encoding="utf-8") as f:
+                        output_str = f.read()
+            except Exception as err:
+                print(f"Error reading template {template}: {err}\n", file=sys.stderr)
+                use_minimal = True
+
+            if use_minimal:
+                print("Using internal minimal HTML that refers to d3's web site. JavaScript " +
+                      "loaded this way from a local file may be blocked unless your " +
+                      "browser has relaxed permissions. Run with '--allow-download' to fetch" +
+                      "the full D3 HTML template.", file=sys.stderr)
+                output_str = MINIMAL_HTML
+
+            elif template_md5sum:
+                assert output_str is not None
+                download_md5sum = hashlib.md5(output_str.encode("utf-8")).hexdigest()
+                if download_md5sum != template_md5sum:
+                    s = None
+                    while s not in ["y", "n"]:
+                        s = input(f"""Unexpected template md5sum.
+{download_md5sum} != {template_md5sum}, for:
+{output_str}
+continue?[yn] """).lower()
+                    if s == "n":
+                        sys.exit(1)
+
+            assert output_str is not None
+            output_str = output_str.replace("/** @options_json **/", options_json)
+            output_str = output_str.replace("/** @flamegraph_json **/", stacks_json)
+            output_fn = self.args.output or "flamegraph.html"
+        else:
+            output_str = stacks_json
+            output_fn = self.args.output or "stacks.json"
+
+        if output_fn == "-":
+            sys.stdout.write(output_str)
+        else:
+            print(f"dumping data to {output_fn}")
+            with open(output_fn, "w", encoding="utf-8") as out:
+                out.write(output_str)
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(description="Create flame graphs using perf python module.")
+    parser.add_argument("-f", "--format", default="html", choices=["json", "html"],
+                        help="output file format")
+    parser.add_argument("-o", "--output", help="output file name")
+    parser.add_argument("--template",
+                        default="/usr/share/d3-flame-graph/d3-flamegraph-base.html",
+                        help="path to flame graph HTML template")
+    parser.add_argument("--colorscheme", default="blue-green",
+                        help="flame graph color scheme", choices=["blue-green", "orange"])
+    parser.add_argument("-i", "--input", help="input perf.data file")
+    parser.add_argument("--allow-download", default=False, action="store_true",
+                        help="allow unprompted downloading of HTML template")
+    parser.add_argument("-e", "--event", default="", dest="event_name", type=str,
+                        help="specify the event to generate flamegraph for")
+
+    cli_args = parser.parse_args()
+    cli = FlameGraphCLI(cli_args)
+    cli.run()
-- 
2.54.0.rc2.533.g4f5dca5207-goog



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox