thinkpad-acpi: rework brightness support

Refactor and redesign the brightness control backend...

In order to fix bugzilla #11750...

Add a new brightness control mode: support direct NVRAM checkpointing
of the backlight level (i.e. store directly to NVRAM without the need
for UCMS calls), and use that together with the EC-based control.
Disallow UCMS+EC, thus avoiding races with the SMM firmware.

Switch the models that define HBRV (EC Brightness Value) in the DSDT
to the new mode.  These are: T40-T43, R50-R52, R50e, R51e, X31-X41.

Change the default for all other IBM ThinkPads to UCMS-only.  The
Lenovo models already default to UCMS-only.

Reported-by: Alexey Fisher <bug-track@fisher-privat.net>
Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Signed-off-by: Len Brown <len.brown@intel.com>

1 files changed, 218 insertions, 99 deletions

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 4eec77032a71..ba3682c5cde0 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -192,6 +192,7 @@ enum {
 #define TPACPI_DBG_RFKILL       0x0004
 #define TPACPI_DBG_HKEY         0x0008
 #define TPACPI_DBG_FAN          0x0010
+#define TPACPI_DBG_BRGHT        0x0020
 
 #define onoff(status, bit) ((status) & (1 << (bit)) ? "on" : "off")
 #define enabled(status, bit) ((status) & (1 << (bit)) ? "enabled" : "disabled")
@@ -274,7 +275,6 @@ static struct {
 
 static struct {
         u16 hotkey_mask_ff:1;
-        u16 bright_cmos_ec_unsync:1;
 } tp_warned;
 
 struct thinkpad_id_data {
@@ -5526,6 +5526,20 @@ static struct ibm_struct ecdump_driver_data = {
 
 #define TPACPI_BACKLIGHT_DEV_NAME "thinkpad_screen"
 
+/*
+ * ThinkPads can read brightness from two places: EC HBRV (0x31), or
+ * CMOS NVRAM byte 0x5E, bits 0-3.
+ *
+ * EC HBRV (0x31) has the following layout
+ *   Bit 7: unknown function
+ *   Bit 6: unknown function
+ *   Bit 5: Z: honour scale changes, NZ: ignore scale changes
+ *   Bit 4: must be set to zero to avoid problems
+ *   Bit 3-0: backlight brightness level
+ *
+ * brightness_get_raw returns status data in the HBRV layout
+ */
+
 enum {
         TP_EC_BACKLIGHT = 0x31,
 
@@ -5535,108 +5549,164 @@ enum {
         TP_EC_BACKLIGHT_MAPSW = 0x20,
 };
 
+enum tpacpi_brightness_access_mode {
+        TPACPI_BRGHT_MODE_AUTO = 0,     /* Not implemented yet */
+        TPACPI_BRGHT_MODE_EC,           /* EC control */
+        TPACPI_BRGHT_MODE_UCMS_STEP,    /* UCMS step-based control */
+        TPACPI_BRGHT_MODE_ECNVRAM,      /* EC control w/ NVRAM store */
+        TPACPI_BRGHT_MODE_MAX
+};
+
 static struct backlight_device *ibm_backlight_device;
-static int brightness_mode;
+
+static enum tpacpi_brightness_access_mode brightness_mode =
+                TPACPI_BRGHT_MODE_MAX;
+
 static unsigned int brightness_enable = 2; /* 2 = auto, 0 = no, 1 = yes */
 
 static struct mutex brightness_mutex;
 
-/*
- * ThinkPads can read brightness from two places: EC 0x31, or
- * CMOS NVRAM byte 0x5E, bits 0-3.
- *
- * EC 0x31 has the following layout
- *   Bit 7: unknown function
- *   Bit 6: unknown function
- *   Bit 5: Z: honour scale changes, NZ: ignore scale changes
- *   Bit 4: must be set to zero to avoid problems
- *   Bit 3-0: backlight brightness level
- *
- * brightness_get_raw returns status data in the EC 0x31 layout
- */
-static int brightness_get_raw(int *status)
+/* NVRAM brightness access,
+ * call with brightness_mutex held! */
+static unsigned int tpacpi_brightness_nvram_get(void)
 {
-        u8 lec = 0, lcmos = 0, level = 0;
+        u8 lnvram;
 
-        if (brightness_mode & 1) {
-                if (!acpi_ec_read(TP_EC_BACKLIGHT, &lec))
-                        return -EIO;
-                level = lec & TP_EC_BACKLIGHT_LVLMSK;
-        };
-        if (brightness_mode & 2) {
-                lcmos = (nvram_read_byte(TP_NVRAM_ADDR_BRIGHTNESS)
-                         & TP_NVRAM_MASK_LEVEL_BRIGHTNESS)
-                        >> TP_NVRAM_POS_LEVEL_BRIGHTNESS;
-                lcmos &= (tp_features.bright_16levels)? 0x0f : 0x07;
-                level = lcmos;
-        }
-
-        if (brightness_mode == 3) {
-                *status = lec;  /* Prefer EC, CMOS is just a backing store */
-                lec &= TP_EC_BACKLIGHT_LVLMSK;
-                if (lec == lcmos)
-                        tp_warned.bright_cmos_ec_unsync = 0;
-                else {
-                        if (!tp_warned.bright_cmos_ec_unsync) {
-                                printk(TPACPI_ERR
-                                        "CMOS NVRAM (%u) and EC (%u) do not "
-                                        "agree on display brightness level\n",
-                                        (unsigned int) lcmos,
-                                        (unsigned int) lec);
-                                tp_warned.bright_cmos_ec_unsync = 1;
-                        }
+        lnvram = (nvram_read_byte(TP_NVRAM_ADDR_BRIGHTNESS)
+                  & TP_NVRAM_MASK_LEVEL_BRIGHTNESS)
+                  >> TP_NVRAM_POS_LEVEL_BRIGHTNESS;
+        lnvram &= (tp_features.bright_16levels) ? 0x0f : 0x07;
+
+        return lnvram;
+}
+
+static void tpacpi_brightness_checkpoint_nvram(void)
+{
+        u8 lec = 0;
+        u8 b_nvram;
+
+        if (brightness_mode != TPACPI_BRGHT_MODE_ECNVRAM)
+                return;
+
+        vdbg_printk(TPACPI_DBG_BRGHT,
+                "trying to checkpoint backlight level to NVRAM...\n");
+
+        if (mutex_lock_killable(&brightness_mutex) < 0)
+                return;
+
+        if (unlikely(!acpi_ec_read(TP_EC_BACKLIGHT, &lec)))
+                goto unlock;
+        lec &= TP_EC_BACKLIGHT_LVLMSK;
+        b_nvram = nvram_read_byte(TP_NVRAM_ADDR_BRIGHTNESS);
+
+        if (lec != ((b_nvram & TP_NVRAM_MASK_LEVEL_BRIGHTNESS)
+                             >> TP_NVRAM_POS_LEVEL_BRIGHTNESS)) {
+                /* NVRAM needs update */
+                b_nvram &= ~(TP_NVRAM_MASK_LEVEL_BRIGHTNESS <<
+                                TP_NVRAM_POS_LEVEL_BRIGHTNESS);
+                b_nvram |= lec;
+                nvram_write_byte(b_nvram, TP_NVRAM_ADDR_BRIGHTNESS);
+                dbg_printk(TPACPI_DBG_BRGHT,
+                           "updated NVRAM backlight level to %u (0x%02x)\n",
+                           (unsigned int) lec, (unsigned int) b_nvram);
+        } else
+                vdbg_printk(TPACPI_DBG_BRGHT,
+                           "NVRAM backlight level already is %u (0x%02x)\n",
+                           (unsigned int) lec, (unsigned int) b_nvram);
+
+unlock:
+        mutex_unlock(&brightness_mutex);
+}
+
+
+/* call with brightness_mutex held! */
+static int tpacpi_brightness_get_raw(int *status)
+{
+        u8 lec = 0;
+
+        switch (brightness_mode) {
+        case TPACPI_BRGHT_MODE_UCMS_STEP:
+                *status = tpacpi_brightness_nvram_get();
+                return 0;
+        case TPACPI_BRGHT_MODE_EC:
+        case TPACPI_BRGHT_MODE_ECNVRAM:
+                if (unlikely(!acpi_ec_read(TP_EC_BACKLIGHT, &lec)))
                         return -EIO;
-                }
-        } else {
-                *status = level;
+                *status = lec;
+                return 0;
+        default:
+                return -ENXIO;
         }
+}
+
+/* call with brightness_mutex held! */
+/* do NOT call with illegal backlight level value */
+static int tpacpi_brightness_set_ec(unsigned int value)
+{
+        u8 lec = 0;
+
+        if (unlikely(!acpi_ec_read(TP_EC_BACKLIGHT, &lec)))
+                return -EIO;
+
+        if (unlikely(!acpi_ec_write(TP_EC_BACKLIGHT,
+                                (lec & TP_EC_BACKLIGHT_CMDMSK) |
+                                (value & TP_EC_BACKLIGHT_LVLMSK))))
+                return -EIO;
+
+        return 0;
+}
+
+/* call with brightness_mutex held! */
+static int tpacpi_brightness_set_ucmsstep(unsigned int value)
+{
+        int cmos_cmd, inc;
+        unsigned int current_value, i;
+
+        current_value = tpacpi_brightness_nvram_get();
+
+        if (value == current_value)
+                return 0;
+
+        cmos_cmd = (value > current_value) ?
+                        TP_CMOS_BRIGHTNESS_UP :
+                        TP_CMOS_BRIGHTNESS_DOWN;
+        inc = (value > current_value) ? 1 : -1;
+
+        for (i = current_value; i != value; i += inc)
+                if (issue_thinkpad_cmos_command(cmos_cmd))
+                        return -EIO;
 
         return 0;
 }
 
 /* May return EINTR which can always be mapped to ERESTARTSYS */
-static int brightness_set(int value)
+static int brightness_set(unsigned int value)
 {
-        int cmos_cmd, inc, i, res;
-        int current_value;
-        int command_bits;
+        int res;
 
         if (value > ((tp_features.bright_16levels)? 15 : 7) ||
             value < 0)
                 return -EINVAL;
 
+        vdbg_printk(TPACPI_DBG_BRGHT,
+                        "set backlight level to %d\n", value);
+
         res = mutex_lock_killable(&brightness_mutex);
         if (res < 0)
                 return res;
 
-        res = brightness_get_raw(&current_value);
-        if (res < 0)
-                goto errout;
-
-        command_bits = current_value & TP_EC_BACKLIGHT_CMDMSK;
-        current_value &= TP_EC_BACKLIGHT_LVLMSK;
-
-        cmos_cmd = value > current_value ?
-                        TP_CMOS_BRIGHTNESS_UP :
-                        TP_CMOS_BRIGHTNESS_DOWN;
-        inc = (value > current_value)? 1 : -1;
-
-        res = 0;
-        for (i = current_value; i != value; i += inc) {
-                if ((brightness_mode & 2) &&
-                    issue_thinkpad_cmos_command(cmos_cmd)) {
-                        res = -EIO;
-                        goto errout;
-                }
-                if ((brightness_mode & 1) &&
-                    !acpi_ec_write(TP_EC_BACKLIGHT,
-                                   (i + inc) | command_bits)) {
-                        res = -EIO;
-                        goto errout;;
-                }
+        switch (brightness_mode) {
+        case TPACPI_BRGHT_MODE_EC:
+        case TPACPI_BRGHT_MODE_ECNVRAM:
+                res = tpacpi_brightness_set_ec(value);
+                break;
+        case TPACPI_BRGHT_MODE_UCMS_STEP:
+                res = tpacpi_brightness_set_ucmsstep(value);
+                break;
+        default:
+                res = -ENXIO;
         }
 
-errout:
         mutex_unlock(&brightness_mutex);
         return res;
 }
@@ -5645,21 +5715,34 @@ errout:
 
 static int brightness_update_status(struct backlight_device *bd)
 {
-        /* it is the backlight class's job (caller) to handle
-         * EINTR and other errors properly */
-        return brightness_set(
+        unsigned int level =
                 (bd->props.fb_blank == FB_BLANK_UNBLANK &&
                  bd->props.power == FB_BLANK_UNBLANK) ?
-                                bd->props.brightness : 0);
+                                bd->props.brightness : 0;
+
+        dbg_printk(TPACPI_DBG_BRGHT,
+                        "backlight: attempt to set level to %d\n",
+                        level);
+
+        /* it is the backlight class's job (caller) to handle
+         * EINTR and other errors properly */
+        return brightness_set(level);
 }
 
 static int brightness_get(struct backlight_device *bd)
 {
         int status, res;
 
-        res = brightness_get_raw(&status);
+        res = mutex_lock_killable(&brightness_mutex);
         if (res < 0)
-                return 0; /* FIXME: teach backlight about error handling */
+                return 0;
+
+        res = tpacpi_brightness_get_raw(&status);
+
+        mutex_unlock(&brightness_mutex);
+
+        if (res < 0)
+                return 0;
 
         return status & TP_EC_BACKLIGHT_LVLMSK;
 }
@@ -5709,7 +5792,7 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
         }
 
         if (!brightness_enable) {
-                dbg_printk(TPACPI_DBG_INIT,
+                dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
                            "brightness support disabled by "
                            "module parameter\n");
                 return 1;
@@ -5724,20 +5807,38 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
         if (b == 16)
                 tp_features.bright_16levels = 1;
 
-        if (!brightness_mode) {
-                if (thinkpad_id.vendor == PCI_VENDOR_ID_LENOVO)
-                        brightness_mode = 2;
-                else
-                        brightness_mode = 3;
+        /*
+         * Check for module parameter bogosity, note that we
+         * init brightness_mode to TPACPI_BRGHT_MODE_MAX in order to be
+         * able to detect "unspecified"
+         */
+        if (brightness_mode > TPACPI_BRGHT_MODE_MAX)
+                return -EINVAL;
 
-                dbg_printk(TPACPI_DBG_INIT, "selected brightness_mode=%d\n",
-                        brightness_mode);
-        }
+        /* TPACPI_BRGHT_MODE_AUTO not implemented yet, just use default */
+        if (brightness_mode == TPACPI_BRGHT_MODE_AUTO ||
+            brightness_mode == TPACPI_BRGHT_MODE_MAX) {
+                if (thinkpad_id.vendor == PCI_VENDOR_ID_IBM) {
+                        /*
+                         * IBM models that define HBRV probably have
+                         * EC-based backlight level control
+                         */
+                        if (acpi_evalf(ec_handle, NULL, "HBRV", "qd"))
+                                /* T40-T43, R50-R52, R50e, R51e, X31-X41 */
+                                brightness_mode = TPACPI_BRGHT_MODE_ECNVRAM;
+                        else
+                                /* all other IBM ThinkPads */
+                                brightness_mode = TPACPI_BRGHT_MODE_UCMS_STEP;
+                } else
+                        /* All Lenovo ThinkPads */
+                        brightness_mode = TPACPI_BRGHT_MODE_UCMS_STEP;
 
-        if (brightness_mode > 3)
-                return -EINVAL;
+                dbg_printk(TPACPI_DBG_BRGHT,
+                           "selected brightness_mode=%d\n",
+                           brightness_mode);
+        }
 
-        if (brightness_get_raw(&b) < 0)
+        if (tpacpi_brightness_get_raw(&b) < 0)
                 return 1;
 
         if (tp_features.bright_16levels)
@@ -5751,7 +5852,8 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
                 printk(TPACPI_ERR "Could not register backlight device\n");
                 return PTR_ERR(ibm_backlight_device);
         }
-        vdbg_printk(TPACPI_DBG_INIT, "brightness is supported\n");
+        vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
+                        "brightness is supported\n");
 
         ibm_backlight_device->props.max_brightness =
                                 (tp_features.bright_16levels)? 15 : 7;
@@ -5761,13 +5863,25 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
         return 0;
 }
 
+static void brightness_suspend(pm_message_t state)
+{
+        tpacpi_brightness_checkpoint_nvram();
+}
+
+static void brightness_shutdown(void)
+{
+        tpacpi_brightness_checkpoint_nvram();
+}
+
 static void brightness_exit(void)
 {
         if (ibm_backlight_device) {
-                vdbg_printk(TPACPI_DBG_EXIT,
+                vdbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_BRGHT,
                             "calling backlight_device_unregister()\n");
                 backlight_device_unregister(ibm_backlight_device);
         }
+
+        tpacpi_brightness_checkpoint_nvram();
 }
 
 static int brightness_read(char *p)
@@ -5814,6 +5928,9 @@ static int brightness_write(char *buf)
                         return -EINVAL;
         }
 
+        tpacpi_disclose_usertask("procfs brightness",
+                        "set level to %d\n", level);
+
         /*
          * Now we know what the final level should be, so we try to set it.
          * Doing it this way makes the syscall restartable in case of EINTR
@@ -5827,6 +5944,8 @@ static struct ibm_struct brightness_driver_data = {
         .read = brightness_read,
         .write = brightness_write,
         .exit = brightness_exit,
+        .suspend = brightness_suspend,
+        .shutdown = brightness_shutdown,
 };
 
 /*************************************************************************
@@ -7465,10 +7584,10 @@ module_param_named(fan_control, fan_control_allowed, bool, 0);
 MODULE_PARM_DESC(fan_control,
                  "Enables setting fan parameters features when true");
 
-module_param_named(brightness_mode, brightness_mode, int, 0);
+module_param_named(brightness_mode, brightness_mode, uint, 0);
 MODULE_PARM_DESC(brightness_mode,
                  "Selects brightness control strategy: "
-                 "0=auto, 1=EC, 2=CMOS, 3=both");
+                 "0=auto, 1=EC, 2=UCMS, 3=EC+NVRAM");
 
 module_param(brightness_enable, uint, 0);
 MODULE_PARM_DESC(brightness_enable,