Thanks, Hans:
On 7/12/22 14:38, Hans de Goede wrote:
On some new laptop designs a new Nvidia specific WMI interface is present
which gives info about panel brightness control and may allow controlling
the brightness through this interface when the embedded controller is used
for brightness control.
When this WMI interface is present and indicates that the EC is used,
then this interface should be used for brightness control.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/acpi/Kconfig | 1 +
drivers/acpi/video_detect.c | 35 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/gma500/Kconfig | 2 ++
drivers/gpu/drm/i915/Kconfig | 2 ++
include/acpi/video.h | 1 +
5 files changed, 41 insertions(+)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1e34f846508f..c372385cfc3f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -212,6 +212,7 @@ config ACPI_VIDEO
tristate "Video"
depends on X86 && BACKLIGHT_CLASS_DEVICE
depends on INPUT
+ depends on ACPI_WMI
select THERMAL
help
This driver implements the ACPI Extensions For Display Adapters
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 8c2863403040..7b89dc9a04a2 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -75,6 +75,35 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
return AE_OK;
}
+#define WMI_BRIGHTNESS_METHOD_SOURCE 2
+#define WMI_BRIGHTNESS_MODE_GET 0
+#define WMI_BRIGHTNESS_SOURCE_EC 2
+
+struct wmi_brightness_args {
+ u32 mode;
+ u32 val;
+ u32 ret;
+ u32 ignored[3];
+};
+
+static bool nvidia_wmi_ec_supported(void)
+{
+ struct wmi_brightness_args args = {
+ .mode = WMI_BRIGHTNESS_MODE_GET,
+ .val = 0,
+ .ret = 0,
+ };
+ struct acpi_buffer buf = { (acpi_size)sizeof(args), &args };
+ acpi_status status;
+
+ status = wmi_evaluate_method("603E9613-EF25-4338-A3D0-C46177516DB7", 0,
+ WMI_BRIGHTNESS_METHOD_SOURCE, &buf, &buf);
+ if (ACPI_FAILURE(status))
+ return false;
+
+ return args.ret == WMI_BRIGHTNESS_SOURCE_EC;
+}
+
The code duplication here with nvidia-wmi-ec-backlight.c is a little
unfortunate. Can we move the constants, struct definition, and WMI GUID
from that file to a header file that's used both by the EC backlight
driver and the ACPI video driver?
I was thinking it might be nice to add a wrapper around
wmi_brightness_notify() in nvidia-wmi-ec-backlight.c that does this
source == WMI_BRIGHTNESS_SOURCE_EC test, and then export it so that it
can be called both here and in the EC backlight driver's probe routine,
but then I guess that would make video.ko depend on
nvidia-wmi-ec-backlight.ko, which seems wrong. It also seems wrong to
implement the WMI plumbing in the ACPI video driver, and export it so
that the EC backlight driver can use it, so I guess I can live with the
duplication of the relatively simple WMI stuff here, it would just be
nice to not have to define all of the API constants, structure, and GUID
twice.
/* 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)
@@ -518,6 +547,7 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
{
static DEFINE_MUTEX(init_mutex);
+ static bool nvidia_wmi_ec_present;
static bool native_available;
static bool init_done;
static long video_caps;
@@ -530,6 +560,7 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
ACPI_UINT32_MAX, find_video, NULL,
&video_caps, NULL);
+ nvidia_wmi_ec_present = nvidia_wmi_ec_supported();
init_done = true;
}
if (native)
@@ -547,6 +578,10 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
if (acpi_backlight_dmi != acpi_backlight_undef)
return acpi_backlight_dmi;
+ /* Special cases such as nvidia_wmi_ec and apple gmux. */
+ if (nvidia_wmi_ec_present)
+ return acpi_backlight_nvidia_wmi_ec;
Should there also be a change to the EC backlight driver to call
acpi_video_get_backlight_type() and make sure we get
acpi_backlight_nvidia_wmi_ec? I don't see such a change in this patch
series; I could implement it (and test it) against your patch if there's
some reason you didn't do so with the current patchset.
+
/* On systems with ACPI video use either native or ACPI video. */
if (video_caps & ACPI_VIDEO_BACKLIGHT) {
/*
diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index 0cff20265f97..807b989e3c77 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -7,6 +7,8 @@ config DRM_GMA500
select ACPI_VIDEO if ACPI
select BACKLIGHT_CLASS_DEVICE if ACPI
select INPUT if ACPI
+ select X86_PLATFORM_DEVICES if ACPI
+ select ACPI_WMI if ACPI
I'm not sure I understand why the Intel DRM drivers pick up the
additional platform/x86 and WMI dependencies here. ACPI_VIDEO already
depends on these, doesn't it? If Kconfig doesn't otherwise automatically
pull in an option's dependencies when selecting that option, then
shouldn't Nouveau's Kconfig get updated as well? It selects ACPI_VIDEO
in some configuration cases. (It looks like amdgpu doesn't currently
select ACPI_VIDEO, maybe because it doesn't depend on it the way the
Intel drivers do: there are several AMD+NVIDIA iGPU/dGPU designs out
there which use this backlight interface.)
help
Say yes for an experimental 2D KMS framebuffer driver for the
Intel GMA500 (Poulsbo), Intel GMA600 (Moorestown/Oak Trail) and
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 7ae3b7d67fcf..3efce05d7b57 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -23,6 +23,8 @@ config DRM_I915
# but for select to work, need to select ACPI_VIDEO's dependencies, ick
select BACKLIGHT_CLASS_DEVICE if ACPI
select INPUT if ACPI
+ select X86_PLATFORM_DEVICES if ACPI
+ select ACPI_WMI if ACPI
select ACPI_VIDEO if ACPI
select ACPI_BUTTON if ACPI
select SYNC_FILE
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 0625806d3bbd..91578e77ac4e 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -48,6 +48,7 @@ enum acpi_backlight_type {
acpi_backlight_video,
acpi_backlight_vendor,
acpi_backlight_native,
+ acpi_backlight_nvidia_wmi_ec,
};
#if IS_ENABLED(CONFIG_ACPI_VIDEO)
