linux-kernel - Re: [PATCH 1/2] platform/loongarch: laptop: Get brightness setting from EC on probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAhV-H6j1OT9D8ZBtyEP=Mu5+m=t0ebUvuC=gVeNsoPizwK1TQ@mail.gmail.com>
Date: Wed, 4 Jun 2025 21:56:20 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Yao Zi <ziyao@...root.org>
Cc: Jianmin Lv <lvjianmin@...ngson.cn>, WANG Xuerui <kernel@...0n.name>, 
	linux-kernel@...r.kernel.org, loongarch@...ts.linux.dev, 
	Mingcong Bai <jeffbai@...c.io>, Kexy Biscuit <kexybiscuit@...c.io>, stable@...r.kernel.org
Subject: Re: [PATCH 1/2] platform/loongarch: laptop: Get brightness setting
 from EC on probe

On Tue, Jun 3, 2025 at 2:44 PM Yao Zi <ziyao@...root.org> wrote:
>
> On Tue, Jun 03, 2025 at 12:11:48PM +0800, Huacai Chen wrote:
> > On Sat, May 31, 2025 at 7:39 PM Yao Zi <ziyao@...root.org> wrote:
> > >
> > > Previously 1 is unconditionally taken as current brightness value. This
> > > causes problems since it's required to restore brightness settings on
> > > resumption, and a value that doesn't match EC's state before suspension
> > > will cause surprising changes of screen brightness.
> > laptop_backlight_register() isn't called at resuming, so I think your
> > problem has nothing to do with suspend (S3).
>
> It does have something to do with it. In loongson_hotkey_resume() which
> is called when leaving S3 (suspension), the brightness is restored
> according to props.brightness,
>
>         bd = backlight_device_get_by_type(BACKLIGHT_PLATFORM);
>         if (bd) {
>                 loongson_laptop_backlight_update(bd) ?
>                 pr_warn("Loongson_backlight: resume brightness failed") :
>                 pr_info("Loongson_backlight: resume brightness %d\n", bd->props
> .brightness);
>         }
>
> and without this patch, props.brightness is always set to 1 when the
> driver probes, but actually (at least with the firmware on my laptop)
> the screen brightness is set to 80 instead of 1 on cold boot, IOW, a
> brightness value that doesn't match hardware state is set to
> props.brightness.
>
> On resumption, loongson_hotkey_resume() restores the brightness
> settings according to props.brightness. But as the value isn't what is
> used by hardware before suspension. the screen brightness will look very
> different (1 v.s. 80) comparing to the brightness before suspension.
>
> Some dmesg proves this as well, without this patch it says
>
>         loongson_laptop: Loongson_backlight: resume brightness 1
>
> but before suspension, reading
> /sys/class/backlight/loongson3_laptop/actual_brightness yields 80.
OK, that makes sense. But the commit message can still be improved, at
least replace suspension/resumption with suspend/resume. You can grep
them at Documentation/power.

Huacai

>
> > But there is really a problem about hibernation (S4): the brightness
> > is 1 during booting, but when switching to the target kernel, the
> > brightness may jump to the old value.
> >
> > If the above case is what you meet, please update the commit message.
> >
> > Huacai
>
> Thanks,
> Yao Zi
>
> > >
> > > Let's get brightness from EC and take it as the current brightness on
> > > probe of the laptop driver to avoid the surprising behavior. Tested on
> > > TongFang L860-T2 3A5000 laptop.
> > >
> > > Cc: stable@...r.kernel.org
> > > Fixes: 6246ed09111f ("LoongArch: Add ACPI-based generic laptop driver")
> > > Signed-off-by: Yao Zi <ziyao@...root.org>
> > > ---
> > >  drivers/platform/loongarch/loongson-laptop.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/platform/loongarch/loongson-laptop.c b/drivers/platform/loongarch/loongson-laptop.c
> > > index 99203584949d..828bd62e3596 100644
> > > --- a/drivers/platform/loongarch/loongson-laptop.c
> > > +++ b/drivers/platform/loongarch/loongson-laptop.c
> > > @@ -392,7 +392,7 @@ static int laptop_backlight_register(void)
> > >         if (!acpi_evalf(hotkey_handle, &status, "ECLL", "d"))
> > >                 return -EIO;
> > >
> > > -       props.brightness = 1;
> > > +       props.brightness = ec_get_brightness();
> > >         props.max_brightness = status;
> > >         props.type = BACKLIGHT_PLATFORM;
> > >
> > > --
> > > 2.49.0
> > >
> > >