[PATCH] drm/amd: Only allow one entity to control ABM
Christian König
ckoenig.leichtzumerken at gmail.com
Fri Feb 16 14:47:15 UTC 2024
Am 16.02.24 um 15:42 schrieb Mario Limonciello:
> 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.
Yeah, but even then the design to let this be controlled by an userspace
deamon is questionable. Stuff like that is handled inside the kernel and
not exposed to userspace usually.
Regards,
Christian.
>
>>
>> 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