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

getvcp returns DDCRC_ALL_RESPONSES_NULL since 1.3.0 #305

Closed
koko-ng opened this issue Mar 19, 2023 · 5 comments
Closed

getvcp returns DDCRC_ALL_RESPONSES_NULL since 1.3.0 #305

koko-ng opened this issue Mar 19, 2023 · 5 comments
Labels

Comments

@koko-ng
Copy link

koko-ng commented Mar 19, 2023

Hi!
Thanks for this very useful tool, it has been working very well so far, however since the recent update from 0.9.9 to 1.4.1 on Fedora it unfortunately stopped working with my HP MP34d over USB-C.

I bisected and found the incriminating commit to be d2ebdd2, unfortunately I don't know the protocol well enough to find what causes the problem. However I found that this change fixes my issue, I can open a PR if you think it's an appropriate fix.

diff --git a/src/ddc/ddc_packet_io.c b/src/ddc/ddc_packet_io.c
index 1f661c10..03efa1c0 100644
--- a/src/ddc/ddc_packet_io.c
+++ b/src/ddc/ddc_packet_io.c
@@ -698,7 +698,7 @@ ddc_write_read_with_retry(
                   if (retryable) {
                      if (ddcrc_null_response_ct == 1 && get_output_level() >= DDCA_OL_VERBOSE)
                         f0printf(fout(), "Extended delay as recovery from DDC Null Response...\n");
-                     tsd_set_sleep_multiplier_ct(ddcrc_null_response_ct++);
+                     tsd_set_sleep_multiplier_ct(++ddcrc_null_response_ct);
                      sleep_multiplier_incremented = true;
                      // replaces: call_dynamic_tuned_sleep_i2c(SE_DDC_NULL, ddcrc_null_response_ct);
                   }

Best ✨

@rockowitz rockowitz added the bug label Mar 20, 2023
@rockowitz
Copy link
Owner

Some monitors misuse the DDC Null Response to indicate that a feature is unsupported. You've encountered a bug in the code that handles that situation. To help me understand the problem properly and and assess its severity, please submit the output from the following commands as attachments:

ddcutuil detect --verbose for both 1.4.1 and 0.9.9 (or some earlier version that doesn't have the problem)
ddcutil interrogate for 1.4.1 and, if that fails for 0.9.9 (or some earlier version that doesn't have the problem)
a ddcutil command exhibiting the problem

Thank you.

@koko-ng
Copy link
Author

koko-ng commented Apr 1, 2023

Hi, sorry for the late answer, I don't always have this monitor at hand.
Anyways, here are the logs: https://gist.github.com/koko-ng/a681a792206137b8bbc94d8b5a580aa7
Let me know if there is anything else I can do to help! :)

Thanks

@rockowitz
Copy link
Owner

I have just uploaded an extensive set of changes to branch 1.5.0-dev. Included in them are changes that are intended to address the problems you've encountered.

The root problem you hit is that the detection phase of ddcutil (apparently) incorrectly determined that your monitor misuses a DDC Null Message to indicate unsupported features. There was special recovery code in the retry logic (recall that DDC communication is unreliable) for DDC Null Messages, and it was no longer working properly.

Please try using the development branch. It may be that, with the default settings, things just work. However, the monitor may return a DDC Null Message during probing, leading to the monitor being marked invalid and a message that monitor communication failed. In that case, first try using option --sleep-multiplier 2.0 to give the monitor more time to formulate proper responses. Second, try using option --dsa2. This is a new dynamic sleep algorithm that adjusts the sleep multiplier up (as needed) and down (insofar as possible) on a per monitor basis. (Note: if you specify both --sleep-multiplier and --dsa2 on the same command, ddcutil discards what it knows about the monitor and starts its adjustment from the sleep multiplier specified.)

If you encounter problems, as a first step, please submit the output of for failed program executions. Thank you.

@koko-ng
Copy link
Author

koko-ng commented Apr 22, 2023

Thanks for the update, it works well with the latest version (commit 0b6971c) without any special parameters.

@koko-ng koko-ng closed this as completed Apr 22, 2023
@rockowitz
Copy link
Owner

@koko-ng Thank you for letting me know that the recent changes have addressed your problem.

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

No branches or pull requests

2 participants