Hi
2021. augusztus 25., szerda 0:04 keltezéssel, Daniel Dadap írta:
> A number of upcoming notebook computer designs drive the internal
> display panel's backlight PWM through the Embedded Controller (EC).
> This EC-based backlight control can be plumbed through to an ACPI
> "WMAA" method interface, which in turn can be wrapped by WMI with
> the GUID handle 603E9613-EF25-4338-A3D0-C46177516DB7.
>
> Add a new driver, aliased to the WMAA WMI GUID, to expose a sysfs
> backlight class driver to control backlight levels on systems with
> EC-driven backlights.
>
> Signed-off-by: Aaron Plattner <aplattner@xxxxxxxxxx>
> Signed-off-by: Daniel Dadap <ddadap@xxxxxxxxxx>
> ---
>
> v2: Convert to WMI subsystem driver, suggested by Mario Limonciello
> <mario.limonciello@xxxxxxxxxxx>; various cleanups suggested by
> Barnabás Pőcze <pobrn@xxxxxxxxxxxxxx>
> v3: Address assorted style nits raised by Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx> in response to a related patch; remove
> additional behavior change to WMI subsystem from patch series as
> recommended by Hans de Goede <hdegoede@xxxxxxxxxx>
>
> MAINTAINERS | 6 +
> drivers/platform/x86/Kconfig | 10 ++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/wmaa-backlight-wmi.c | 184 ++++++++++++++++++++++
> 4 files changed, 201 insertions(+)
> create mode 100644 drivers/platform/x86/wmaa-backlight-wmi.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bbaecde94aa0..fd7362a86c6d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20008,6 +20008,12 @@ L: linux-wireless@xxxxxxxxxxxxxxx
> S: Odd fixes
> F: drivers/net/wireless/wl3501*
>
> +WMAA BACKLIGHT DRIVER
> +M: Daniel Dadap <ddadap@xxxxxxxxxx>
> +L: platform-driver-x86@xxxxxxxxxxxxxxx
> +S: Supported
> +F: drivers/platform/x86/wmaa-backlight-wmi.c
> +
> WOLFSON MICROELECTRONICS DRIVERS
> L: patches@xxxxxxxxxxxxxxxxxxxxx
> S: Supported
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index d12db6c316ea..e54449c16d03 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -113,6 +113,16 @@ config PEAQ_WMI
> help
> Say Y here if you want to support WMI-based hotkeys on PEAQ 2-in-1s.
>
> +config WMAA_BACKLIGHT_WMI
> + tristate "ACPI WMAA Backlight Driver"
> + depends on ACPI_WMI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + This driver provides a sysfs backlight interface for notebook
> + systems which expose the WMAA ACPI method and an associated WMI
> + wrapper to drive LCD backlight levels through the system's
> + Embedded Controller.
> +
> config XIAOMI_WMI
> tristate "Xiaomi WMI key driver"
> depends on ACPI_WMI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 7ee369aab10d..109c1714237d 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_INTEL_WMI_SBL_FW_UPDATE) += intel-wmi-sbl-fw-update.o
> obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o
> obj-$(CONFIG_MXM_WMI) += mxm-wmi.o
> obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o
> +obj-$(CONFIG_WMAA_BACKLIGHT_WMI) += wmaa-backlight-wmi.o
> obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o
> obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
>
> diff --git a/drivers/platform/x86/wmaa-backlight-wmi.c b/drivers/platform/x86/wmaa-backlight-wmi.c
> new file mode 100644
> index 000000000000..b607d3f88fc2
> --- /dev/null
> +++ b/drivers/platform/x86/wmaa-backlight-wmi.c
> @@ -0,0 +1,184 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/backlight.h>
> +#include <linux/module.h>
> +#include <linux/wmi.h>
> +
> +#define WMAA_WMI_GUID "603E9613-EF25-4338-A3D0-C46177516DB7"
> +
> +struct wmi_wmaa_priv {
> + struct backlight_device *backlight;
> +};
> +
> +enum wmaa_method {
> + WMAA_BRIGHTNESS_LEVEL = 1,
> + WMAA_BRIGHTNESS_SOURCE = 2,
> +};
> +
> +enum wmaa_get_or_set {
> + WMAA_GET = 0,
> + WMAA_SET = 1,
> + WMAA_GET_MAX = 2, // for WMAA_BRIGHTNESS_LEVEL only
/* this type of comments is preferred */
> +};
> +
> +enum wmaa_source {
> + WMAA_SOURCE_CLEAR = 0,
> + WMAA_SOURCE_GPU = 1,
> + WMAA_SOURCE_EC = 2,
> + WMAA_SOURCE_AUX = 3,
> + WMAA_SOURCE_COUNT
> +};
> +
> +struct wmaa_args {
> + u32 set;
> + u32 val;
> + u32 ret;
> + u32 ignored[3];
> +};
> +
> +static const struct wmi_device_id wmaa_backlight_wmi_id_table[] = {
> + { .guid_string = WMAA_WMI_GUID },
> + { },
> +};
> +
> +static struct wmi_device *wdev;
> +
> +static int wmi_call_wmaa(enum wmaa_method method, enum wmaa_get_or_set set, u32 *val)
> +{
> + struct wmaa_args args = {
> + .set = set,
> + .val = 0,
> + .ret = 0,
> + };
> + struct acpi_buffer buf = { (acpi_size)sizeof(args), &args };
> + acpi_status status;
> +
> + if (set == WMAA_SET)
> + args.val = *val;
> +
> + status = wmidev_evaluate_method(wdev, 0, method, &buf, &buf);
> + if (ACPI_FAILURE(status)) {
> + pr_err("ACPI WMAA failed with %s\n", acpi_format_exception(status));
I think this could be `dev_err()` instead.
> + return -EIO;
> + }
> +
> + if (set != WMAA_SET)
> + *val = args.ret;
> +
> + return 0;
> +}
> +
> +static int wmaa_get_brightness(u32 *level)
> +{
> + return wmi_call_wmaa(WMAA_BRIGHTNESS_LEVEL, WMAA_GET, level);
> +}
> +
> +static int wmaa_set_brightness(u32 level)
> +{
> + return wmi_call_wmaa(WMAA_BRIGHTNESS_LEVEL, WMAA_SET, &level);
> +}
> +
> +static int wmaa_backlight_update_status(struct backlight_device *bd)
> +{
> + return wmaa_set_brightness(bd->props.brightness);
> +}
> +
> +static int wmaa_backlight_get_brightness(struct backlight_device *bd)
> +{
> + u32 level;
> + int ret;
> +
> + ret = wmaa_get_brightness(&level);
> +
> + WARN_ON(ret != 0);
> + return ret == 0 ? level : 0;
> +}
> +
> +static const struct backlight_ops wmaa_backlight_ops = {
> + .update_status = wmaa_backlight_update_status,
> + .get_brightness = wmaa_backlight_get_brightness,
> +};
> +
> +static int wmaa_get_max_brightness(u32 *level)
> +{
> + return wmi_call_wmaa(WMAA_BRIGHTNESS_LEVEL, WMAA_GET_MAX, level);
> +}
> +
> +static int wmaa_get_brightness_source(u32 *source)
> +{
> + return wmi_call_wmaa(WMAA_BRIGHTNESS_SOURCE, WMAA_GET, source);
> +}
> +
> +static int wmaa_backlight_wmi_probe(struct wmi_device *w, const void *ctx)
> +{
> + struct backlight_properties props = {0};
> + struct wmi_wmaa_priv *priv;
> + u32 source;
> + int ret;
> +
> + priv = devm_kmalloc(&w->dev, sizeof(*priv), GFP_KERNEL);
> + if(!priv)
> + return -ENOMEM;
> +
> + wdev = w;
It seems odd to me that half of the state is per-device, but the other half is global.
> +
> + ret = wmaa_get_brightness_source(&source);
> + if (ret)
> + goto done;
> +
> + if (source != WMAA_SOURCE_EC) {
> + ret = -ENODEV;
> + goto done;
> + }
> +
> + // Register a backlight handler
> + props.type = BACKLIGHT_PLATFORM;
> + ret = wmaa_get_max_brightness(&props.max_brightness);
> + if (ret)
> + goto done;
> +
> + ret = wmaa_get_brightness(&props.brightness);
> + if (ret)
> + goto done;
> +
> + priv->backlight = backlight_device_register("wmaa_backlight",
> + NULL, NULL, &wmaa_backlight_ops, &props);
Have you looked at `devm_backlight_device_register()`?
> + if (IS_ERR(priv->backlight))
> + return PTR_ERR(priv->backlight);
> +
> + dev_set_drvdata(&w->dev, priv);
> +
> +done:
> + return ret;
> +}
> +
> +static void wmaa_backlight_wmi_remove(struct wmi_device *wdev)
> +{
> + struct wmi_wmaa_priv *priv = dev_get_drvdata(&wdev->dev);
> +
> + backlight_device_unregister(priv->backlight);
> +}
> +
> +static struct wmi_driver wmaa_backlight_wmi_driver = {
> + .driver = {
> + .name = "wmaa-backlight",
> + },
> + .probe = wmaa_backlight_wmi_probe,
> + .remove = wmaa_backlight_wmi_remove,
> + .id_table = wmaa_backlight_wmi_id_table,
> +};
> +
> +module_wmi_driver(wmaa_backlight_wmi_driver);
> +
> +MODULE_AUTHOR("Aaron Plattner <aplattner@xxxxxxxxxx>");
> +MODULE_AUTHOR("Daniel Dadap <ddadap@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("WMAA Backlight WMI driver");
> +MODULE_LICENSE("GPL v2");
> +
> +MODULE_ALIAS("wmi:"WMAA_WMI_GUID);
> --
> 2.20.1
Best regards,
Barnabás Pőcze
