Re: [PATCH] drm/amd/display: fix ABM disablement
On 2023-11-22 17:24, Hamza Mahfooz wrote: > On recent versions of DMUB firmware, if we want to completely disable > ABM we have to pass ABM_LEVEL_IMMEDIATE_DISABLE as the requested ABM > level to DMUB. Otherwise, LCD eDP displays are unable to reach their > maximum brightness levels. So, to fix this whenever the user requests an > ABM level of 0 pass ABM_LEVEL_IMMEDIATE_DISABLE to DMUB instead. Also, > to keep the user's experience consistent map ABM_LEVEL_IMMEDIATE_DISABLE > to 0 when a user tries to read the requested ABM level. > > Signed-off-by: Hamza Mahfooz Reviewed-by: Harry Wentland Harry > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +--- > 1 file changed, 5 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 5d9496db0ecb..8cb92d941cd9 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -6230,7 +6230,7 @@ int amdgpu_dm_connector_atomic_set_property(struct > drm_connector *connector, > dm_new_state->underscan_enable = val; > ret = 0; > } else if (property == adev->mode_info.abm_level_property) { > - dm_new_state->abm_level = val; > + dm_new_state->abm_level = val ?: ABM_LEVEL_IMMEDIATE_DISABLE; > ret = 0; > } > > @@ -6275,7 +6275,8 @@ int amdgpu_dm_connector_atomic_get_property(struct > drm_connector *connector, > *val = dm_state->underscan_enable; > ret = 0; > } else if (property == adev->mode_info.abm_level_property) { > - *val = dm_state->abm_level; > + *val = (dm_state->abm_level != ABM_LEVEL_IMMEDIATE_DISABLE) ? > + dm_state->abm_level : 0; > ret = 0; > } > > @@ -6348,7 +6349,8 @@ void amdgpu_dm_connector_funcs_reset(struct > drm_connector *connector) > state->pbn = 0; > > if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) > - state->abm_level = amdgpu_dm_abm_level; > + state->abm_level = amdgpu_dm_abm_level ?: > + ABM_LEVEL_IMMEDIATE_DISABLE; > > __drm_atomic_helper_connector_reset(connector, >base); > }
Re: [PATCH] drm/amd/display: fix ABM disablement
On 11/22/23 17:24, Hamza Mahfooz wrote: On recent versions of DMUB firmware, if we want to completely disable ABM we have to pass ABM_LEVEL_IMMEDIATE_DISABLE as the requested ABM level to DMUB. Otherwise, LCD eDP displays are unable to reach their maximum brightness levels. So, to fix this whenever the user requests an ABM level of 0 pass ABM_LEVEL_IMMEDIATE_DISABLE to DMUB instead. Also, to keep the user's experience consistent map ABM_LEVEL_IMMEDIATE_DISABLE to 0 when a user tries to read the requested ABM level. Signed-off-by: Hamza Mahfooz Cc: sta...@vger.kernel.org # 6.1+ --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +--- 1 file changed, 5 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 5d9496db0ecb..8cb92d941cd9 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6230,7 +6230,7 @@ int amdgpu_dm_connector_atomic_set_property(struct drm_connector *connector, dm_new_state->underscan_enable = val; ret = 0; } else if (property == adev->mode_info.abm_level_property) { - dm_new_state->abm_level = val; + dm_new_state->abm_level = val ?: ABM_LEVEL_IMMEDIATE_DISABLE; ret = 0; } @@ -6275,7 +6275,8 @@ int amdgpu_dm_connector_atomic_get_property(struct drm_connector *connector, *val = dm_state->underscan_enable; ret = 0; } else if (property == adev->mode_info.abm_level_property) { - *val = dm_state->abm_level; + *val = (dm_state->abm_level != ABM_LEVEL_IMMEDIATE_DISABLE) ? + dm_state->abm_level : 0; ret = 0; } @@ -6348,7 +6349,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) state->pbn = 0; if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) - state->abm_level = amdgpu_dm_abm_level; + state->abm_level = amdgpu_dm_abm_level ?: + ABM_LEVEL_IMMEDIATE_DISABLE; __drm_atomic_helper_connector_reset(connector, >base); } -- Hamza
Re: [PATCH] drm/amd/display: fix ABM disablement
On 11/22/23 17:45, Pillai, Aurabindo wrote: [AMD Official Use Only - General] Does amdgpu_dm_connector_funcs_reset() get called on wakeup from suspend According to my research/testing drm_connector's rest() callback only gets called when the connector gets created. ? Users would want the system to have the same brightness level before suspending. -- Regards, Jay *From:* Mahfooz, Hamza *Sent:* Wednesday, November 22, 2023 5:24 PM *To:* amd-gfx@lists.freedesktop.org *Cc:* Wentland, Harry ; Li, Sun peng (Leo) ; Siqueira, Rodrigo ; Koenig, Christian ; Hung, Alex ; Pillai, Aurabindo ; Wu, Hersen ; Lin, Wayne ; Mahfooz, Hamza *Subject:* [PATCH] drm/amd/display: fix ABM disablement On recent versions of DMUB firmware, if we want to completely disable ABM we have to pass ABM_LEVEL_IMMEDIATE_DISABLE as the requested ABM level to DMUB. Otherwise, LCD eDP displays are unable to reach their maximum brightness levels. So, to fix this whenever the user requests an ABM level of 0 pass ABM_LEVEL_IMMEDIATE_DISABLE to DMUB instead. Also, to keep the user's experience consistent map ABM_LEVEL_IMMEDIATE_DISABLE to 0 when a user tries to read the requested ABM level. Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +--- 1 file changed, 5 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 5d9496db0ecb..8cb92d941cd9 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6230,7 +6230,7 @@ int amdgpu_dm_connector_atomic_set_property(struct drm_connector *connector, dm_new_state->underscan_enable = val; ret = 0; } else if (property == adev->mode_info.abm_level_property) { - dm_new_state->abm_level = val; + dm_new_state->abm_level = val ?: ABM_LEVEL_IMMEDIATE_DISABLE; ret = 0; } @@ -6275,7 +6275,8 @@ int amdgpu_dm_connector_atomic_get_property(struct drm_connector *connector, *val = dm_state->underscan_enable; ret = 0; } else if (property == adev->mode_info.abm_level_property) { - *val = dm_state->abm_level; + *val = (dm_state->abm_level != ABM_LEVEL_IMMEDIATE_DISABLE) ? + dm_state->abm_level : 0; ret = 0; } @@ -6348,7 +6349,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) state->pbn = 0; if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) - state->abm_level = amdgpu_dm_abm_level; + state->abm_level = amdgpu_dm_abm_level ?: + ABM_LEVEL_IMMEDIATE_DISABLE; __drm_atomic_helper_connector_reset(connector, >base); } -- 2.42.1 -- Hamza
Re: [PATCH] drm/amd/display: fix ABM disablement
[AMD Official Use Only - General] Does amdgpu_dm_connector_funcs_reset() get called on wakeup from suspend ? Users would want the system to have the same brightness level before suspending. -- Regards, Jay From: Mahfooz, Hamza Sent: Wednesday, November 22, 2023 5:24 PM To: amd-gfx@lists.freedesktop.org Cc: Wentland, Harry ; Li, Sun peng (Leo) ; Siqueira, Rodrigo ; Koenig, Christian ; Hung, Alex ; Pillai, Aurabindo ; Wu, Hersen ; Lin, Wayne ; Mahfooz, Hamza Subject: [PATCH] drm/amd/display: fix ABM disablement On recent versions of DMUB firmware, if we want to completely disable ABM we have to pass ABM_LEVEL_IMMEDIATE_DISABLE as the requested ABM level to DMUB. Otherwise, LCD eDP displays are unable to reach their maximum brightness levels. So, to fix this whenever the user requests an ABM level of 0 pass ABM_LEVEL_IMMEDIATE_DISABLE to DMUB instead. Also, to keep the user's experience consistent map ABM_LEVEL_IMMEDIATE_DISABLE to 0 when a user tries to read the requested ABM level. Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +--- 1 file changed, 5 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 5d9496db0ecb..8cb92d941cd9 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6230,7 +6230,7 @@ int amdgpu_dm_connector_atomic_set_property(struct drm_connector *connector, dm_new_state->underscan_enable = val; ret = 0; } else if (property == adev->mode_info.abm_level_property) { - dm_new_state->abm_level = val; + dm_new_state->abm_level = val ?: ABM_LEVEL_IMMEDIATE_DISABLE; ret = 0; } @@ -6275,7 +6275,8 @@ int amdgpu_dm_connector_atomic_get_property(struct drm_connector *connector, *val = dm_state->underscan_enable; ret = 0; } else if (property == adev->mode_info.abm_level_property) { - *val = dm_state->abm_level; + *val = (dm_state->abm_level != ABM_LEVEL_IMMEDIATE_DISABLE) ? + dm_state->abm_level : 0; ret = 0; } @@ -6348,7 +6349,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) state->pbn = 0; if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) - state->abm_level = amdgpu_dm_abm_level; + state->abm_level = amdgpu_dm_abm_level ?: + ABM_LEVEL_IMMEDIATE_DISABLE; __drm_atomic_helper_connector_reset(connector, >base); } -- 2.42.1
[PATCH] drm/amd/display: fix ABM disablement
On recent versions of DMUB firmware, if we want to completely disable ABM we have to pass ABM_LEVEL_IMMEDIATE_DISABLE as the requested ABM level to DMUB. Otherwise, LCD eDP displays are unable to reach their maximum brightness levels. So, to fix this whenever the user requests an ABM level of 0 pass ABM_LEVEL_IMMEDIATE_DISABLE to DMUB instead. Also, to keep the user's experience consistent map ABM_LEVEL_IMMEDIATE_DISABLE to 0 when a user tries to read the requested ABM level. Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +--- 1 file changed, 5 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 5d9496db0ecb..8cb92d941cd9 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6230,7 +6230,7 @@ int amdgpu_dm_connector_atomic_set_property(struct drm_connector *connector, dm_new_state->underscan_enable = val; ret = 0; } else if (property == adev->mode_info.abm_level_property) { - dm_new_state->abm_level = val; + dm_new_state->abm_level = val ?: ABM_LEVEL_IMMEDIATE_DISABLE; ret = 0; } @@ -6275,7 +6275,8 @@ int amdgpu_dm_connector_atomic_get_property(struct drm_connector *connector, *val = dm_state->underscan_enable; ret = 0; } else if (property == adev->mode_info.abm_level_property) { - *val = dm_state->abm_level; + *val = (dm_state->abm_level != ABM_LEVEL_IMMEDIATE_DISABLE) ? + dm_state->abm_level : 0; ret = 0; } @@ -6348,7 +6349,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) state->pbn = 0; if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) - state->abm_level = amdgpu_dm_abm_level; + state->abm_level = amdgpu_dm_abm_level ?: + ABM_LEVEL_IMMEDIATE_DISABLE; __drm_atomic_helper_connector_reset(connector, >base); } -- 2.42.1