Fix errors when connecting Dell Alienware display to Linux laptop

Cargo.toml

@@ -32,6 +32,7 @@ ddc-macos = "0.2.2"
 
 [target.'cfg(target_os = "linux")'.dependencies]
 ddc-i2c = "0.2"
+udev = "0.2"
 uinput = "0.1"
 
 [target.'cfg(target_os = "windows")'.dependencies]

README.md

@@ -22,6 +22,26 @@ The app should function on MacOS, Windows, and Linux.
    brew install haimgel/tools/display_switch
    ```
 
+### Linux: device permissions
+
+Enable user access to I<sup>2</sup>C and input devices. Run as root:
+
+```bash
+groupadd i2c
+echo 'KERNEL=="i2c-[0-9]*", GROUP="i2c"' >> /etc/udev/rules.d/10-local_i2c_group.rules
+groupadd uinput
+echo 'KERNEL=="uinput", GROUP="uinput", MODE="0660"' >> /etc/udev/rules.d/10-local_uinput_group.rules
+udevadm control --reload-rules && udevadm trigger
+```
+
+Then add your user to the i2c and uinput groups:
+
+```bash
+sudo usermod -aG i2c,uinput $(whoami)
+```
+
+And finally restart your session.
+
 ## Configuration
 
 The configuration is pretty similar on all platforms:
@@ -180,19 +200,6 @@ Copy built executable:
 ```bash
   cp target/release/display_switch /usr/local/bin/
 ```
-Enable read/write access to i2c devices for users in `i2c` group. Run as root :
-
-```bash
-groupadd i2c
-echo 'KERNEL=="i2c-[0-9]*", GROUP="i2c"' >> /etc/udev/rules.d/10-local_i2c_group.rules
-udevadm control --reload-rules && udevadm trigger
-```
-
-Then add your user to the i2c group :
-
-```
-sudo usermod -aG i2c $(whoami)
-```
 
 Create a systemd unit file in your user directory (`/home/$USER/.config/systemd/user/display-switch.service`) with contents
 

src/app.rs

@@ -22,7 +22,7 @@ impl usb::UsbCallback for App {
         if device_id == self.config.usb_device {
             info!("Monitored device ({:?}) is connected", &self.config.usb_device);
             std::thread::spawn(|| {
-                wake_displays().map_err(|err| error!("{:?}", err));
+                wake_displays().map_err(|err| warn!("Couldn't wake displays: {:?}", err));
             });
             display_control::switch(&self.config, SwitchDirection::Connect);
         }

src/display_control.rs

@@ -8,13 +8,26 @@ use crate::input_source::InputSource;
 use anyhow::{Error, Result};
 use ddc_hi::{Ddc, Display, Handle};
 
+#[cfg(target_os = "linux")]
+use std::collections::HashMap;
 use std::collections::HashSet;
 use std::process::{Command, Stdio};
 use std::{thread, time};
 
+#[cfg(target_os = "linux")]
+use std::ffi::OsStr;
+#[cfg(target_os = "linux")]
+use std::os::unix::fs::MetadataExt;
+#[cfg(target_os = "linux")]
+use std::sync::{Mutex, OnceLock};
+#[cfg(target_os = "linux")]
+use udev::DeviceType::CharacterDevice;
+
 /// VCP feature code for input select
 const INPUT_SELECT: u8 = 0x60;
 const RETRY_DELAY_MS: u64 = 3000;
+const GET_CURRENT_INPUT_RETRY_ATTEMPTS: usize = 5;
+const GET_CURRENT_INPUT_RETRY_DELAY_MS: u64 = 100;
 
 fn display_name(display: &Display, index: Option<usize>) -> String {
     // Different OSes populate different fields of ddc-hi-rs info structure differently. Create
@@ -47,8 +60,31 @@ fn are_display_names_unique(displays: &[Display]) -> bool {
     displays.iter().all(|display| hash.insert(display_name(display, None)))
 }
 
+fn get_current_input(handle: &mut Handle, display_name: &str) -> Result<ddc_hi::VcpValue> {
+    let retry_delay = time::Duration::from_millis(GET_CURRENT_INPUT_RETRY_DELAY_MS);
+
+    for attempt in 1.. {
+        match handle.get_vcp_feature(INPUT_SELECT) {
+            Ok(raw_source) => return Ok(raw_source),
+            Err(err) => {
+                if attempt >= GET_CURRENT_INPUT_RETRY_ATTEMPTS {
+                    return Err(err);
+                }
+
+                debug!(
+                    "Failed to get current input for display {} on attempt {}/{}: {:?}",
+                    display_name, attempt, GET_CURRENT_INPUT_RETRY_ATTEMPTS, err
+                );
+                thread::sleep(retry_delay);
+            }
+        }
+    }
+
+    unreachable!()
+}
+
 fn try_switch_display(handle: &mut Handle, display_name: &str, input: InputSource) {
-    match handle.get_vcp_feature(INPUT_SELECT) {
+    match get_current_input(handle, display_name) {
         Ok(raw_source) => {
             if raw_source.value() & 0xff == input.value() {
                 info!("Display {} is already set to {}", display_name, input);
@@ -70,8 +106,75 @@ fn try_switch_display(handle: &mut Handle, display_name: &str, input: InputSourc
     }
 }
 
+#[cfg(target_os = "linux")]
+fn display_connector_name(display: &Display) -> Option<String> {
+    let Handle::I2cDevice(i2c) = &display.handle;
+    let file = i2c.inner_ref().inner_ref();
+    let devnum = file.metadata().ok()?.rdev();
+
+    let context = udev::Context::new().ok()?;
+    let mut device = context.device_from_devnum(CharacterDevice, devnum).ok()?;
+
+    loop {
+        if device.subsystem() == Some(OsStr::new("drm")) {
+            let name = device.sysname().to_str()?;
+            if name.starts_with("card") && name.contains('-') {
+                return Some(name.to_owned());
+            }
+        }
+
+        device = device.parent()?;
+    }
+}
+
+#[cfg(target_os = "linux")]
+fn is_laptop_connector_name(name: &str) -> bool {
+    name.contains("-eDP-") || name.contains("-LVDS-")
+}
+
+#[cfg(target_os = "linux")]
+fn filtered_displays() -> Vec<Display> {
+    static SKIPPED_DISPLAYS: OnceLock<Mutex<HashMap<String, bool>>> = OnceLock::new();
+    let mut skipped_displays = SKIPPED_DISPLAYS
+        .get_or_init(|| Mutex::new(HashMap::new()))
+        .lock()
+        .unwrap_or_else(|poisoned| poisoned.into_inner());
+
+    let mut displays = Vec::new();
+    for display in Display::enumerate() {
+        let display_id = display.info.id.clone();
+        if let Some(was_skipped) = skipped_displays.get(&display_id).copied() {
+            if !was_skipped {
+                displays.push(display);
+            }
+            continue;
+        }
+
+        let connector = display_connector_name(&display);
+        let is_skipped = connector.as_deref().is_some_and(is_laptop_connector_name);
+        skipped_displays.insert(display_id, is_skipped);
+        if !is_skipped {
+            displays.push(display);
+            continue;
+        }
+
+        info!(
+            "Skipping built-in laptop display {} on connector '{}'",
+            display_name(&display, None),
+            connector.unwrap(),
+        );
+    }
+
+    displays
+}
+
+#[cfg(not(target_os = "linux"))]
+fn filtered_displays() -> Vec<Display> {
+    Display::enumerate()
+}
+
 fn displays() -> Vec<Display> {
-    let displays = Display::enumerate();
+    let displays = filtered_displays();
     if !displays.is_empty() {
         return displays;
     }
@@ -85,7 +188,7 @@ fn displays() -> Vec<Display> {
         delay_duration.as_secs()
     );
     thread::sleep(delay_duration);
-    Display::enumerate()
+    filtered_displays()
 }
 
 pub fn log_current_source() {
@@ -97,9 +200,9 @@ pub fn log_current_source() {
     let unique_names = are_display_names_unique(&displays);
     for (index, mut display) in displays.into_iter().enumerate() {
         let display_name = display_name(&display, if unique_names { None } else { Some(index + 1) });
-        match display.handle.get_vcp_feature(INPUT_SELECT) {
+        match get_current_input(&mut display.handle, &display_name) {
             Ok(raw_source) => {
-                let source = InputSource::from(raw_source.value());
+                let source = InputSource::from(raw_source);
                 info!("Display {} is currently set to {}", display_name, source);
             }
             Err(err) => {

src/input_source.rs

@@ -3,6 +3,7 @@
 // This code is licensed under MIT license (see LICENSE.txt for details)
 //
 
+use ddc_hi::VcpValue;
 use paste::paste;
 use serde::de::Error;
 use serde::{Deserialize, Deserializer};
@@ -116,6 +117,13 @@ impl From<u16> for InputSource {
     }
 }
 
+impl From<VcpValue> for InputSource {
+    fn from(value: VcpValue) -> Self {
+        // Some VDUs don't zero the SNC high-byte like input source (0x60).
+        Self::from(value.sl as u16)
+    }
+}
+
 impl fmt::Display for SymbolicInputSource {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{:?}", self)

src/platform/wake_displays.rs

@@ -40,14 +40,16 @@ pub fn wake_displays() -> Result<()> {
 
 #[cfg(target_os = "linux")]
 pub fn wake_displays() -> Result<()> {
+    use anyhow::Context;
     use std::{thread, time};
     use uinput::event::controller::Controller::Mouse;
     use uinput::event::controller::Mouse::Left;
     use uinput::event::relative::Position::X;
     use uinput::event::relative::Relative::Position;
     use uinput::event::Event::{Controller, Relative};
 
-    let mut device = uinput::default()?
+    let mut device = uinput::default()
+            .context("can't open default uinput device")?
             .name("display-switch")?
             // It's necessary to enable any mouse button. Otherwise Relative events would not work.
             .event(Controller(Mouse(Left)))?