[PATCH 0/2] drm/client: Clear stale cmdline modes before the 1024x768 clone pass

[PATCH 0/2] drm/client: Clear stale cmdline modes before the 1024x768 clone pass

[Hugo Sales]

drm_client_target_cloned() first tries to clone with the mode each
connector got from the kernel's video= option, then falls back to
1024x768 on every connector. The fallback keeps the modes the first
pass left behind, so a connector that has a video= mode set and no
1024x768 mode can make it report a clone with modes that differ. On a
single-CRTC device one connector then gets no CRTC

Patch 1 frees those modes before the fallback. Patch 2 adds a KUnit
test that fails without it

Sashiko flagged this while reviewing my fbdev probe series [1]. I
haven't hit it on hardware: it needs a single-CRTC device with two
connected outputs and video= set on only some of them, and it only shows
on the fbdev console. The KUnit test reproduces it, but how often it
happens on real systems is unconfirmed

[1] https://lore.kernel.org/all/20261024165355.2782145-1-hugo@hsal.es/

Hugo Sales (2):
  drm/client: Clear cmdline modes before the 1024x768 clone pass
  drm/tests: client-modeset: Test cloning with video= on one connector

 drivers/gpu/drm/drm_client_modeset.c          |   2 +
 .../gpu/drm/tests/drm_client_modeset_test.c   | 109 +++++++++++++++++-
 2 files changed, 110 insertions(+), 1 deletion(-)


base-commit: a1e63ab695ad76850ab9787b9eb6aab695dc2965
-- 
2.56.0

[PATCH 1/2] drm/client: Clear cmdline modes before the 1024x768 clone pass

[Hugo Sales]

drm_client_target_cloned() first tries to clone with each connector's
cmdline mode. That pass fills modes[] as it goes and stops at the first
enabled connector without a cmdline mode, or ends with modes that don't
match. Either way the earlier slots keep their cmdline modes

The 1024x768 pass that follows only writes a slot when the connector has
a matching mode, and relies on an empty slot to spot a connector it
can't clone. A connector with a mode from video= and no 1024x768 mode
keeps its cmdline mode instead, so the function returns true with modes
that differ. drm_client_target_preferred() then never runs, and on a
single-CRTC device drm_client_pick_crtcs() leaves one of the connectors
without a CRTC

Free the modes from the cmdline pass before looking for 1024x768

Fixes: 1d42bbc8f7f9 ("drm/fbdev: fix cloning on fbcon")
Signed-off-by: Hugo Sales <hugo@hsal.es>
---
Sashiko flagged this while reviewing my fbdev probe series [1]. The bug
predates that series, so I'm sending the fix on its own. I haven't hit
it on hardware. The KUnit test in the next patch fails without this fix
and passes with it

DRM KUnit on UML passes 672/672, also with KASAN and with kmemleak.
kmemleak reports six objects leaked by drm_bridge tests, which it also
reports on the base without this series

[1] https://lore.kernel.org/all/20261024165355.2782145-1-hugo@hsal.es/
---
 drivers/gpu/drm/drm_client_modeset.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 0080a8e9556a5..5e45beea66438 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -343,6 +343,8 @@ static bool drm_client_target_cloned(struct drm_device *dev,
 		return true;
 	}
 
+	modes_destroy(dev, modes, connector_count);
+
 	/* try and find a 1024x768 mode on each connector */
 	can_clone = true;
 	dmt_mode = drm_mode_find_dmt(dev, 1024, 768, 60, false);
-- 
2.56.0

[PATCH 2/2] drm/tests: client-modeset: Test cloning with video= on one connector

[Hugo Sales]

Add a drm_client_target_cloned() test with two connectors. The first
one has a video= mode and only 1920x1080, the second one has no video=
mode and only 1024x768. The connectors share no mode, so the function
must not report them as clonable

Without the previous commit the first connector keeps its cmdline mode
through the 1024x768 pass and the test fails

Signed-off-by: Hugo Sales <hugo@hsal.es>
---
 .../gpu/drm/tests/drm_client_modeset_test.c   | 109 +++++++++++++++++-
 1 file changed, 108 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c
index ec58fe064d862..a8b316fbe8433 100644
--- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -10,6 +10,7 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_kunit_helpers.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_modes.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_probe_helper.h>
@@ -195,7 +196,113 @@ static struct kunit_suite drm_test_pick_cmdline_test_suite = {
 	.test_cases = drm_test_pick_cmdline_tests
 };
 
