Also write to bl_power when setting brightness.

Commit Message

Matthias Hopf Aug. 18, 2009, 5:07 p.m. UTC

Switches backlight on/off on controllers that support this api.
---
 src/drmmode_display.c |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

Comments

Matthew Garrett Aug. 18, 2009, 5:19 p.m. UTC | #1

On Tue, Aug 18, 2009 at 07:07:23PM +0200, Matthias Hopf wrote:
> +    len = snprintf(val, BACKLIGHT_VALUE_LEN, "%d\n",
> +		   level > 0 ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN);

Doesn't this make it impossible to use the minimum value? I don't think 
that's a good tradeoff. The alternative (bumping everything up by one 
and adding an extra 0 level to turn the backlight off) means that 
there'll be two equivalent levels on hardware where the power attribute 
doesn't work. I think you're better off not doing this, and just leaving 
backlight enable/disable up to DPMS.

Matthias Hopf Aug. 19, 2009, 8:58 a.m. UTC | #2

On Aug 18, 09 18:19:56 +0100, Matthew Garrett wrote:
> On Tue, Aug 18, 2009 at 07:07:23PM +0200, Matthias Hopf wrote:
> > +    len = snprintf(val, BACKLIGHT_VALUE_LEN, "%d\n",
> > +		   level > 0 ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN);
> 
> Doesn't this make it impossible to use the minimum value? I don't think 
> that's a good tradeoff. The alternative (bumping everything up by one 
> and adding an extra 0 level to turn the backlight off) means that 
> there'll be two equivalent levels on hardware where the power attribute 
> doesn't work. I think you're better off not doing this, and just leaving 
> backlight enable/disable up to DPMS.

You're probably right. Let's ignore this for now.

Matthias

Patch

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 4fb20d9..f41b978 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -109,6 +109,12 @@  static char *backlight_interfaces[] = {
 /* Enough for 10 digits of backlight + '\n' + '\0' */
 #define BACKLIGHT_VALUE_LEN 12
 
+/* Constants from include/linux/fb.h */
+enum {
+	FB_BLANK_UNBLANK   = 0,
+	FB_BLANK_POWERDOWN = 4
+};
+
 static void
 drmmode_backlight_set(xf86OutputPtr output, int level)
 {
@@ -121,22 +127,37 @@  drmmode_backlight_set(xf86OutputPtr output, int level)
     if (! drmmode_output->backlight_iface || level < 0)
 	return;
 
-    len = snprintf(val, BACKLIGHT_VALUE_LEN, "%d\n", level);
     sprintf(path, "%s/%s/brightness",
 	    BACKLIGHT_CLASS, drmmode_output->backlight_iface);
+    len = snprintf(val, BACKLIGHT_VALUE_LEN, "%d\n", level);
     fd = open(path, O_RDWR);
     if (fd == -1) {
 	xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s for backlight "
 		   "control: %s\n", path, strerror(errno));
 	return;
     }
-
     ret = write(fd, val, len);
     if (ret == -1) {
 	xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "write to %s for backlight "
 		   "control failed: %s\n", path, strerror(errno));
     }
+    close(fd);
 
+    sprintf(path, "%s/%s/bl_power",
+	    BACKLIGHT_CLASS, drmmode_output->backlight_iface);
+    len = snprintf(val, BACKLIGHT_VALUE_LEN, "%d\n",
+		   level > 0 ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN);
+    fd = open(path, O_RDWR);
+    if (fd == -1) {
+	xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s for backlight "
+		   "control: %s\n", path, strerror(errno));
+	return;
+    }
+    ret = write(fd, val, len);
+    if (ret == -1) {
+	xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "write to %s for backlight "
+		   "control failed: %s\n", path, strerror(errno));
+    }
     close(fd);
 }