All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/amd/display: Keep eDP native timing after resume with video=
@ 2026-10-14 22:30 Hugo Sales
  2026-10-14 22:30 ` [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS Hugo Sales
  2026-10-14 22:30 ` [PATCH 2/2] drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback Hugo Sales
  0 siblings, 2 replies; 6+ messages in thread
From: Hugo Sales @ 2026-10-14 22:30 UTC (permalink / raw)
  To: amd-gfx
  Cc: dri-devel, linux-kernel, harry.wentland, sunpeng.li, siqueira,
	alexander.deucher, Hugo Sales

On a 3840x2400 eDP panel booted with video=eDP-1:1920x1200@60, an HDMI
hotplug followed by suspend/resume brings the panel back with the
1920x1200 mode as its raw timing. The image ends up squeezed into the
top-left corner. Photos, hardware details and the test loop are in
https://gitlab.freedesktop.org/drm/amd/-/issues/5983

The hotplug makes fbdev emulation reprobe every connector with its
framebuffer size as the limit, so connector->modes loses the 3840x2400
mode. On resume, create_stream_for_sink() finds no preferred mode in the
list and scales onto the first mode instead. Patch 1 falls back to the
encoder's native_mode, which amdgpu_dm_get_native_mode() copies from the
preferred mode before the size limits apply. Patch 2 adds KUnit tests
for it

The fallback only applies to eDP and LVDS, where amdgpu turns scaling on
by itself. On other connectors the preferred mode can also be missing
because mode_valid rejected it, and then scaling onto the first valid
mode is right. Those connectors still hit this bug when userspace sets
the scaling mode property, which this series doesn't fix

A fuller amdgpu fix would always take the eDP and LVDS native timing
from the encoder, and search connector->modes only when the encoder has
none. I kept this to a fallback so it changes less, but I can respin it
that way if you'd prefer

On amd-staging-drm-next, with an HDMI hotplug and suspend/resume
(pm_test=platform) in a loop, 28 of 29 resumes came back with the wrong
timing without the patch, and the other showed it briefly. With it, 169
of 169 were fine. A backport to 7.2.8 went from 103 of 103 bad to 1363
of 1363 good. The new KUnit tests fail without the fix, and the
connector suite passes, apart from dm_test_fs_caps_hdmi_vsdb and
dm_test_fs_caps_pcon_whitelist, which already fail on this tree

The cause could also be fixed in DRM core, for every connector, by
making drm_client_modeset_probe() probe with the device's limits and
apply the framebuffer size only when fbdev picks its own mode. That
touches more shared code, since fbdev's other mode pickers would then
need size checks too

Hugo Sales (2):
  drm/amd/display: Fall back to the native mode, not the first, on
    eDP/LVDS
  drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback

 .../display/amdgpu_dm/amdgpu_dm_connector.c   |  28 +++++
 .../tests/amdgpu_dm_connector_test.c          | 112 ++++++++++++++++++
 2 files changed, 140 insertions(+)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS
  2026-10-14 22:30 [PATCH 0/2] drm/amd/display: Keep eDP native timing after resume with video= Hugo Sales
