Re: [PATCH v2] drm/panel-edp: disable the backlight at probe

[Doug Anderson]

Hi,

On Mon, Nov 2, 2026 at 5:15 AM Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:
>
> On 10/30/26 02:40, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Oct 26, 2026 at 6:14 PM Haipeng Lin via B4 Relay
> > <devnull+jimmie.lin.gmail.com@xxxxxxxxxx> wrote:
> >>
> >> From: Haipeng Lin <jimmie.lin@xxxxxxxxx>
> >>
> >> pwm-backlight can preserve the enabled state left by firmware to
> >> support a seamless display handoff. However, panel-edp does not support
> >> such a handoff. When probing a generic eDP panel, it powers the panel
> >> on to read its EDID. If display pipeline binding takes longer than the
> >> runtime-PM autosuspend delay, the panel is then powered off while the
> >> backlight remains enabled.
>
> So we don't support handoff _now_ but we may certainly support it someday.

Yup, that's fair.


> >> 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 backlight at probe, after acquiring it and before enabling
> >> runtime PM. This clears the firmware's enabled state so the DRM panel
> >> core turns the backlight on after panel power and video are ready at
> >> the first modeset.
> >>
> >> This also turns off the firmware display at probe on systems where it
> >> previously remained lit until the first modeset.
> >>
> >> Verified on the Surface Pro 11 LCD by delaying display pipeline bind
> >> past the autosuspend timeout. BL_EN now drops at probe, before panel
> >> runtime autosuspend, 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
> >> Suggested-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> >> Assisted-by: LLM
> >> 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 v2: patch provides the same full brightness range on boot
> >> (stalled or not) as the v1 patch. BL_EN drops at panel-edp probe,
> >> ~1s before VDD cut, rises ~0.5s after panel power at the first modeset.
> >>
> >> Tracing and functional testing on the SP11 shows unchanged sequencing
> >> for fast boots (without artificial delay), DPMS off/on, and system
> >> suspend/resume.
> >>
> >> Panels with shared/always-on supplies and no panel enable GPIO
> >> may continue displaying the firmware framebuffer during the
> >> runtime suspend before first modeset. With this change panel-edp
> >> disables the backlight obtained through the backlight phandle at probe.
> >> The DRM panel core enables the backlight again at the first modeset.
> >> I have not tested this configuration.
> >>
> >> Testing on other panel-edp systems particularly with pwm-backlight,
> >> would be appreciated.
> >> ---
> >> Changes in v2:
> >> - Disables the backlight once at probe instead of
> >>    in panel_edp_suspend() following Doug Anderson's suggestion.
> >> - Only disable the backlight obtained through the backlight phandle.
> >>    The driver's AUX backlight setup is untouched.
> >> - Link to v1: https://lore.kernel.org/r/20261024-sp11-bl-upstream-1-v1-1-d39bfaf9afbc@xxxxxxxxx
> >> ---
> >>   drivers/gpu/drm/panel/panel-edp.c | 11 +++++++++++
> >>   1 file changed, 11 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> >> index 94bc54c3833a..d019f4165674 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>
> >> @@ -918,6 +919,16 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
> >>          if (err)
> >>                  return err;
> >>
> >> +       /*
> >> +        * Backlight drivers may preserve the state left by firmware,
> >> +        * but panel-edp does not support seamless handoff.
> >> +        * Runtime PM may power off the panel before the first modeset,
> >> +        * leaving the backlight on.
> >> +        * Disable it here so the DRM panel core enables it in the correct
> >> +        * order at the first modeset.
> >> +        */
> >> +       backlight_disable(panel->base.backlight);
> >
> > While still feeling a little ugly, I can't think of anything better to
> > do in this case. Thus I'd say:
> >
> > Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> >
> > I'd prefer to get a second opinion from someone else in the DRM
> > community to make sure I'm not missing something, though. Neil,
> > Maxime, or Linus W maybe?
>
> I lack some knowledge, but when the display pipeline is probed
> who's in charge of disabling the backlight when suspending ?

I'm afraid I don't understand this question.


> Why don't we handle the backlight state in the PM ops ?

That was what Haipeng proposed in v1. ...but it felt wrong to me. The
core panel code is supposed to manage the backlight and it handles
this in most cases. The only problem needing a fix was at bootup.

-Doug