Less strict requirements for video device detection

PATCH Applied Stefan Bader 3 participants 9 messages Aug 20, 2009 – Oct 13, 2009 ACPI

Share & tools

Stefan Bader

author

2009-08-20 09:14 UTC #0 lore raw

Hardware: Acer 6920G (from a bug report)

Another case of a broken BIOS. In this case there are several definitions for video bus devices but only one has _DOS and _DOD defined. All other definitions only have _DOD. In the past (2.6.27) _ADR was not evaluated to make sure of using a present video device, but with that bug brightness could be changed.

Now the video bus having _DOS and _DOD is detected as not being present. The other definitions are not considered because they are lacking the _DOS method. Using the attached patch, would cause the detection code to consider the other definitions and has been tested to enable backlight control.

Would this be an acceptable approach? From the ACPI spec it rather sounds like _DOD and _DOS must be present for a device for display switching and _DOS would indicate possible backlight control as well. So the question might not be so much is it the right thing than is it safe enough to allow more compatibility with broken implementations without causing other problems...

-Stefan

Quoted 1 line

From 302f148a5eee5226b15c1dec1dbdfb8326bfa17a Mon Sep 17 00:00:00 2001

From: Stefan Bader <[email protected]>
Date: Thu, 20 Aug 2009 10:45:58 +0200
Subject: [PATCH] acpi: video: Loosen strictness of video bus detection code

BugLink http://bugs.launchpad.net/bugs/333386

Currently a video bus device must (beside other criteria) define _DOD and _DOS methods to be considered a video device.
Some broken BIOSes prevented working backlight control by only defining both for one (non-existing bus) and only _DOD for the rest. With this patch in place the other bus definitions were considered too and backlight control started to work again.

Signed-off-by Stefan Bader <[email protected]>

---
 drivers/acpi/video.c        |    2 +-
 drivers/acpi/video_detect.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

drivers/acpi/video.c +1 -1

index 8851315..f9efeb8 100644
@@ -1083,7 +1083,7 @@ static int acpi_video_bus_check(struct acpi_video_bus *video)
 	 */
 
 	/* Does this device support video switching? */
-	if (video->cap._DOS) {
+	if (video->cap._DOS || video->cap._DOD) {
 		video->flags.multihead = 1;
 		status = 0;
 	}

drivers/acpi/video_detect.c +1 -1

index 7cd2b63..bee5e34 100644
@@ -82,7 +82,7 @@ long acpi_is_video_device(struct acpi_device *device)
 		return 0;
 
 	/* Does this device able to support video switching ? */
-	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) &&
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) ||
 	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOS", &h_dummy)))
 		video_caps |= ACPI_VIDEO_OUTPUT_SWITCHING;

Signature · 1 line

1.5.4.3

Zhang Rui

2009-08-21 01:19 UTC #1 lore raw

Quoted 5 lines · Stefan Bader

Hardware: Acer 6920G (from a bug report)
Another case of a broken BIOS. In this case there are several definitions for video bus devices but only one has _DOS and _DOD defined. All other definitions only have _DOD.

I have seen such kind of BIOS too.

Quoted 10 lines

In the past (2.6.27) _ADR was not evaluated to make sure of using a present video device, but with that bug brightness could be changed.
Now the video bus having _DOS and _DOD is detected as not being present. The other definitions are not considered because they are lacking the _DOS method. Using the attached patch, would cause the detection code to consider the other definitions and has been tested to enable backlight control.
Would this be an acceptable approach?

I think so. I generated a similar patch before, but didn't sent it out for some reason.
My suggestion is that we should also print out a warning message if _DOS is missed, what do you think?

thanks,
rui

Quoted 8 lines

From the ACPI spec it rather sounds like
_DOD and _DOS must be present for a device for display switching and _DOS would indicate possible backlight control as well. So the question might not be so much is it the right thing than is it safe enough to allow more compatibility with broken implementations without causing other problems...
-Stefan

Stefan Bader

author

2009-08-21 10:00 UTC #2 lore raw

Re: Less strict requirements for video device detection (v2)

Quoted 13 lines · Zhang Rui

