Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display redetection starting/Display redetection finished #391

Open
digitaltrails opened this issue Mar 10, 2024 · 13 comments
Open

Display redetection starting/Display redetection finished #391

digitaltrails opened this issue Mar 10, 2024 · 13 comments

Comments

@digitaltrails
Copy link

When ddcutil-service optionally uses polling for hotplug detection, two messages are logged to the journal for every call to ddc_redetect_displays().

Display redetection starting.
Display redetection finished.

Ideally I don't want detect to be logged unless something has gone wrong or I've increased the level of logging above the default.

But I accept that polling by detect is not a normal use-case (although I think the plasma widget ddcci_plasmoid also does polling, so that's two applications so far).

@rockowitz
Copy link
Owner

From my point of view display redetection is a significant non-error event. Is has ddcutil log level DDCA_SYSLOG_NOTICE. You can prevent its being logged by passing DDCA_SYSLOG_WARNING to ddca_init2().

If you don't want to use libddcutil's display status callback facility, you could create a (relatively) simple loop in your own code that watches for display related hotplug events, and only call ddca_redetect_displays() when one occurs.

@digitaltrails
Copy link
Author

digitaltrails commented Mar 11, 2024

If you don't want to use libddcutil's display status callback facility, you could create a (relatively) simple loop in your own code that watches for display related hotplug events, and only call ddca_redetect_displays() when one occurs.

Sounds like something I should do to lower the overheads, maybe I could then check more frequently. Is there some code in ddcutil I should look to for some guidance.

I guess I need something that works without DRM because that's the only reason I'm polling, to support non-DRM/buggy-DRM systems.

@rockowitz
Copy link
Owner

Look at src/ddc/ddc_watch_displays.c, particularly functions ddc_start_watch_displays(), ddc_stop_watch_displays(), and ddc_watch_displays_using_udev(). Most of what's there can be stripped out; handling DPMS events, the use of DRM to keep track of what has changed, the complex bookkeeping. What's important for you in ddc_watch_displays_using_udev() is the polling loop that every few seconds calls udev_monitor_receive_device(). If successful, test for a display hotplug event, do whatever you need to do to signal that redetection is necessary, then call udev_device_unref(). One thing you want to watch out for is the possibility of 2 hotplug events in immediate succession, one that disconnects a monitor and second that connects it. For your purposes you probably could get by with checking for multiple events within a couple seconds and only signal a hotplug event once in that case.

@digitaltrails
Copy link
Author

Given the progress on #390, perhaps DRM is more reliable than I first thought. If almost all drivers now properly support connectivity detection via DRM, almost everyone will be covered by libddcutil.

Perhaps it's now unnecessary for the service to do connectivity polling to support what may only be a small number of legacy cases (and those users might not even care about connectivity detection). I'll hold off on doing any more work on polling using detect.

I suppose there is the issue that DPMS state change is not always correctly reported. I suspect this polling will have to continue. (The echo detect doesn't help with DPMS).

@digitaltrails
Copy link
Author

My own attempt at udev code only works if I also echo detect > /sys/class/drm/card0-DP-?/status. With hindsight, it's quite likely udev/drm might not work around a problem with udev/drm.

I think I should just wait for libddcutil 2.1.x where x is the version that writes detect to /sys/class/drm/card0-DP-?/status.

And I should leave my old poll-detect code as is for anyone using earlier versions of libddcutil or anyone stuck with non-DRM drivers.

@rockowitz
Copy link
Owner

Writing "detect" to the sysfs status attribute was a quick check that I was on the right track. As you've noted, its not really a solution since it requires root. DRM can be instructed via its API to update the information it exposes in /sys, but that too seems to require root privileges. I'm working on querying DRM directly using its API. in hopes that information is always current.

@rockowitz
Copy link
Owner

After much hacking around with querying drm directly, I found an unprivileged operation that forces edid data to be updated. From the documentation for function drmModeGetConnector() in xf86drmMode.h: "This will do a forced probe on the connector to retrieve remote information such as EDIDs from the display device," I've incorporated this function in a subsystem that queries DRM to get the EDID, sleep state, etc.

For test purposes, try using the following options:

  • --enable-try-get-edid-from-sysfs: if possible, obtain the EDID from sysfs during display detection
  • --f15: if the EDID is obtained from sysfs during display detection, verify that it is correct by reading the EDID using I2C
  • --f6: force drm monitor state redetection during ddca_redetect_displays()

Is EDID read from sysfs always correct now?

However, I'm coming to think that the overhead and complexity of ensuring that the EDID in sysfs is correct negates the performance improvement over just always reading the EDID directly from the monitor. But for now I'd like to be certain that I've addressed the problem of stale information in sysfs.

@digitaltrails
Copy link
Author

I'm getting a -3014 from ddca_register_display_status_callback(). I suspect it's because all_sysfs_i2c_info_drm() isn't finding anything that satisfies if (str_starts_with(info->adapter_class, "0x03")). I'm a bit rusty on this stuff, I'll take a harder look later.

@digitaltrails
Copy link
Author

digitaltrails commented Mar 25, 2024

info->adapter_class is always NULL.

(all_sysfs_i2c_info_drm        ) Starting  rescan=false
(all_sysfs_i2c_info_drm        )           busno=0, adapter_class=(null), adapter_path=(null)
(all_sysfs_i2c_info_drm        )           busno=1, adapter_class=(null), adapter_path=(null)
(all_sysfs_i2c_info_drm        )           busno=2, adapter_class=(null), adapter_path=(null)
(all_sysfs_i2c_info_drm        )           busno=3, adapter_class=(null), adapter_path=(null)
(all_sysfs_i2c_info_drm        )           busno=4, adapter_class=(null), adapter_path=(null)
(all_sysfs_i2c_info_drm        )           busno=5, adapter_class=(null), adapter_path=(null)
(all_sysfs_i2c_info_drm        )           busno=6, adapter_class=(null), adapter_path=(null)
(all_sysfs_i2c_info_drm        ) Done      Returning: false. 

@digitaltrails
Copy link
Author

Added name:

ddcutil-service: Display redetection starting.
(all_sysfs_i2c_info_drm        ) Starting  rescan=false
(all_sysfs_i2c_info_drm        )           busno=0, adapter_class=(null), adapter_path=(null) name=NVIDIA i2c adapter 3 at 8:00.0
(all_sysfs_i2c_info_drm        )           busno=1, adapter_class=(null), adapter_path=(null) name=NVIDIA i2c adapter 4 at 8:00.0
(all_sysfs_i2c_info_drm        )           busno=2, adapter_class=(null), adapter_path=(null) name=NVIDIA i2c adapter 5 at 8:00.0
(all_sysfs_i2c_info_drm        )           busno=3, adapter_class=(null), adapter_path=(null) name=NVIDIA i2c adapter 6 at 8:00.0
(all_sysfs_i2c_info_drm        )           busno=4, adapter_class=(null), adapter_path=(null) name=SMBus PIIX4 adapter port 0 at 0b00
(all_sysfs_i2c_info_drm        )           busno=5, adapter_class=(null), adapter_path=(null) name=SMBus PIIX4 adapter port 2 at 0b00
(all_sysfs_i2c_info_drm        )           busno=6, adapter_class=(null), adapter_path=(null) name=SMBus PIIX4 adapter port 1 at 0b20
(all_sysfs_i2c_info_drm        ) Done      Returning: false. 
(ddcutil-service:23642): ddcutil-service-DEBUG: 15:33:49.319: Poll for display connection changes

@rockowitz
Copy link
Owner

Status code DDCRC_INVALID_OPERATION (-3014) is returned by ddca_register_display_status_callback() when it appears that not all display drivers implement DRM, which is required when watching for display connection/disconnection.

I'd need to fire up my test system with a Nvidia card to confirm, but I'm pretty sure that function all_sysfs_i2c_info_drm() used by ddca_resgister_display_status_callback(), is failing because of how the Nvidia driver populates sysfs. To confirm, the output of struct Sysfs_I2C_Info by option --f2 would have NULL for the adapter_path and adapter_class.

There are actually several different functions that accumulated which test whether all display drivers implement DRM. If you execute **ddcutil det --trcfunc submaster_initializer you will see the results of all_displays_drm_using_drm_api()), check_all_video_adapters_implement_drm() as well as all_sysfs_i2c_info_drm(). Can you confirm that the first two functions return true while the third return false?

@digitaltrails
Copy link
Author

After powering up the PC and then hot plugging the Beng:

./src/ddcutil det --trcfunc submaster_initializer | tee log2
(submaster_initializer         ) Starting  parsed_cmd = 0x557d684105e0
(submaster_initializer         )           all_displays_drm_using drm_api() returned true
(submaster_initializer         )           check_all_video_adapters_implement_drm() returned true
(submaster_initializer         )           all_sysfs_i2c_info_drm() returned false
(submaster_initializer         )           drm_enabled = true, sys_drm_connectors = 0x557d6840edc0
(submaster_initializer         ) Done      Returning: true. 
Display 1
   I2C bus:  /dev/i2c-0
   DRM connector:           card0-DP-1
   EDID synopsis:
      Mfg id:               HWP - Hewlett Packard
      Model:                HP ZR24w
      Product code:         10345  (0x2869)
      Serial number:        CNT00811J6
      Binary serial number: 16843009 (0x01010101)
      Manufacture year:     2010,  Week: 8
   VCP version:         2.2

Display 2
   I2C bus:  /dev/i2c-1
   EDID synopsis:
      Mfg id:               BNQ - UNK
      Model:                BenQ T2200HD
      Product code:         30502  (0x7726)
      Serial number:        2C803527SL0
      Binary serial number: 21573 (0x00005445)
      Manufacture year:     2008,  Week: 51
   VCP version:         2.0

Display 3
   I2C bus:  /dev/i2c-3
   DRM connector:           card0-DP-3
   EDID synopsis:
      Mfg id:               GSM - Goldstar Company Ltd (LG)
      Model:                LG HDR 4K
      Product code:         30471  (0x7707)
      Serial number:        
      Binary serial number: 86395 (0x0001517b)
      Manufacture year:     2019,  Week: 4
   VCP version:         2.1

@rockowitz
Copy link
Owner

rockowitz commented Mar 26, 2024

I've replaced function all_sysfs_i2c_info_drm() with check_all_video_adapters_implement_drm() and pushed the changes to 2.1.5-dev, so ddca_register_display_status_callback() should now work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants