Messages in this thread | | | Date | Fri, 25 Mar 2022 08:22:29 +0100 | Subject | Re: [PATCH v2 3/25] drm/amdgpu: Disable ABM when AC mode | From | Christian König <> |
| |
Hi Ryan,
we should try to avoid that and if it isn't possible at least use some constant like ACPI_AC_CLASS.
Could be that the information isn't available otherwise. Alex should know more about that.
Regards, Christian.
Am 25.03.22 um 08:09 schrieb Lin, Tsung-hua (Ryan): > [AMD Official Use Only] > > Hi Christian, > > There is already a string comparison in the same function. I just reference that to port this solution. > > > > #define ACPI_AC_CLASS "ac_adapter" > > > static int amdgpu_acpi_event(struct notifier_block *nb, > unsigned long val, > void *data) > { > struct amdgpu_device *adev = container_of(nb, struct amdgpu_device, acpi_nb); > struct acpi_bus_event *entry = (struct acpi_bus_event *)data; > > + if (strcmp(entry->device_class, "battery") == 0) { > + adev->pm.ac_power = power_supply_is_system_supplied() > 0; > + } > > if (strcmp(entry->device_class, ACPI_AC_CLASS) == 0) { <-------------------here! > if (power_supply_is_system_supplied() > 0) > DRM_DEBUG_DRIVER("pm: AC\n"); > else > DRM_DEBUG_DRIVER("pm: DC\n"); > > amdgpu_pm_acpi_event_handler(adev); > } > > /* Check for pending SBIOS requests */ > return amdgpu_atif_handler(adev, entry); > } > > Thanks, > Ryan Lin. > > -----Original Message----- > From: Koenig, Christian <Christian.Koenig@amd.com> > Sent: Friday, March 25, 2022 2:58 PM > To: Lin, Tsung-hua (Ryan) <Tsung-hua.Lin@amd.com>; Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; David1.Zhou@amd.com; airlied@linux.ie; daniel@ffwll.ch; seanpaul@chromium.org; bas@basnieuwenhuizen.nl; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; sashal@kernel.org; markyacoub@google.com; victorchengchi.lu@amd.com; ching-shih.li@amd.corp-partner.google.com; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; ddavenport@chromium.org; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; Li, Leon <Leon.Li@amd.com> > Subject: Re: [PATCH v2 3/25] drm/amdgpu: Disable ABM when AC mode > > Am 25.03.22 um 05:05 schrieb Ryan Lin: >> Disable ABM feature when the system is running on AC mode to get the >> more perfect contrast of the display. >> >> Signed-off-by: Ryan Lin <tsung-hua.lin@amd.com> >> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 4 ++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 + >> drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c | 58 ++++++++++++------- >> drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 1 + >> 4 files changed, 42 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c >> index c560c1ab62ecb..bc8bb9aad2e36 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c >> @@ -822,6 +822,10 @@ static int amdgpu_acpi_event(struct notifier_block *nb, >> struct amdgpu_device *adev = container_of(nb, struct amdgpu_device, acpi_nb); >> struct acpi_bus_event *entry = (struct acpi_bus_event *)data; >> >> + if (strcmp(entry->device_class, "battery") == 0) { > String comparison in a hot path is not something we usually like to see in the kernel. > > Isn't there any other way to detect that? Like a flag or similar? > > Regards, > Christian. > >> + adev->pm.ac_power = power_supply_is_system_supplied() > 0; >> + } >> + >> if (strcmp(entry->device_class, ACPI_AC_CLASS) == 0) { >> if (power_supply_is_system_supplied() > 0) >> DRM_DEBUG_DRIVER("pm: AC\n"); >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index abfcc1304ba0c..3a0afe7602727 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -3454,6 +3454,7 @@ int amdgpu_device_init(struct amdgpu_device >> *adev, >> >> adev->gfx.gfx_off_req_count = 1; >> adev->pm.ac_power = power_supply_is_system_supplied() > 0; >> + adev->pm.old_ac_power = true; >> >> atomic_set(&adev->throttling_logging_enabled, 1); >> /* >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c >> b/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c >> index 54a1408c8015c..478a734b66926 100644 >> --- a/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c >> @@ -23,6 +23,8 @@ >> * >> */ >> >> +#include <linux/power_supply.h> >> +#include "amdgpu.h" >> #include "dmub_abm.h" >> #include "dce_abm.h" >> #include "dc.h" >> @@ -51,6 +53,7 @@ >> #define DISABLE_ABM_IMMEDIATELY 255 >> >> >> +extern uint amdgpu_dm_abm_level; >> >> static void dmub_abm_enable_fractional_pwm(struct dc_context *dc) >> { >> @@ -117,28 +120,6 @@ static void dmub_abm_init(struct abm *abm, uint32_t backlight) >> dmub_abm_enable_fractional_pwm(abm->ctx); >> } >> >> -static unsigned int dmub_abm_get_current_backlight(struct abm *abm) >> -{ >> - struct dce_abm *dce_abm = TO_DMUB_ABM(abm); >> - unsigned int backlight = REG_READ(BL1_PWM_CURRENT_ABM_LEVEL); >> - >> - /* return backlight in hardware format which is unsigned 17 bits, with >> - * 1 bit integer and 16 bit fractional >> - */ >> - return backlight; >> -} >> - >> -static unsigned int dmub_abm_get_target_backlight(struct abm *abm) -{ >> - struct dce_abm *dce_abm = TO_DMUB_ABM(abm); >> - unsigned int backlight = REG_READ(BL1_PWM_TARGET_ABM_LEVEL); >> - >> - /* return backlight in hardware format which is unsigned 17 bits, with >> - * 1 bit integer and 16 bit fractional >> - */ >> - return backlight; >> -} >> - >> static bool dmub_abm_set_level(struct abm *abm, uint32_t level) >> { >> union dmub_rb_cmd cmd; >> @@ -148,6 +129,9 @@ static bool dmub_abm_set_level(struct abm *abm, uint32_t level) >> int edp_num; >> uint8_t panel_mask = 0; >> >> + if (power_supply_is_system_supplied() > 0) >> + level = 0; >> + >> get_edp_links(dc->dc, edp_links, &edp_num); >> >> for (i = 0; i < edp_num; i++) { >> @@ -170,6 +154,36 @@ static bool dmub_abm_set_level(struct abm *abm, uint32_t level) >> return true; >> } >> >> +static unsigned int dmub_abm_get_current_backlight(struct abm *abm) { >> + struct dce_abm *dce_abm = TO_DMUB_ABM(abm); >> + unsigned int backlight = REG_READ(BL1_PWM_CURRENT_ABM_LEVEL); >> + struct dc_context *dc = abm->ctx; >> + struct amdgpu_device *adev = dc->driver_context; >> + >> + if (adev->pm.ac_power != adev->pm.old_ac_power) { >> + dmub_abm_set_level(abm, amdgpu_dm_abm_level); >> + adev->pm.ac_power = power_supply_is_system_supplied() > 0; >> + adev->pm.old_ac_power = adev->pm.ac_power; >> + } >> + >> + /* return backlight in hardware format which is unsigned 17 bits, with >> + * 1 bit integer and 16 bit fractional >> + */ >> + return backlight; >> +} >> + >> +static unsigned int dmub_abm_get_target_backlight(struct abm *abm) { >> + struct dce_abm *dce_abm = TO_DMUB_ABM(abm); >> + unsigned int backlight = REG_READ(BL1_PWM_TARGET_ABM_LEVEL); >> + >> + /* return backlight in hardware format which is unsigned 17 bits, with >> + * 1 bit integer and 16 bit fractional >> + */ >> + return backlight; >> +} >> + >> static bool dmub_abm_init_config(struct abm *abm, >> const char *src, >> unsigned int bytes, >> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h >> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h >> index f6e0e7d8a0077..de459411a0e83 100644 >> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h >> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h >> @@ -445,6 +445,7 @@ struct amdgpu_pm { >> uint32_t smu_prv_buffer_size; >> struct amdgpu_bo *smu_prv_buffer; >> bool ac_power; >> + bool old_ac_power; >> /* powerplay feature */ >> uint32_t pp_feature; >>
| |