Quoted 3 lines · Stefan Bader
Hardware: Acer 6920G (from a bug report)
Another case of a broken BIOS. In this case there are several definitions for video bus devices but only one has _DOS and _DOD defined. All other definitions only have _DOD.
I have seen such kind of BIOS too.
Quoted 5 lines
In the past (2.6.27) _ADR was not evaluated to make sure of using a present video device, but with that bug brightness could be changed.
Now the video bus having _DOS and _DOD is detected as not being present. The other definitions are not considered because they are lacking the _DOS method. Using the attached patch, would cause the detection code to consider the other definitions and has been tested to enable backlight control.

Quoted 6 lines

Quoted 1 line
Would this be an acceptable approach?
I think so. I generated a similar patch before, but didn't sent it out for some reason.
My suggestion is that we should also print out a warning message if _DOS is missed, what do you think?

Some indication about the problem can't hurt. Probably not in acpi_is_video_device as that would trigger for even unused devices.
So I added a warning to acpi_video_bus_check for the case when _DOS is missing. The case of _DOS being present but _DOD not might also be worth a warning but (though the check in acpi_is_video_device prevented this) would have been accepted by the current code.

-Stefan

Quoted 9 lines

thanks,
rui
Quoted 5 lines
From the ACPI spec it rather sounds like
_DOD and _DOS must be present for a device for display switching and _DOS would indicate possible backlight control as well. So the question might not be so much is it the right thing than is it safe enough to allow more compatibility with broken implementations without causing other problems...
-Stefan

Signature · 1 line

When all other means of communication fail, try words!

Quoted 1 line

From 6b483015524f67dee3ae2f08f3c0cef27c9d84c6 Mon Sep 17 00:00:00 2001

From: Stefan Bader <[email protected]>
Date: Fri, 21 Aug 2009 11:03:05 +0200
Subject: [PATCH] acpi: video: Loosen strictness of video bus detection code

BugLink http://bugs.launchpad.net/bugs/333386

Signed-off-by Stefan Bader <[email protected]>

---
 drivers/acpi/video.c        |    7 ++++++-
 drivers/acpi/video_detect.c |    2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

drivers/acpi/video.c +6 -1

index 8851315..acd4636 100644
@@ -1083,7 +1083,12 @@ static int acpi_video_bus_check(struct acpi_video_bus *video)
 	 */
 
 	/* Does this device support video switching? */
-	if (video->cap._DOS) {
+	if (video->cap._DOS || video->cap._DOD) {
+		if (!video->cap._DOS) {
+			printk(KERN_WARNING PREFIX
+				"BIOS bug: %s declares _DOD but not _DOS\n",
+				acpi_device_bid(video->device));
+		}
 		video->flags.multihead = 1;
 		status = 0;
 	}

drivers/acpi/video_detect.c +1 -1

index 7cd2b63..bee5e34 100644
@@ -82,7 +82,7 @@ long acpi_is_video_device(struct acpi_device *device)
 		return 0;
 
 	/* Does this device able to support video switching ? */
-	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) &&
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) ||
 	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOS", &h_dummy)))
 		video_caps |= ACPI_VIDEO_OUTPUT_SWITCHING;

Signature · 1 line

1.5.4.3

Zhang Rui

2009-08-24 01:20 UTC #3 lore raw

Re: Less strict requirements for video device detection (v2)

Quoted 21 lines · Stefan Bader

Quoted 20 lines · Zhang Rui
Quoted 5 lines · Stefan Bader
Hardware: Acer 6920G (from a bug report)
Another case of a broken BIOS. In this case there are several definitions for video bus devices but only one has _DOS and _DOD defined. All other definitions only have _DOD.
I have seen such kind of BIOS too.
Quoted 10 lines
In the past (2.6.27) _ADR was not evaluated to make sure of using a present video device, but with that bug brightness could be changed.
Now the video bus having _DOS and _DOD is detected as not being present. The other definitions are not considered because they are lacking the _DOS method. Using the attached patch, would cause the detection code to consider the other definitions and has been tested to enable backlight control.
Would this be an acceptable approach?

Quoted 8 lines

