Re: [PATCH] drm/panel-edp: disable the backlight before powering off panel

[Doug Anderson]

Hi,

On Sat, Oct 24, 2026 at 5:24 PM Haipeng Lin via B4 Relay
<devnull+jimmie.lin.gmail.com@xxxxxxxxxx> wrote:
>
> From: Haipeng Lin <jimmie.lin@xxxxxxxxx>
>
> If the display pipeline takes longer than the runtime-PM autosuspend
> delay (1 s) to bind after the panel probes, panel-edp's runtime suspend
> powers the panel off while the backlight is still on: pwm-backlight
> inherits the enable state left by the firmware at probe and keeps the
> enable GPIO high. The panel supply is thus cut without first disabling
> the backlight, leaving BL_EN asserted across a panel power cycle.
>
> On the Surface Pro 11 LCD (LGD0779), leaving BL_EN high across this
> power cycle appears to leave the backlight controller in an incorrect
> state: brightness is pinned near maximum with a greatly reduced
> dimming range. Subsequent brightness changes update the PWM duty, but
> do not restore it to normal. Cycling the backlight off and on at
> unchanged brightness restores the full range.
>
> The first drm_panel_enable() does not recover from this because
> pwm-backlight still considers itself enabled and leaves BL_EN high.
>
> Disable the attached backlight in panel_edp_suspend() before marking
> AUX unpowered and cutting panel power. This ensures that the first
> modeset enables the backlight after panel power and video are ready.
> When DRM has already disabled the backlight, it remains off.
>
> Verified on the Surface Pro 11 LCD by delaying display pipeline bind
> past the autosuspend timeout. BL_EN now drops before panel power is
> cut, and brightness control works at the first modeset.
>
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> Link: https://github.com/dwhinham/archiso-aarch64-sp11/issues/41
> Assisted-by: claude-fable:5
> Signed-off-by: Haipeng Lin <jimmie.lin@xxxxxxxxx>
> ---
> Can reproduce on an installed system on Surface Pro 11 LCD (X1P-64-100,
> LGD0779 LCD) by booting with modprobe.blacklist=phy_qcom_qmp_combo then
> loading that module manually after ~20s. This delays DPU component bind
> past the panel's runtime autosuspend timeout.
>
> A/B tested against the same v7.2 tree:
>
> - Unpatched: panel VDD off 3.5s..20.5s while BL_EN stays high.
> after the first modeset brightness is pinned near max brightness with
> a "collapsed" dimming range. sysfs brightness writes still reprogram
> the PMIC LPG. One bl_power off/on cycle at unchanged brightness
> restores the full range.
>
> - Patched: BL_EN drops ~300us before the VDD cut and rises again after
> panel power+link at the first modeset. full brightness range works.
>
> Tracing and functional testing on the SP11 shows unchanged sequencing
> for fast boots, DPMS off/on, and system suspend/resume.
>
> Two cases would benefit from testing on other hardware:
> 1. AUX-controlled backlights
> The disable is placed before drm_dp_dpcd_set_powered(false) and
> regulator_disable() so the panel remains accessible if disabling its
> backlight requires AUX writes. But I have not tested this since my
> LCD is GPIO/PWM based.
>
> 2. Panels with shared/always-on supplies and no panel enable GPIO
> These panels may continue displaying the firmware framebuffer during
> the pre-first-modeset runtime suspend. This change will turn off their
> attached backlight during that interval, potentially making the boot
> display go dark earlier. This matches the driver's powered-off state,
> but I have not tested that configuration.
>
> Testing on other panel-edp systems particularly with pwm-backlight,
> would be appreciated.
> ---
>  drivers/gpu/drm/panel/panel-edp.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 94bc54c3833a..87f9c13380be 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -21,6 +21,7 @@
>   * DEALINGS IN THE SOFTWARE.
>   */
>
> +#include <linux/backlight.h>
>  #include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
> @@ -413,6 +414,18 @@ static int panel_edp_suspend(struct device *dev)
>  {
>         struct panel_edp *p = dev_get_drvdata(dev);
>
> +       /*
> +        * eDP panel power sequencing requires the backlight to be off
> +        * whenever the panel is powered off. Before the first modeset the
> +        * backlight driver may still hold the enable state inherited from
> +        * the firmware, and some panels' backlight controllers come up in a
> +        * wrong dimming mode if their enable line stays high across a panel
> +        * power cycle. Sync the backlight off here so that the backlight
> +        * enable from drm_panel_enable() performs a true power-on in the
> +        * correct order.
> +        */
> +       backlight_disable(p->base.backlight);

The above doesn't look right to me. In general, the backlight should
be owned by the core DRM panel code. Jamming this in here looks like a
hack. We'll also be double-disabling the backlight in most cases since
the core drm_panel code will be disabling it _and_ we'll be disabling
it.

I guess the problem here is that the PWM backlight code tries to
preserve the backlight to handle panel drivers that support a seamless
handoff from the bootloader to the kernel. Since some panel drivers
handle that and others (like panel-edp) don't, it's hard for the core
to magically do the right thing.

I think the right answer is to disable the backlight at probe time for
panel-edp. This is a one-time thing and just to handle the bootloader
handoff, so having the workaround run once at bootup is better.

-Doug