Linux ACPI
 help / color / mirror / Atom feed
From: Daniel Dadap <ddadap@nvidia.com>
To: Hans de Goede <johannes.goede@oss.qualcomm.com>
Cc: Lee Jones <lee@kernel.org>, Daniel Thompson <danielt@kernel.org>,
	Jingoo Han <jingoohan1@gmail.com>, Helge Deller <deller@gmx.de>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	linux-fbdev@vger.kernel.org,
	linux-acpi <linux-acpi@vger.kernel.org>,
	"platform-driver-x86@vger.kernel.org"
	<platform-driver-x86@vger.kernel.org>
Subject: Re: Plan to fix nvidia_wmi_ec backlight issues, help wanted
Date: Tue, 5 May 2026 10:51:02 -0500	[thread overview]
Message-ID: <afoR5jUHLPXpYJ8a@ddadap-lakeline.nvidia.com> (raw)
In-Reply-To: <f10c039b-e9ef-4aae-8291-8f1e71074586@oss.qualcomm.com>

Thanks, Hans -

On Tue, May 05, 2026 at 11:52:55AM +0200, Hans de Goede wrote:
> <resend adding pdx86 list, sorry>
> 
> Hi All,
> 
> A while ago Nvidia introduced a new Nvidia specific firmware method
> for controlling the backlight on laptops with runtime switchable
> Nvidia hybrid graphics.
> 
> This is supported through the nvidia-wmi-ec-backlight driver, but has
> never worked 100%.
> 
> The new WMI firmware interface has a method to ask the firmware if
> the backlight is controlled though the EC and this the new WMI interface
> should be used; or if it is directly controlled by the GPU and the GPU
> driver's native backlight support should be used.
> 
> There are several bugs in the implementation of this on various laptops:
> 
> 1. The method to get the backlight control source sometimes does not
> report the correct value.
> 
> 2. On some laptop models which method (native-gpu vs nvidia-wmi) works
> changes at runtime, e.g. when plugging in a charger.
> 
> Known affected laptop models/bug reports about this:
> - Acer Predator PH315-55: needs acpi_backlight=native
> - Dell G15 5515 with RTX 3050: *needs* acpi_backlight=native
> - Dell G15 5515 with RTX 3060: *breaks* with acpi_backlight=native
> - Lenovo Legion Slim 7 2021
> - https://bugzilla.kernel.org/show_bug.cgi?id=217026
> - https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/work_items/784
> - https://gitlab.freedesktop.org/drm/amd/-/issues/4512
> - https://bugzilla.suse.com/show_bug.cgi?id=1217750