Quoted 4 lines
I think so. I generated a similar patch before, but didn't sent it out for some reason.
My suggestion is that we should also print out a warning message if _DOS is missed, what do you think?
Some indication about the problem can't hurt. Probably not in acpi_is_video_device as that would trigger for even unused devices. So I added a warning to acpi_video_bus_check for the case when _DOS is missing.

how about using printk(KERN_WARNING FW_BUG "blabla")?

thanks,
rui

Quoted 19 lines

The case of _DOS being present but _DOD not might also be worth a warning but (though the check in acpi_is_video_device prevented this) would have been accepted by the current code.
-Stefan
Quoted 12 lines
thanks,
rui
Quoted 8 lines
From the ACPI spec it rather sounds like
_DOD and _DOS must be present for a device for display switching and _DOS would indicate possible backlight control as well. So the question might not be so much is it the right thing than is it safe enough to allow more compatibility with broken implementations without causing other problems...
-Stefan

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Stefan Bader

author

2009-08-24 14:44 UTC #4 lore raw

Re: Less strict requirements for video device detection (v3)

Quoted 14 lines · Zhang Rui

Quoted 13 lines · Stefan Bader
Quoted 12 lines · Zhang Rui
Quoted 3 lines · Stefan Bader
Hardware: Acer 6920G (from a bug report)
Another case of a broken BIOS. In this case there are several definitions for video bus devices but only one has _DOS and _DOD defined. All other definitions only have _DOD.
I have seen such kind of BIOS too.
Quoted 6 lines
In the past (2.6.27) _ADR was not evaluated to make sure of using a present video device, but with that bug brightness could be changed.
Now the video bus having _DOS and _DOD is detected as not being present. The other definitions are not considered because they are lacking the _DOS method. Using the attached patch, would cause the detection code to consider the other definitions and has been tested to enable backlight control.
Would this be an acceptable approach?

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
I think so. I generated a similar patch before, but didn't sent it out for some reason.
My suggestion is that we should also print out a warning message if _DOS is missed, what do you think?
Some indication about the problem can't hurt. Probably not in acpi_is_video_device as that would trigger for even unused devices. So I added a warning to acpi_video_bus_check for the case when _DOS is missing.
how about using printk(KERN_WARNING FW_BUG "blabla")?

I am not biased on that.

-Stefan

Quoted 16 lines

thanks,
rui
Quoted 12 lines
The case of _DOS being present but _DOD not might also be worth a warning but (though the check in acpi_is_video_device prevented this) would have been accepted by the current code. -Stefan
Quoted 8 lines
thanks,
rui
Quoted 5 lines
From the ACPI spec it rather sounds like
_DOD and _DOS must be present for a device for display switching and _DOS would indicate possible backlight control as well. So the question might not be so much is it the right thing than is it safe enough to allow more compatibility with broken implementations without causing other problems...
-Stefan

Signature · 1 line

When all other means of communication fail, try words!

Quoted 1 line

From 6b483015524f67dee3ae2f08f3c0cef27c9d84c6 Mon Sep 17 00:00:00 2001

From: Stefan Bader <[email protected]>
Date: Fri, 21 Aug 2009 11:03:05 +0200
Subject: [PATCH] acpi: video: Loosen strictness of video bus detection code

BugLink http://bugs.launchpad.net/bugs/333386

Signed-off-by Stefan Bader <[email protected]>

---
 drivers/acpi/video.c        |    7 ++++++-
 drivers/acpi/video_detect.c |    2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

drivers/acpi/video.c +6 -1

index 8851315..acd4636 100644
@@ -1083,7 +1083,12 @@ static int acpi_video_bus_check(struct acpi_video_bus *video)
 	 */
 
 	/* Does this device support video switching? */
-	if (video->cap._DOS) {
+	if (video->cap._DOS || video->cap._DOD) {
+		if (!video->cap._DOS) {
+			printk(KERN_WARNING FW_BUG
+				"ACPI(%s) defines _DOD but not _DOS\n",
+				acpi_device_bid(video->device));
+		}
 		video->flags.multihead = 1;
 		status = 0;
 	}

drivers/acpi/video_detect.c +1 -1

index 7cd2b63..bee5e34 100644
@@ -82,7 +82,7 @@ long acpi_is_video_device(struct acpi_device *device)
 		return 0;
 
 	/* Does this device able to support video switching ? */
-	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) &&
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) ||
 	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOS", &h_dummy)))
 		video_caps |= ACPI_VIDEO_OUTPUT_SWITCHING;

