[PATCH] drm/i915/backlight: Keep ACPI backlight requests while the panel is off

[Zak Brighton Knight]

With the ACPI video backlight (acpi_video0), brightness changes reach
i915 as OpRegion ASLE requests: the firmware's _BCM method writes the
level to the mailbox and raises the ASLE interrupt.
intel_backlight_set_acpi() returns early when the connector has no
crtc, so a request made while the panel is off is dropped. The ACPI
video driver still reports the new level, but the panel comes back on
at the old one.

The connector has no crtc whenever userspace turns the screen off by
disabling the crtc and detaching the connector from it, which is what
mutter does for its power saving mode. GNOME's login screen makes this
easy to hit: it dims the screen when idle and turns it off a little
later, and on the next key press it restores the brightness while the
screen is still off. The screen comes back dim, and the next brightness
key press jumps from the level the backlight device reports.

On a ThinkPad T520 (Ivy Bridge, LVDS, coreboot), setting acpi_video0
to 10, turning the screen off through mutter, setting 14 and turning
the screen back on gives, with drm.debug=0x6:

  brightness 10, screen on:
    updating opregion backlight 73/255
    [CONNECTOR:92:LVDS-1] set backlight level = 444
  screen off:
    [CONNECTOR:92:LVDS-1] set backlight PWM = 0
  brightness 14, screen off:
    updating opregion backlight 170/255
  screen on:
    [CONNECTOR:92:LVDS-1] set backlight PWM = 444

after which acpi_video0 reads brightness 14 and actual_brightness 10.

The check goes back to commit 752aa88a1e78 ("drm/i915: make backlight
functions take a connector"). Commit 260d8f98ef9d ("drm/i915: Remove
most INVALID_PIPE checks from the backlight code") dropped it from the
native path, but kept it for BIOS requests, which can arrive during
driver init before the modeset readout, when there is no crtc yet. The
native path, intel_panel_set_backlight(), stores the level whatever the
connector state and leaves it to the next backlight enable.

Do the same for ACPI requests: always store the level, and skip only
the hardware update when there is no crtc, which still covers the
driver init case.

The problem was found and root-caused, and this patch was drafted, with
the help of an AI coding assistant (Claude, by Anthropic).

Fixes: 752aa88a1e78 ("drm/i915: make backlight functions take a connector")
Signed-off-by: Zak Brighton Knight <zakbrightonknight@xxxxxxxxx>
---
Tested on the T520 (i7-3840QM, Ivy Bridge, coreboot 24.08 with SeaBIOS)
with v7.4-rc1 plus this patch, built with PROVE_LOCKING, under GNOME 50
(mutter 50.6). intel_backlight_set_acpi() is the same in drm-intel-next
and drm-tip today, and the patch applies to both.

- The sequence above now ends with "set backlight PWM = 1035" when the
  screen comes back on, and acpi_video0 reads 14 for both brightness
  and actual_brightness. The same from 13 down to 6.
- Setting 12 while the screen is off, then suspending to RAM and
  resuming before turning the screen on: acpi_video re-sends 12 on
  resume ("Restoring backlight state"), and the screen comes back at 12.
- Suspend and resume with the screen on keeps the level, the level
  systemd restores at boot is applied, and there were no lockdep
  reports or warnings.

One case still comes back wrong, differently from before. On this
machine _BQC reports the PWM duty cycle, so reading actual_brightness
while the panel is off makes acpi_video cache its lowest level (2%),
and it re-sends that on resume. Without this patch, that request is
dropped and the panel comes back at the level from before the screen
went off; with it, the panel comes back at 2%. Mutter only reads
brightness, so I have only hit this from a test script.

Not tested: the driver init case the check was kept for. coreboot only
sends requests from _BCM, or from the brightness keys before the OS has
called _BCL, so I couldn't make it send one between the backlight
setup and the modeset readout. Not tested either on other platforms or
with other compositors.

https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15987 reports
the same symptom on a MacBookPro11,1: the brightness is wrong after the
display sleeps, and acpi_backlight=native fixes it. It may be this bug,
but I can't test that machine.

 drivers/gpu/drm/i915/display/intel_backlight.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
index b128896cb..2310a9258 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -310,13 +310,7 @@ void intel_backlight_set_acpi(const struct drm_connector_state *conn_state,
 	struct intel_panel *panel = &connector->panel;
 	u32 hw_level;
 
-	/*
-	 * Lack of crtc may occur during driver init because
-	 * connection_mutex isn't held across the entire backlight
-	 * setup + modeset readout, and the BIOS can issue the
-	 * requests at any time.
-	 */
-	if (!panel->backlight.present || !conn_state->crtc)
+	if (!panel->backlight.present)
 		return;
 
 	mutex_lock(&display->backlight.lock);
@@ -332,7 +326,15 @@ void intel_backlight_set_acpi(const struct drm_connector_state *conn_state,
 					 panel->backlight.level,
 					 panel->backlight.device->props.max_brightness);
 
-	if (panel->backlight.enabled)
+	/*
+	 * Lack of crtc may occur during driver init because
+	 * connection_mutex isn't held across the entire backlight
+	 * setup + modeset readout, and the BIOS can issue the
+	 * requests at any time. There's no crtc either while
+	 * userspace has the panel turned off. Keep the level
+	 * anyway, it's applied when the backlight is next enabled.
+	 */
+	if (panel->backlight.enabled && conn_state->crtc)
 		intel_panel_actually_set_backlight(conn_state, hw_level);
 
 	mutex_unlock(&display->backlight.lock);

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.56.0