Re: [systemd-devel] [PATCH] backlight: let udev properties override clamping

2015-02-02 Thread Lennart Poettering 
On Sat, 31.01.15 09:30, Topi Miettinen (toiwo...@gmail.com) wrote:

 On 01/29/15 00:09, Lennart Poettering wrote:
  On Wed, 28.01.15 23:51, Topi Miettinen (toiwo...@gmail.com) wrote:
  
  diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
  index 1271a66..df53b75 100644
  --- a/src/backlight/backlight.c
  +++ b/src/backlight/backlight.c
  @@ -373,6 +373,7 @@ int main(int argc, char *argv[]) {
   
   if (streq(argv[1], load)) {
   _cleanup_free_ char *value = NULL;
  +const char *clamp;
   
   if (!shall_restore_state())
   return EXIT_SUCCESS;
  @@ -390,7 +391,9 @@ int main(int argc, char *argv[]) {
   return EXIT_FAILURE;
   }
   
  -clamp_brightness(device, value, max_brightness);
  +clamp = udev_device_get_property_value(device, 
  ID_BACKLIGHT_CLAMP);
  +if (clamp == NULL || streq(clamp, 1))
  
  Please use parse_boolean() for this.
 
 OK.
 
  
  I think it would be better to name this ID_BACKLIGHT_CLAMP_MIN or so.
 
 _MIN as in minimum? The brightness is also clamped to maximum brightness
 advertised by the device.

Ah, I was confused for a moment, got lost between making this a prop
that ovverides the clamping parameter, and making this just a boolean
prop for turning this off and on... YOu made it the later, which is
fine, and hence disregard my comments about _MIN...

Your newer patch is merged now. THanks!

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] backlight: let udev properties override clamping

2015-02-02 Thread Lennart Poettering 
On Sun, 01.02.15 08:49, Topi Miettinen (toiwo...@gmail.com) wrote:

 On my computer, the minimum brightness enforced by clamping in
 backlight is too bright.
 
 Let udev property ID_BACKLIGHT_CLAMP control whether the brightness
 is clamped or not.

Applied! Thanks!

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] backlight: let udev properties override clamping

