[PATCH 0/2] drm/amd/display: Fix streamless connector update and ABM modesets

[PATCH 0/2] drm/amd/display: Fix streamless connector update and ABM modesets

[David Weber]

Hi,

This series contains two independent fixes for connector property updates
in amdgpu_dm.

The first patch fixes a reproduced NULL-stream dereference and is marked
for stable.

The second avoids the visible panel blank caused by modesetting for every
ABM level change. This is useful for desktop environments that want to
adjust ABM dynamically for AC, battery and low-battery profiles without
making the screen go black briefly on every profile transition.

It would be nice to have the ABM fix in stable kernels too, but I am not
sure whether this behavior issue justifies a stable backport. I therefore
kept its Fixes tag but did not explicitly Cc stable.

David

David Weber (2):
  drm/amd/display: Skip connector updates without a stream
  drm/amd/display: Avoid modeset for ABM changes

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
2.54.0

[PATCH 1/2] drm/amd/display: Skip connector updates without a stream

[David Weber]

Atomic DPMS can leave a connector assigned to an enabled but inactive
CRTC after the driver has removed its DC stream. A later atomic commit
can then change HDR_OUTPUT_METADATA or Broadcast RGB while leaving the
CRTC mode, routing and active state unchanged.

That property-only commit does not require a modeset, so it reaches the
connector stream-update path with a NULL stream. An HDR metadata change
passes the update bailout and reaches dc_stream_get_status(NULL), while
a Broadcast RGB change dereferences the stream earlier when comparing
its output color space.

Skip stream updates until a stream exists. The connector state remains
stored, and an enabling modeset applies it to the newly created stream.

The crash was reproduced on a Phoenix1 system with DCN 3.1.4 and Linux
7.2-rc5 using a one-shot AI-generated DRM atomic reproducer:

https://pastebin.com/KxT1BcSs

The reproducer first sets HDR_OUTPUT_METADATA on an active CRTC, then
sets CRTC ACTIVE=0 to remove the DC stream while keeping the connector
routed to the CRTC, and finally changes HDR_OUTPUT_METADATA from one
non-NULL blob to another. The last commit does not require a modeset and
reached dc_stream_get_status(NULL) from amdgpu_dm_atomic_commit_tail().
The same sequence completed without crashing with this fix applied.

Fixes: b232d4ed92ea ("drm/amd/display: Only force modesets when toggling HDR")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: David Weber <weber.aulendorf@gmail.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1820547b1dde..13101a6be437 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -11519,6 +11519,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_commit *state)
 
 		dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
 		dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+		/* DPMS-off leaves the connector routed to a streamless CRTC. */
+		if (!dm_new_crtc_state->stream)
+			continue;
 
 		scaling_changed = is_scaling_state_different(dm_new_con_state,
 							     dm_old_con_state);
-- 
2.54.0

[PATCH 2/2] drm/amd/display: Avoid modeset for ABM changes

[David Weber]

ABM connector property changes currently set connectors_changed on the
associated CRTC. DRM treats that flag as requiring a modeset, so each
ABM change tears down and recreates the stream and visibly blanks the
panel.

drm_atomic_get_crtc_state() already adds the affected CRTC to the
atomic state. dm_update_crtc_state() then copies the connector ABM level
to the CRTC state, allowing the commit tail to program it with the
existing stream update path.

Stop marking the CRTC as connectors_changed for ABM changes. Continue
doing so for scaling changes.

This makes runtime ABM changes practical for desktop power management,
for example when switching between AC, battery and low-battery
profiles.

Tested on a Phoenix1 system with DCN 3.1.4 and an internal eDP panel.
Changing ABM through the panel_power_savings sysfs property was
confirmed visually and by measuring the resulting change in system
power consumption. It no longer caused the black-screen flicker from
the forced modeset.

