amdgpu, dc, backlight brightness

[Andriy Gapon]

On 29/04/2020 21:15, Alex Deucher wrote:
> I think it varies from OEM to OEM and whatever windows required at the
> time.  The sbios may also do different things depending on the osi
> string passed to ACPI.  Originally, ACPI handled it all directly.
> Then we got some laptops which which did the whole event via ATIF
> thing (even that I think varied based on the .  Later, I think the
> keys just produced brightness events and it was up to the OS to do
> something with them so the user's environment would catch the events
> and adjust the backlight via the standard OS backlight control
> interface.  We never hooked up the ATIF stuff to DC since I don't
> recall ever running into any laptops that used it for backlight
> control (the code was carried over from radeon when we forked amdgpu).

Thank you for the information!
Indeed, I see that there are so many quirks in how brightness keys are handled
by firmware.  Ranging from actually changing the brightness to posting ACPI
events to posting key codes.  And then those options are not exclusive of each
other.

Just in case, I've written a bit of code for ATIF handler to control the
backlight in the DC case.  I doubt that it is very useful, it was mostly an
exercise for myself.

----------------------------------------------------------------------------
commit ed2ca1d7e3fbdb641d9a1bc2de9b88e2927ff1bd
Author: Andriy Gapon <avg at FreeBSD.org>
Date:   Thu Apr 30 14:47:11 2020 +0300

    amdgpu_acpi: perform automatic backlight adjustment in the DC case too

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 04bbd8f41441c..62fbae1177091 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -444,7 +444,6 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev,

 		DRM_DEBUG_DRIVER("ATIF: %d pending SBIOS requests\n", count);

-		/* todo: add DC handling */
 		if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) &&
 		    !amdgpu_device_has_dc_support(adev)) {
 			struct amdgpu_encoder *enc = atif->encoder_for_bl;
@@ -463,6 +462,34 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev,
 #endif
 			}
 		}
+#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) ||
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
+		if ((req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) &&
+		    amdgpu_device_has_dc_support(adev)) {
+			struct amdgpu_display_manager *dm = &adev->dm;
+			struct backlight_device *bd = dm->backlight_dev;
+
+			if (bd) {
+				DRM_DEBUG_DRIVER("Changing brightness to %d\n",
+						 req.backlight_level);
+
+				/*
+				 * Newer Linux has
+				 * backlight_device_set_brightness, but it is
+				 * hardwired to post BACKLIGHT_UPDATE_SYSFS.
+				 */
+				mutex_lock(&bd->ops_lock);
+				if (bd->ops &&
+				    req.backlight_level <= bd->props.max_brightness) {
+					bd->props.brightness = req.backlight_level;
+					backlight_update_status(bd);
+				}
+				mutex_unlock(&bd->ops_lock);
+#if 0
+				backlight_generate_event(bd, BACKLIGHT_UPDATE_HOTKEY);
+#endif
+			}
+		}
+#endif
 		if (req.pending & ATIF_DGPU_DISPLAY_EVENT) {
 			if ((adev->flags & AMD_IS_PX) &&
 			    amdgpu_atpx_dgpu_req_power_for_displays()) {



-- 
Andriy Gapon