| 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
| ||
|
Date: Thu, 4 Mar 2021 15:35:19 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Luke D Jones <luke@...nes.dev>, platform-driver-x86@...r.kernel.org
Cc: corentin.chary@...il.com, mgross@...ux.intel.com,
acpi4asus-user@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] WMI: asus: Reduce G14 and G15 match to min product name
Hi,
On 2/27/21 11:20 AM, Luke D Jones wrote:
> This patch reduces the product match for GA401 series laptops to
> the minimum string required.
>
> The GA401 series of laptops has a lengthy list of product
> variations in the 2020 series and the 2021 series refresh
> is using the same base product ID of GA401.
>
> The same is also true for the GA502 series, and the new GA503
> series which is added in this patch as a variant of the G15.
Thank you for your patch.
I msut say that I find it very strange that 2021 series laptops need
to use the Asus vendor specific WMI interface for backlight control.
I see that the GA401 GA502 and GA503 models are all models with
AMD 4000/5000 series CPUs + Nvidia 2060 series GPUs.
So I guess it may be possible that this is the right thing
to do, and I do realize that we are already doing this for the
listed models. But it seems weird.
Modern laptops almost always use the native backlight control
build into the drm/kms driver. And in some special cases
(hybrid GPU setups) they might use the good old ACPI-video
interface. But using vendor specific interfaces sounds very
wrong to me. That is something which was typically done on
pre Windows XP era hardware.
Have you tried passing acpi_backlight=video on the kernel commandline?
What is the output of ls /sys/class/backlight before and after this
patch?
What is the output of ls /sys/class/backlight when using
acpi_backlight=video on the kernel commandline?
If the ls output shows multiple interfaces have you tried using all
listed interfaces directly from sysfs / the commandline ?
(perhaps userspace is picking the wrong interface in the case there
are multiple interfaces?)
Note what you are doing now is the equivalent of passing
acpi_backlight=vendor, which again is a weird thing to do on
recent / new hardware.
Also your commit message seems to lack a lot of details like:
1. Do you own an effected or multiple affected models yourself on
which you tested this?
2. Was this tested by others on other models of these series?
3. I assume this was discussed with others in some mailinglist /
forum discussion please provide links to this discussion.
4. Has this been tested with with both the nouveau and the
nvidia binary driver or only with the nvidia binary driver ?
5. What were the symptoms / problems noticed before making this change
and how do things work after making the change?
Regards,
Hans
>
> Signed-off-by: Luke D Jones <luke@...nes.dev>
> ---
> drivers/platform/x86/asus-nb-wmi.c | 57 ++++--------------------------
> 1 file changed, 6 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index d41d7ad14be0..f4db67c3eba2 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -427,73 +427,28 @@ static const struct dmi_system_id asus_quirks[] = {
> },
> {
> .callback = dmi_matched,
> - .ident = "ASUSTeK COMPUTER INC. GA401IH",
> + .ident = "ASUSTeK COMPUTER INC. GA401",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "GA401IH"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "GA401"),
> },
> .driver_data = &quirk_asus_vendor_backlight,
> },
> {
> .callback = dmi_matched,
> - .ident = "ASUSTeK COMPUTER INC. GA401II",
> + .ident = "ASUSTeK COMPUTER INC. GA502",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "GA401II"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "GA502"),
> },
> .driver_data = &quirk_asus_vendor_backlight,
> },
> {
> .callback = dmi_matched,
> - .ident = "ASUSTeK COMPUTER INC. GA401IU",
> + .ident = "ASUSTeK COMPUTER INC. GA503",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "GA401IU"),
> - },
> - .driver_data = &quirk_asus_vendor_backlight,
> - },
> - {
> - .callback = dmi_matched,
> - .ident = "ASUSTeK COMPUTER INC. GA401IV",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "GA401IV"),
> - },
> - .driver_data = &quirk_asus_vendor_backlight,
> - },
> - {
> - .callback = dmi_matched,
> - .ident = "ASUSTeK COMPUTER INC. GA401IVC",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "GA401IVC"),
> - },
> - .driver_data = &quirk_asus_vendor_backlight,
> - },
> - {
> - .callback = dmi_matched,
> - .ident = "ASUSTeK COMPUTER INC. GA502II",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "GA502II"),
> - },
> - .driver_data = &quirk_asus_vendor_backlight,
> - },
> - {
> - .callback = dmi_matched,
> - .ident = "ASUSTeK COMPUTER INC. GA502IU",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "GA502IU"),
> - },
> - .driver_data = &quirk_asus_vendor_backlight,
> - },
> - {
> - .callback = dmi_matched,
> - .ident = "ASUSTeK COMPUTER INC. GA502IV",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "GA502IV"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "GA503"),
> },
> .driver_data = &quirk_asus_vendor_backlight,
> },
>
Powered by blists - more mailing lists