[systemd-devel] [PATCH 2/2] backlight: Avoid restoring brightness to an unreadably dim level

[Lennart Poettering]

On Tue, 11.03.14 21:16, Josh Triplett (josh at joshtriplett.org) wrote:

> Some systems turn the backlight all the way off at the lowest levels.
> Clamp saved brightness to at least 1 or 5% of max_brightness.  This
> avoids preserving an unreadably dim screen, which would otherwise force
> the user to disable state restoration.

Applied, made a minor change though: I downgrade the warning message to
debug, since this isn' really something to warn about..

> ---
> v2: Send the right patch this time.  Factor clamping into a separate
>     function.  Make failures in clamping non-fatal.  Use unsigned rather
>     than unsigned long long.
> 
>  src/backlight/backlight.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
> index 81470b3..44308e0 100644
> --- a/src/backlight/backlight.c
> +++ b/src/backlight/backlight.c
> @@ -192,6 +192,48 @@ static bool validate_device(struct udev *udev, struct udev_device *device) {
>          return true;
>  }
>  
> +/* Some systems turn the backlight all the way off at the lowest levels.
> + * clamp_brightness clamps the saved brightness to at least 1 or 5% of
> + * max_brightness.  This avoids preserving an unreadably dim screen, which
> + * would otherwise force the user to disable state restoration. */
> +static void clamp_brightness(struct udev_device *device, char **value) {
> +        int r;
> +        const char *max_brightness_str;
> +        unsigned brightness, max_brightness, new_brightness;
> +
> +        max_brightness_str = udev_device_get_sysattr_value(device, "max_brightness");
> +        if (!max_brightness_str) {
> +                log_warning("Failed to read max_brightness attribute; not checking saved brightness");
> +                return;
> +        }
> +
> +        r = safe_atou(*value, &brightness);
> +        if (r < 0) {
> +                log_warning("Failed to parse brightness \"%s\": %s", *value, strerror(-r));
> +                return;
> +        }
> +
> +        r = safe_atou(max_brightness_str, &max_brightness);
> +        if (r < 0) {
> +                log_warning("Failed to parse max_brightness \"%s\": %s", max_brightness_str, strerror(-r));
> +                return;
> +        }
> +
> +        new_brightness = MAX3(brightness, 1U, max_brightness/20);
> +        if (new_brightness != brightness) {
> +                char *old_value = *value;
> +
> +                r = asprintf(value, "%u", new_brightness);
> +                if (r < 0) {
> +                        log_warning("Failed to format new brightness: %s", strerror(-r));
> +                        return;
> +                }
> +
> +                log_warning("Saved brightness %s too low; increasing to %s", old_value, *value);
> +                free(old_value);
> +        }
> +}
> +
>  int main(int argc, char *argv[]) {
>          _cleanup_udev_unref_ struct udev *udev = NULL;
>          _cleanup_udev_device_unref_ struct udev_device *device = NULL;
> @@ -306,6 +348,8 @@ int main(int argc, char *argv[]) {
>                          return EXIT_FAILURE;
>                  }
>  
> +                clamp_brightness(device, &value);
> +
>                  r = udev_device_set_sysattr_value(device, "brightness", value);
>                  if (r < 0) {
>                          log_error("Failed to write system attribute: %s", strerror(-r));


Lennart

-- 
Lennart Poettering, Red Hat