Ville Syrj�l� writes:

On Thu, Nov 22, 2012 at 07:28:44PM +0100, Egbert Eich wrote:

...

Something similar should be done for drm_edid_is_valid() - even if the driver doesn't bother (for instance because this function is only called once when the device structures are initialized).

The current code ignores the error state for extension blocks i guess it should not if we want to avoid having repreated logging of errors in the extension blocks.

I'm not sure. The current code dump all failed extension block, doesn't it? Maybe we actually do want that to happen, at least w/ debugs enabled.

Yes, it does. It drops them silently. I was a bit unclear - what I meant was: we ignore the result for the final error state we keep to be able to track which errors we still want to log.

Then there are various retry loops in the code, which may or may not be necessary. I have a feeling some of them may have been attempts at papering over a bug that I fixed [1] in the i2c bitbanging code. But if they are necessary, I'm not sure we really appreciate repeated dumps of the same block.

[1] https://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=8ee...

Right, I know this patch :) I'm also not sure about the retries. I guess for the base block we want at least one retry in case the display is in an intermediate state where it will not send the base block. Apart from this I've seen many EDID failures however I don't recall seeing one which got fixed by repeated reads. However to see if those loops fix any issue we may want to keep all messages enabled for the time being.

Cheers, Egbert.