On 2021-11-24 13:55, Alex Deucher wrote: > On Wed, Nov 24, 2021 at 1:52 PM Harry Wentland <harry.wentland@xxxxxxx> wrote: >> >> On 2021-11-24 11:36, Alex Deucher wrote: >>> Update the bios scratch register when updating the backlight >>> level. Some platforms apparently read this scratch register >>> and do additional operations in their hotkey handlers. >>> >>> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1518>>>> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx> >> >> Maybe dce_driver_set_backlight should handle this but doing this >> in amdgpu also works. I don't know if I have a preference either >> way. > > Might be helpful for other OSes unless they handle the bios scratch > registers some other way. Also does dce_driver_set_backlight() handle > OLED and aux controlled backlights? > No, that has its own codepath and other OSes tend to always use the ABM path which uses DMCU/DMUB for programming backlight. I'm not sure if they let FW handle that or deal with it in the DM for the OS. Hence your approach is possibly more sensible in amdgpu. Harry > Alex > >> >> Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx> >> >> Harry >> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 12 ++++++++++++ >>> drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h | 2 ++ >>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++++ >>> 3 files changed, 18 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c >>> index 96b7bb13a2dd..12a6b1c99c93 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c >>> @@ -1569,6 +1569,18 @@ void amdgpu_atombios_scratch_regs_engine_hung(struct amdgpu_device *adev, >>> WREG32(adev->bios_scratch_reg_offset + 3, tmp); >>> } >>> >>> +void amdgpu_atombios_scratch_regs_set_backlight_level(struct amdgpu_device *adev, >>> + u32 backlight_level) >>> +{ >>> + u32 tmp = RREG32(adev->bios_scratch_reg_offset + 2); >>> + >>> + tmp &= ~ATOM_S2_CURRENT_BL_LEVEL_MASK; >>> + tmp |= (backlight_level << ATOM_S2_CURRENT_BL_LEVEL_SHIFT) & >>> + ATOM_S2_CURRENT_BL_LEVEL_MASK; >>> + >>> + WREG32(adev->bios_scratch_reg_offset + 2, tmp); >>> +} >>> + >>> bool amdgpu_atombios_scratch_need_asic_init(struct amdgpu_device *adev) >>> { >>> u32 tmp = RREG32(adev->bios_scratch_reg_offset + 7); >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h >>> index 8cc0222dba19..27e74b1fc260 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h >>> @@ -185,6 +185,8 @@ bool amdgpu_atombios_has_gpu_virtualization_table(struct amdgpu_device *adev); >>> void amdgpu_atombios_scratch_regs_lock(struct amdgpu_device *adev, bool lock); >>> void amdgpu_atombios_scratch_regs_engine_hung(struct amdgpu_device *adev, >>> bool hung); >>> +void amdgpu_atombios_scratch_regs_set_backlight_level(struct amdgpu_device *adev, >>> + u32 backlight_level); >>> bool amdgpu_atombios_scratch_need_asic_init(struct amdgpu_device *adev); >>> >>> void amdgpu_atombios_copy_swap(u8 *dst, u8 *src, u8 num_bytes, bool to_le); >>> 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 40a4269770fa..05e4a0952a2b 100644 >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> @@ -51,6 +51,7 @@ >>> #include <drm/drm_hdcp.h> >>> #endif >>> #include "amdgpu_pm.h" >>> +#include "amdgpu_atombios.h" >>> >>> #include "amd_shared.h" >>> #include "amdgpu_dm_irq.h" >>> @@ -3918,6 +3919,9 @@ static int amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm, >>> caps = dm->backlight_caps[bl_idx]; >>> >>> dm->brightness[bl_idx] = user_brightness; >>> + /* update scratch register */ >>> + if (bl_idx == 0) >>> + amdgpu_atombios_scratch_regs_set_backlight_level(dm->adev, dm->brightness[bl_idx]); >>> brightness = convert_brightness_from_user(&caps, dm->brightness[bl_idx]); >>> link = (struct dc_link *)dm->backlight_link[bl_idx]; >>> >>> >>
