[v3] apple-gmux: preserve brightness using EFI

[PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Atharva Tiwari 1 month, 2 weeks ago

Make apple-gmux save the current backlight brightness to EFI on shutdown,
so the brightness level is preserved across reboots.

(tested on iMac20,1)

Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
---
v3:
- Only save the brightness when it differs from the boot brightness.
v2:
- Used correct size for efi_data
---
 drivers/platform/x86/apple-gmux.c | 61 +++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 1417e230edbd..08b9f48c11b8 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -22,6 +22,7 @@
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/debugfs.h>
+#include <linux/efi.h>
 #include <acpi/video.h>
 #include <asm/io.h>
 
@@ -78,6 +79,11 @@ struct apple_gmux_data {
 	/* debugfs data */
 	u8 selected_port;
 	struct dentry *debug_dentry;
+
+	/* efi data */
+	efi_status_t efi_status;
+	u32 efi_attr;
+	u16 efi_data;
 };
 
 static struct apple_gmux_data *apple_gmux_data;
@@ -107,6 +113,9 @@ struct apple_gmux_config {
 
 # define MMIO_GMUX_MAX_BRIGHTNESS	0xffff
 
+#define EFI_BRIGHTNESS_NAME		L"backlight-level"
+#define EFI_BRIGHTNESS_GUID		EFI_GUID(0x7c436110, 0xab2a, 0x4bbb, 0xa8, 0x80, 0xfe, 0x41, 0x99, 0x5c, 0x9f, 0x82)
+
 static u8 gmux_pio_read8(struct apple_gmux_data *gmux_data, int port)
 {
 	return inb(gmux_data->iostart + port);
@@ -751,6 +760,35 @@ static void gmux_fini_debugfs(struct apple_gmux_data *gmux_data)
 	debugfs_remove_recursive(gmux_data->debug_dentry);
 }
 
+#ifdef CONFIG_EFI
+MODULE_IMPORT_NS("EFIVAR");
+static void gmux_init_efi(struct apple_gmux_data *gmux_data)
+{
+	unsigned long size = sizeof(gmux_data->efi_data);
+
+	gmux_data->efi_status = EFI_UNSUPPORTED;
+
+	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_SET_VARIABLE))
+		return;
+
+	if (efivar_lock())
+		return;
+
+	gmux_data->efi_status = efivar_get_variable(EFI_BRIGHTNESS_NAME, &EFI_BRIGHTNESS_GUID,
+					&gmux_data->efi_attr, &size, &gmux_data->efi_data);
+
+	efivar_unlock();
+}
+
+#else /* CONFIG_EFI */
+
+static void gmux_init_efi(struct apple_gmux_data *gmux_data)
+{
+	return;
+}
+
+#endif /* CONFIG_EFI */
+
 static int gmux_suspend(struct device *dev)
 {
 	struct pnp_dev *pnp = to_pnp_dev(dev);
@@ -960,6 +998,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	}
 
 	gmux_init_debugfs(gmux_data);
+	gmux_init_efi(gmux_data);
+
 	return 0;
 
 err_register_handler:
@@ -1012,6 +1052,26 @@ static void gmux_remove(struct pnp_dev *pnp)
 	kfree(gmux_data);
 }
 
