nouveau exposes backlight controls in presence of ACPI

nouveau exposes backlight controls in presence of ACPI

[Aaron Sowry]

[-- Attachment #1.1: Type: text/plain, Size: 315 bytes --]

It seems to be de facto standard to only expose kernel-level backlight controls in the absence of ACPI, however nouveau does this regardless of whether ACPI equivalents are available or not. This will confuse userspace in some situations. Does this need patching or is there some reason for this behaviour?

/Aaron

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

[-- Attachment #2: Type: text/plain, Size: 181 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: nouveau exposes backlight controls in presence of ACPI

[Ben Skeggs]

On Fri, 2010-10-29 at 17:49 +0200, Aaron Sowry wrote:
> It seems to be de facto standard to only expose kernel-level backlight controls in the absence of ACPI, however nouveau does this regardless of whether ACPI equivalents are available or not. This will confuse userspace in some situations. Does this need patching or is there some reason for this behaviour?
Yes, it appears that you're correct here.  I took a look into what is
supposed to happen, and it looks fairly trivial to implement and stop
nouveau providing nv_backlight if video.ko is already providing it.

However.  There's also the platform-specific modules (thinkpad etc) that
will provide their own backlight methods if the standard ACPI mechanism
isn't available.  I didn't see any immediately obvious way of knowing
whether or not they were being provided.

I'm not too certain the best way to deal with this, any ideas? :)

Ben.
> 
> /Aaron
> _______________________________________________
> Nouveau mailing list
> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: nouveau exposes backlight controls in presence of ACPI

[Aaron Sowry]

[-- Attachment #1.1.1: Type: text/plain, Size: 1508 bytes --]

> However.  There's also the platform-specific modules (thinkpad etc) that
> will provide their own backlight methods if the standard ACPI mechanism
> isn't available.  I didn't see any immediately obvious way of knowing
> whether or not they were being provided.
> 
> I'm not too certain the best way to deal with this, any ideas? :)

See attached patch, which prevents nouveau registering a backlight control if ACPI already provides one. If there are no glaring errors in the code/implementation then I would like to submit it.

Dealing with platform-specific modules is tricky because it looks like nouveau initialises itself quite early on in the boot process:

[aaron@jules ~]$ dmesg | grep ACPI
...
[    3.359377] nouveau 0000:01:00.0: power state changed by ACPI to D0
[    3.359560] nouveau 0000:01:00.0: power state changed by ACPI to D0
[    4.860678] [drm] nouveau 0000:01:00.0: Detected ACPI backlight support, not registering control
[   14.206997] ACPI: WMI: Skipping duplicate GUID 05901221-D566-11D1-B2F0-00A0C9062910
[   14.207693] ACPI: WMI: Mapper loaded
[   17.686907] thinkpad_acpi: ThinkPad ACPI Extras v0.24
[   17.689517] thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver
[   17.756039] thinkpad_acpi: Standard ACPI backlight interface available, not loading native one.

My suggestion would be that it is the responsibility of user-space to select an appropriate ACPI backlight control.

/Aaron

[-- Attachment #1.1.2: nouveau_state.patch --]
[-- Type: text/x-diff, Size: 839 bytes --]

--- kernel-2.6.35.fc14.orig/drivers/gpu/drm/nouveau/nouveau_state.c	2010-11-02 13:04:25.433000083 +0100
+++ kernel-2.6.35.fc14.new/drivers/gpu/drm/nouveau/nouveau_state.c	2010-11-02 15:18:26.116693011 +0100
@@ -23,6 +23,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include <linux/acpi.h>
 #include <linux/swab.h>
 #include <linux/slab.h>
 #include "drmP.h"
@@ -600,9 +601,13 @@ nouveau_card_init(struct drm_device *dev
 			goto out_irq;
 	}
 
+	if (acpi_video_backlight_support())
+		NV_INFO(dev, "Detected ACPI backlight support, not registering control\n");
+	else {
 	ret = nouveau_backlight_init(dev);
-	if (ret)
-		NV_ERROR(dev, "Error %d registering backlight\n", ret);
+		if (ret)
+			NV_ERROR(dev, "Error %d registering backlight\n", ret);
+	}
 
 	nouveau_fbcon_init(dev);
 	drm_kms_helper_poll_init(dev);

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

[-- Attachment #2: Type: text/plain, Size: 181 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: nouveau exposes backlight controls in presence of ACPI

[Calvin Walton]

On Tue, 2010-11-02 at 13:58 +1000, Ben Skeggs wrote:
> On Fri, 2010-10-29 at 17:49 +0200, Aaron Sowry wrote:
> > It seems to be de facto standard to only expose kernel-level
> backlight controls in the absence of ACPI, however nouveau does this
> regardless of whether ACPI equivalents are available or not. This will
> confuse userspace in some situations. Does this need patching or is
> there some reason for this behaviour?
> Yes, it appears that you're correct here.  I took a look into what is
> supposed to happen, and it looks fairly trivial to implement and stop
> nouveau providing nv_backlight if video.ko is already providing it.
> 
> However.  There's also the platform-specific modules (thinkpad etc) that
> will provide their own backlight methods if the standard ACPI mechanism
> isn't available.  I didn't see any immediately obvious way of knowing
> whether or not they were being provided.
> 
> I'm not too certain the best way to deal with this, any ideas? :)

It was my understanding that the future direction the kernel was going
to go with this is that all detected backlight devices would be
exported, with a tag in sysfs describing the type of interface
(platform, firmware, direct hardware, whichever). Then it would be up to
userspace to pick the correct backlight device to use based on the tags.
Nothing stopping a user from manipulating a different one directly, with
possible interesting side-effects, of course.

This doesn't seem to be ready yet, though... I've only seen some related
discussion on LKML a while ago.

Note that the thinkpad-acpi module at the moment is also checking for
the ACPI video device to be loaded, and will disable itself in that
case. So the case that causes problems right now is if you have e.g.
both thinkpad-acpi and nouveau, but no ACPI video device?

-- 
Calvin Walton <calvin.walton-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: nouveau exposes backlight controls in presence of ACPI

[Aaron Sowry]

[-- Attachment #1.1: Type: text/plain, Size: 1853 bytes --]

> It was my understanding that the future direction the kernel was going
> to go with this is that all detected backlight devices would be
> exported, with a tag in sysfs describing the type of interface
> (platform, firmware, direct hardware, whichever). Then it would be up to
> userspace to pick the correct backlight device to use based on the tags.
> Nothing stopping a user from manipulating a different one directly, with
> possible interesting side-effects, of course.

Interesting. I guess this would be the best long-term solution, but short-term we can't proceed based on speculation. Nor can we really trust userspace to make the correct decisions at the moment, as the controls don't normally provide enough information to be able to do that reliably.

> Note that the thinkpad-acpi module at the moment is also checking for
> the ACPI video device to be loaded, and will disable itself in that
> case. So the case that causes problems right now is if you have e.g.
> both thinkpad-acpi and nouveau, but no ACPI video device?

The problem in my case is when both ACPI and nouveau controls are exposed - gnome-power-manager prioritises the nouveau control over ACPI (I've raised a bug for this too) and because my card isn't supported properly by the driver, I get a range of approximately useless brightness levels. As I see it, we have 3 related issues when it comes to backlight control:

0) Fixing NV50 backlight support in nouveau for those chipsets with max_brightness > 1025
1) Determining under which circumstances we should expose kernel-level controls (IMHO only when no better alternatives are available)
2) Determining how userspace should select an appropriate control when more than one is available

Matthew Garrett addresses some of those issues in a fairly decent rant here:
http://mjg59.livejournal.com/127103.html

/Aaron

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

[-- Attachment #2: Type: text/plain, Size: 181 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: nouveau exposes backlight controls in presence of ACPI

[Ben Skeggs]

On Tue, 2010-11-02 at 15:51 +0100, Aaron Sowry wrote:
> > However.  There's also the platform-specific modules (thinkpad etc) that
> > will provide their own backlight methods if the standard ACPI mechanism
> > isn't available.  I didn't see any immediately obvious way of knowing
> > whether or not they were being provided.
> > 
> > I'm not too certain the best way to deal with this, any ideas? :)
> 
> See attached patch, which prevents nouveau registering a backlight control if ACPI already provides one. If there are no glaring errors in the code/implementation then I would like to submit it.
This is much like what I intended to commit.  I guess it doesn't hurt to
do so, it doesn't help with all cases yet but if it fixes issues for
some people, why not.

Ben.
> 
> Dealing with platform-specific modules is tricky because it looks like nouveau initialises itself quite early on in the boot process:
> 
> [aaron@jules ~]$ dmesg | grep ACPI
> ...
> [    3.359377] nouveau 0000:01:00.0: power state changed by ACPI to D0
> [    3.359560] nouveau 0000:01:00.0: power state changed by ACPI to D0
> [    4.860678] [drm] nouveau 0000:01:00.0: Detected ACPI backlight support, not registering control
> [   14.206997] ACPI: WMI: Skipping duplicate GUID 05901221-D566-11D1-B2F0-00A0C9062910
> [   14.207693] ACPI: WMI: Mapper loaded
> [   17.686907] thinkpad_acpi: ThinkPad ACPI Extras v0.24
> [   17.689517] thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver
> [   17.756039] thinkpad_acpi: Standard ACPI backlight interface available, not loading native one.
> 
> My suggestion would be that it is the responsibility of user-space to select an appropriate ACPI backlight control.
> 
> /Aaron