Ville Syrj�l� writes:

Me neither. I just figured it might reduce the chance of false positives. But if you say that can't happen, I'll take your word for it.

...

Regarding memcmp() you are definitely right, I will change the code.

...

Also the comment is somehow misleading. It talks about the base block even though we're looking at the extension block.

Reason for this patch: I had a bug report for a monitor announcing extension blocks in the extension block flag of the base block (over 200!) although it wasn't EDDC capable. For some reason it got past the ACK check when the segment number was written to address 0x30 and happily transferred the base block for any odd numbered block and some garbage for even ones.

That's nasty. When I saw your patch I was immediately thinking that this is caused by the IGNORE_NAK flag, but then I read the current code, and realized that we're not using that flag. It was used in the original EDDC patch, and I was worried that this kind of bad behaviour would be possible if use the flag. But it seems I underestimated how crappy the monitor hardwar can be.

Right. This flag would only make sense for the base block (to reset an EDDC capable monitor but not fail on non-EDDC capable ones). In fact I had a patch to add this, but then I realized that none of the I2C implemenations in the gfx drivers honored this flag. But then I reread the VESA spec for EDDC, this requires that the segment address in the monitor has to be reset on a STOP condition.

So if there ever is the case that a monitor has a segment > 0 selected when drm_read_edid() is called for the first time (because of a system crash for instance) the validation will fail and causes a 2nd read attempt for which the segment address should be reset by the previous transfer. Regarding crappiness of monitor hardware: catch me at a bar and I can tell you more stories ... ;p

...

The only reliable way we found to catch this condition early was to check if block 2 had the header of a base block which will happen when the display cannot deal with the segment number.

This was what I tried to summarize in very few words - maybe i should reword it a bit.

Right. The idea seems reasonable, I just found the comment somehow a bit confusing when I was reading the code following it.

So maybe something like 'Test if base block ..., by checking whether the extension block is a duplicate the base block.' Although the use of memcmp() will already make things much clearer.

Right. I've already reworked it. Will send out.

Cheers, Egbert.