ACPI / video: update the skip case for acpi_video_device_in_dod
PATCH 1/1 ACPI / video: update the skip case for acpi_video_device_in_dod
If the firmware has declared more than 8 video output devices, and the one that control the internal panel's backlight is listed after the first 8 output devices, the _DOD will not include it due to the current i915 operation region implementation. As a result, we will not create a backlight device for it while we should. Solve this problem by special case the firmware that has 8+ output devices in that if we see such a firmware, we do not test if the device is in _DOD list. The creation of the backlight device will also enable the firmware to emit events on backlight hotkey press when the acpi_osi= cmdline option is specified on those affected ASUS laptops.
--- drivers/acpi/video.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/acpi/video.c +8 -2
@@ -155,6 +155,7 @@ struct acpi_video_bus {u8 dos_setting;struct acpi_video_enumerated_device *attached_array;u8 attached_count; +u8 child_count;struct acpi_video_bus_cap cap;struct acpi_video_bus_flags flags;struct list_head video_device_list;
@@ -1176,8 +1177,12 @@ static bool acpi_video_device_in_dod(struct acpi_video_device *device)struct acpi_video_bus *video = device->video;int i; -/* If we have a broken _DOD, no need to test */ -if (!video->attached_count) +/* +* If we have a broken _DOD or we have more than 8 output devices +* under the graphics controller node that we can't proper deal with +* in the operation region code currently, no need to test. +*/ +if (!video->attached_count || video->child_count >8 )return true;for (i =0 ; i < video->attached_count; i++) {
@@ -1430,6 +1435,7 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,dev_err (&dev->dev,"Can't attach device\n" );break ; } + video->child_count++; }return status; }
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 11 lines · Aaron Lu
If the firmware has declared more than 8 video output devices, and the one that control the internal panel's backlight is listed after the first 8 output devices, the
_DODwill not include it due to the current i915 operation region implementation. As a result, we will not create a backlight device for it while we should. Solve this problem by special case the firmware that has 8+ output devices in that if we see such a firmware, we do not test if the device is in_DODlist. The creation of the backlight device will also enable the firmware to emit events on backlight hotkey press when the acpi_osi= cmdline option is specified on those affected ASUS laptops.
Quoted 6 lines
Reported-and-tested-by Oleksij Rempel <[email protected]>Reported-and-tested-by Dmitry Tunin <[email protected]>Reported-and-tested-by Jimbo <[email protected]>Signed-off-by Aaron Lu <[email protected]>
Obviously I'd like to have the proper information in the opregion DIDL/CADL fields, but it's not going to happen soon, nor is it going to fit under stable size limits.
I like the simplicity here. FWIW,
Quoted 42 lines
--- drivers/acpi/video.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/acpi/video.c +8
-2@@ -155,6 +155,7 @@ struct acpi_video_bus {u8 dos_setting;struct acpi_video_enumerated_device *attached_array;u8 attached_count; +u8 child_count;struct acpi_video_bus_cap cap;struct acpi_video_bus_flags flags;struct list_head video_device_list;@@ -1176,8 +1177,12 @@ static bool acpi_video_device_in_dod(struct acpi_video_device *device)struct acpi_video_bus *video = device->video;int i; -/* If we have a broken _DOD, no need to test */ -if (!video->attached_count) +/* +* If we have a broken _DOD or we have more than 8 output devices +* under the graphics controller node that we can't proper deal with +* in the operation region code currently, no need to test. +*/ +if (!video->attached_count || video->child_count >8 )return true;for (i =0 ; i < video->attached_count; i++) {@@ -1430,6 +1435,7 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,dev_err (&dev->dev,"Can't attach device\n" );break ; } + video->child_count++; }return status; }Signature · 1 line
2.1.0
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 19 lines · Jani Nikula
Quoted 11 lines · Aaron Lu
If the firmware has declared more than 8 video output devices, and the one that control the internal panel's backlight is listed after the first 8 output devices, the
_DODwill not include it due to the current i915 operation region implementation. As a result, we will not create a backlight device for it while we should. Solve this problem by special case the firmware that has 8+ output devices in that if we see such a firmware, we do not test if the device is in_DODlist. The creation of the backlight device will also enable the firmware to emit events on backlight hotkey press when the acpi_osi= cmdline option is specified on those affected ASUS laptops.Quoted 6 lines
Reported-and-tested-by Oleksij Rempel <[email protected]>Reported-and-tested-by Dmitry Tunin <[email protected]>Reported-and-tested-by Jimbo <[email protected]>Signed-off-by Aaron Lu <[email protected]>
Quoted 7 lines
Obviously I'd like to have the proper information in the opregion DIDL/CADL fields, but it's not going to happen soon, nor is it going to fit under stable size limits.
I like the simplicity here. FWIW,
Acked-by Jani Nikula <[email protected]>
Thanks :-)
BTW, this patch is more of a fix for my previous commit 0b8db271f159 ("ACPI / video: check _DOD list when creating backlight devices") that made the acpi_video interface disappear on these affected laptops so I would like to fix it quickly. And I agree that a proper implementation of the DIDL/CADL list is needed so that people can get rid of the acpi_osi= cmdline option they currently need to get hotkey events.
Regards,
Aaron
Quoted 31 lines
Quoted 31 lines
--- drivers/acpi/video.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/acpi/video.c +7
-2@@ -155,6 +155,7 @@ struct acpi_video_bus {u8 dos_setting;struct acpi_video_enumerated_device *attached_array;u8 attached_count; +u8 child_count;struct acpi_video_bus_cap cap;struct acpi_video_bus_flags flags;struct list_head video_device_list;@@ -1176,8 +1177,12 @@ static bool acpi_video_device_in_dod(struct acpi_video_device *device)struct acpi_video_bus *video = device->video;int i; -/* If we have a broken _DOD, no need to test */ -if (!video->attached_count) +/* +* If we have a broken _DOD or we have more than 8 output devices +* under the graphics controller node that we can't proper deal with +* in the operation region code currently, no need to test. +*/ +if (!video->attached_count || video->child_count >8 )return true;
Quoted 13 lines
Quoted 12 lines
drivers/acpi/video.c +1
-0for (i =0 ; i < video->attached_count; i++) {@@ -1430,6 +1435,7 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,dev_err (&dev->dev,"Can't attach device\n" );break ; } + video->child_count++; }return status; }Signature · 1 line
2.1.0
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 28 lines · Aaron Lu
Quoted 19 lines · Jani Nikula
Quoted 11 lines · Aaron Lu
If the firmware has declared more than 8 video output devices, and the one that control the internal panel's backlight is listed after the first 8 output devices, the
_DODwill not include it due to the current i915 operation region implementation. As a result, we will not create a backlight device for it while we should. Solve this problem by special case the firmware that has 8+ output devices in that if we see such a firmware, we do not test if the device is in_DODlist. The creation of the backlight device will also enable the firmware to emit events on backlight hotkey press when the acpi_osi= cmdline option is specified on those affected ASUS laptops.Quoted 6 lines
Reported-and-tested-by Oleksij Rempel <[email protected]>Reported-and-tested-by Dmitry Tunin <[email protected]>Reported-and-tested-by Jimbo <[email protected]>Signed-off-by Aaron Lu <[email protected]>Quoted 7 lines
Obviously I'd like to have the proper information in the opregion DIDL/CADL fields, but it's not going to happen soon, nor is it going to fit under stable size limits.
I like the simplicity here. FWIW,
Acked-by Jani Nikula <[email protected]>
Quoted 8 lines
Thanks :-)
BTW, this patch is more of a fix for my previous commit
0b8db271f159("ACPI / video: check_DODlist when creating backlight devices") that made theacpi_videointerface disappear on these affected laptops so I would like to fix it quickly. And I agree that a proper implementation of the DIDL/CADL list is needed so that people can get rid of the acpi_osi= cmdline option they currently need to get hotkey events.
Patch applied, thanks!
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.