[PATCH v4 2/2] drm/amd: Add abm level drm property

Wentland, Harry Harry.Wentland at amd.com
Wed Nov 21 20:41:53 UTC 2018



On 2018-11-20 11:00 a.m., 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:
> Store ABM level in crtc state to accommodate dc
> v4:
> Fix ABM saving on dpms cycle
> 
> Signed-off-by: David Francis <David.Francis at amd.com>

Series is
Reviewed-by: Harry Wentland <harry.wentland at amd.com>

Harry

> ---
>  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 | 36 ++++++++++++++++---
>  .../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, 53 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..95b1106e0662 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 =
> @@ -4579,6 +4600,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>  
>  		dc_stream_attach->adjust = acrtc_state->adjust;
>  		dc_stream_attach->vrr_infopacket = acrtc_state->vrr_infopacket;
> +		dc_stream_attach->abm_level = acrtc_state->abm_level;
>  
>  		if (false == commit_planes_to_stream(dm->dc,
>  							plane_states_constructed,
> @@ -4759,7 +4781,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 +4797,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 +4818,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 +5156,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