+static void gmux_shutdown(struct pnp_dev *pnp)
+{
+	struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
+	u16 brightness = (u16)gmux_get_brightness(gmux_data->bdev);
+
+#ifdef CONFIG_EFI
+	if (gmux_data->efi_status == EFI_SUCCESS && gmux_data->efi_data != brightness) {
+		gmux_data->efi_status = efivar_set_variable(EFI_BRIGHTNESS_NAME,
+						&EFI_BRIGHTNESS_GUID,
+						gmux_data->efi_attr,
+						sizeof(brightness),
+						&brightness);
+		if (gmux_data->efi_status != EFI_SUCCESS)
+			pr_info("Unable to save brightness: 0x%lx\n", gmux_data->efi_status);
+	}
+#endif /* CONFIG_EFI */
+
+	gmux_remove(pnp);
+}
+
 static const struct pnp_device_id gmux_device_ids[] = {
 	{GMUX_ACPI_HID, 0},
 	{"", 0}
@@ -1026,6 +1086,7 @@ static struct pnp_driver gmux_pnp_driver = {
 	.name		= "apple-gmux",
 	.probe		= gmux_probe,
 	.remove		= gmux_remove,
+	.shutdown	= gmux_shutdown,
 	.id_table	= gmux_device_ids,
 	.driver		= {
 			.pm = &gmux_dev_pm_ops,
-- 
2.43.0

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by kernel test robot 1 month, 2 weeks ago

Hi Atharva,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.19 next-20260211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Atharva-Tiwari/apple-gmux-preserve-brightness-using-EFI/20260211-173626
base:   linus/master
patch link:    https://lore.kernel.org/r/20260211092640.3231-1-atharvatiwarilinuxdev%40gmail.com
patch subject: [PATCH v3] apple-gmux: preserve brightness using EFI
config: x86_64-randconfig-011-20260211 (https://download.01.org/0day-ci/archive/20260212/202602120036.rVaXQsI0-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260212/202602120036.rVaXQsI0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602120036.rVaXQsI0-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/platform/x86/apple-gmux.c: In function 'gmux_shutdown':
>> drivers/platform/x86/apple-gmux.c:1058:13: warning: unused variable 'brightness' [-Wunused-variable]
    1058 |         u16 brightness = (u16)gmux_get_brightness(gmux_data->bdev);
         |             ^~~~~~~~~~


vim +/brightness +1058 drivers/platform/x86/apple-gmux.c

  1054	
  1055	static void gmux_shutdown(struct pnp_dev *pnp)
  1056	{
  1057		struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
> 1058		u16 brightness = (u16)gmux_get_brightness(gmux_data->bdev);
  1059	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Lukas Wunner 1 month, 2 weeks ago

On Wed, Feb 11, 2026 at 02:56:38PM +0530, Atharva Tiwari wrote:
> Make apple-gmux save the current backlight brightness to EFI on shutdown,
> so the brightness level is preserved across reboots.

This version moves the EFI handling back to the gmux driver.  Why?

I found version 2 (which separated EFI handling into a separate component)
much better because it will allow other drivers such as i915 and amdgpu
to update the brightness if they're the ones driving the backlight,
instead of gmux.

> +static void gmux_shutdown(struct pnp_dev *pnp)
> +{
> +	struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
> +	u16 brightness = (u16)gmux_get_brightness(gmux_data->bdev);
> +
> +#ifdef CONFIG_EFI
> +	if (gmux_data->efi_status == EFI_SUCCESS && gmux_data->efi_data != brightness) {
> +		gmux_data->efi_status = efivar_set_variable(EFI_BRIGHTNESS_NAME,

Per section 21 of Documentation/process/coding-style.rst, #ifdef inside
functions should be avoided.

Thanks,

Lukas

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Atharva Tiwari 2 weeks, 4 days ago

> This version moves the EFI handling back to the gmux driver.  Why?

Because the platform maintainer just NACKed the EFI backend driver, so i
cant do anything

https://lore.kernel.org/all/c2d14818-1c34-47c7-a210-1f7c737f0bc9@kernel.org/

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Lukas Wunner 2 weeks, 3 days ago

On Sun, Mar 15, 2026 at 04:15:03AM +0530, Atharva Tiwari wrote:
> > This version moves the EFI handling back to the gmux driver.  Why?
> 
> Because the platform maintainer just NACKed the EFI backend driver, so i
> cant do anything
> 
> https://lore.kernel.org/all/c2d14818-1c34-47c7-a210-1f7c737f0bc9@kernel.org/

The NAK was only because of writing to the EFI variable in frequent
intervals.  I'd assume Hans is fine if it is synced only once on
shutdown.

Splitting out the EFI handling into a separate component in
drivers/firmware/efi/ as you did in this version:

https://lore.kernel.org/all/20260206125641.12983-2-atharvatiwarilinuxdev@gmail.com/

... is much cleaner as it allows code reuse by i915 and other drivers
which may be controlling brightness on MacBooks or iMacs without a gmux.
So I'd really prefer if you went back to that, but sync only on shutdown
and only if brightness has changed, per Hans' request.

BTW, I did another test on my MacBook9,1 with brightness set to maximum
on macOS, then booting into Linux.  The EFI variable as set by macOS
contains:

07 00 00 80 ff 03

... but /sys/class/backlight/gmux_backlight/brightness contains the
value 110400 (0x1af40), which obviously exceeds USHRT_MAX.  So I'm not
convinced that writing that raw value to the EFI variable actually yields
correct results.

Thanks,

Lukas

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Atharva Tiwari 2 weeks, 3 days ago

> ... but /sys/class/backlight/gmux_backlight/brightness contains the
> value 110400 (0x1af40), which obviously exceeds USHRT_MAX.  So I'm not

How did this even happen?, AFIK the brightness value uses gmux_get_brightness
and that masks the value with 0xFFFF so this should be impossible?

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Armin Wolf 2 weeks, 4 days ago

Am 14.03.26 um 23:45 schrieb Atharva Tiwari:

>> This version moves the EFI handling back to the gmux driver.  Why?
> Because the platform maintainer just NACKed the EFI backend driver, so i
> cant do anything
>
> https://lore.kernel.org/all/c2d14818-1c34-47c7-a210-1f7c737f0bc9@kernel.org/
>
After reading this, i wonder why this functionality should live inside the kernel after all.
Systemd seems to already store the backlight brightness during shutdown/reboot, so maybe
having a small oneshot service that synchronizes those values with the EFI variable during
shutdown/reboot might be easier?

Thanks,
Armin Wolf

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Atharva Tiwari 2 weeks, 3 days ago

> Systemd seems to already store the backlight brightness during shutdown/reboot, so maybe
> having a small oneshot service that synchronizes those values with the EFI variable during
> shutdown/reboot might be easier?

This is logical only if we assume every system uses systemd, and that isint the case, and trying
to port the same logical to every alternative to systemd will take more time and effort, than to just
embed this logic in the kernel.

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Armin Wolf 2 weeks, 3 days ago

Am 15.03.26 um 23:08 schrieb Atharva Tiwari:

>> Systemd seems to already store the backlight brightness during shutdown/reboot, so maybe
>> having a small oneshot service that synchronizes those values with the EFI variable during
>> shutdown/reboot might be easier?
> This is logical only if we assume every system uses systemd, and that isint the case, and trying
> to port the same logical to every alternative to systemd will take more time and effort, than to just
> embed this logic in the kernel.

Such a daemon should also work with other init systems, it only needs to handle SIGTERM.

Thanks,
Armin Wolf

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Atharva Tiwari 2 weeks, 2 days ago

> Such a daemon should also work with other init systems, it only needs to handle SIGTERM.

Even if i create a daemon that works with all init systems, i would still need to upstream the
daemon to all init systems, which will ofc take a lot of time. and also i dont get why embedding
this logic in the kernel is a bad idea.

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Hans de Goede 1 month, 2 weeks ago

Hi,

On 11-Feb-26 10:26, Atharva Tiwari wrote:
> Make apple-gmux save the current backlight brightness to EFI on shutdown,
> so the brightness level is preserved across reboots.
> 
> (tested on iMac20,1)
> 
> Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
> ---
> v3:
> - Only save the brightness when it differs from the boot brightness.

Thanks this looks much better.

> v2:
> - Used correct size for efi_data
> ---
>  drivers/platform/x86/apple-gmux.c | 61 +++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index 1417e230edbd..08b9f48c11b8 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -22,6 +22,7 @@
>  #include <linux/pci.h>
>  #include <linux/vga_switcheroo.h>
>  #include <linux/debugfs.h>
> +#include <linux/efi.h>
>  #include <acpi/video.h>
>  #include <asm/io.h>
>  
> @@ -78,6 +79,11 @@ struct apple_gmux_data {
>  	/* debugfs data */
>  	u8 selected_port;
>  	struct dentry *debug_dentry;
> +
> +	/* efi data */
> +	efi_status_t efi_status;
> +	u32 efi_attr;
> +	u16 efi_data;
>  };
>  
>  static struct apple_gmux_data *apple_gmux_data;
> @@ -107,6 +113,9 @@ struct apple_gmux_config {
>  
>  # define MMIO_GMUX_MAX_BRIGHTNESS	0xffff
>  
> +#define EFI_BRIGHTNESS_NAME		L"backlight-level"
> +#define EFI_BRIGHTNESS_GUID		EFI_GUID(0x7c436110, 0xab2a, 0x4bbb, 0xa8, 0x80, 0xfe, 0x41, 0x99, 0x5c, 0x9f, 0x82)
> +
>  static u8 gmux_pio_read8(struct apple_gmux_data *gmux_data, int port)
>  {
>  	return inb(gmux_data->iostart + port);
> @@ -751,6 +760,35 @@ static void gmux_fini_debugfs(struct apple_gmux_data *gmux_data)
>  	debugfs_remove_recursive(gmux_data->debug_dentry);
>  }
>  
> +#ifdef CONFIG_EFI
> +MODULE_IMPORT_NS("EFIVAR");
> +static void gmux_init_efi(struct apple_gmux_data *gmux_data)
> +{
> +	unsigned long size = sizeof(gmux_data->efi_data);
> +
> +	gmux_data->efi_status = EFI_UNSUPPORTED;
> +
> +	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_SET_VARIABLE))
> +		return;
> +
> +	if (efivar_lock())
> +		return;
> +
> +	gmux_data->efi_status = efivar_get_variable(EFI_BRIGHTNESS_NAME, &EFI_BRIGHTNESS_GUID,
> +					&gmux_data->efi_attr, &size, &gmux_data->efi_data);
> +
> +	efivar_unlock();
> +}
> +
> +#else /* CONFIG_EFI */
> +
> +static void gmux_init_efi(struct apple_gmux_data *gmux_data)
> +{
> +	return;
> +}
> +
> +#endif /* CONFIG_EFI */
> +
>  static int gmux_suspend(struct device *dev)
>  {
>  	struct pnp_dev *pnp = to_pnp_dev(dev);
> @@ -960,6 +998,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
>  	}
>  
>  	gmux_init_debugfs(gmux_data);
> +	gmux_init_efi(gmux_data);
> +
>  	return 0;
>  
>  err_register_handler:
> @@ -1012,6 +1052,26 @@ static void gmux_remove(struct pnp_dev *pnp)
>  	kfree(gmux_data);
>  }
>  
> +static void gmux_shutdown(struct pnp_dev *pnp)
> +{
> +	struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
> +	u16 brightness = (u16)gmux_get_brightness(gmux_data->bdev);
> +
> +#ifdef CONFIG_EFI
> +	if (gmux_data->efi_status == EFI_SUCCESS && gmux_data->efi_data != brightness) {
> +		gmux_data->efi_status = efivar_set_variable(EFI_BRIGHTNESS_NAME,
> +						&EFI_BRIGHTNESS_GUID,
> +						gmux_data->efi_attr,
> +						sizeof(brightness),
> +						&brightness);


I think this is missing efivar_lock() + unlock() calls ?

Regards,

Hans




> +		if (gmux_data->efi_status != EFI_SUCCESS)
> +			pr_info("Unable to save brightness: 0x%lx\n", gmux_data->efi_status);
> +	}
> +#endif /* CONFIG_EFI */
> +
> +	gmux_remove(pnp);
> +}
> +
>  static const struct pnp_device_id gmux_device_ids[] = {
>  	{GMUX_ACPI_HID, 0},
>  	{"", 0}
> @@ -1026,6 +1086,7 @@ static struct pnp_driver gmux_pnp_driver = {
>  	.name		= "apple-gmux",
>  	.probe		= gmux_probe,
>  	.remove		= gmux_remove,
> +	.shutdown	= gmux_shutdown,
>  	.id_table	= gmux_device_ids,
>  	.driver		= {
>  			.pm = &gmux_dev_pm_ops,

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Atharva Tiwari 1 month, 2 weeks ago

> I think this is missing efivar_lock() + unlock() calls ?

actually, efivar_set_variable already holds the lock, so efivar_lock isint
required.

See: drivers/firmware/efi/vars.c:242 and drivers/firmware/efi/vars.c:184

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Hans de Goede 1 month, 2 weeks ago

Hi,

On 11-Feb-26 13:30, Atharva Tiwari wrote:
>> I think this is missing efivar_lock() + unlock() calls ?
> 
> actually, efivar_set_variable already holds the lock, so efivar_lock isint
> required.
> 
> See: drivers/firmware/efi/vars.c:242 and drivers/firmware/efi/vars.c:184

Ah I did not know that, that is somewhat inconsistent but not
a problem with this patch:

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Atharva Tiwari 2 weeks, 5 days ago

> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Sorry to disturb, but its been 1 month since the patch,
and it is still not applied. can you please apply it.

Re: [PATCH v3] apple-gmux: preserve brightness using EFI

Posted by Lukas Wunner 2 weeks, 4 days ago

On Sat, Mar 14, 2026 at 01:09:56AM +0530, Atharva Tiwari wrote:
> > Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> 
> Sorry to disturb, but its been 1 month since the patch,
> and it is still not applied. can you please apply it.

Kernel development is consensus-driven.  Patches are usually not applied
as long as there are outstanding unaddressed comments.  Unfortunately
you did not reply at all to the comments I offered:

https://lore.kernel.org/all/aYx6ocvaFHunfP5f@wunner.de/

Moreover patches are never applied as long as there are unaddressed
0-day build warnings:

https://lore.kernel.org/all/202602120036.rVaXQsI0-lkp@intel.com/

Please resubmit a patch which addresses the build issue and comments
you received.  If you disagree with certain comments, explain why.

Thanks,

Lukas