Re: [Nouveau] [PATCH v5 17/31] ACPI: video: Add Nvidia WMI EC brightness control detection (v3)

[Daniel Dadap <ddadap@nvidia.com>]

On 8/25/22 9:37 AM, 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.
>
> Changes in v2:
> - Use the new shared nvidia-wmi-ec-backlight.h header for the
>    WMI firmware API definitions
> - ACPI_VIDEO can now be enabled on non X86 too,
>    adjust the Kconfig changes to match this.
>
> Changes in v3:
> - Use WMI_BRIGHTNESS_GUID define
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/acpi/Kconfig           |  1 +
>   drivers/acpi/video_detect.c    | 37 ++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/gma500/Kconfig |  2 ++
>   drivers/gpu/drm/i915/Kconfig   |  2 ++
>   include/acpi/video.h           |  1 +
>   5 files changed, 43 insertions(+)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 7802d8846a8d..44ad4b6bd234 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -212,6 +212,7 @@ config ACPI_VIDEO
>   	tristate "Video"
>   	depends on BACKLIGHT_CLASS_DEVICE
>   	depends on INPUT
> +	depends on ACPI_WMI || !X86
>   	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 cc9d0d91e268..4dc7fb865083 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -32,6 +32,7 @@
>   #include <linux/dmi.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
> +#include <linux/platform_data/x86/nvidia-wmi-ec-backlight.h>
>   #include <linux/types.h>
>   #include <linux/workqueue.h>
>   #include <acpi/video.h>
> @@ -75,6 +76,36 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
>   	return AE_OK;
>   }
>   
> +/* This depends on ACPI_WMI which is X86 only */
> +#ifdef CONFIG_X86


This could probably also provide the { return false; } stub which you 
have for non-x86 if the kernel is built without nvidia-wmi-ec-backight, 
e.g.:

#if defined(CONFIG_X86) && (defined(CONFIG_NVIDIA_WMI_EC_BACKLIGHT) || 
defined(CONFIG_NVIDIA_WMI_EC_BACKLIGHT_MODULE))

Although I suppose that would break things if somebody has a kernel that 
originally had NVIDIA_WMI_EC_BACKLIGHT=n in Kconfig, and then builds the 
nvidia-wmi-ec-backlight driver out-of-tree later. I don't know whether 
that's intended to be a supported use case, so I guess it is fine either 
way.


> +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(WMI_BRIGHTNESS_GUID, 0,
> +				     WMI_BRIGHTNESS_METHOD_SOURCE, &buf, &buf);
> +	if (ACPI_FAILURE(status))
> +		return false;
> +
> +	/*
> +	 * If brightness is handled by the EC then nvidia-wmi-ec-backlight
> +	 * should be used, else the GPU driver(s) should be used.
> +	 */
> +	return args.ret == WMI_BRIGHTNESS_SOURCE_EC;
> +}
> +#else
> +static bool nvidia_wmi_ec_supported(void)
> +{
> +	return false;
> +}
> +#endif
> +
>   /* 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)
> @@ -541,6 +572,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;
> @@ -553,6 +585,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)
> @@ -570,6 +603,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;
> +
>   	/* 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
>   	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)