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.
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
