system76 / firmware-open

System76 Open Firmware
Other
949 stars 86 forks source link

DAC keyboard backlight level is weird #458

Open murtezayesil opened 1 year ago

murtezayesil commented 1 year ago

6 levels of keyboard backlight brightness increase in a weird way. when changing brightness with Fn+F4. New firmware was flashed today morning (2023-08-10) but the firmware was downloaded 4 days ago.

Steps to reproduce

  1. Get a lemp11
  2. Go to settings and install the firmware through GNOME system settings
  3. After few reboots, login to gnome-shell and change keyboard brightness with Fn+F4
  4. Observe that brightness indicator increases like shown in actual behaviour

Expected behavior

More consistent increase in keyboard backlight: back light level 0: <....................> 0% backlight level 1: <####................> 20% backlight level 2: <########............> 40% backlight level 3: <############........> 60% backlight level 4: <################....> 80% backlight level 6: <####################> 100%

Actual behavior

back light level 0: <....................> 0% backlight level 1: <##########..........> 50% backlight level 2: <###########.........> 56% backlight level 3: <#############.......> 65% backlight level 4: <###############.....> 75% backlight level 6: <####################> 100%

Additional info

Screen capture of it in action: https://github.com/system76/firmware-open/assets/24689015/a08438ce-38cb-4b3a-8473-6be5ee6a7d97

crawfxrd commented 1 year ago

This is intentional.

https://github.com/system76/ec/blob/13dd6a10381e3973006fd68958a7b57c2068b488/src/board/system76/common/kbled.c#L10-L29

Introduced with https://github.com/system76/ec/commit/4c9d3197b872, which changed the reporting from the pre-defined levels to the actual DAC value.

jacobgkau commented 1 year ago

@crawfxrd I noticed this while testing the lemp10 firmware update, thanks for pointing me to this issue. Are we planning to address it in any way? Not sure if we need @system76/ux input, but it does come across as strange behavior compared to the old intervals.

Old: Screencast from 08-23-2023 11:30:58 AM.webm

New: simplescreenrecorder-2023-08-23_12.18.33.webm

jacobgkau commented 1 year ago

Does the new method have any advantages like more accurate/consistent reporting with custom keyboard brightness levels outside of the predefined steps?

maria-komarova commented 1 year ago

It does seem somewhat unexpected and puzzling. What does it mean to see the reporting correspond to the actual DAC value? If we stick to this, we might want to communicate to people what and why is happening. If I would see this, I'd be puzzled and won't be able to tell what is going on. I might even decide that something is broken.

jackpot51 commented 1 year ago

I think it is typical in other systems to map between the apparent brightness and the hardware details of how that is achieved. Here, we are exposing hardware details that I don't think are useful to the user. We should probably try to map things so that the brightness steps shown in the GUI are closer to even, or at least to the steps of the color keyboards. I'm not sure whether this should be done in the EC firmware or in the kernel driver.

murtezayesil commented 12 months ago

Does the new method have any advantages like more accurate/consistent reporting with custom keyboard brightness levels outside of the predefined steps?

Current way of keyboard brightness indicator doesn't much with how bright the keyboard backlight is. At supposed 50%, backlight is so dim that it is only useful to make out where each keys are in a room with total darkness. In my opinion, this should be indicates as 20% rather than 50%, which was the case before the firmware update on lemp11. At supposed 56% brightness, keys are much brighter.

https://github.com/system76/firmware-open/assets/24689015/5c00aa6a-940c-4d9e-b1ca-8e8f9b30082b