Hi 2023. július 12., szerda 20:44 keltezéssel, Hans de Goede <hdegoede@xxxxxxxxxx> írta: > Hi, > > On 7/12/23 03:07, Barnabás Pőcze wrote: > > Hi > > > > > > 2023. július 11., kedd 11:42 keltezéssel, Hans de Goede <hdegoede@xxxxxxxxxx> írta: > > > >> [...] > >>>> > >>>> If settings below 60 are no good, then the correct way to handle > >>>> this is to limit the range to 0 - (255-60) and add / substract > >>>> 60 when setting / getting the brightness. > >>>> > >>>> E.g. do something like this: > >>>> > >>>> #define SCREENPAD_MIN_BRIGHTNESS 60 > >>>> #define SCREENPAD_MAX_BRIGHTNESS 255 > >>>> > >>>> props.max_brightness = SCREENPAD_MAX_BRIGHTNESS - > >>>> SCREENPAD_MIN_BRIGHTNESS; > >>>> > >>>> And in update_screenpad_bl_status() do: > >>>> > >>>> ctrl_param = bd->props.brightness + SCREENPAD_MIN_BRIGHTNESS; > >>>> > >>>> And when reading the brightness substract SCREENPAD_MIN_BRIGHTNESS, > >>>> clamping to a minimum value of 0. > >>>> > >>>> This avoids a dead-zone in the brightness range between 0-60 . > >>> > >>> Hi Hans, I think this is the wrong thing to do. > >>> > >>> The initial point of that first `brightness = 60;` is only to set it to > >>> an acceptable brightness on boot. We don't want to prevent the user > >>> from going below that brightness at all - this is just to ensure the > >>> screen is visible on boot if the brightness was under that value, and > >>> it is usually only under that value if it was set to off before > >>> shutdown/reboot. > >>> > >>> It's not to try and put the range between 60-255, it's just to make the > >>> screen visible on boot if it was off previously. The folks who have > >>> tested this have found that this is the desired behaviour they expect. > >> > >> I see. > >> > >> So if I understand things correctly then 60 is a good default, > >> but the screen can go darker and still be usable. > >> > >> But 1 leads to an unusable screen, so we still need > >> a SCREENPAD_MIN_BRIGHTNESS to avoid the screen being able > >> to go so dark that it is no longer usable and then still > >> move the range a bit, but just not by 60, but by some > >> other number (something in the 10-30 range I guess?) > >> > >> Combined with adding a: > >> > >> #define SCREENPAD_DEFAULT_BRIGHTNESS 60 > >> > >> And at boot when the read back brightness < > >> SCREENPAD_MIN_BRIGHTNESS set it to SCREENPAD_DEFAULT_BRIGHTNESS. > >> > >> We really want to avoid users to be able to set an unusable > >> low brightness level. As mentioned in the new panel brightness > >> API proposal: > >> > >> https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86d2e@xxxxxxxxxx/ > >> > >> "3. The meaning of 0 is not clearly defined, it can be either off, > >> or minimum brightness at which the display is still readable > >> (in a low light environment)" > >> > >> and the plan going forward is to: > >> > >> "Unlike the /sys/class/backlight/foo/brightness this brightness property > >> has a clear definition for the value 0. The kernel must ensure that 0 > >> means minimum brightness (so 0 should _never_ turn the backlight off). > >> If necessary the kernel must enforce a minimum value by adding > >> an offset to the value seen in the property to ensure this behavior." > >> > >> So I really want to see this new backlight driver implement the > >> new planned behavior for 0 from the start, with 0 meaning minimum > >> *usable* brightness. > >> > >> Regards, > >> > >> Hans > > > > > > Sorry for hijacking this thread, but then how can I turn backlight off? > > Documentation/ABI/stable/sysfs-class-backlight > > What: /sys/class/backlight/<backlight>/bl_power > Date: April 2005 > KernelVersion: 2.6.12 > Contact: Richard Purdie <rpurdie@xxxxxxxxx> > Description: > Control BACKLIGHT power, values are FB_BLANK_* from fb.h > > - FB_BLANK_UNBLANK (0) : power on. > - FB_BLANK_POWERDOWN (4) : power off > > Although it is better to actually disable video output on the connector, > this leads to much bigger power savings. Under X, this can typically be > done by hitting the lock-screen option, e.g. "Windows-logo-key + L" under > GNOME. Super+L locks the screen for me on GNOME, which is decidedly *not* what I want to achieve. I just want to disable the backlight on the laptop panel without any other changes whatsoever. Writing 4 to /sys/class/backlight/<backlight>/bl_power does essentially what I want. > > On the console you can do: > > echo 1 > /sys/class/graphics/fb0/blank > > To really put the panel in low power mode. I never doubted it could still be done. I guess what I wanted to say is "convenience". Using the brightness slider / hotkeys is very convenient, much more convenient than messing with getting the right permissions to be able to write to /sys/class/backlight/<backlight>/bl_power or /sys/class/graphics/fb0/blank. > > > > > I quite liked how I was able to turn my laptop display (almost) completely off > > with the brightness hotkeys / brightness slider in gnome-shell, and I was quite > > annoyed when this was changed in gnome-settings-daemon and forced the minimum > > brightness to be 1% of max_brightness. > > Right, there are 2 problems with this: > > 1. Using brightness control to disable the backlight is not reliabl. Many > backlight control methods only go to some low setting not to completely off. > This differs from model laptop to model laptop. Also e.g. amdgpu and radeonhd > have always ensured that brightness never completely turns of the backlight. > > The plan going forward is to try and consistently have 0 mean minimum > backlight and not backlight off, instead of the current some models 0 = off, > some models 0 is works fine in a not too brightly lit room. That is good to know, I did not realize that was the case thanks to my intel_backlight bias... > > 2. Users sometimes turn of the backlight through e.g. the GNOME system menu > slider and then have no way to turn it back on (on devices without (working) > brightness hotkeys (they cannot use the slider since they can no longer see it) > This scenario is a real problem which used to result in quite a few bug reports. Of course I understand why that change was made in e.g. gnome-settings-daemon. However, the same way gnome-tweaks has a switch for enabling volume levels greater than 100%, there could be a switch for enabling the full brightness scale. > > > Also, "minimum brightness at which the display is still readable" is not really > > well-defined > > True, as mentioned above the minimum might only be good enough to > e.g. read text in a somewhat low lit room, but typically it at > least leaves things visible enough to allow a user to change > the brightness to a better setting. > > What we don't want is brightness settings so low that the backlight is > essentially off (does not even show any light in a fully dark room). > > > so it can (will) happen that the minimum brightness values don't match, > > so it is theoretically possible that I cannot set both my laptop panel and external > > monitor to the same desired brightness level. Or am I missing something? > > This already is the case, some monitors may not go as low (or high) as you want, > some laptops also already don't go as low as you want. Fair point, but restricting the range decreases the chances even more. > > Expecting to be able to match monitor and laptop panel brightness at both > ends of the brightness range simply is not realistic. > [...] In conclusion, I appreciate your answer, there were some things I did not know. I suppose, after all, it might indeed not be a wise idea to shoehorn turning off backlight into brightness control. Regards, Barnabás Pőcze
