| 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
| ||
|
Message-ID: <8f1143f0-e70f-4187-9a2c-4e825ed9c63f@linux.dev>
Date: Thu, 18 Sep 2025 09:37:07 -0700
From: Matthew Schwartz <matthew.schwartz@...ux.dev>
To: Mario Limonciello <mario.limonciello@....com>, 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 10:55 AM, Mario Limonciello wrote:
> 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.
Was this applied somewhere or still waiting on more comments?
Thanks
>
>> ---
>> 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:
>> *
>
Powered by blists - more mailing lists