I believe there were also a couple of cases where different builds of the
same notebook (as far as DMI quirks tables could detect) would be driven
by different methods, exacerbating problem (1) because adding a quirtk to
fix one notebook would break other versions of the "same" notebook. (Oh,
maybe that's the two G15 5515 bugs linked above.)

> 
> It turns out that under Windows both a WMI backlight driver and a GPU
> native backlight driver get installed and Windows simply always calls
> both when the backlight changes working around these kind of firmware
> bugs.
> 

Yes, it's frustrating that so many notebooks have implementations of this
interface that have bugs that get masked by Windows always using both of
the interfaces. I wonder whether Windows behaves this way because bugs of
this type are so prevalent, or if the bugs are prevalent because Windows
always behaved that way so nobody noticed.

> When this first came up, about 2 years ago, I came up with the below
> plan to fix this. Nvidia was supposed to work in implementing this,
> but so far an implementation of this plan has not happened.
> 
> Note I do not have time to work on this myself. I'm posting this here
> in the hope that either Nvidia will pick this up after all; or that
> someone else can make this happen.
> 
> I'm more then happy to help answering any questions which may come up
> while implementing this; and to review the resulting patches.
>

Sorry about that - I totally forgot about this proposal. I'm still happy
to help here, although if someone else is especially eager to implement
these changes, I'm also happy to answer questions, test, review, and help
in other ways as bandwidth allows. I have some clarifying questions about
the plan - it's been some time since we talked about this, so I apologize
if I've already asked these questions and you've already answered them.
 
> 
> The plan
> ========
> 
> 1. Modify the core backlight code to allow registering a backlight-device
> for in kernel use only, disabling the registering of a class device under
> /sys/class/backlight .
> 
> 2. Add a new backlight-aggregate.c backlight driver, which exports a
> devm_backlight_aggregate_register_or_add() function. Drivers can call
> this passing in an existing backlight-device (with its sysfs interface
> disabled). This either creates a singleton /sys/class/backlight/aggregate
> device, or adds the passed in backlight to the existing singleton instance
> if it already exists.
> 
> This new driver always exposes a range of 0-255 to userspace. This acts
> as a proxie for any backlight-devices registered to be part of
> the aggregate, forwarding any brightness set calls to all backlights,
> scaled to their actual range. For reads this will report the last set
> brightness value (starting with the value of the first registered
> backlight-device).
> 
> 3. Add a new nvidia_wmi_ec_and_native type to drivers/acpi/video_detect.c
> for both DMI quirks and commandline handling.
> 

So by default both the native and GPU drivers would register a "normal"
backlight handler, and if the "both" quirk is set (or if requested via
kernel command line) they'll register with the aggregate device?

Would it make sense to make this more generic, with a parameter which
can be set via command line or quirks for which backlight devices to
aggregate? And then maybe instead of a new "nvidia_wmi_ec_and_native"
backlight type in video_detect.c, it could be called "aggregate" or
something else along those lines, used in combination with the list of
backlight types to aggregate.

Have we seen systems where both the iGPU and dGPU register their own
native handlers? I vaguely recall seeing such a system, but I do not
keep careful notes about this sort of thing, so I'm not certain. If
there are indeed such systems, then "native" might not be specific
enough, and it may be necessary to explicitly name backlight drivers
individually in order to aggregate them.

> 
> 4. Modify acpi_video_backlight_use_native() to also return true if
> the __acpi_video_get_backlight_type() call there returns the new
> acpi_backlight_nvidia_wmi_ec_and_native.

> 5. Add a new devm_backlight_device_register_native() helper which
> calls __acpi_video_get_backlight_type(true, NULL) and if that returns
> the new nvidia_wmi_ec_and_native type then registers the passed in
> backlight with the flag to not register a sysfs class interface and
> then on success calls the new devm_backlight_aggregate_register_or_add()
> with the just registered backlight device; and if the returned type
> instead is acpi_backlight_native register the passed in backlight
> normally through devm_backlight_device_register()
> 
> 5. Modify the i915 and amdgpu drivers to use the new
> devm_backlight_device_register_native() helper. Since this new helper
> checks the backlight-type itself, drop acpi_video_backlight_use_native()
> checks *if* the registration is the only thing guarded by that check.
> 
> 6. drivers/platform/x86/nvidia-wmi-ec-backlight.c to not return
> early from probe when acpi_video_get_backlight_type() returns
> the new nvidia_wmi_ec_and_native type and for that type make it
> registers its backlight with the flag to not register a sysfs class
> interface and then on success calls the new
> devm_backlight_aggregate_register_or_add().
> 

There still isn't anything tying a particular sysfs backlight interface
node to a specific display panel, right? Is there a plan to eventually
support this? How would this interact with that plan? I'm just a little
worried about designing ourselves into a corner that makes things harder
to untangle individual devices later if the need arises.

> Regards,
> 
> Hans
> 

  reply	other threads:[~2026-05-05 15:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05  9:52 Plan to fix nvidia_wmi_ec backlight issues, help wanted Hans de Goede
2026-05-05 15:51 ` Daniel Dadap [this message]
2026-05-06  7:46   ` Armin Wolf
  -- strict thread matches above, loose matches on Subject: below --
2026-05-05  9:50 Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afoR5jUHLPXpYJ8a@ddadap-lakeline.nvidia.com \
    --to=ddadap@nvidia.com \
    --cc=danielt@kernel.org \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jingoohan1@gmail.com \
    --cc=johannes.goede@oss.qualcomm.com \
    --cc=lee@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox