Merge lp:~fourdollars/unity-settings-daemon/fix-lowest-brightness into lp:unity-settings-daemon/14.04

[Shih-Yuan Lee]

power: On raw backlight types, clamp the value to a minumum

Please help to review this change.
This change has been applied in xenial.
I am backporting it into trusty.

[Iain Lane (laney)]

You said on the bug that this isn't necessary, so closing it

Unmerged revisions

4049. By Shih-Yuan Lee on 2015-11-20

power: On raw backlight types, clamp the value to a minumum

Backlight class brightness 0 is to disable the backlight since kernel 3.18.
This behavior is by design for the backlight control in i915 kernel module.
http://lists.freedesktop.org/archives/intel-gfx/2014-August/050642.html

However gnome-settings-daemon has included some changeset to fix this issue.
https://bugzilla.gnome.org/show_bug.cgi?id=744278

unity-settings-daemon should include the same changeset to fix this issue. (LP: #1381625)

=== modified file 'plugins/power/gpm-common.c'
--- plugins/power/gpm-common.c	2013-12-04 23:55:26 +0000
+++ plugins/power/gpm-common.c	2015-11-20 08:06:29 +0000
@@ -1248,7 +1248,7 @@
 #endif
         if (get_primary_output (rr_screen) != NULL)
                 return TRUE;
-        path = gsd_backlight_helper_get_best_backlight ();
+        path = gsd_backlight_helper_get_best_backlight (NULL);
         if (path == NULL)
                 return FALSE;
 

=== modified file 'plugins/power/gsd-backlight-helper.c'
--- plugins/power/gsd-backlight-helper.c	2013-01-23 22:53:38 +0000
+++ plugins/power/gsd-backlight-helper.c	2015-11-20 08:06:29 +0000
@@ -21,6 +21,7 @@
 
 #include "config.h"
 
+#include <stdlib.h>
 #include <unistd.h>
 #include <glib-object.h>
 #include <locale.h>
@@ -41,13 +42,16 @@
 static gboolean
 gsd_backlight_helper_write (const gchar *filename, gint value, GError **error)
 {
+	gchar *filename_path = NULL;
 	gchar *text = NULL;
 	gint retval;
 	gint length;
 	gint fd = -1;
 	gboolean ret = TRUE;
 
-	fd = open (filename, O_WRONLY);
+	filename_path = g_build_filename (filename, "brightness", NULL);
+
+	fd = open (filename_path, O_WRONLY);
 	if (fd < 0) {
 		ret = FALSE;
 		g_set_error (error, 1, 0, "failed to open filename: %s", filename);
@@ -69,9 +73,64 @@
 	if (fd >= 0)
 		close (fd);
 	g_free (text);
+	g_free (filename_path);
 	return ret;
 }
 
+static gint
+gsd_backlight_helper_read_value (const gchar *filename, GError **error)
+{
+	gchar *contents = NULL;
+	gint value;
+
+	if (g_file_get_contents (filename, &contents, NULL, error))
+		value = atoi (contents);
+	else
+		value = -1;
+	g_free (contents);
+
+	return value;
+}
+
+static gint
+gsd_backlight_helper_get (const gchar *filename, GError **error)
+{
+	gchar *filename_path = NULL;
+	gint value;
+
+	filename_path = g_build_filename (filename, "brightness", NULL);
+	value = gsd_backlight_helper_read_value (filename_path, error);
+	g_free (filename_path);
+	return value;
+}
+
+static gint
+gsd_backlight_helper_get_max (const gchar *filename, GError **error)
+{
+	gchar *filename_path = NULL;
+	gint value;
+
+	filename_path = g_build_filename (filename, "max_brightness", NULL);
+	value = gsd_backlight_helper_read_value (filename_path, error);
+	g_free (filename_path);
+	return value;
+}
+
+static gint
+clamp_minimum (gint max, gint value)
+{
+	gint minimum;
+	/* If the interface has less than 100 possible values, it's
+	 * likely that 0 doesn't turn the backlight off so we let 0 be
+	 * set in that case. */
+	if (max > 99)
+		minimum = 1;
+	else
+		minimum = 0;
+
+	return MAX (value, minimum);
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -80,13 +139,11 @@
 	gint euid;
 	guint retval = 0;
 	GError *error = NULL;
-	gboolean ret = FALSE;
 	gint set_brightness = -1;
 	gboolean get_brightness = FALSE;
 	gboolean get_max_brightness = FALSE;
 	gchar *filename = NULL;
-	gchar *filename_file = NULL;
-	gchar *contents = NULL;
+	GsdBacklightType type;
 
 	const GOptionEntry options[] = {
 		{ "set-brightness", '\0', 0, G_OPTION_ARG_INT, &set_brightness,
@@ -121,7 +178,7 @@
 	}
 
 	/* find device */
-	filename = gsd_backlight_helper_get_best_backlight ();
+	filename = gsd_backlight_helper_get_best_backlight (&type);
 	if (filename == NULL) {
 		retval = GSD_BACKLIGHT_HELPER_EXIT_CODE_NO_DEVICES;
 		g_print ("%s: %s\n",
@@ -132,9 +189,9 @@
 
 	/* GetBrightness */
 	if (get_brightness) {
-		filename_file = g_build_filename (filename, "brightness", NULL);
-		ret = g_file_get_contents (filename_file, &contents, NULL, &error);
-		if (!ret) {
+		gint value;
+		value = gsd_backlight_helper_get (filename, &error);
+		if (value < 0) {
 			g_print ("%s: %s\n",
 				 "Could not get the value of the backlight",
 				 error->message);
@@ -144,16 +201,16 @@
 		}
 
 		/* just print the contents to stdout */
-		g_print ("%s", contents);
+		g_print ("%d", value);
 		retval = GSD_BACKLIGHT_HELPER_EXIT_CODE_SUCCESS;
 		goto out;
 	}
 
 	/* GetSteps */
 	if (get_max_brightness) {
-		filename_file = g_build_filename (filename, "max_brightness", NULL);
-		ret = g_file_get_contents (filename_file, &contents, NULL, &error);
-		if (!ret) {
+		gint value;
+		value = gsd_backlight_helper_get_max (filename, &error);
+		if (value < 0) {
 			g_print ("%s: %s\n",
 				 "Could not get the maximum value of the backlight",
 				 error->message);
@@ -163,7 +220,7 @@
 		}
 
 		/* just print the contents to stdout */
-		g_print ("%s", contents);
+		g_print ("%d", value);
 		retval = GSD_BACKLIGHT_HELPER_EXIT_CODE_SUCCESS;
 		goto out;
 	}
@@ -180,8 +237,22 @@
 
 	/* SetBrightness */
 	if (set_brightness != -1) {
-		filename_file = g_build_filename (filename, "brightness", NULL);
-		ret = gsd_backlight_helper_write (filename_file, set_brightness, &error);
+		gboolean ret = FALSE;
+		gint max = gsd_backlight_helper_get_max (filename, &error);
+
+		if (max < 0) {
+			g_print ("%s: %s\n",
+				 "Could not get the maximum value of the backlight",
+				 error->message);
+			g_error_free (error);
+			retval = GSD_BACKLIGHT_HELPER_EXIT_CODE_ARGUMENTS_INVALID;
+			goto out;
+		}
+
+		if (type == GSD_BACKLIGHT_TYPE_RAW)
+			set_brightness = clamp_minimum (max, set_brightness);
+
+		ret = gsd_backlight_helper_write (filename, set_brightness, &error);
 		if (!ret) {
 			g_print ("%s: %s\n",
 				 "Could not set the value of the backlight",
@@ -196,8 +267,6 @@
 	retval = GSD_BACKLIGHT_HELPER_EXIT_CODE_SUCCESS;
 out:
 	g_free (filename);
-	g_free (filename_file);
-	g_free (contents);
 	return retval;
 }
 

=== modified file 'plugins/power/gsd-backlight-linux.c'
--- plugins/power/gsd-backlight-linux.c	2013-03-27 11:59:18 +0000
+++ plugins/power/gsd-backlight-linux.c	2015-11-20 08:06:29 +0000
@@ -44,7 +44,7 @@
 #endif /* HAVE_GUDEV */
 
 char *
-gsd_backlight_helper_get_best_backlight (void)
+gsd_backlight_helper_get_best_backlight (GsdBacklightType *type)
 {
 #ifdef HAVE_GUDEV
 	gchar *path = NULL;
@@ -59,14 +59,23 @@
 	/* search the backlight devices and prefer the types:
 	 * firmware -> platform -> raw */
 	path = gsd_backlight_helper_get_type (devices, "firmware");
-	if (path != NULL)
+	if (path != NULL) {
+		if (type)
+			*type = GSD_BACKLIGHT_TYPE_FIRMWARE;
 		goto out;
+	}
 	path = gsd_backlight_helper_get_type (devices, "platform");
-	if (path != NULL)
+	if (path != NULL) {
+		if (type)
+			*type = GSD_BACKLIGHT_TYPE_PLATFORM;
 		goto out;
+	}
 	path = gsd_backlight_helper_get_type (devices, "raw");
-	if (path != NULL)
+	if (path != NULL) {
+		if (type)
+			*type = GSD_BACKLIGHT_TYPE_RAW;
 		goto out;
+	}
 out:
 	g_object_unref (client);
 	g_list_foreach (devices, (GFunc) g_object_unref, NULL);

=== modified file 'plugins/power/gsd-backlight-linux.h'
--- plugins/power/gsd-backlight-linux.h	2013-01-23 22:53:38 +0000
+++ plugins/power/gsd-backlight-linux.h	2015-11-20 08:06:29 +0000
@@ -19,4 +19,10 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-char *gsd_backlight_helper_get_best_backlight (void);
+typedef enum {
+	GSD_BACKLIGHT_TYPE_FIRMWARE,
+	GSD_BACKLIGHT_TYPE_PLATFORM,
+	GSD_BACKLIGHT_TYPE_RAW,
+} GsdBacklightType;
+
+char *gsd_backlight_helper_get_best_backlight (GsdBacklightType *type);