Upstreaming

[flukejones]

Hi there, are you intending to upstream this to the kernel? If not, why not? And can I help you do this? (I've already contributed a lot of patches).

[Plippo]

I've hesitated because I fear the patch would probably not be accepted as it could be seen as a "misuse" of the led interface by using it for a screen (which is not really an LED). I myself would also prefer using the backlight interface for the ScreenPad, but there is currently no good support in the desktop environments for having two active backlight devices controlling different screens, so using the backlight interface would break the default experience for all users (as they could no longer control the main screen using the backlight keys on the keyboard or the system applet - at least that was the problem I had using Gnome). Unfortunately I don't have much time right now so I'm not able to look into improving support in the various desktop environments.

[Plippo]

Thank you anyway for offering to help! If you think there is a chance that a patch using the led interface could be accepted, we could of course give it a try.

[dragonnn]

Once you get the right email list and find the right CC the kernels are really nice and if they have another idea how this should be done they will help you and guide you through. It would be sad if such work get add some point forgotten or incompatible with new kernels if you stop maintaining it. I encourage you to upstream it :).

[chowdri]

Is it possible to add lightbar support through this led interface? Parsing through the source code, I found definitions, functions and structures indicating a lightbar.

[Plippo]

Is it possible to add lightbar support through this led interface? Parsing through the source code, I found definitions, functions and structures indicating a lightbar.

Well the kernel’s led interface is just a generic interface to support all kinds of LEDs, so probably supporting a lightbar would be possible (I have to admit I don't really know what a lightbar is, but I suppose it is some kind of LED). But I would rather keep this issue here on the topic of upstreaming.

[chowdri]

Is it possible to add lightbar support through this led interface? Parsing through the source code, I found definitions, functions and structures indicating a lightbar.

Well the kernel’s led interface is just a generic interface to support all kinds of LEDs, so probably supporting a lightbar would be possible (I have to admit I don't really know what a lightbar is, but I suppose it is some kind of LED). But I would rather keep this issue here on the topic of upstreaming.

Yes, I thought so. On UX581GV, the laptop provides a lightbar with multiple LEDs to indicate battery charge status or the performance mode. On windows, the lightbar also allows for alexa light effects.

I find the performance mode indication helpful to let me know if the laptop is running on performance or powersave mode with or without the charger.

[chowdri]

Is it possible to add lightbar support through this led interface? Parsing through the source code, I found definitions, functions and structures indicating a lightbar.

Well the kernel’s led interface is just a generic interface to support all kinds of LEDs, so probably supporting a lightbar would be possible (I have to admit I don't really know what a lightbar is, but I suppose it is some kind of LED). But I would rather keep this issue here on the topic of upstreaming.

I found this link indicating patches to make some lightbar work: https://lore.kernel.org/lkml/20170921112001.14788-1-maxime.bellenge@gmail.com/ I tried to add some of the patches, but couldn't get it to work. Not sure if the DEVID is correct.

[anonsquest]

It seems that this asus-wmi module prevents me from getting platform profile support and custom fan curves which is handled by asus-wmi. The newer asus-wmi module has the performance power profile option but this current one does not.

[anonsquest]

It seems that this asus-wmi module prevents me from getting platform profile support and custom fan curves which is handled by asus-wmi. The newer asus-wmi module has the performance power profile option but this current one does not.

I found a workaround with this module not working with newer kernels

   $ cat /usr/src/asus-wmi-1.0/inc/asus-wmi.h
    /* SPDX-License-Identifier: GPL-2.0 /
    #ifndef PLATFORM_DATA_X86_ASUS_WMI_H_EXT
    #define PLATFORM_DATA_X86_ASUS_WMI_H_EXT

    #define ASUS_WMI_DEVID_SCREENPAD    0x00050031
    #define ASUS_WMI_DEVID_SCREENPAD_LIGHT    0x00050032

    #endif    / __PLATFORM_DATA_X86_ASUS_WMI_H_EXT */

All I did was uncomment all the lines and save and restart my laptop.

So it does seem like 0x00050031and 0x00050032 are the correct targets for my laptop (GX551QS)as suggested by this post: https://github.com/s-light/ASUS-ZenBook-Pro-Duo-UX581GV/issues/1#issuecomment-633611760

Hope this helps someone else having similar issues.

[jibsaramnim]

It would be really nice if this could all be upstreamed so we can remove the whole manual build/install steps, making these laptops even more accessible to less technical people who still wish to run Linux (and just in general smoothen the experience, of course).

I wonder if there is a way to rely on something like a GNOME extension to support cases where more than one display can have its brightness adjusted, which would be a nice way to implement all of this the "correct' way. Something like this, perhaps.

Out of curiosity, how involved would it be to have a test branch with the "correct" backlight implementation? I know there is a backlight branch but that seems quite outdated, I'm not sure if that would even still work.

[jibsaramnim]

A small follow-up to my previous comment; I was curious so have actually gone ahead and written a rudimentary implementation by basically looking at how it was already done for a main monitor and adapt this for a secondary display. I have this work done in a branch over in my fork here. There's probably some cleaning up to do as-well as some better ways to handle certain things as I'm really not all that familiar with C, but in its current form it does actually work.

Unfortunately GNOME does not yet support controlling brightness of multiple displays, and the extension (or actually, the DDC utility itself) does not support internal displays, so with this more "technically correct" implementation, on GNOME you're left not being able to control the brightness of both displays through the GUI, only one. The good news is that there is currently a WIP PR for a first implementation related to this, but it's about a year old and there seems to be some progress left to be done yet. I'm not actually sure if KDE or other DE's already have this properly implemented, that'd be interesting to see.

The command line methods (e.g. echo 255 | sudo tee /sys/class/backlight/screenpad_backlight/brightness) do work as expected though, so that's useful. If you'd like to try it out, you can fetch the files from my branch and install it like usual, except I've bumped the version to 2.0 as it's quite a big rewrite with fundamental changes, so you'll have to create a new folder /usr/src/asus-wmi-2.0/ and dkms build and install this new 2.0 variant. Be sure to (temporarily) remove the 1.0 version, too.

I'm thinking that this "technically correct" implementation would be a nice one to submit upstream, but as it might take some more time before DE(s) correctly support multi-displays like this, it might also be nice to have a slightly patched version that just synchronizes brightness levels between both monitors, basically how the GNOME extension handles it currently, but without the overhead of it having to force this synchronization.

If anyone else has any ideas, thoughts and/or feedback, please do let me know!

(all of this was done and tested on an UX482EA. If you have a different model, I'd love to hear how this might work on yours!)

[flukejones]

@jibsaramnim I can help you to create a patch on mainline kernel and submit upstream if you like? It's 100% better to get this stuff in to mainline rather than have people relying on various dkms modules in different stages of sync with upstream.

[jibsaramnim]

I can help you to create a patch on mainline kernel and submit upstream if you like?

I'm planning on re-doing my work based on Kernel 6.1, the current patches need modifications to support those anyway, so I'd like to bring my "this could theoretically be upstreamed" stuff in line with that. After that I would love an extra pair of eyes to help make it as good as it can be, and to have it actually upstreamed yes. I've never submitted anything to the Linux Kernel before, so that part does feel very daunting.

It's 100% better to get this stuff in to mainline rather than have people relying on various dkms modules in different stages of sync with upstream.

Agreed! If you're ok with it, I'd love it if you could send me an email or so so we can work out the details of how to make this happen as smoothly as possible.

Thanks so much!