Signature · 1 line

1.5.4.3

Zhang Rui

Acked-by

2009-08-25 01:09 UTC #5 lore raw

Re: Less strict requirements for video device detection (v3)

Acked-by Zhang Rui <[email protected]>

Quoted 28 lines · Stefan Bader

Quoted 27 lines · Zhang Rui
Quoted 25 lines · Stefan Bader
Quoted 16 lines · Zhang Rui
Quoted 5 lines · Stefan Bader
Hardware: Acer 6920G (from a bug report)
Another case of a broken BIOS. In this case there are several definitions for video bus devices but only one has _DOS and _DOD defined. All other definitions only have _DOD.
I have seen such kind of BIOS too.
Quoted 8 lines
In the past (2.6.27) _ADR was not evaluated to make sure of using a present video device, but with that bug brightness could be changed.
Now the video bus having _DOS and _DOD is detected as not being present. The other definitions are not considered because they are lacking the _DOS method. Using the attached patch, would cause the detection code to consider the other definitions and has been tested to enable backlight control.
Quoted 5 lines
Quoted 1 line
Would this be an acceptable approach?
I think so. I generated a similar patch before, but didn't sent it out for some reason.
My suggestion is that we should also print out a warning message if _DOS is missed, what do you think?
Some indication about the problem can't hurt. Probably not in acpi_is_video_device as that would trigger for even unused devices. So I added a warning to acpi_video_bus_check for the case when _DOS is missing.

Quoted 28 lines

Quoted 1 line
how about using printk(KERN_WARNING FW_BUG "blabla")?
I am not biased on that.
-Stefan
Quoted 21 lines
thanks,
rui
Quoted 17 lines
The case of _DOS being present but _DOD not might also be worth a warning but (though the check in acpi_is_video_device prevented this) would have been accepted by the current code.
-Stefan
Quoted 11 lines
thanks,
rui
Quoted 8 lines
From the ACPI spec it rather sounds like
_DOD and _DOS must be present for a device for display switching and _DOS would indicate possible backlight control as well. So the question might not be so much is it the right thing than is it safe enough to allow more compatibility with broken implementations without causing other problems...
-Stefan

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Stefan Bader

author Acked-by Zhang Rui

2009-09-30 20:23 UTC #6 lore raw

Re: Less strict requirements for video device detection (v3)

Resending. It feels like being lost.

-Stefan

Quoted 34 lines · Zhang Rui

Acked-by Zhang Rui <[email protected]>
Quoted 31 lines · Stefan Bader
Quoted 13 lines · Zhang Rui
Quoted 12 lines · Stefan Bader
Quoted 11 lines · Zhang Rui
Quoted 3 lines · Stefan Bader
Hardware: Acer 6920G (from a bug report)
Another case of a broken BIOS. In this case there are several definitions for video bus devices but only one has _DOS and _DOD defined. All other definitions only have _DOD.
I have seen such kind of BIOS too.
Quoted 5 lines
In the past (2.6.27) _ADR was not evaluated to make sure of using a present video device, but with that bug brightness could be changed.
Now the video bus having _DOS and _DOD is detected as not being present. The other definitions are not considered because they are lacking the _DOS method. Using the attached patch, would cause the detection code to consider the other definitions and has been tested to enable backlight control.
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Would this be an acceptable approach?
I think so. I generated a similar patch before, but didn't sent it out for some reason.
My suggestion is that we should also print out a warning message if _DOS is missed, what do you think?
Some indication about the problem can't hurt. Probably not in acpi_is_video_device as that would trigger for even unused devices. So I added a warning to acpi_video_bus_check for the case when _DOS is missing.
how about using printk(KERN_WARNING FW_BUG "blabla")?
I am not biased on that.
-Stefan
Quoted 6 lines
thanks,
rui
Quoted 3 lines
The case of _DOS being present but _DOD not might also be worth a warning but (though the check in acpi_is_video_device prevented this) would have been accepted by the current code. -Stefan

