| Message ID | 548E9559.3080707@intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Mon, 15 Dec 2014, Aaron Lu <aaron.lu@intel.com> wrote: > 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. > > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=70241 > Reported-and-tested-by: Oleksij Rempel <linux@rempel-privat.de> > Reported-and-tested-by: Dmitry Tunin <hanipouspilot@gmail.com> > Reported-and-tested-by: Jimbo <jaime.91@hotmail.es> > Cc: stable@vger.kernel.org > Signed-off-by: Aaron Lu <aaron.lu@intel.com> 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 <jani.nikula@intel.com> > --- > drivers/acpi/video.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c > index 8b844931b936..c72e79d2c5ad 100644 > --- a/drivers/acpi/video.c > +++ b/drivers/acpi/video.c > @@ -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; > } > -- > 2.1.0 >
On 12/15/2014 05:02 PM, Jani Nikula wrote: > On Mon, 15 Dec 2014, Aaron Lu <aaron.lu@intel.com> wrote: >> 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. >> >> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=70241 >> Reported-and-tested-by: Oleksij Rempel <linux@rempel-privat.de> >> Reported-and-tested-by: Dmitry Tunin <hanipouspilot@gmail.com> >> Reported-and-tested-by: Jimbo <jaime.91@hotmail.es> >> Cc: stable@vger.kernel.org >> Signed-off-by: Aaron Lu <aaron.lu@intel.com> > > 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 <jani.nikula@intel.com> 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 > >> --- >> drivers/acpi/video.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c >> index 8b844931b936..c72e79d2c5ad 100644 >> --- a/drivers/acpi/video.c >> +++ b/drivers/acpi/video.c >> @@ -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; >> } >> -- >> 2.1.0 >> > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, December 16, 2014 11:21:15 AM Aaron Lu wrote: > On 12/15/2014 05:02 PM, Jani Nikula wrote: > > On Mon, 15 Dec 2014, Aaron Lu <aaron.lu@intel.com> wrote: > >> 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. > >> > >> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=70241 > >> Reported-and-tested-by: Oleksij Rempel <linux@rempel-privat.de> > >> Reported-and-tested-by: Dmitry Tunin <hanipouspilot@gmail.com> > >> Reported-and-tested-by: Jimbo <jaime.91@hotmail.es> > >> Cc: stable@vger.kernel.org > >> Signed-off-by: Aaron Lu <aaron.lu@intel.com> > > > > 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 <jani.nikula@intel.com> > > 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. Patch applied, thanks!
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 8b844931b936..c72e79d2c5ad 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -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; }
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. Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=70241 Reported-and-tested-by: Oleksij Rempel <linux@rempel-privat.de> Reported-and-tested-by: Dmitry Tunin <hanipouspilot@gmail.com> Reported-and-tested-by: Jimbo <jaime.91@hotmail.es> Cc: stable@vger.kernel.org Signed-off-by: Aaron Lu <aaron.lu@intel.com> --- drivers/acpi/video.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)