| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <edec5950-cec8-b647-ccb1-ba48f9b3bbb0@daynix.com>
Date: Mon, 24 Oct 2022 21:58:57 +0900
From: Akihiko Odaki <akihiko.odaki@...nix.com>
To: Hans de Goede <hdegoede@...hat.com>
Cc: David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Jonathan Corbet <corbet@....net>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
"Lee, Chun-Yi" <jlee@...e.com>, Mark Gross <markgross@...nel.org>,
Corentin Chary <corentin.chary@...il.com>,
Cezary Jackiewicz <cezary.jackiewicz@...il.com>,
Matthew Garrett <mjg59@...f.ucam.org>,
Pali Rohár <pali@...nel.org>,
Jonathan Woithe <jwoithe@...t42.net>,
Ike Panhc <ike.pan@...onical.com>,
Daniel Dadap <ddadap@...dia.com>,
Kenneth Chan <kenneth.t.chan@...il.com>,
Mattia Dongili <malattia@...ux.it>,
Henrique de Moraes Holschuh <hmh@....eng.br>,
Azael Avalos <coproscefalo@...il.com>,
Lee Jones <lee@...nel.org>,
Daniel Thompson <daniel.thompson@...aro.org>,
Jingoo Han <jingoohan1@...il.com>,
Helge Deller <deller@....de>,
Robert Moore <robert.moore@...el.com>,
dri-devel@...ts.freedesktop.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
intel-gfx@...ts.freedesktop.org,
platform-driver-x86@...r.kernel.org,
acpi4asus-user@...ts.sourceforge.net,
ibm-acpi-devel@...ts.sourceforge.net, linux-fbdev@...r.kernel.org,
devel@...ica.org
Subject: Re: [PATCH 00/22] Fallback to native backlight
On 2022/10/24 20:53, Hans de Goede wrote:
> Hi Akihiko,
>
> On 10/24/22 13:34, Akihiko Odaki wrote:
>> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
>> helper") and following commits made native backlight unavailable if
>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>> unavailable, which broke the backlight functionality on Lenovo ThinkPad
>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>> cases.
>
> I appreciate your work on this, but what this in essence does is
> it allows 2 backlight drivers (vendor + native) to get registered
> for the same panel again. While the whole goal of the backlight refactor
> series landing in 6.1 was to make it so that there always is only
> *1* backlight device registered instead of (possibly) registering
> multiple and letting userspace figure it out. It is also important
> to only always have 1 backlight device per panel for further
> upcoming changes.
>
> So nack for this solution, sorry.
>
> I am aware that this breaks backlight control on some Chromebooks,
> this was already reported and I wrote a long reply explaining why
> things are done the way they are done now and also suggesting
> 2 possible (much simpler) fixes, see:
> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>
> Unfortunately the reported has not followed-up on this and
> I don't have the hardware to test this myself.
>
> Can you please try implementing 1 of the fixes suggested there
> and then submit that upstream ?
>
> Regards,
>
> Hans
>
Hi Hans,
Thanks for reviewing and letting me know the prior attempt.
In this case, there is only a native backlight device and no vendor
backlight device so the duplication of backlight devices does not
happen. I think it is better to handle such a case without quirks.
I understand it is still questionable to cover the case by allowing
duplication when both of a vendor backlight device and native one. To
explain my understanding and reasoning for *not* trying to apply the
de-duplication rule to the vendor/native combination, let me first
describe that the de-duplication which happens in
acpi_video_get_backlight_type() is a heuristics and limited.
As the background of acpi_video_get_backlight_type(), there is an
assumption that it should be common that both of the firmware,
implementing ACPI, and the kernel have code to drive backlight. In the
case, the more reliable one should be picked instead of using both, and
that is what the statements in `if (video_caps & ACPI_VIDEO_BACKLIGHT)`
does.
However, the method has two limitations:
1. It does not cover the case where two backlight devices with the same
type exist.
2. The underlying assumption does not apply to vendor/native combination.
Regarding the second limitation, I don't even understand the difference
between vendor and native. My guess is that a vendor backlight device
uses vendor-specific ACPI interface, and a native one directly uses
hardware registers. If my guess is correct, the difference between
vendor and native does not imply that both of them are likely to exist
at the same time. As the conclusion, there is no more motivation to try
to de-duplicate the vendor/native combination than to try to
de-duplicate combination of devices with a single type.
Of course, it is better if we could also avoid registering two devices
with one type for one physical device. Possibly we can do so by
providing a parameter to indicate that it is for the same "internal"
backlight to devm_backlight_device_register(), and let the function
check for the duplication. However, this rule may be too restrict, or
may have problems I missed.
Based on the discussion above, we can deduce three possibilities:
a. There is no reason to distinguish vendor and native in this case, and
we can stick to my current proposal.
b. There is a valid reason to distinguish vendor and native, and we can
adopt the same strategy that already adopted for ACPI video/vendor
combination.
c. We can implement de-duplication in devm_backlight_device_register().
d. The other possible options are not worth, and we can just implement
quirks specific to Chromebook/coreboot.
In case b, it should be noted that vendor and native backlight device do
not require ACPI video, and CONFIG_ACPI_VIDEO may not be enabled. In the
case, the de-duplication needs to be implemented in backlight class device.
Regards,
Akihiko Odaki
>
>
>
>
>
>
>
>
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@...nix.com>
>>
>> Akihiko Odaki (22):
>> drm/i915/opregion: Improve backlight request condition
>> ACPI: video: Introduce acpi_video_get_backlight_types()
>> LoongArch: Use acpi_video_get_backlight_types()
>> platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>> platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>> platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>> platform/x86: nvidia-wmi-ec-backlight: Use
>> acpi_video_get_backlight_types()
>> platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>> platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>> platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>> platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>> ACPI: video: Remove acpi_video_get_backlight_type()
>> ACPI: video: Fallback to native backlight
>>
>> Documentation/gpu/todo.rst | 8 +--
>> drivers/acpi/acpi_video.c | 2 +-
>> drivers/acpi/video_detect.c | 54 ++++++++++---------
>> drivers/gpu/drm/i915/display/intel_opregion.c | 3 +-
>> drivers/platform/loongarch/loongson-laptop.c | 4 +-
>> drivers/platform/x86/acer-wmi.c | 2 +-
>> drivers/platform/x86/asus-laptop.c | 2 +-
>> drivers/platform/x86/asus-wmi.c | 4 +-
>> drivers/platform/x86/compal-laptop.c | 2 +-
>> drivers/platform/x86/dell/dell-laptop.c | 2 +-
>> drivers/platform/x86/eeepc-laptop.c | 2 +-
>> drivers/platform/x86/fujitsu-laptop.c | 4 +-
>> drivers/platform/x86/ideapad-laptop.c | 2 +-
>> drivers/platform/x86/intel/oaktrail.c | 2 +-
>> drivers/platform/x86/msi-laptop.c | 2 +-
>> drivers/platform/x86/msi-wmi.c | 2 +-
>> .../platform/x86/nvidia-wmi-ec-backlight.c | 2 +-
>> drivers/platform/x86/panasonic-laptop.c | 2 +-
>> drivers/platform/x86/samsung-laptop.c | 2 +-
>> drivers/platform/x86/sony-laptop.c | 2 +-
>> drivers/platform/x86/thinkpad_acpi.c | 4 +-
>> drivers/platform/x86/toshiba_acpi.c | 2 +-
>> drivers/video/backlight/backlight.c | 18 +++++++
>> include/acpi/video.h | 21 ++++----
>> include/linux/backlight.h | 1 +
>> 25 files changed, 85 insertions(+), 66 deletions(-)
>>
>
Powered by blists - more mailing lists