Quoted 8 lines

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
thanks,
rui
Quoted 4 lines
From the ACPI spec it rather sounds like
_DOD and _DOS must be present for a device for display switching and _DOS would indicate possible backlight control as well. So the question might not be so much is it the right thing than is it safe enough to allow more compatibility with broken implementations without causing other problems...
-Stefan
From dbd8ac9a49a1b11eb7c5a49cfd44a93f1a5dd921 Mon Sep 17 00:00:00 2001

From: Stefan Bader <[email protected]>
Date: Fri, 21 Aug 2009 11:03:05 +0200
Subject: [PATCH] acpi: video: Loosen strictness of video bus detection code

BugLink http://bugs.launchpad.net/bugs/333386

Signed-off-by Stefan Bader <[email protected]>

Acked-by Zhang Rui <[email protected]>

---
 drivers/acpi/video.c        |    7 ++++++-
 drivers/acpi/video_detect.c |    2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

drivers/acpi/video.c +6 -1

index a4fddb2..518910e 100644
@@ -1109,7 +1109,12 @@ static int acpi_video_bus_check(struct acpi_video_bus *video)
 	 */
 
 	/* Does this device support video switching? */
-	if (video->cap._DOS) {
+	if (video->cap._DOS || video->cap._DOD) {
+		if (!video->cap._DOS) {
+			printk(KERN_WARNING FW_BUG
+				"ACPI(%s) defines _DOD but not _DOS\n",
+				acpi_device_bid(video->device));
+		}
 		video->flags.multihead = 1;
 		status = 0;
 	}

drivers/acpi/video_detect.c +1 -1

index 7032f25..575593a 100644
@@ -84,7 +84,7 @@ long acpi_is_video_device(struct acpi_device *device)
 		return 0;
 
 	/* Does this device able to support video switching ? */
-	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) &&
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) ||
 	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOS", &h_dummy)))
 		video_caps |= ACPI_VIDEO_OUTPUT_SWITCHING;

Signature · 1 line

1.5.4.3

Zhang Rui

Acked-by

2009-10-09 01:32 UTC #7 lore raw

Re: Less strict requirements for video device detection (v3)

Quoted 4 lines · Stefan Bader

Resending. It feels like being lost.
-Stefan

Hi, len,

can you apply this patch please? thanks.

From: Stefan Bader <[email protected]>
Subject: [PATCH] acpi: video: Loosen strictness of video bus detection code

BugLink http://bugs.launchpad.net/bugs/333386

Signed-off-by Stefan Bader <[email protected]>

Acked-by Zhang Rui <[email protected]>

---
 drivers/acpi/video.c        |    7 ++++++-
 drivers/acpi/video_detect.c |    2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

drivers/acpi/video.c +6 -1

index a4fddb2..518910e 100644
@@ -1109,7 +1109,12 @@ static int acpi_video_bus_check(struct acpi_video_bus *video)
 	 */
 
 	/* Does this device support video switching? */
-	if (video->cap._DOS) {
+	if (video->cap._DOS || video->cap._DOD) {
+		if (!video->cap._DOS) {
+			printk(KERN_WARNING FW_BUG
+				"ACPI(%s) defines _DOD but not _DOS\n",
+				acpi_device_bid(video->device));
+		}
 		video->flags.multihead = 1;
 		status = 0;
 	}

drivers/acpi/video_detect.c +1 -1

index 7032f25..575593a 100644
@@ -84,7 +84,7 @@ long acpi_is_video_device(struct acpi_device *device)
 		return 0;
 
 	/* Does this device able to support video switching ? */
-	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) &&
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) ||
 	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOS", &h_dummy)))
 		video_caps |= ACPI_VIDEO_OUTPUT_SWITCHING;

Signature · 1 line

1.5.4.3

Len Brown

reviewer Applied

2009-10-13 06:53 UTC #8 lore raw

Re: Less strict requirements for video device detection (v3)

Quoted 1 line

[PATCH] acpi: video: Loosen strictness of video bus detection code

applied

thanks,
Len Brown, Intel Open Source Technology Center