First report: G14 / 2020 - integrated graphics only (NVIDIA disabled) # cat /proc/cmdlineBOOT_IMAGE=(hd0,gpt2)/vmlinuz-5.10.19-200.fc33.x86_64 root=UUID=c3fe67fb-a740-440b-bc2d-e7cfc9f19a42 ro rootflags=subvol=@ rd.luks.uuid=luks-cf5c7a8a-d5da-45ae-b556-db9c78b359e5 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 acpi_backlight=video
1. # ls /sys/class/backlight acpi_video0 amdgpu_bl0 2. acpi_video0 max_brightness 49, brightness is changing amdgpu_bl0 max_brightness 255, brightness is changing Both equal bright at max value # cat /proc/cmdlineBOOT_IMAGE=(hd0,gpt2)/vmlinuz-5.10.19-200.fc33.x86_64 root=UUID=c3fe67fb-a740-440b-bc2d-e7cfc9f19a42 ro rootflags=subvol=@ rd.luks.uuid=luks-cf5c7a8a-d5da-45ae-b556-db9c78b359e5 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 acpi_backlight=native
3. # ls /sys/class/backlight amdgpu_bl0 4. amdgpu_bl0 max_brightness 255, brightness is changing # cat /proc/cmdlineBOOT_IMAGE=(hd0,gpt2)/vmlinuz-5.10.19-200.fc33.x86_64 root=UUID=c3fe67fb-a740-440b-bc2d-e7cfc9f19a42 ro rootflags=subvol=@ rd.luks.uuid=luks-cf5c7a8a-d5da-45ae-b556-db9c78b359e5 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 acpi_backlight=vendor
5. # ls /sys/class/backlight amdgpu_bl0 6. amdgpu_bl0 max_brightness 255, brightness is changing 5.10 kernel includes the quirk patches. Were you wanting data with that patch removed completely? (not just the submitted one) From what you've said, I think I expected acpi_backlight=video to work regardless of the patch being applied. Regards, Luke. On Fri, Mar 5, 2021 at 10:36, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
<note adding the pdx86 list back to the Cc> Hi Luke, On 3/5/21 10:21 AM, Luke Jones wrote:Hello Hans, Many thanks for taking the time to review this and provide some knowledge. Most of what has come about in this patch is through direct user with a fairly large community I maintain on Discord centered around a utility I wrote to make ROG laptop users lives a bit better - https://gitlab.com/asus-linux. The community has been beneficial to getting some realtime testing done, shortening time. In the case of the 2021 Ryzen laptops I'm pleased you asked questions. I asked the single G15 laptop owner we have to check functionality and yes, we don't need that code. It appears there were some crossed wires in conversations. I will amend the patch keeping the glob style got 2020 G14 and G15 as we do have concrete anecdotal evidence that all *those* models, and some un-added ones need the quirk.If possible I would like to see some more research done on those 2020 models too. Specifically it would be good to get the following info: 1. Output of "ls /sys/class/backlight" with "acpi_backlight=video" on the kernel commandline.2. Testing of *all* the present backlight devices with "acpi_backlight=video"on the kernel commandline. The user can test this by doing: cd /sys/class/backlight/$backlight-name cat max_brightness echo $value-between-0-and-max_brightness > brightness echo $another-value-between-0-and-max_brightness > brightness etc. And then see if the brightness of the screen actually changes. 3. Output of "ls /sys/class/backlight" with "acpi_backlight=native" on the kernel commandline.4. Testing of *all* the present backlight devices with "acpi_backlight=native"on the kernel commandline (see 2.) 5. Output of "ls /sys/class/backlight" with "acpi_backlight=vendor" on the kernel commandline.6. Testing of *all* the present backlight devices with "acpi_backlight=vendor"on the kernel commandline (see 2.) My hope/expectation is that acpi_backlight=video will also work, because as said using acpi_backlight=vendor is somewhat weird for 2020 / 2021 laptop models. Also typically the acpi-video backlight interface will have a larger max_brightness giving finer grained (more steps) brightness control. Regards, HansOn Thu, Mar 4, 2021 at 15:35, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: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 changeand how do things work after making the change? Regards, HansSigned-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.cindex 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, },
