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

Mario Limonciello mario.limonciello at amd.com
Fri Feb 16 14:42:30 UTC 2024


On 2/16/2024 08:38, Christian König wrote:
> 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?

We're talking about activating the ABM algorithm when the system is in 
power saving mode; not from inactivity.  This allows the user to squeeze 
out some extra power "just" in that situation.

But given the comments on the other patch, I tend to agree with Harry's 
proposal instead that we just drop the DRM property entirely as there 
are no consumers of it.

> 
> 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