maximbaz / wluma

Automatic brightness adjustment based on screen contents and ALS
ISC License
623 stars 26 forks source link

[Bug]: Can't find display, but shows up in ddcutil detect #101

Closed braye closed 5 months ago

braye commented 5 months ago

Steps for reproducing the issue

Run wluma with an LG Ultragear 27GP83B and a Dell U2415.

What is the buggy behavior?

The LG monitor is not detected - the Dell shows up fine.

What is the expected behavior?

The monitor should be detected, and its brightness adjusted.

Logs

RUST_LOG=trace wluma
[2024-03-11T23:41:19Z DEBUG wluma] Using Config {
        als: Iio {
            path: "/sys/bus/iio/devices",
            thresholds: {
                800: "outdoors",
                0: "night",
                20: "dark",
                80: "dim",
                250: "normal",
                500: "bright",
            },
        },
        output: [
            DdcUtil(
                DdcUtilOutput {
                    name: "DELL U2415 CFV9N85Q0HWS",
                    capturer: None,
                    min_brightness: 1,
                },
            ),
            DdcUtil(
                DdcUtilOutput {
                    name: "LG ULTRAGEAR 103NTPC9A464",
                    capturer: None,
                    min_brightness: 1,
                },
            ),
        ],
    }
[2024-03-11T23:41:19Z TRACE ddc_hi] DisplayInfo::from_edid(I2cDevice, 22794)
[2024-03-11T23:41:19Z TRACE ddc_hi] DisplayInfo::from_edid(I2cDevice, 22795)
[2024-03-11T23:41:24Z TRACE ddc_hi] DisplayInfo::from_capabilities(I2cDevice, 22795)
[2024-03-11T23:41:24Z DEBUG wluma::brightness::ddcutil] Discovered displays: ["DELL U2415 CFV9N85Q0HWS"]
[2024-03-11T23:41:24Z DEBUG wluma::brightness::ddcutil] Using display 'DELL U2415 CFV9N85Q0HWS' for config 'DELL U2415 CFV9N85Q0HWS'
[2024-03-11T23:41:24Z TRACE ddc_hi] DisplayInfo::from_edid(I2cDevice, 22794)
[2024-03-11T23:41:24Z TRACE ddc_hi] DisplayInfo::from_edid(I2cDevice, 22795)
[2024-03-11T23:41:25Z ERROR wluma::brightness::controller] Unable to get brightness value: DDC/CI error: invalid DDC/CI length
[2024-03-11T23:41:25Z DEBUG wluma::brightness::ddcutil] Discovered displays: []
[2024-03-11T23:41:25Z WARN  wluma] Skipping 'LG ULTRAGEAR 103NTPC9A464' as it might be disconnected: Unable to find display
[2024-03-11T23:41:25Z INFO  wluma] Continue adjusting brightness and wluma will learn your preference over time.

Version

4.3.0 - nixpkgs

Environment

Linux nixos 6.7.9 #1-NixOS SMP PREEMPT_DYNAMIC Wed Mar  6 14:54:01 UTC 2024 x86_64 GNU/Linux

ddcutil output:

Display 1
   I2C bus:  /dev/i2c-10
   DRM connector:           card0-DP-1
   EDID synopsis:
      Mfg id:               GSM - Goldstar Company Ltd (LG)
      Model:                LG ULTRAGEAR
      Product code:         23507  (0x5bd3)
      Serial number:        103NTPC9A464
      Binary serial number: 316464 (0x0004d430)
      Manufacture year:     2021,  Week: 3
   VCP version:         2.1

Display 2
   I2C bus:  /dev/i2c-11
   DRM connector:           card0-DP-2
   EDID synopsis:
      Mfg id:               DEL - Dell Inc.
      Model:                DELL U2415
      Product code:         41144  (0xa0b8)
      Serial number:        CFV9N85Q0HWS
      Binary serial number: 810047315 (0x30485753)
      Manufacture year:     2018,  Week: 21
   VCP version:         2.1

27GP83B MCCS Capabilities string:

(
    prot(monitor)
    type(lcd)
    model(GP83B)
    cmds(01 02 03 0C E3 F3)
    vcp(
        02 04 05 08 10 12 
        14(05 08 0B ) 
        16 18 1A 52 
        60(11 12 0F 10 ) 
        AC AE B2 B6 C0 C6 C8 C9 
        D6(01 04) 
        DF 62 8D F4 
        F5(01 02 03 04) 
        F6(00 01 02) 
        4D 4E 4F 
        15(01 06 11 13 14 15 18 19 20 22 23 24 28 29 32 48) 
        F7(00 01 02 03) 
        F8(00 01) 
        F9 EF 
        FA(00 01) 
        FD(00 01) 
        FE(00 01 02) 
        FF
    )
    mccs_ver(2.1)
    mswhql(1)
)

U2415 Capabilities string:

(
    prot(monitor)
    type(LCD)
    model(U2415)
    cmds(01 02 03 07 0C E3 F3)
    vcp(
        02 04 05 08 10 12 
        14(04 0B 05 06 08 09 0C) 
        16 18 1A 52 
        60(0F 10 11 12) 
        AA(01 02 04) 
        AC AE B2 B6 C6 C8 C9 
        D6(01 04 05) 
        DC(00 02 03 05) 
        DF E0 E1 
        E2(00 01 02 04 14 19 0C 0D 0F 10 11 13) 
        F0(00 08) 
        F1(01 02) 
        F2 FD
    )
    mswhql(1)
    asset_eep(40)
    mccs_ver(2.1)
)

I suspect this may be a bug in the `mccs_caps` crate with regards to parsing the capabilities string. Running the `ddc-hi` example code gives an error: "Failed to parse MCCS capabilities". I really don't know where to begin with fixing that, though.
maximbaz commented 5 months ago

