linux-kernel - Re: [PATCH v2] drm/amd/display: Only restore backlight after amdgpu_dm_init or dm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ccdc6dda-8341-4b77-a571-e0642499e3f7@amd.com>
Date: Thu, 11 Sep 2025 12:55:49 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Matthew Schwartz <matthew.schwartz@...ux.dev>, chiahsuan.chung@....com,
 alexander.deucher@....com, harry.wentland@....com, simona@...ll.ch,
 airlied@...il.com, sunpeng.li@....com
Cc: amd-gfx@...ts.freedesktop.org, zaeem.mohamed@....com, misyl@...ggi.es,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/amd/display: Only restore backlight after
 amdgpu_dm_init or dm_resume

On 9/11/25 12:48 PM, Matthew Schwartz wrote:
> On clients that utilize AMD_PRIVATE_COLOR properties for HDR support,
> brightness sliders can include a hardware controlled portion and a
> gamma-based portion. This is the case on the Steam Deck OLED when using
> gamescope with Steam as a client.
> 
> When a user sets a brightness level while HDR is active, the gamma-based
> portion and/or hardware portion are adjusted to achieve the desired
> brightness. However, when a modeset takes place while the gamma-based
> portion is in-use, restoring the hardware brightness level overrides the
> user's overall brightness level and results in a mismatch between what
> the slider reports and the display's current brightness.
> 
> To avoid overriding gamma-based brightness, only restore HW backlight
> level after boot or resume. This ensures that the backlight level is
> set correctly after the DC layer resets it while avoiding interference
> with subsequent modesets.
> 
> Fixes: 7875afafba84 ("drm/amd/display: Fix brightness level not retained over reboot")
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4551
> Signed-off-by: Matthew Schwartz <matthew.schwartz@...ux.dev>

Reviewed-by: Mario Limonciello <mario.limonciello@....com>

Haven't picked it up, will wait a few days for more comments.

> ---
> v2: Drop set_backlight_level and use dm->restore_backlight in
> amdgpu_dm_commit_streams
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 ++++++++----
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  7 +++++++
>   2 files changed, 15 insertions(+), 4 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 7808a647a306c..2a5fa85505e84 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2037,6 +2037,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>   
>   	dc_hardware_init(adev->dm.dc);
>   
> +	adev->dm.restore_backlight = true;
> +
>   	adev->dm.hpd_rx_offload_wq = hpd_rx_irq_create_workqueue(adev);
>   	if (!adev->dm.hpd_rx_offload_wq) {
>   		drm_err(adev_to_drm(adev), "failed to create hpd rx offload workqueue.\n");
> @@ -3407,6 +3409,7 @@ static int dm_resume(struct amdgpu_ip_block *ip_block)
>   		dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
>   
>   		dc_resume(dm->dc);
> +		adev->dm.restore_backlight = true;
>   
>   		amdgpu_dm_irq_resume_early(adev);
>   
> @@ -9802,7 +9805,6 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
>   	bool mode_set_reset_required = false;
>   	u32 i;
>   	struct dc_commit_streams_params params = {dc_state->streams, dc_state->stream_count};
> -	bool set_backlight_level = false;
>   
>   	/* Disable writeback */
>   	for_each_old_connector_in_state(state, connector, old_con_state, i) {
> @@ -9922,7 +9924,6 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
>   			acrtc->hw_mode = new_crtc_state->mode;
>   			crtc->hwmode = new_crtc_state->mode;
>   			mode_set_reset_required = true;
> -			set_backlight_level = true;
>   		} else if (modereset_required(new_crtc_state)) {
>   			drm_dbg_atomic(dev,
>   				       "Atomic commit: RESET. crtc id %d:[%p]\n",
> @@ -9979,13 +9980,16 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
>   	 * to fix a flicker issue.
>   	 * It will cause the dm->actual_brightness is not the current panel brightness
>   	 * level. (the dm->brightness is the correct panel level)
> -	 * So we set the backlight level with dm->brightness value after set mode
> +	 * So we set the backlight level with dm->brightness value after initial
> +	 * set mode. Use restore_backlight flag to avoid setting backlight level
> +	 * for every subsequent mode set.
>   	 */
> -	if (set_backlight_level) {
> +	if (dm->restore_backlight) {
>   		for (i = 0; i < dm->num_of_edps; i++) {
>   			if (dm->backlight_dev[i])
>   				amdgpu_dm_backlight_set_level(dm, i, dm->brightness[i]);
>   		}
> +		dm->restore_backlight = false;
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index b937da0a4e4a0..6aae51c1beb36 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -610,6 +610,13 @@ struct amdgpu_display_manager {
>   	 */
>   	u32 actual_brightness[AMDGPU_DM_MAX_NUM_EDP];
>   
> +	/**
> +	 * @restore_backlight:
> +	 *
> +	 * Flag to indicate whether to restore backlight after modeset.
> +	 */
> +	bool restore_backlight;
> +
>   	/**
>   	 * @aux_hpd_discon_quirk:
>   	 *