Fixes: c5892a102182 ("drm/amd/display: Fix dmesg warning from setting abm level")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: David Weber <weber.aulendorf@gmail.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 13101a6be437..40099bff153a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -13168,7 +13168,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 		struct dm_connector_state *dm_old_con_state = to_dm_connector_state(old_con_state);
 		struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state);
 
-		/* Skip connectors that are disabled or part of modeset already. */
+		/* Skip connectors without a CRTC. */
 		if (!new_con_state->crtc)
 			continue;
 
@@ -13179,8 +13179,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 			goto fail;
 		}
 
-		if (dm_old_con_state->abm_level != dm_new_con_state->abm_level ||
-		    dm_old_con_state->scaling != dm_new_con_state->scaling)
+		if (dm_old_con_state->scaling != dm_new_con_state->scaling)
 			new_crtc_state->connectors_changed = true;
 	}
 
-- 
2.54.0

Re: [PATCH 1/2] drm/amd/display: Skip connector updates without a stream

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The function `amdgpu_dm_atomic_commit_tail` uses `kzalloc` with `GFP_KERNEL` and acquires a `mutex`, both of which can sleep, despite being an atomic commit path.
- [High] AMDGPU never updates the privacy screen hardware state because privacy screen updates force a modeset, but the commit tail skips privacy screen updates during modesets.
--

--- Patch [1]: [PATCH 1/2] drm/amd/display: Skip connector updates without a stream ---
commit 3a3a91e5bab6bdb82ebcebf89c73cd8b740ca904
Author: David Weber <weber.aulendorf@gmail.com>

drm/amd/display: Skip connector updates without a stream
    
Atomic DPMS can leave a connector assigned to an enabled but inactive
CRTC after the driver has removed its DC stream. A later atomic commit
can then change HDR_OUTPUT_METADATA or Broadcast RGB while leaving the
CRTC mode, routing and active state unchanged.

This patch skips stream updates until a stream exists, preventing null
pointer dereferences when the connector stream-update path is reached
with a NULL stream.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 567bf04979ee5..712e8d3e0a01d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5050,6 +5050,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_commit *state)
>  
>  		dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
>  		dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);

[Severity: High]
This isn't a bug introduced by this patch, but does this skip logic prevent
privacy screen updates?

In amdgpu_dm_connector_atomic_check(), any change to the privacy screen
software state forces a modeset:

    new_crtc_state->mode_changed = true;

However, earlier in this loop in amdgpu_dm_atomic_commit_tail(), connectors
undergoing a modeset are skipped:

    /* Skip any modesets/resets */
    if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state))
        continue;

Since the only call to update the privacy screen hardware is located at the
very end of this loop:

    drm_connector_update_privacy_screen(new_con_state);

Will this cause privacy screen updates to be permanently skipped because they
force a modeset that bypasses the update code?

> +		/* DPMS-off leaves the connector routed to a streamless CRTC. */
> +		if (!dm_new_crtc_state->stream)
> +			continue;
>  
>  		scaling_changed = is_scaling_state_different(dm_new_con_state,
>  							     dm_old_con_state);

[Severity: High]
This is a pre-existing issue, but are there sleeping functions called from
this atomic context?

Later in amdgpu_dm_atomic_commit_tail(), kzalloc is called with GFP_KERNEL:

    dummy_updates = kzalloc(sizeof(struct dc_surface_update) * MAX_SURFACES, GFP_KERNEL);

And then a mutex is acquired:

    mutex_lock(&dm->dc_lock);

Since drm_atomic_helper_commit_tail() and its variants execute in an atomic
context where sleeping is forbidden, could this kzalloc(..., GFP_KERNEL) and
mutex_lock() cause kernel warnings, system instability, or potential
deadlocks?

Also, if the memory allocation fails, the subsequent hardware programming in
dc_update_planes_and_stream() is silently skipped. Can this lead to an
unrecoverable state mismatch since commit_tail cannot return error codes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813112634.11455-1-weber.aulendorf@gmail.com?part=1