2015-01-31 Thread Topi Miettinen 
On 01/29/15 00:09, Lennart Poettering wrote:
 On Wed, 28.01.15 23:51, Topi Miettinen (toiwo...@gmail.com) wrote:
 
 diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
 index 1271a66..df53b75 100644
 --- a/src/backlight/backlight.c
 +++ b/src/backlight/backlight.c
 @@ -373,6 +373,7 @@ int main(int argc, char *argv[]) {
  
  if (streq(argv[1], load)) {
  _cleanup_free_ char *value = NULL;
 +const char *clamp;
  
  if (!shall_restore_state())
  return EXIT_SUCCESS;
 @@ -390,7 +391,9 @@ int main(int argc, char *argv[]) {
  return EXIT_FAILURE;
  }
  
 -clamp_brightness(device, value, max_brightness);
 +clamp = udev_device_get_property_value(device, 
 ID_BACKLIGHT_CLAMP);
 +if (clamp == NULL || streq(clamp, 1))
 
 Please use parse_boolean() for this.

OK.

 
 I think it would be better to name this ID_BACKLIGHT_CLAMP_MIN or so.

_MIN as in minimum? The brightness is also clamped to maximum brightness
advertised by the device.

-Topi

 
 Otherwise looks fine to me.
 
 Lennart
 

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] backlight: let udev properties override clamping

2015-01-31 Thread Topi Miettinen 
On my computer, the minimum brightness enforced by clamping in
backlight is too bright.

Let udev property ID_BACKLIGHT_CLAMP control whether the brightness
is clamped or not.
---
 man/systemd-backli...@.service.xml | 9 -
 src/backlight/backlight.c  | 5 -
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/man/systemd-backli...@.service.xml 
b/man/systemd-backli...@.service.xml
index 453afbf..21c6437 100644
--- a/man/systemd-backli...@.service.xml
+++ b/man/systemd-backli...@.service.xml
@@ -58,7 +58,14 @@
 is a service that restores the display backlight
 brightness at early boot and saves it at shutdown. On
 disk, the backlight brightness is stored in
-filename/var/lib/systemd/backlight//filename./para
+filename/var/lib/systemd/backlight//filename. During
+loading, if udev property
+optionID_BACKLIGHT_CLAMP/option is not set to
+false value, the brightness is clamped to a value of
+at least 1 or 5% of maximum brightness, whichever is
+greater. This restriction will be removed when the
+kernel allows user space to reliably set a brightness
+value which does not turn off the display./para
 /refsect1
 
 refsect1
diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
index 1271a66..2d357db 100644
--- a/src/backlight/backlight.c
+++ b/src/backlight/backlight.c
@@ -373,6 +373,7 @@ int main(int argc, char *argv[]) {
 
 if (streq(argv[1], load)) {
 _cleanup_free_ char *value = NULL;
+const char *clamp;
 
 if (!shall_restore_state())
 return EXIT_SUCCESS;
@@ -390,7 +391,9 @@ int main(int argc, char *argv[]) {
 return EXIT_FAILURE;
 }
 
-clamp_brightness(device, value, max_brightness);
+clamp = udev_device_get_property_value(device, 
ID_BACKLIGHT_CLAMP);
+if (clamp == NULL || parse_boolean(clamp))
+clamp_brightness(device, value, max_brightness);
 
 r = udev_device_set_sysattr_value(device, brightness, value);
 if (r  0) {
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] backlight: let udev properties override clamping

2015-01-31 Thread Zbigniew Jędrzejewski-Szmek 
On Wed, Jan 28, 2015 at 11:51:47PM +0200, Topi Miettinen wrote:
 On my computer, the minimum brightness enforced by clamping in
 backlight is too bright.
 
 Let udev property ID_BACKLIGHT_CLAMP control whether the brightness
 is clamped or not.
 ---
  man/systemd-backli...@.service.xml | 11 ++-
  src/backlight/backlight.c  |  5 -
  2 files changed, 14 insertions(+), 2 deletions(-)
 
 diff --git a/man/systemd-backli...@.service.xml 
 b/man/systemd-backli...@.service.xml
 index 453afbf..4105685 100644
 --- a/man/systemd-backli...@.service.xml
 +++ b/man/systemd-backli...@.service.xml
 @@ -58,7 +58,16 @@
  is a service that restores the display backlight
  brightness at early boot and saves it at shutdown. On
  disk, the backlight brightness is stored in
 -filename/var/lib/systemd/backlight//filename./para
 +filename/var/lib/systemd/backlight//filename. During
 +loading, if udev property ID_BACKLIGHT_CLAMP is not
 +present or is set to literal1/literal, the
 +brightness is clamped to at least value
 +literal1/literal or 5% of maximum brightness. This
 +is to avoid an unpowered display due to poor API
 +design in kernel (unfixed as of 2015-01-28) that does
 +not allow user space to know the difference between
 +lowest brightness and powering off the
 +backlight./para
Since you're going to submit a new version anyway, please reword this bit.
Maybe something like this:

  During loading, if udev property optionID_BACKLIGHT_CLAMPoption
  is not set to a false value, brightness is clamped to a value of at
  least 1 or 5% of the maximum brightness, whichever is greater. This
  restriction will be removed when the kernel allows user space to
  reliably set a brightness value which does not turn off the display.

Zbyszek

  /refsect1
  
  refsect1
 diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
 index 1271a66..df53b75 100644
 --- a/src/backlight/backlight.c
 +++ b/src/backlight/backlight.c
 @@ -373,6 +373,7 @@ int main(int argc, char *argv[]) {
  
  if (streq(argv[1], load)) {
  _cleanup_free_ char *value = NULL;
 +const char *clamp;
  
  if (!shall_restore_state())
  return EXIT_SUCCESS;
 @@ -390,7 +391,9 @@ int main(int argc, char *argv[]) {
  return EXIT_FAILURE;
  }
  
 -clamp_brightness(device, value, max_brightness);
 +clamp = udev_device_get_property_value(device, 
 ID_BACKLIGHT_CLAMP);
 +if (clamp == NULL || streq(clamp, 1))
 +clamp_brightness(device, value, max_brightness);
  
  r = udev_device_set_sysattr_value(device, brightness, 
 value);
  if (r  0) {
 -- 
 2.1.4
 
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] backlight: let udev properties override clamping

2015-01-28 Thread Topi Miettinen 
On my computer, the minimum brightness enforced by clamping in
backlight is too bright.

Let udev property ID_BACKLIGHT_CLAMP control whether the brightness
is clamped or not.
---
 man/systemd-backli...@.service.xml | 11 ++-
 src/backlight/backlight.c  |  5 -
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/man/systemd-backli...@.service.xml 
b/man/systemd-backli...@.service.xml
index 453afbf..4105685 100644
--- a/man/systemd-backli...@.service.xml
+++ b/man/systemd-backli...@.service.xml
@@ -58,7 +58,16 @@
 is a service that restores the display backlight
 brightness at early boot and saves it at shutdown. On
 disk, the backlight brightness is stored in
-filename/var/lib/systemd/backlight//filename./para
+filename/var/lib/systemd/backlight//filename. During
+loading, if udev property ID_BACKLIGHT_CLAMP is not
+present or is set to literal1/literal, the
+brightness is clamped to at least value
+literal1/literal or 5% of maximum brightness. This
+is to avoid an unpowered display due to poor API
+design in kernel (unfixed as of 2015-01-28) that does
+not allow user space to know the difference between
+lowest brightness and powering off the
+backlight./para
 /refsect1
 
 refsect1
diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
index 1271a66..df53b75 100644
--- a/src/backlight/backlight.c
+++ b/src/backlight/backlight.c
@@ -373,6 +373,7 @@ int main(int argc, char *argv[]) {
 
 if (streq(argv[1], load)) {
 _cleanup_free_ char *value = NULL;
+const char *clamp;
 
 if (!shall_restore_state())
 return EXIT_SUCCESS;
@@ -390,7 +391,9 @@ int main(int argc, char *argv[]) {
 return EXIT_FAILURE;
 }
 
-clamp_brightness(device, value, max_brightness);
+clamp = udev_device_get_property_value(device, 
ID_BACKLIGHT_CLAMP);
+if (clamp == NULL || streq(clamp, 1))
+clamp_brightness(device, value, max_brightness);
 
 r = udev_device_set_sysattr_value(device, brightness, value);
 if (r  0) {
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] backlight: let udev properties override clamping

2015-01-28 Thread Lennart Poettering 
On Wed, 28.01.15 23:51, Topi Miettinen (toiwo...@gmail.com) wrote:

 diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
 index 1271a66..df53b75 100644
 --- a/src/backlight/backlight.c
 +++ b/src/backlight/backlight.c
 @@ -373,6 +373,7 @@ int main(int argc, char *argv[]) {
  
  if (streq(argv[1], load)) {
  _cleanup_free_ char *value = NULL;
 +const char *clamp;
  
  if (!shall_restore_state())
  return EXIT_SUCCESS;
 @@ -390,7 +391,9 @@ int main(int argc, char *argv[]) {
  return EXIT_FAILURE;
  }
  
 -clamp_brightness(device, value, max_brightness);
 +clamp = udev_device_get_property_value(device, 
 ID_BACKLIGHT_CLAMP);
 +if (clamp == NULL || streq(clamp, 1))

Please use parse_boolean() for this.

I think it would be better to name this ID_BACKLIGHT_CLAMP_MIN or so.

Otherwise looks fine to me.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel