Add option to skip checksum validation during DDC read (this helps with some displays) about betterdisplay

[waydabber]

Ok, I see you updated your post and that the offset is always parroted back. I am not sure why that is, it might be the Thunderbolt controller doing it as it translates stuff (if you did not mean USB-C connection using DisplayPort alt mode instead of native Thunderbolt), not sure.

[waydabber]

(note to self: issue needs investigation/confirmation)

[waydabber]

Hi @vjraitila

It seems to me that DDC reading works in general (at least with the display I am testing), for image adjustments. Sadly none of my currently used displays actually support DDC volume control anymore (the one I used for testing that is broken), so this still might be a specific issue with volume control which I can't test right now. Can you confirm whether reading on startup works for Contrast for example but specifically it does not work for Volume?

Thank you!

[waydabber]

This seems to work (probably), see the discussion. If not, I'll reopen this issue.

[waydabber]

Repurposed this ticket to something else based on the discussion.

[waydabber]

[vjraitila]

Let me know if I can help with some specifics. I did play around with the DDC/CI driver in Linux and these are the results I got retrieving the brightness, contrast and volume, respectively:

[159381.287631] ddcci: sending to 10:6e:6e: 01 10
[159381.400169] ddcci: received from 10:6e:6e: [8/8] 02 00 10 00 00 64 00 19
[159381.400185] ddcci: sending to 10:6e:6e: 01 12
[159381.510195] ddcci: received from 10:6e:6e: [8/8] 02 00 12 00 00 64 00 46
[159381.510210] ddcci: sending to 10:6e:6e: 01 62
[159381.620189] ddcci: received from 10:6e:6e: [8/8] 02 00 62 00 00 64 00 0a

Each of the settings the current value (last byte) was returned accurately. If I can find the time, I might also dig into the checksum part, but I'm not at all familiar with the protocol. I'm just curious in what way exactly this particular monitor misbehaves.

[waydabber]

You can try this and change the setting mentioned: https://github.com/waydabber/BetterDisplay/releases/download/v2.0.0-pre-release/BetterDisplay-v2.3.4-b30363-pre.zip

[vjraitila]

In this build (at least) the volume and contrast are indeed successfully read from the monitor upon startup. However, brightness is set to 100%.

I think I'll keep tinkering with this a bit as the way the checksum is calculated in the Linux driver here, it does pass. Does an open source tool exist for MacOS which does the checksum and would be easy to add some debugging output?

[waydabber]

Hi, the brighntess is set to 100% because the combined brightness (this combined hardware and software brightness/dimming methods) is active and is by default restored. If you disable the combined brightness setting, the brightness slider will solely use DDC (if available).

[vjraitila]

The ability to disable checksum validation might be helpful in some circumstances. But out of curiosity, I dug a bit deeper in case this turns out to be helpful for someone.

Get VCP volume request in both Linux (ddcutil) and MacOS (m1dcc - with a checksum fix):

6E 51 82 01 62 DE
^  ^  ^  ^  ^  ^      
|  |  |  |  |  |
|  |  |  |  |  CHK
|  |  |  |  VCP opcode (Volume)
|  |  |  Get VCP Feature COMMAND
|  |  Length
|  Source address
Destination address

The checksum CHK is calculated by xorring all preceding bytes as per spec. This succeeds both in Linux and MacOS.

PS. To my eyes, m1ddc calculates the checksum for the read request incorrectly (expected: 0xDEh, actual: 0x8Fh). I mean the algorithm is clearly not right, but probably the monitor does not care. It also seems that IOAVServiceWriteI2C fills in the destination address automatically and the source address is provided as a separate parameter in the function invocation. Only the last 4 bytes are passed in as data.

VCP volume response in Linux (ddcutil):

6E 88 02 00 62 00 00 64 00 09 BB

Adding the destination address to the beginning of the packet, the result is:

6F 6E 88 02 00 62 00 00 64 00 09 BB
^  ^  ^  ^  ^  ^  ^  ^  ^  ^  ^  ^      
|  |  |  |  |  |  |  |  |  |  |  |
|  |  |  |  |  |  |  |  |  |  |  CHK'
|  |  |  |  |  |  |  |  |  |  Present value Low byte
|  |  |  |  |  |  |  |  |  Present value High byte
|  |  |  |  |  |  |  |  Maximum value Low byte
|  |  |  |  |  |  |  Maximum value High byte
|  |  |  |  |  |  VCP type code (Set parameter)
|  |  |  |  |  VCP opcode (Volume) from Feature request message
|  |  |  |  Result Code (NoError)
|  |  |  VCP Feature reply op code
|  |  Length
|  Source address
Destination address

This looks per spec to me. The checksum CHK' is calculated the same as CHK except the destination address (first byte) is replaced with a virtual address of 0x50h, giving:

50 6E 88 02 00 62 00 00 64 00 09 BB

The checksum (0xBBh) then matches.

However, in MacOS (m1ddc) IOAVServiceReadI2C returns:

6E 51 02 00 62 00 00 64 00 09 BB
   ^ 
   |
   ERROR

PS. The input buffer size in m1ddc calling IOAVServiceReadI2C also seems to be off-by-one, but that is not the actual problem.

Notably the response in MacOS is otherwise similar to the one I get with Linux, except the value of the 'length' byte is incorrect. Hence I do not have the necessary information in order to calculate the checksum and I do not think the monitor itself is to blame.

There is something fishy about the third argument called 'offset' in the IOAVServiceReadI2C function. The value passed in here seems to match the one returned in the 'length' byte which is not right. Furthermore, not knowing the length of the returned payload is obviously problematic in other ways. I'm aware that this is a private API, but passing the host (source) address as a parameter to a field called 'offset' feels strange. It is more clear in IOAVServiceWriteI2C where the function signature is different.

[vjraitila]

As pure speculation, I would say that the IOAVServiceReadI2C API - assuming the function signature is accurate - is not designed to process arbitrary VCP Feature request responses and somewhat works by accident. An argument called 'offset' could refer to multi-part requests, applicable to e.g. display Capability requests and "Table" commands in the spec.

Simple VCP Feature requests and replies to not have this feature.

[waydabber]

Hi @vjraitila - the relevant APIs work just fine, checksum is also returned but simply does not match for some displays - but this is a display firmware issue, not an API issue (the return package is otherwise valid as the opcode and returned values are right). DDC capability requests also work (this feature is present in the current version of BetterDisplay, use betterdisplaycli get -ddcCapabilities to get the parsed output or betterdisplaycli get -ddcCapabilitiesString to get the raw string - just did not add an UI for it yet, but I plan to add a proper UI and some help for the user to autoconfigure DDC stuff).

Note: Since the v2.3.x (current release) I completely rewrote the entire I2C/DDC/EDID subsystem from scratch both for Intel and Apple Silicon (using a unified code for them) and optimized how stuff works. This is not yet available in a pre-release form though.

[waydabber]

Hmm. Sorry, I did not read your previous comment when I answered. I have displays with which the checksum matches reliably and with other displays it usually doesn't so that's why I was thinking it is a display dependent thing - and I get the same results with Intel which works completely differently API-wise. But now I am not sure, so I'll need to look at this again. :)

Sidenote: m1ddc might generally calculate the checksum wrong (older versions of BetterDisplay did so for sore, so the same issue might be present in m1ddc, I'll need to check it. If you find any issue with m1ddc, feel free to do a PR).

Regarding the length byte, I found it a hit and miss but mostly working. The latest BD versions (especially for capabilities strings where the result is multi-part and offsets are important to stich stuff together) look at length in some cases but also look for other indicators (like a 0x00 byte in the payload part) to figure out how chunks are put together as some displays report length differently (some report the length of the current message, some the remaining bytes until the end of the capabilities string, irrespective of the current chunk's useful size).

[vjraitila]

I was planning to do a PR to m1ddc, but since IOAVServiceReadI2C behaves irrationally I figured it would not be that useful anyway. FWIW, the request checksum in m1ddc omits the 'source address' byte and also xors once with itself. The other issue I noticed was that it uses an output buffer size of 12 instead of 11 when calling IOAVServiceReadI2C adding one byte of garbage into the response.

The reason I suspected something other than the display was to blame is because under Linux my LG 38WN95C-W works perfectly according to spec, including matching checksums - as demonstrated in my earlier comments. This was further reinforced by seeing that m1ddc, the only source code I can examine, does not in my mind follow the spec and the usage of IOAVServiceReadI2C being dubious (esp. the 'offset' argument).

Anyway, I'm just a guy poking around this stuff and just read through the VESA DDC/CI Standard yesterday - so what do I know :). I'm also wondering why people use these obscure, private APIs when Apple also provides this. Wouldn't that work?

[waydabber]

Apple has no documented API at all for I2C communication on Apple Silicon Macs. There is no mention of deprecating the standard IOKit way and it indeed works fine on Intel macs but since Apple Silicon macs do not have IOFramebuffer class I/O registry entries with associated things like IOFramebufferI2CInterface objects, IOI2CInterface simply does nothing.

About m1ddc: it was just an early iteration of trying to make things work by modifying a code put together by the community. Since then it started to have a life of its own with various PRs added to it. It's development is entirely driven by the community, which is great - so really if you want to improve it, just post a PR and if it works, I'll merge it. I am working mainly in swift and don't plan on improving m1ddc myself in any way. Will possibly create a standalone tool though in Swift that should have these features and work both for Intel and Apple Silicon, but just did not get there (note: betterdisplaycli basically does everything that's needed I think but it requires a running BetterDisplay and I understand not everybody wants that).

[waydabber]

Ok, just quickly tested two displays with DDC read on luminance on Apple Silicon. Here are the results I got with IOAVServiceReadI2C:

Display 1:

DDC reply data as follows: [110, 136, 2, 0, 16, 0, 0, 100, 0, 38, 230]

Display 2:

Malformed DDC reply received [119, 104, 113, 108, 40, 49, 41, 41, 0, 221, 68] received for Display 9 (GP27-FUS)
DDC reply data as follows: [110, 136, 2, 0, 16, 0, 0, 100, 0, 10, 202]

Display 2 is finnicky usually, it sent some garbage for the first attempt but the second attempt was fine.

Checksum was enforced for both displays. As you see the second byte is 136 (0x88) so that's the expected value. Not sure why you are getting 0x51, it must be a communication error of some kind.

This is the checksum check that is being used when checksum is enforced:

Self.checksum([0x50] + replyData.dropLast()) == replyData.last

This is the xor checksum function I am using:

  private static func checksum(_ data: [UInt8]) -> UInt8 {
    data.reduce(0, ^)
  }

[waydabber]

(update: I see the silly log output there with received [...] received - fixed that quickly lol)

[vjraitila]

Just to make it clear that we have an identical test case and running the same code, can you get 0x88 as length when executing

$ ./m1ddc display 2 get volume

as well?

[waydabber]

Sorry, I did not try m1ddc, was testing only the IOAVServiceReadI2C call on my M3 Mac (using BetterDisplay's current Framebuffer library that handles the I2C stuff as well for both platforms). But I don't see why the length would be different, this data should be returned by the display as a response. The displays I have here right now don't support volume control at all so I can't test that specific case.

[vjraitila]

Okay. Then I'm stumped and have to investigate even deeper as my initial analysis was wrong and there is something else going on.

For me whatever I put in the offset gets "echoed" back in the length byte. That is the reason for the value 0x51 (as that is the value m1ddc uses). This is entirely consistent on every invocation and happens on both MacBook Air M1 and M2, without a CalDigit dock in between and attaching the display directly. It also does the same when getting luminance.

There is a difference that the Macs are connected via Thunderbolt whereas the Linux box uses a DisplayPort connection.

[waydabber]

I see. I don't fully understand why would you specify an offset for a Get operation (it does not expect an offset). Offset makes only sense for the capabilities string (or table operations but I am not sure there is any display out there that supports that - it is left unimplemented in BetterDisplay). Even with the capabilities string, the returned length value meaning varies by implementation.

[vjraitila]

Now you are just repeating what I said earlier :). I already deduced what the offset is for by skimming through the spec. However, m1ddc always puts the source (host) address 0x51 there even for simple VCP feature requests.

That was my point about the call to IOAVServiceReadI2C not making sense.

Regardless, that is probably not the issue here as your behavior differs anyway. And it behaves the same for me regardless of what I put in the offset. It would probably make sense to put 0x0 in there, but that just results in 0x0 being returned/echoed as length.

And yeah, I read somewhere that only the lowest bits are considered for the length. But that is already the case with 0x88 (length: 8) as well.

I was curious about VCP feature requests specifically and why they follow the spec to the T in Linux and not in MacOS. These not just working without disabling checksums in BetterDisplay led me down the rabbit hole.

[waydabber]

Ok, I see. I am not sure why m1ddc would do that - the code is based on some early reverse engineering/experimenting, trying to make the undocumented IOAVServiceReadI2C work and we probably did not understand something. But I correct myself, the offset value in the IOAVServiceReadI2C should not be (I think) used for DDC at all, as the offset is sent as part of the request for the capabilities string.

But I just tried, with the displays I am using (connected via DisplayPort and HDMI), it does not matter whether what I put into the offset field for a Get operation, so it must be just an interesting coincidence that the display you are using parrots back the offset value when using Thunderbolt. Does it work fine when you provide 0 as an offset?

[vjraitila]

Yeah, I already started disassembling IOKit to see what it actually does, but realized that I also have a life :). Let's see if I can get down to it at some point - probably not.

(System Information reports the display being connected in TB3 mode and the display does support it)

[waydabber]

You might want to try with pure DisplayPort as well and see what happens.

I think m1ddc adds the data address to the IOAVServiceReadI2C because of the similarity in function signature with IOAVServiceWriteI2C where third uint32_t is indeed the data address. This probably bothered no-one though as it should not make a difference for any of the use cases m1ddc covers (except with your display apparently) - this offset is useful for other (non-DDC) I2C operations where there is no IOAVServiceWriteI2C beforehand to send the offset (as like with a DDC request - BD uses this offset field for non-DDC I2C stuff where it's needed).