[PATCH v3 2/2] drm/amd: Add abm level drm property
Samantha McVey
samantham at posteo.net
Fri Nov 16 21:52:26 UTC 2018
David,
Your patch works great. If you are not aware, the ABM does not work
anymore after the display turns off and back on again. Setting ABM level
to 0 and back to 4 makes it start working again though. This happens
when my display turns off automatically, but you can replicate this by
running `xset dpms force off`.
On 15-11-18 07:20, David.Francis at amd.com (David Francis) wrote:
> Adaptive Backlight Management (ABM) is a feature
> that reduces backlight level to save power, while
> increasing pixel contrast and pixel luminance
> to maintain readability and image quality.
>
> ABM will adjust in response to the
> pixel luminance of the displayed content.
>
> ABM is made available as a drm property on eDP
> monitors called "abm level", which ranges from 0 to 4.
> When this property is set to 0, ABM is off. Levels 1
> to 4 represent different ranges of backlight reduction.
> At higher levels both the backlight reduction and pixel
> adjustment will be greater.
>
> ABM requires DMCU firmware, which is currently available for
> Raven ASICs only. If the feature does not work, please
> ensure your firmware is up to date.
>
> v2:
> Fix commit message, only attach property if DMCU loaded
> v3:
> Storre ABM level in crtc state to accommodate dc
>
> Signed-off-by: David Francis <David.Francis at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 5 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 ++
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35 ++++++++++++++++---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 ++
> drivers/gpu/drm/amd/display/dc/core/dc.c | 11 +++++-
> drivers/gpu/drm/amd/display/dc/dc.h | 1 +
> 6 files changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 7d6a36bca9dd..ced8cefa223b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -637,6 +637,11 @@ int amdgpu_display_modeset_create_props(struct amdgpu_device *adev)
> "freesync_capable");
> if (!adev->mode_info.freesync_capable_property)
> return -ENOMEM;
> + adev->mode_info.abm_level_property =
> + drm_property_create_range(adev->ddev, 0,
> + "abm level", 0, 4);
> + if (!adev->mode_info.abm_level_property)
> + return -ENOMEM;
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 1627dd3413c7..2938635c0fc1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -342,6 +342,8 @@ struct amdgpu_mode_info {
> struct drm_property *freesync_property;
> /* it is used to know about display capability of freesync mode */
> struct drm_property *freesync_capable_property;
> + /* Adaptive Backlight Modulation (power feature) */
> + struct drm_property *abm_level_property;
> /* hardcoded DFP edid from BIOS */
> struct edid *bios_hardcoded_edid;
> int bios_hardcoded_edid_size;
> 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 f71febb4210d..090a602f3014 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2920,6 +2920,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
> state->adjust = cur->adjust;
> state->vrr_infopacket = cur->vrr_infopacket;
> state->freesync_enabled = cur->freesync_enabled;
> + state->abm_level = cur->abm_level;
>
> /* TODO Duplicate dc_stream after objects are stream object is flattened */
>
> @@ -3038,6 +3039,9 @@ int amdgpu_dm_connector_atomic_set_property(struct drm_connector *connector,
> } else if (property == adev->mode_info.freesync_capable_property) {
> dm_new_state->freesync_capable = val;
> ret = 0;
> + } else if (property == adev->mode_info.abm_level_property) {
> + dm_new_state->abm_level = val;
> + ret = 0;
> }
>
> return ret;
> @@ -3086,7 +3090,11 @@ int amdgpu_dm_connector_atomic_get_property(struct drm_connector *connector,
> } else if (property == adev->mode_info.freesync_capable_property) {
> *val = dm_state->freesync_capable;
> ret = 0;
> + } else if (property == adev->mode_info.abm_level_property) {
> + *val = dm_state->abm_level;
> + ret = 0;
> }
> +
> return ret;
> }
>
> @@ -3151,6 +3159,7 @@ amdgpu_dm_connector_atomic_duplicate_state(struct drm_connector *connector)
>
> new_state->freesync_capable = state->freesync_capable;
> new_state->freesync_enable = state->freesync_enable;
> + new_state->abm_level = state->abm_level;
>
> return &new_state->base;
> }
> @@ -3904,6 +3913,12 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
> drm_object_attach_property(&aconnector->base.base,
> adev->mode_info.freesync_capable_property, 0);
> }
> +
> + if (connector_type == DRM_MODE_CONNECTOR_eDP &&
> + dc_is_dmcu_initialized(adev->dm.dc)) {
> + drm_object_attach_property(&aconnector->base.base,
> + adev->mode_info.abm_level_property, 0);
> + }
> }
>
> static int amdgpu_dm_i2c_xfer(struct i2c_adapter *i2c_adap,
> @@ -4410,6 +4425,7 @@ static bool commit_planes_to_stream(
> struct dc_stream_state *dc_stream = dm_new_crtc_state->stream;
> struct dc_stream_update *stream_update =
> kzalloc(sizeof(struct dc_stream_update), GFP_KERNEL);
> + unsigned int abm_level;
>
> if (!stream_update) {
> BREAK_TO_DEBUGGER();
> @@ -4442,6 +4458,11 @@ static bool commit_planes_to_stream(
> stream_update->adjust = &dc_stream->adjust;
> }
>
> + if (dm_new_crtc_state->abm_level != dm_old_crtc_state->abm_level) {
> + abm_level = dm_new_crtc_state->abm_level;
> + stream_update->abm_level = &abm_level;
> + }
> +
> for (i = 0; i < new_plane_count; i++) {
> updates[i].surface = plane_states[i];
> updates[i].gamma =
> @@ -4759,7 +4780,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
> }
> }
>
> - /* Handle scaling and underscan changes*/
> + /* Handle scaling, underscan, and abm changes*/
> for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) {
> struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state);
> struct dm_connector_state *dm_old_con_state = to_dm_connector_state(old_con_state);
> @@ -4775,11 +4796,14 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
> if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state))
> continue;
>
> - /* Skip anything that is not scaling or underscan changes */
> - if (!is_scaling_state_different(dm_new_con_state, dm_old_con_state))
> - continue;
>
> dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> + dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
> +
> + /* Skip anything that is not scaling or underscan changes */
> + if (!is_scaling_state_different(dm_new_con_state, dm_old_con_state) &&
> + (dm_new_crtc_state->abm_level == dm_old_crtc_state->abm_level))
> + continue;
>
> update_stream_scaling_settings(&dm_new_con_state->base.crtc->mode,
> dm_new_con_state, (struct dc_stream_state *)dm_new_crtc_state->stream);
> @@ -4793,6 +4817,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
>
> dm_new_crtc_state->stream->adjust = dm_new_crtc_state->adjust;
> dm_new_crtc_state->stream->vrr_infopacket = dm_new_crtc_state->vrr_infopacket;
> + dm_new_crtc_state->stream->abm_level = dm_new_crtc_state->abm_level;
>
> /*TODO How it works with MPO ?*/
> if (!commit_planes_to_stream(
> @@ -5130,6 +5155,8 @@ static int dm_update_crtcs_state(struct amdgpu_display_manager *dm,
> set_freesync_on_stream(dm, dm_new_crtc_state,
> dm_new_conn_state, new_stream);
>
> + dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
> +
> if (dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
> dc_is_stream_scaling_unchanged(new_stream, dm_old_crtc_state->stream)) {
> new_crtc_state->mode_changed = 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 d6960644d714..3c2fbfac938f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -234,6 +234,8 @@ struct dm_crtc_state {
> bool freesync_enabled;
> struct dc_crtc_timing_adjust adjust;
> struct dc_info_packet vrr_infopacket;
> +
> + int abm_level;
> };
>
> #define to_dm_crtc_state(x) container_of(x, struct dm_crtc_state, base)
> @@ -255,6 +257,7 @@ struct dm_connector_state {
> bool underscan_enable;
> bool freesync_enable;
> bool freesync_capable;
> + uint8_t abm_level;
> };
>
> #define to_dm_connector_state(x)\
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 1d8bd554869b..dba6b57830c7 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -1686,6 +1686,15 @@ void dc_resume(struct dc *dc)
> core_link_resume(dc->links[i]);
> }
>
> +bool dc_is_dmcu_initialized(struct dc *dc)
> +{
> + struct dmcu *dmcu = dc->res_pool->dmcu;
> +
> + if (dmcu)
> + return dmcu->funcs->is_dmcu_initialized(dmcu);
> + return false;
> +}
> +
> bool dc_submit_i2c(
> struct dc *dc,
> uint32_t link_index,
> @@ -1810,4 +1819,4 @@ void get_clock_requirements_for_state(struct dc_state *state, struct AsicStateEx
> info->dcfClockDeepSleep = (unsigned int)state->bw.dcn.clk.dcfclk_deep_sleep_khz;
> info->fClock = (unsigned int)state->bw.dcn.clk.fclk_khz;
> info->phyClock = (unsigned int)state->bw.dcn.clk.phyclk_khz;
> -}
> \ No newline at end of file
> +}
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
> index 6b0988310138..dea8bc39c688 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -742,5 +742,6 @@ void dc_set_power_state(
> struct dc *dc,
> enum dc_acpi_cm_power_state power_state);
> void dc_resume(struct dc *dc);
> +bool dc_is_dmcu_initialized(struct dc *dc);
>
> #endif /* DC_INTERFACE_H_ */
More information about the amd-gfx
mailing list