linux-kernel - Re: [PATCH] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <53e5cc5d-f220-4e4c-8ee4-159a7d8f329a@amd.com>
Date: Wed, 10 Sep 2025 16:32:10 -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] drm/amd/display: Only restore backlight after
 amdgpu_dm_init or dm_resume

On 9/10/25 4:27 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>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 +++++++++--
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  7 +++++++
>   2 files changed, 16 insertions(+), 2 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..3651315760759 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);
>   
> @@ -9922,7 +9925,8 @@ 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;
> +			if (dm->restore_backlight)
> +				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 +9983,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) {

Is there really still a need for the local variable in this function 
'set_backlight_level'?

It seems to me with this change you can just use dm->restore_backlight 
for state tracking at all times.
>   		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:
>   	 *