@ 2026-10-14 22:30 ` Hugo Sales
  2026-10-14 22:39   ` sashiko-bot
  2026-10-14 22:30 ` [PATCH 2/2] drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback Hugo Sales
  1 sibling, 1 reply; 6+ messages in thread
From: Hugo Sales @ 2026-10-14 22:30 UTC (permalink / raw)
  To: amd-gfx
  Cc: dri-devel, linux-kernel, harry.wentland, sunpeng.li, siqueira,
	alexander.deucher, Hugo Sales

Currently, create_stream_for_sink() takes the native timing for a scaled
mode from the preferred mode in connector->modes. After fbdev emulation
probes with size limits below the native resolution, as it does when
video= sets a smaller mode, that mode is no longer in the list

If that probe is the last one before a suspend, for example after an
HDMI hotplug, the resume finds no preferred mode, and the panel is
driven with the scaled mode's timing instead of its own

amdgpu_dm_get_native_mode() copies the preferred mode to the encoder on
every probe, before the size limits apply. Fall back to that copy when
connector->modes has no preferred mode. Limit this to eDP and LVDS,
where amdgpu turns scaling on by itself. On other connectors mode_valid
can also leave the preferred mode out, and then the first valid mode is
the right fallback

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5983
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Hugo Sales <hugo@hsal.es>
---
 .../display/amdgpu_dm/amdgpu_dm_connector.c   | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
index 428a5f937..a1dc3e287 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
@@ -257,6 +257,8 @@ STATIC_IFN_KUNIT void update_subconnector_property(struct amdgpu_dm_connector *a
 EXPORT_IF_KUNIT(update_subconnector_property);
 
 STATIC_IFN_KUNIT int amdgpu_dm_connector_get_modes(struct drm_connector *connector);
+STATIC_IFN_KUNIT struct drm_encoder *amdgpu_dm_connector_to_encoder(
+	struct drm_connector *connector);
 
 STATIC_IFN_KUNIT void amdgpu_dm_fbc_init(struct drm_connector *connector)
 {
@@ -1470,6 +1472,7 @@ create_stream_for_sink(struct drm_connector *connector,
 	struct dc_stream_state *stream = NULL;
 	struct drm_display_mode mode;
 	struct drm_display_mode saved_mode;
+	struct drm_display_mode native_mode;
 	struct drm_display_mode *freesync_mode = NULL;
 	bool native_mode_found = false;
 	bool recalculate_timing = false;
@@ -1529,6 +1532,31 @@ create_stream_for_sink(struct drm_connector *connector,
 			break;
 		}
 	}
+
+	/*
+	 * After a probe with size limits smaller than the preferred mode,
+	 * connector->modes no longer has that mode, and the first mode left
+	 * would become the stream timing. fbdev emulation probes with limits
+	 * that video= can set below the native size. The encoder has a copy of
+	 * the preferred mode from amdgpu_dm_get_native_mode(), made on every
+	 * probe before the limits apply, so use that copy. Only do this for eDP
+	 * and LVDS panels, which take only their native timing. On other
+	 * connectors mode_valid can also reject the preferred mode, and then
+	 * the first valid mode is the right one
+	 */
+	if (!native_mode_found && aconnector && !aconnector->mst_root && aconnector->drm_edid &&
+	    (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
+	     connector->connector_type == DRM_MODE_CONNECTOR_LVDS)) {
+		struct drm_encoder *encoder = amdgpu_dm_connector_to_encoder(connector);
+
+		if (encoder && to_amdgpu_encoder(encoder)->native_mode.clock) {
+			drm_mode_copy(&native_mode, &to_amdgpu_encoder(encoder)->native_mode);
+			drm_mode_set_crtcinfo(&native_mode, CRTC_INTERLACE_HALVE_V);
+			preferred_mode = &native_mode;
+			native_mode_found = true;
+		}
+	}
+
 	if (!native_mode_found)
 		preferred_mode = list_first_entry_or_null(
 				&connector->modes,
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback
  2026-10-14 22:30 [PATCH 0/2] drm/amd/display: Keep eDP native timing after resume with video= Hugo Sales
  2026-10-14 22:30 ` [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS Hugo Sales
@ 2026-10-14 22:30 ` Hugo Sales
  2026-10-14 22:44   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Hugo Sales @ 2026-10-14 22:30 UTC (permalink / raw)
  To: amd-gfx
  Cc: dri-devel, linux-kernel, harry.wentland, sunpeng.li, siqueira,
	alexander.deucher, Hugo Sales

Leave only a scaled mode in connector->modes and put the larger native
mode in the encoder, as a size-limited probe does. On eDP and LVDS,
create_stream_for_sink() must build a stream with the native timing and
the scaled mode as its source size. On HDMI it must keep the first mode
in the list as the timing

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Hugo Sales <hugo@hsal.es>
---
 .../tests/amdgpu_dm_connector_test.c          | 112 ++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
index 56fd133bc..55e2c3849 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
@@ -4547,6 +4547,115 @@ static void dm_test_create_stream_hdmi_vsif(struct kunit *test)
 	dc_sink_release(sink);
 }
 
+/*
+ * Build a stream after a probe with size limits below the native mode, such as
+ * fbdev emulation's. That probe leaves only a 1920x1200 scaled mode in the
+ * connector's mode list, while the encoder keeps the 3840x2400 native mode
+ */
+static struct dc_stream_state *
+dm_test_native_mode_pruned_stream(struct kunit *test, int connector_type)
+{
+	static const u8 edid[EDID_LENGTH];
+	struct dm_test_stream_ctx *ctx = dm_test_stream_ctx_alloc(test);
+	struct amdgpu_encoder *aenc;
+	struct dc_stream_state *stream;
+	struct drm_display_mode *native;
+
+	ctx->aconnector->base.connector_type = connector_type;
+
+	aenc = drmm_kzalloc(ctx->drm, sizeof(*aenc), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, aenc);
+	KUNIT_ASSERT_EQ(test, drmm_encoder_init(ctx->drm, &aenc->base, NULL,
+						DRM_MODE_ENCODER_TMDS, NULL), 0);
+	KUNIT_ASSERT_EQ(test, drm_connector_attach_encoder(&ctx->aconnector->base, &aenc->base), 0);
+
+	native = &aenc->native_mode;
+	native->clock = 592500;
+	native->hdisplay = 3840;
+	native->hsync_start = 3888;
+	native->hsync_end = 3920;
+	native->htotal = 4000;
+	native->vdisplay = 2400;
+	native->vsync_start = 2403;
+	native->vsync_end = 2409;
+	native->vtotal = 2469;
+	/* Cached by the first, unlimited probe */
+	drm_mode_copy(&ctx->aconnector->freesync_vid_base, native);
+
+	drm_mode_copy(ctx->mode, native);
+	ctx->mode->hdisplay = 1920;
+	ctx->mode->vdisplay = 1200;
+	drm_mode_set_crtcinfo(ctx->mode, CRTC_INTERLACE_HALVE_V);
+	dm_test_stream_add_mode(test, ctx, 0);
+	ctx->dm_state->scaling = RMX_FULL;
+
+	ctx->aconnector->drm_edid = drm_edid_alloc(edid, sizeof(edid));
+	KUNIT_ASSERT_NOT_NULL(test, ctx->aconnector->drm_edid);
+
+	stream = create_stream_for_sink(&ctx->aconnector->base, ctx->mode,
+					ctx->dm_state, NULL, 8,
+					PIXEL_ENCODING_RGB, false);
+
+	drm_edid_free(ctx->aconnector->drm_edid);
+	KUNIT_ASSERT_NOT_NULL(test, stream);
+
+	return stream;
+}
+
+/**
+ * dm_test_create_stream_native_mode_pruned_edp - Test eDP gets native timing
+ * @test: The KUnit test context
+ *
+ * The scaled mode must still be driven with the panel's native timing, kept in
+ * the encoder
+ */
+static void dm_test_create_stream_native_mode_pruned_edp(struct kunit *test)
+{
+	struct dc_stream_state *stream =
+		dm_test_native_mode_pruned_stream(test, DRM_MODE_CONNECTOR_eDP);
+
+	KUNIT_EXPECT_EQ(test, (int)stream->timing.h_addressable, 3840);
+	KUNIT_EXPECT_EQ(test, (int)stream->timing.v_addressable, 2400);
+	KUNIT_EXPECT_EQ(test, (int)stream->src.width, 1920);
+	KUNIT_EXPECT_EQ(test, (int)stream->src.height, 1200);
+	dc_stream_release(stream);
+}
+
+/**
+ * dm_test_create_stream_native_mode_pruned_lvds - Test LVDS gets native timing
+ * @test: The KUnit test context
+ */
+static void dm_test_create_stream_native_mode_pruned_lvds(struct kunit *test)
+{
+	struct dc_stream_state *stream =
+		dm_test_native_mode_pruned_stream(test, DRM_MODE_CONNECTOR_LVDS);
+
+	KUNIT_EXPECT_EQ(test, (int)stream->timing.h_addressable, 3840);
+	KUNIT_EXPECT_EQ(test, (int)stream->timing.v_addressable, 2400);
+	KUNIT_EXPECT_EQ(test, (int)stream->src.width, 1920);
+	KUNIT_EXPECT_EQ(test, (int)stream->src.height, 1200);
+	dc_stream_release(stream);
+}
+
+/**
+ * dm_test_create_stream_native_mode_pruned_hdmi - Test HDMI uses the first mode
+ * @test: The KUnit test context
+ *
+ * On a connector that is not a panel, mode_valid may have dropped the native
+ * mode, so the first mode left in the list must drive the timing
+ */
+static void dm_test_create_stream_native_mode_pruned_hdmi(struct kunit *test)
+{
+	struct dc_stream_state *stream =
+		dm_test_native_mode_pruned_stream(test, DRM_MODE_CONNECTOR_HDMIA);
+
+	KUNIT_EXPECT_EQ(test, (int)stream->timing.h_addressable, 1920);
+	KUNIT_EXPECT_EQ(test, (int)stream->timing.v_addressable, 1200);
+	KUNIT_EXPECT_EQ(test, (int)stream->src.width, 1920);
+	KUNIT_EXPECT_EQ(test, (int)stream->src.height, 1200);
+	dc_stream_release(stream);
+}
+
 /* Tests for amdgpu_dm_connector_detect() */
 
 /*
@@ -10246,6 +10355,9 @@ static struct kunit_case amdgpu_dm_connector_tests[] = {
 	KUNIT_CASE(dm_test_create_stream_dp_vsc_colorimetry),
 	KUNIT_CASE(dm_test_create_stream_dp_colorimetry_quirk),
 	KUNIT_CASE(dm_test_create_stream_hdmi_vsif),
+	KUNIT_CASE(dm_test_create_stream_native_mode_pruned_edp),
+	KUNIT_CASE(dm_test_create_stream_native_mode_pruned_lvds),
+	KUNIT_CASE(dm_test_create_stream_native_mode_pruned_hdmi),
 	/* amdgpu_dm_connector_detect */
 	KUNIT_CASE(dm_test_detect_force_on),
 	KUNIT_CASE(dm_test_detect_force_on_digital),
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS
  2026-10-14 22:30 ` [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS Hugo Sales
@ 2026-10-14 22:39   ` sashiko-bot
  2026-10-14 22:52     ` Hugo Sales
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-10-14 22:39 UTC (permalink / raw)
  To: Hugo Sales; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm: uninitialized stack memory read in create_stream_for_sink()
--

commit c9acc168fa3f685b9a0bbaf87f6642a89b782aef
Author: Hugo Sales <hugo@hsal.es>
drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS

This commit updates create_stream_for_sink() to fall back to the native
mode stored in the encoder when the preferred mode is no longer present
in the connector's mode list. This is limited to eDP and LVDS panels where
amdgpu handles scaling automatically.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5983

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> index 428a5f9377c62..a1dc3e28764cf 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> @@ -1470,6 +1472,7 @@ create_stream_for_sink(struct drm_connector *connector,
>  	struct dc_stream_state *stream = NULL;
>  	struct drm_display_mode mode;
>  	struct drm_display_mode saved_mode;
> +	struct drm_display_mode native_mode;
>  	struct drm_display_mode *freesync_mode = NULL;
>  	bool native_mode_found = false;
>  	bool recalculate_timing = false;

[ ... ]

> @@ -1529,6 +1532,31 @@ create_stream_for_sink(struct drm_connector *connector,
>  			break;
>  		}
>  	}
> +
> +	/*
> +	 * After a probe with size limits smaller than the preferred mode,
> +	 * connector->modes no longer has that mode, and the first mode left
> +	 * would become the stream timing. fbdev emulation probes with limits
> +	 * that video= can set below the native size. The encoder has a copy of
> +	 * the preferred mode from amdgpu_dm_get_native_mode(), made on every
> +	 * probe before the limits apply, so use that copy. Only do this for eDP
> +	 * and LVDS panels, which take only their native timing. On other
> +	 * connectors mode_valid can also reject the preferred mode, and then
> +	 * the first valid mode is the right one
> +	 */
> +	if (!native_mode_found && aconnector && !aconnector->mst_root && aconnector->drm_edid &&
> +	    (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> +	     connector->connector_type == DRM_MODE_CONNECTOR_LVDS)) {
> +		struct drm_encoder *encoder = amdgpu_dm_connector_to_encoder(connector);
> +
> +		if (encoder && to_amdgpu_encoder(encoder)->native_mode.clock) {
> +			drm_mode_copy(&native_mode, &to_amdgpu_encoder(encoder)->native_mode);

[Severity: Medium]
Does this result in an uninitialized stack memory read?

The native_mode variable is declared on the stack without initialization.
When passed to drm_mode_copy(), that function starts by attempting to
preserve the existing list node pointers of the destination:

void drm_mode_copy(struct drm_display_mode *dst, 
                   const struct drm_display_mode *src)
{
        struct list_head head = dst->head;

        *dst = *src;
        dst->head = head;
}

Because native_mode is uninitialized, reading dst->head accesses garbage
data. While this won't corrupt an active list (since the garbage is written
back to a local variable that is never linked), it still triggers undefined
behavior and can cause Kernel Memory Sanitizer (KMSAN) warnings.

Could we initialize native_mode when it is declared to avoid this?
For example: struct drm_display_mode native_mode = { 0 };

> +			drm_mode_set_crtcinfo(&native_mode, CRTC_INTERLACE_HALVE_V);
> +			preferred_mode = &native_mode;
> +			native_mode_found = true;
> +		}
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014223041.171317-1-hugo@hsal.es?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback
  2026-10-14 22:30 ` [PATCH 2/2] drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback Hugo Sales
@ 2026-10-14 22:44   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-14 22:44 UTC (permalink / raw)
  To: Hugo Sales; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/amd/display: uninitialized stack memory read in drm_mode_copy()
--

commit 852bbf13f64b2d512665604270ff18f1f920af2e
Author: Hugo Sales <hugo@hsal.es>

drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback

Leave only a scaled mode in connector->modes and put the larger native
mode in the encoder, as a size-limited probe does. On eDP and LVDS,
create_stream_for_sink() must build a stream with the native timing and
the scaled mode as its source size. On HDMI it must keep the first mode
in the list as the timing.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> index 56fd133bc4056..55e2c38499f4a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> @@ -4547,6 +4547,115 @@ static void dm_test_create_stream_hdmi_vsif(struct kunit *test)
[ ... ]
> +static struct dc_stream_state *
> +dm_test_native_mode_pruned_stream(struct kunit *test, int connector_type)
> +{
[ ... ]
> +	stream = create_stream_for_sink(&ctx->aconnector->base, ctx->mode,
> +					ctx->dm_state, NULL, 8,
> +					PIXEL_ENCODING_RGB, false);

[Severity: Medium]
Does this call to create_stream_for_sink() trigger a read of uninitialized
stack memory?

In create_stream_for_sink(), the variable native_mode is declared on the
stack without initialization:

amdgpu_dm_connector.c:create_stream_for_sink() {
	...
	struct drm_display_mode native_mode;
	...
	if (!native_mode_found && aconnector && ...) {
		...
		drm_mode_copy(&native_mode, &to_amdgpu_encoder(encoder)->native_mode);
		...
	}
}

The drm_mode_copy() function begins by saving the destination's list head:

drm_modes.c:drm_mode_copy() {
	struct list_head head = dst->head;
	...
}

Because native_mode is uninitialized, this reads uninitialized stack memory,
which could trigger KMSAN panics on systems with memory sanitizers enabled.
Should native_mode be zero-initialized or initialized using drm_mode_init()
first?

> +
> +	drm_edid_free(ctx->aconnector->drm_edid);
> +	KUNIT_ASSERT_NOT_NULL(test, stream);
> +
> +	return stream;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014223041.171317-1-hugo@hsal.es?part=2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS
  2026-10-14 22:39   ` sashiko-bot
@ 2026-10-14 22:52     ` Hugo Sales
  0 siblings, 0 replies; 6+ messages in thread
From: Hugo Sales @ 2026-10-14 22:52 UTC (permalink / raw)
  To: sashiko-reviews, sashiko-bot; +Cc: dri-devel

> Should native_mode be zero-initialized or initialized using drm_mode_init()
> first?

Thanks for the catch. I'll switch to drm_mode_init(), like the mode copy
a few lines above, in v2

> On 10/14/2026 11:39 PM WEST sashiko-bot@kernel.org wrote:
> 
>  
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm: uninitialized stack memory read in create_stream_for_sink()
> --
> 
> commit c9acc168fa3f685b9a0bbaf87f6642a89b782aef
> Author: Hugo Sales <hugo@hsal.es>
> drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS
> 
> This commit updates create_stream_for_sink() to fall back to the native
> mode stored in the encoder when the preferred mode is no longer present
> in the connector's mode list. This is limited to eDP and LVDS panels where
> amdgpu handles scaling automatically.
> 
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5983
> 
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> > index 428a5f9377c62..a1dc3e28764cf 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> > @@ -1470,6 +1472,7 @@ create_stream_for_sink(struct drm_connector *connector,
> >  	struct dc_stream_state *stream = NULL;
> >  	struct drm_display_mode mode;
> >  	struct drm_display_mode saved_mode;
> > +	struct drm_display_mode native_mode;
> >  	struct drm_display_mode *freesync_mode = NULL;
> >  	bool native_mode_found = false;
> >  	bool recalculate_timing = false;
> 
> [ ... ]
> 
> > @@ -1529,6 +1532,31 @@ create_stream_for_sink(struct drm_connector *connector,
> >  			break;
> >  		}
> >  	}
> > +
> > +	/*
> > +	 * After a probe with size limits smaller than the preferred mode,
> > +	 * connector->modes no longer has that mode, and the first mode left
> > +	 * would become the stream timing. fbdev emulation probes with limits
> > +	 * that video= can set below the native size. The encoder has a copy of
> > +	 * the preferred mode from amdgpu_dm_get_native_mode(), made on every
> > +	 * probe before the limits apply, so use that copy. Only do this for eDP
> > +	 * and LVDS panels, which take only their native timing. On other
> > +	 * connectors mode_valid can also reject the preferred mode, and then
> > +	 * the first valid mode is the right one
> > +	 */
> > +	if (!native_mode_found && aconnector && !aconnector->mst_root && aconnector->drm_edid &&
> > +	    (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> > +	     connector->connector_type == DRM_MODE_CONNECTOR_LVDS)) {
> > +		struct drm_encoder *encoder = amdgpu_dm_connector_to_encoder(connector);
> > +
> > +		if (encoder && to_amdgpu_encoder(encoder)->native_mode.clock) {
> > +			drm_mode_copy(&native_mode, &to_amdgpu_encoder(encoder)->native_mode);
> 
> [Severity: Medium]
> Does this result in an uninitialized stack memory read?
> 
> The native_mode variable is declared on the stack without initialization.
> When passed to drm_mode_copy(), that function starts by attempting to
> preserve the existing list node pointers of the destination:
> 
> void drm_mode_copy(struct drm_display_mode *dst, 
>                    const struct drm_display_mode *src)
> {
>         struct list_head head = dst->head;
> 
>         *dst = *src;
>         dst->head = head;
> }
> 
> Because native_mode is uninitialized, reading dst->head accesses garbage
> data. While this won't corrupt an active list (since the garbage is written
> back to a local variable that is never linked), it still triggers undefined
> behavior and can cause Kernel Memory Sanitizer (KMSAN) warnings.
> 
> Could we initialize native_mode when it is declared to avoid this?
> For example: struct drm_display_mode native_mode = { 0 };
> 
> > +			drm_mode_set_crtcinfo(&native_mode, CRTC_INTERLACE_HALVE_V);
> > +			preferred_mode = &native_mode;
> > +			native_mode_found = true;
> > +		}
> > +	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261014223041.171317-1-hugo@hsal.es?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-15  7:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14 22:30 [PATCH 0/2] drm/amd/display: Keep eDP native timing after resume with video= Hugo Sales
2026-10-14 22:30 ` [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS Hugo Sales
2026-10-14 22:39   ` sashiko-bot
2026-10-14 22:52     ` Hugo Sales
2026-10-14 22:30 ` [PATCH 2/2] drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback Hugo Sales
2026-10-14 22:44   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.