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@xxxxxxxxxx>
> ---
> 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,
> },
>