-kunit_test_suite(drm_test_pick_cmdline_test_suite);
+static int drm_client_target_cloned_add_dmt_mode(struct drm_connector *connector,
+						 int hsize, int vsize)
+{
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_find_dmt(connector->dev, hsize, vsize, 60, false);
+	if (!mode)
+		return 0;
+
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static int drm_client_target_cloned_get_modes_1080p(struct drm_connector *connector)
+{
+	return drm_client_target_cloned_add_dmt_mode(connector, 1920, 1080);
+}
+
+static int drm_client_target_cloned_get_modes_xga(struct drm_connector *connector)
+{
+	return drm_client_target_cloned_add_dmt_mode(connector, 1024, 768);
+}
+
+static const struct drm_connector_helper_funcs drm_client_target_cloned_1080p_helper_funcs = {
+	.get_modes = drm_client_target_cloned_get_modes_1080p,
+};
+
+static const struct drm_connector_helper_funcs drm_client_target_cloned_xga_helper_funcs = {
+	.get_modes = drm_client_target_cloned_get_modes_xga,
+};
+
+/*
+ * Only the first connector has a video= mode, so the cmdline pass stops
+ * at the second one. The first connector has no 1024x768 mode, so the
+ * 1024x768 pass must not report the two connectors as clonable
+ */
+static void drm_test_target_cloned_cmdline_on_one_connector(struct kunit *test)
+{
+	struct drm_connector *connectors[2];
+	const struct drm_display_mode *modes[ARRAY_SIZE(connectors)] = {};
+	struct drm_client_offset offsets[ARRAY_SIZE(connectors)] = {};
+	bool enabled[ARRAY_SIZE(connectors)] = { true, true };
+	struct drm_device *drm;
+	struct device *dev;
+	bool cloned;
+	int i, ret;
+
+	dev = drm_kunit_helper_alloc_device(test);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+
+	drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0,
+						  DRIVER_MODESET);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
+
+	for (i = 0; i < ARRAY_SIZE(connectors); i++) {
+		connectors[i] = drmm_kzalloc(drm, sizeof(*connectors[i]), GFP_KERNEL);
+		KUNIT_ASSERT_NOT_NULL(test, connectors[i]);
+
+		ret = drmm_connector_init(drm, connectors[i],
+					  &drm_client_modeset_connector_funcs,
+					  DRM_MODE_CONNECTOR_Unknown, NULL);
+		KUNIT_ASSERT_EQ(test, ret, 0);
+
+		/* Drop any video= option the kernel was booted with */
+		memset(&connectors[i]->cmdline_mode, 0,
+		       sizeof(connectors[i]->cmdline_mode));
+		connectors[i]->force = DRM_FORCE_UNSPECIFIED;
+	}
+
+	drm_connector_helper_add(connectors[0],
+				 &drm_client_target_cloned_1080p_helper_funcs);
+	drm_connector_helper_add(connectors[1],
+				 &drm_client_target_cloned_xga_helper_funcs);
+
+	KUNIT_ASSERT_TRUE(test,
+			  drm_mode_parse_command_line_for_connector("1920x1080@60",
+								    connectors[0],
+								    &connectors[0]->cmdline_mode));
+
+	mutex_lock(&drm->mode_config.mutex);
+	for (i = 0; i < ARRAY_SIZE(connectors); i++) {
+		ret = drm_helper_probe_single_connector_modes(connectors[i],
+							      1920, 1080);
+		KUNIT_EXPECT_EQ(test, ret, 1);
+	}
+	cloned = drm_client_target_cloned(drm, connectors, ARRAY_SIZE(connectors),
+					  modes, offsets, enabled, 1920, 1080);
+	mutex_unlock(&drm->mode_config.mutex);
+
+	KUNIT_EXPECT_FALSE(test, cloned);
+
+	modes_destroy(drm, modes, ARRAY_SIZE(connectors));
+}
+
+static struct kunit_case drm_test_target_cloned_tests[] = {
+	KUNIT_CASE(drm_test_target_cloned_cmdline_on_one_connector),
+	{}
+};
+
+static struct kunit_suite drm_test_target_cloned_test_suite = {
+	.name = "drm_test_target_cloned",
+	.test_cases = drm_test_target_cloned_tests
+};
+
+kunit_test_suites(&drm_test_pick_cmdline_test_suite,
+		  &drm_test_target_cloned_test_suite);
 
 /*
  * This file is included directly by drm_client_modeset.c so we can't
-- 
2.56.0