[PATCH] drm/amd: Only allow one entity to control ABM

Christian König ckoenig.leichtzumerken at gmail.com
Fri Feb 16 14:38:57 UTC 2024


Am 16.02.24 um 15:07 schrieb Mario Limonciello:
> By exporting ABM to sysfs it's possible that DRM master and software
> controlling the sysfs file fight over the value programmed for ABM.
>
> Adjust the module parameter behavior to control who control ABM:
> -2: DRM
> -1: sysfs (IE via software like power-profiles-daemon)

Well that sounds extremely awkward. Why should a power-profiles-deamon 
has control over the panel power saving features?

I mean we are talking about things like reducing backlight level when 
the is inactivity, don't we?

Regards,
Christian.

> 0-4: User via command line
>
> Also introduce a Kconfig option that allows distributions to choose
> the default policy that is appropriate for them.
>
> Fixes: f97e4303da16 ("drm/amd/display: add panel_power_savings sysfs entry to eDP connectors")
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
> Cc: Hamza Mahfooz <Hamza.Mahfooz at amd.com>
> Cc: Harry Wentland <Harry.Wentland at amd.com>
> Cc: Sun peng (Leo) Li <Sunpeng.Li at amd.com>
>   drivers/gpu/drm/amd/amdgpu/Kconfig            | 72 +++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 23 +++---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  6 +-
>   3 files changed, 90 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index 22d88f8ef527..2ab57ccf6f21 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -80,6 +80,78 @@ config DRM_AMDGPU_WERROR
>   	  Add -Werror to the build flags for amdgpu.ko.
>   	  Only enable this if you are warning code for amdgpu.ko.
>   
> +choice
> +	prompt "Amdgpu panel power Savings"
> +	default AMDGPU_SYSFS_ABM
> +	help
> +		Control the default behavior for adaptive panel power savings.
> +
> +		Panel power savings features will sacrifice color accuracy
> +		in exchange for power savings.
> +
> +		This can be configured for:
> +		- dynamic control by the DRM master
> +		- dynamic control by sysfs nodes
> +		- statically by the user at kernel compile time
> +
> +		This value can also be overridden by the amdgpu.abmlevel
> +		module parameter.
> +
> +config AMDGPU_DRM_ABM
> +	bool "DRM Master control"
> +	help
> +		Export a property called 'abm_level' that can be
> +		manipulated by the DRM master for supported hardware.
> +
> +config AMDGPU_SYSFS_ABM
> +	bool "sysfs control"
> +	help
> +		Export a sysfs file 'panel_power_savings' that can be
> +		manipulated by userspace for supported hardware.
> +
> +config AMDGPU_HARDCODE_ABM0
> +	bool "No Panel power savings"
> +	help
> +		Disable panel power savings.
> +		It can only overridden by the kernel command line.
> +
> +config AMDGPU_HARDCODE_ABM1
> +	bool "25% Panel power savings"
> +	help
> +		Set the ABM panel power savings algorithm to 25%.
> +		It can only overridden by the kernel command line.
> +
> +config AMDGPU_HARDCODE_ABM2
> +	bool "50% Panel power savings"
> +	help
> +		Set the ABM panel power savings algorithm to 50%.
> +		It can only overridden by the kernel command line.
> +
> +config AMDGPU_HARDCODE_ABM3
> +	bool "75% Panel power savings"
> +	help
> +		Set the ABM panel power savings algorithm to 75%.
> +		It can only overridden by the kernel command line.
> +
> +config AMDGPU_HARDCODE_ABM4
> +	bool "100% Panel power savings"
> +	help
> +		Set the ABM panel power savings algorithm to 100%.
> +		It can only overridden by the kernel command line.
> +endchoice
> +
> +config AMDGPU_ABM_POLICY
> +	int
> +	default -2 if AMDGPU_DRM_ABM
> +	default -1 if AMDGPU_SYSFS_ABM
> +	default 0 if AMDGPU_HARDCODE_ABM0
> +	default 1 if AMDGPU_HARDCODE_ABM1
> +	default 2 if AMDGPU_HARDCODE_ABM2
> +	default 3 if AMDGPU_HARDCODE_ABM3
> +	default 4 if AMDGPU_HARDCODE_ABM4
> +	default -1
> +
> +
>   source "drivers/gpu/drm/amd/acp/Kconfig"
>   source "drivers/gpu/drm/amd/display/Kconfig"
>   source "drivers/gpu/drm/amd/amdkfd/Kconfig"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index af7fae7907d7..00d6c8b58716 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -844,17 +844,24 @@ module_param_named(visualconfirm, amdgpu_dc_visual_confirm, uint, 0444);
>    * DOC: abmlevel (uint)
>    * Override the default ABM (Adaptive Backlight Management) level used for DC
>    * enabled hardware. Requires DMCU to be supported and loaded.
> - * Valid levels are 0-4. A value of 0 indicates that ABM should be disabled by
> - * default. Values 1-4 control the maximum allowable brightness reduction via
> - * the ABM algorithm, with 1 being the least reduction and 4 being the most
> - * reduction.
> + * Valid levels are -2 through 4.
>    *
> - * Defaults to -1, or disabled. Userspace can only override this level after
> - * boot if it's set to auto.
> + *  -2: indicates that ABM should be controlled by DRM property 'abm_level.
> + *  -1: indicates that ABM should be controlled by the sysfs file
> + *      'panel_power_savings'.
> + *   0: indicates that ABM should be disabled.
> + * 1-4: control the maximum allowable brightness reduction via
> + *      the ABM algorithm, with 1 being the least reduction and 4 being the most
> + *      reduction.
> + *
> + * Both the DRM property 'abm_level' and the sysfs file 'panel_power_savings'
> + * will only be available on supported hardware configurations.
> + *
> + * The default value is configured by kernel configuration option AMDGPU_ABM_POLICY
>    */
> -int amdgpu_dm_abm_level = -1;
> +int amdgpu_dm_abm_level = CONFIG_AMDGPU_ABM_POLICY;
>   MODULE_PARM_DESC(abmlevel,
> -		 "ABM level (0 = off, 1-4 = backlight reduction level, -1 auto (default))");
> +		 "ABM level (0 = off, 1-4 = backlight reduction level, -1 = sysfs control, -2 = drm control");
>   module_param_named(abmlevel, amdgpu_dm_abm_level, int, 0444);
>   
>   int amdgpu_backlight = -1;
> 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 b9ac3d2f8029..147fe744f82e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6515,7 +6515,7 @@ static void amdgpu_dm_connector_unregister(struct drm_connector *connector)
>   	struct amdgpu_dm_connector *amdgpu_dm_connector = to_amdgpu_dm_connector(connector);
>   
>   	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
> -	    amdgpu_dm_abm_level < 0)
> +	    amdgpu_dm_abm_level == -1)
>   		sysfs_remove_group(&connector->kdev->kobj, &amdgpu_group);
>   
>   	drm_dp_aux_unregister(&amdgpu_dm_connector->dm_dp_aux.aux);
> @@ -6623,7 +6623,7 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector)
>   	int r;
>   
>   	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
> -	    amdgpu_dm_abm_level < 0) {
> +	    amdgpu_dm_abm_level == -1) {
>   		r = sysfs_create_group(&connector->kdev->kobj,
>   				       &amdgpu_group);
>   		if (r)
> @@ -7654,7 +7654,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
>   
>   	if (connector_type == DRM_MODE_CONNECTOR_eDP &&
>   	    (dc_is_dmcu_initialized(adev->dm.dc) ||
> -	     adev->dm.dc->ctx->dmub_srv) && amdgpu_dm_abm_level < 0) {
> +	     adev->dm.dc->ctx->dmub_srv) && amdgpu_dm_abm_level == -2) {
>   		drm_object_attach_property(&aconnector->base.base,
>   				adev->mode_info.abm_level_property, 0);
>   	}



More information about the amd-gfx mailing list