Hello, thanks for the report! I'll have a proper look tomorrow, but I want to quickly ask you if you have considered or even had experience using https://gitlab.com/ddcci-driver-linux/ddcci-driver-linux, and if so what your feedback on that project is.

braye commented 5 months ago

I haven't actually heard of that project until now! In a hilarious coincidence, I was just lamenting how easy it would be to fix this problem if the data was handled in the kernel and could be rewritten/cleaned up before reaching the application.

maximbaz commented 5 months ago

Hehe - if you have time, give it a try, I'm curious if that would solve all your problems. It sounds like a cool idea, although I'm asking this question everyone who reports an issue with DDC in wluma, and curiously nobody seems to be using it, for various reasons - but I still have very little data 😁

braye commented 5 months ago

Well the ddcci module seems to be inconsistent, but when it does create devices for both of the monitors, it does ostensibly work. No warnings in wluma's output - will have to experiment with it more tomorrow.

maximbaz commented 5 months ago

I know you are still testing, so no rush at all, but please do let me know what you end up with.

And to answer your original question - you are totally right that it is the failing capabilities that prevents the monitor from being discovered. This happens because wluma is checking the capabilities in order to see if this is a usable monitor. It's really more of an imposed requirement, not because wluma actually needs any of the capabilities value. It was proven from a few other users that monitors that cannot report capabilities are most likely not usable at all, and in some cases even prevent people from using wluma on docking stations. You can see this post for some additional details e.g. here https://github.com/maximbaz/wluma/issues/72#issuecomment-1537246622

If you would like to try anyway whether removing the capabilities validation would work for your setup, you can basically remove lines 70 and 78 here:

https://github.com/maximbaz/wluma/blob/08984162b112d58c78a390cb06aeca3ab868fc08/src/brightness/ddcutil.rs#L70

braye commented 5 months ago

Sorry, I didn't get a chance to really test this until this evening. The code changes you proposed seems to work, though I did have to wrap the return value of that filter_map in Some().

I changed the brightness and it's reflected in the logs:

[2024-03-19T02:53:20Z DEBUG wluma::brightness::ddcutil] Discovered displays: ["LG ULTRAGEAR 103NTPC9A464", "DELL U2415 CFV9N85Q0HWS"]
[2024-03-19T02:53:20Z DEBUG wluma::brightness::ddcutil] Using display 'DELL U2415 CFV9N85Q0HWS' for config 'DELL U2415 CFV9N85Q0HWS'
[2024-03-19T02:53:20Z DEBUG wluma::brightness::ddcutil] Discovered displays: ["LG ULTRAGEAR 103NTPC9A464", "DELL U2415 CFV9N85Q0HWS"]
[2024-03-19T02:53:20Z DEBUG wluma::brightness::ddcutil] Using display 'LG ULTRAGEAR 103NTPC9A464' for config 'LG ULTRAGEAR 103NTPC9A464'
[2024-03-19T02:53:20Z INFO  wluma] Continue adjusting brightness and wluma will learn your preference over time.
[2024-03-19T02:53:26Z ERROR wluma::brightness::controller] Unable to get brightness value: DDC/CI error: invalid DDC/CI length
[2024-03-19T02:53:26Z ERROR wluma::brightness::controller] Unable to get brightness value: DDC/CI error: invalid DDC/CI length
[2024-03-19T02:53:27Z ERROR wluma::brightness::controller] Unable to get brightness value: DDC/CI error: invalid DDC/CI length
[2024-03-19T02:53:28Z ERROR wluma::brightness::controller] Unable to get brightness value: DDC/CI error: invalid DDC/CI length
[2024-03-19T02:53:32Z DEBUG wluma::predictor::controller] [LG ULTRAGEAR 103NTPC9A464] Learning Entry { lux: "dark", luma: 0, brightness: 40 }
[2024-03-19T02:53:32Z ERROR wluma::brightness::controller] Unable to get brightness value: DDC/CI error: Expected DDC/CI length bit
[2024-03-19T02:53:34Z ERROR wluma::brightness::controller] Unable to get brightness value: DDC/CI error: Expected DDC/CI length bit
[2024-03-19T02:53:37Z DEBUG wluma::predictor::controller] [DELL U2415 CFV9N85Q0HWS] Learning Entry { lux: "dark", luma: 0, brightness: 50 }
[2024-03-19T02:53:40Z ERROR wluma::brightness::controller] Unable to get brightness value: DDC/CI error: Expected DDC/CI length bit

Along with all the (expected) errors from my monitors ;)

Re: the ddcci kernel module - I did a bit of digging and it seems like it won't be upstreamed any time soon because of changes that are (supposedly) coming to sysfs :(

maximbaz commented 5 months ago

Okay, so it sounds like you'd rather use the ddcutil from wluma than the ddcci kernel module. Thanks also for confirming that removing capabilities check helps for your screens.

I'd propose to change that function as per following:

  1. First, only consider monitors that do correctly respond to capabilities check (i.e. exactly as it is in current main branch).
  2. Only when no monitor was found for the given name, as a fallback, repeat the search but now without capabilities check (i.e. as you have in your code locally).

That way we hopefully won't break anyone's setup (as I recall capabilities check primarily serves the purpose to filter out invalid screens when another valid one is also present), and at the same time make it work out of the box for your setup, without introducing any new per-monitor configs that you or anyone else would need to know about and configure.

What do you think? Would you like to send a PR? I don't have an external screen, so I would just trust your tests and green CI :grin:

braye commented 5 months ago

I opened #102 with changes made based on your suggestions - let me know what you think!