* [PATCH] ACPI: video: Fix Apple GMUX backlight detection
@ 2022-12-15 9:41 Hans de Goede
2022-12-15 10:22 ` Lukas Wunner
2022-12-22 16:44 ` Rafael J. Wysocki
0 siblings, 2 replies; 4+ messages in thread
From: Hans de Goede @ 2022-12-15 9:41 UTC (permalink / raw)
To: Rafael J . Wysocki
Cc: Hans de Goede, linux-acpi, platform-driver-x86, Aditya Garg
The apple-gmux driver only binds to old GMUX devices which have an
IORESOURCE_IO resource (using inb()/outb()) rather then memory-mapped
IO (IORESOURCE_MEM).
T2 MacBooks use the new style GMUX devices (with IORESOURCE_MEM access),
so these are not supported by the apple-gmux driver. This is not a problem
since they have working ACPI video backlight support.
But the apple_gmux_present() helper only checks if an ACPI device with
the "APP000B" HID is present, causing acpi_video_get_backlight_type()
to return acpi_backlight_apple_gmux disabling the acpi_video backlight
device.
Add a new apple_gmux_backlight_present() helper which checks that
the "APP000B" device actually is an old GMUX device with an IORESOURCE_IO
resource.
This fixes the acpi_video0 backlight no longer registering on T2 MacBooks.
Note people are working to add support for the new style GMUX to Linux:
https://github.com/kekrby/linux-t2/commits/wip/hybrid-graphics
Once this lands this patch should be reverted so that
acpi_video_get_backlight_type() also prefers the gmux on new style GMUX
MacBooks, but for now this is necessary to avoid regressing backlight
control on T2 Macs.
Fixes: 21245df307cb ("ACPI: video: Add Apple GMUX brightness control detection")
Reported-and-tested-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/video_detect.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index a934bbc9dd37..1b78c7434492 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -34,6 +34,7 @@
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/platform_data/x86/nvidia-wmi-ec-backlight.h>
+#include <linux/pnp.h>
#include <linux/types.h>
#include <linux/workqueue.h>
#include <acpi/video.h>
@@ -105,6 +106,26 @@ static bool nvidia_wmi_ec_supported(void)
}
#endif
+static bool apple_gmux_backlight_present(void)
+{
+ struct acpi_device *adev;
+ struct device *dev;
+
+ adev = acpi_dev_get_first_match_dev(GMUX_ACPI_HID, NULL, -1);
+ if (!adev)
+ return false;
+
+ dev = acpi_get_first_physical_node(adev);
+ if (!dev)
+ return false;
+
+ /*
+ * drivers/platform/x86/apple-gmux.c only supports old style
+ * Apple GMUX with an IO-resource.
+ */
+ return pnp_get_resource(to_pnp_dev(dev), IORESOURCE_IO, 0) != NULL;
+}
+
/* Force to use vendor driver when the ACPI device is known to be
* buggy */
static int video_detect_force_vendor(const struct dmi_system_id *d)
@@ -767,7 +788,7 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
if (nvidia_wmi_ec_present)
return acpi_backlight_nvidia_wmi_ec;
- if (apple_gmux_present())
+ if (apple_gmux_backlight_present())
return acpi_backlight_apple_gmux;
/* Use ACPI video if available, except when native should be preferred. */
--
2.38.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ACPI: video: Fix Apple GMUX backlight detection
2022-12-15 9:41 [PATCH] ACPI: video: Fix Apple GMUX backlight detection Hans de Goede
@ 2022-12-15 10:22 ` Lukas Wunner
2022-12-15 10:50 ` Hans de Goede
2022-12-22 16:44 ` Rafael J. Wysocki
1 sibling, 1 reply; 4+ messages in thread
From: Lukas Wunner @ 2022-12-15 10:22 UTC (permalink / raw)
To: Hans de Goede
Cc: Rafael J . Wysocki, linux-acpi, platform-driver-x86, Aditya Garg
On Thu, Dec 15, 2022 at 10:41:38AM +0100, Hans de Goede wrote:
> The apple-gmux driver only binds to old GMUX devices which have an
> IORESOURCE_IO resource (using inb()/outb()) rather then memory-mapped
> IO (IORESOURCE_MEM).
>
> T2 MacBooks use the new style GMUX devices (with IORESOURCE_MEM access),
> so these are not supported by the apple-gmux driver. This is not a problem
> since they have working ACPI video backlight support.
Interesting.
> +static bool apple_gmux_backlight_present(void)
> +{
> + struct acpi_device *adev;
> + struct device *dev;
> +
> + adev = acpi_dev_get_first_match_dev(GMUX_ACPI_HID, NULL, -1);
> + if (!adev)
> + return false;
> +
> + dev = acpi_get_first_physical_node(adev);
> + if (!dev)
> + return false;
> +
> + /*
> + * drivers/platform/x86/apple-gmux.c only supports old style
> + * Apple GMUX with an IO-resource.
> + */
> + return pnp_get_resource(to_pnp_dev(dev), IORESOURCE_IO, 0) != NULL;
> +}
The T2 is represented by a PCI device with ID 106B:1802.
Instead of the above, how about amending apple_gmux_present()
with a simple check like this:
/* T2 Macs drive GMUX via MMIO, which is unsupported for now */
if (pci_dev_present({{PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x1802)}, {}}))
return false;
Thanks,
Lukas
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ACPI: video: Fix Apple GMUX backlight detection
2022-12-15 10:22 ` Lukas Wunner
@ 2022-12-15 10:50 ` Hans de Goede
0 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2022-12-15 10:50 UTC (permalink / raw)
To: Lukas Wunner
Cc: Rafael J . Wysocki, linux-acpi, platform-driver-x86, Aditya Garg
Hi Lukas,
On 12/15/22 11:22, Lukas Wunner wrote:
> On Thu, Dec 15, 2022 at 10:41:38AM +0100, Hans de Goede wrote:
>> The apple-gmux driver only binds to old GMUX devices which have an
>> IORESOURCE_IO resource (using inb()/outb()) rather then memory-mapped
>> IO (IORESOURCE_MEM).
>>
>> T2 MacBooks use the new style GMUX devices (with IORESOURCE_MEM access),
>> so these are not supported by the apple-gmux driver. This is not a problem
>> since they have working ACPI video backlight support.
>
> Interesting.
>
>
>> +static bool apple_gmux_backlight_present(void)
>> +{
>> + struct acpi_device *adev;
>> + struct device *dev;
>> +
>> + adev = acpi_dev_get_first_match_dev(GMUX_ACPI_HID, NULL, -1);
>> + if (!adev)
>> + return false;
>> +
>> + dev = acpi_get_first_physical_node(adev);
>> + if (!dev)
>> + return false;
>> +
>> + /*
>> + * drivers/platform/x86/apple-gmux.c only supports old style
>> + * Apple GMUX with an IO-resource.
>> + */
>> + return pnp_get_resource(to_pnp_dev(dev), IORESOURCE_IO, 0) != NULL;
>> +}
>
> The T2 is represented by a PCI device with ID 106B:1802.
>
> Instead of the above, how about amending apple_gmux_present()
> with a simple check like this:
>
> /* T2 Macs drive GMUX via MMIO, which is unsupported for now */
> if (pci_dev_present({{PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x1802)}, {}}))
> return false;
Where ever possible I prefer to have the same checks in video_detect.c
as the actual backlight driver uses and:
drivers/platform/x86/apple-gmux.c
refuses to bind because the pnp_get_resource(to_pnp_dev(dev), IORESOURCE_IO, 0)
call fails. So IMHO it is better to do the same thing in video_detect.c .
Specifically likely more then 1 model of MacBook is affected and doing
the same check as apple-gmux.c does will catch them all, while an
(PCI) id based approach often turns into a game of whack-a-mole.
Also changing apple_gmux_present() itself impacts more callers then
just the drivers/acpi_video_detect.c code.
Specifically drivers/gpu/drm/nouveau/nouveau_backlight.c and
drivers/gpu/vga/vga_switcheroo.c also call this function.
And at least in the nouveau case where the check looks like this:
if (apple_gmux_present()) {
NV_INFO_ONCE(drm, "Apple GMUX detected: not registering Nouveau backlight interface\n");
return 0;
}
Not registering the GPU/native backlight device is the right thing to
do on the models with the new GMUX too, since native backlight control
won't work there either.
And I don't know about the check in vga_switcheroo.c. The goal of this
patch is to fix the 6.1 regression introduced by adding
a apple_gmux_present() to video_acpi.c and in its current form it
fixes that regression without introducing any behavioral changes
elsewhere, which seems best for a regressions fix which is intended
to be backported to 6.1 .
Regards,
Hans
p.s.
The nouveau driver not registering its native backlight is actually something
which the amdgpu driver gets wrong, it registers a non working amdgpu_bl0 on
the laptop Aditya ran their tests on. Starting with 6.2 however amdgpu will
honor the return value of acpi_video_get_backlight_type()
(on x86/ACPI platforms).
So the non working amdgpu_bl0 should disappear with 6.2, as long as we get
acpi_video_get_backlight_type() to return the right type, which it does after
this patch (it returns acpi_backlight_video after this patch).
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ACPI: video: Fix Apple GMUX backlight detection
2022-12-15 9:41 [PATCH] ACPI: video: Fix Apple GMUX backlight detection Hans de Goede
2022-12-15 10:22 ` Lukas Wunner
@ 2022-12-22 16:44 ` Rafael J. Wysocki
1 sibling, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2022-12-22 16:44 UTC (permalink / raw)
To: Hans de Goede
Cc: Rafael J . Wysocki, linux-acpi, platform-driver-x86, Aditya Garg
On Thu, Dec 15, 2022 at 10:41 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The apple-gmux driver only binds to old GMUX devices which have an
> IORESOURCE_IO resource (using inb()/outb()) rather then memory-mapped
> IO (IORESOURCE_MEM).
>
> T2 MacBooks use the new style GMUX devices (with IORESOURCE_MEM access),
> so these are not supported by the apple-gmux driver. This is not a problem
> since they have working ACPI video backlight support.
>
> But the apple_gmux_present() helper only checks if an ACPI device with
> the "APP000B" HID is present, causing acpi_video_get_backlight_type()
> to return acpi_backlight_apple_gmux disabling the acpi_video backlight
> device.
>
> Add a new apple_gmux_backlight_present() helper which checks that
> the "APP000B" device actually is an old GMUX device with an IORESOURCE_IO
> resource.
>
> This fixes the acpi_video0 backlight no longer registering on T2 MacBooks.
>
> Note people are working to add support for the new style GMUX to Linux:
> https://github.com/kekrby/linux-t2/commits/wip/hybrid-graphics
>
> Once this lands this patch should be reverted so that
> acpi_video_get_backlight_type() also prefers the gmux on new style GMUX
> MacBooks, but for now this is necessary to avoid regressing backlight
> control on T2 Macs.
>
> Fixes: 21245df307cb ("ACPI: video: Add Apple GMUX brightness control detection")
> Reported-and-tested-by: Aditya Garg <gargaditya08@live.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/acpi/video_detect.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index a934bbc9dd37..1b78c7434492 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -34,6 +34,7 @@
> #include <linux/module.h>
> #include <linux/pci.h>
> #include <linux/platform_data/x86/nvidia-wmi-ec-backlight.h>
> +#include <linux/pnp.h>
> #include <linux/types.h>
> #include <linux/workqueue.h>
> #include <acpi/video.h>
> @@ -105,6 +106,26 @@ static bool nvidia_wmi_ec_supported(void)
> }
> #endif
>
> +static bool apple_gmux_backlight_present(void)
> +{
> + struct acpi_device *adev;
> + struct device *dev;
> +
> + adev = acpi_dev_get_first_match_dev(GMUX_ACPI_HID, NULL, -1);
> + if (!adev)
> + return false;
> +
> + dev = acpi_get_first_physical_node(adev);
> + if (!dev)
> + return false;
> +
> + /*
> + * drivers/platform/x86/apple-gmux.c only supports old style
> + * Apple GMUX with an IO-resource.
> + */
> + return pnp_get_resource(to_pnp_dev(dev), IORESOURCE_IO, 0) != NULL;
> +}
> +
> /* Force to use vendor driver when the ACPI device is known to be
> * buggy */
> static int video_detect_force_vendor(const struct dmi_system_id *d)
> @@ -767,7 +788,7 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
> if (nvidia_wmi_ec_present)
> return acpi_backlight_nvidia_wmi_ec;
>
> - if (apple_gmux_present())
> + if (apple_gmux_backlight_present())
> return acpi_backlight_apple_gmux;
>
> /* Use ACPI video if available, except when native should be preferred. */
> --
Applied as 6.2-rc material, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-12-22 16:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-15 9:41 [PATCH] ACPI: video: Fix Apple GMUX backlight detection Hans de Goede
2022-12-15 10:22 ` Lukas Wunner
2022-12-15 10:50 ` Hans de Goede
2022-12-22 16:44 ` Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.