On 2018-10-17 12:32, Wolfram Sang wrote: > Hi Fabrizio, everyone, > > Thanks for bringing this up! > >> What's the best way to address this? I could pull in the HDMI >> transmitter driver the code to read the EDID back from the monitor so >> that I can fit device specific delays without impacting the generic >> implementation of the EDID readback, but that would replicate some >> code and the driver would not benefit from fixes made to the generic >> implementation. I could change the RCar I2C driver in order to fit a >> new DT parameter (i2c-delay-after-stop-us?), and the driver would put >> in the desired delay after every stop condition, but isn't this >> parameter something every I2C controller would benefit from (now that >> we know we have a use case for it)? What are your thoughts and >> recommendations? > > No need for a property. The I2C standard has a clearly defined value for > that which is named 'tbuf' and is in general the same value as the > minimal low period of the SCL signal. So, it must be handled at the I2C > bus master level. > > Currently, we have no rule at what time drivers have to leave the > master_xfer() callback. Some exit when STOP is still processed, some > when STOP has been sent. I am not aware of a driver respecting tbuf. It > seems worth recommending to respect tbuf. > > I think this needs to be completely handled in the bus master driver. We > have USB-to-I2C bridges which we can control only high level and the > firmware of those need to respect tbuf. I don't see how the I2C core > could support individual drivers here. > > So, that's how I see this situation. Other comments? Other ideas?
From the looks of the picture, it seems that the SoC does respect 'tbuf', but that the DDC pass-through in the HDMI transmitter then destroys the signals such that the monitor has no chance to interpret stuff correctly. Since this is not related to the specific bus driver in use, and since it would be ugly to add some hook that the HDMI transmitter driver could use, methinks that a sane way to describe that the bus driver needs to slow down is to add some DT property that makes the I2C core apply a quirk and thus force some minimum time between stop and start. Just like Fabrizio suggested... Either that, or add some hook in the generic edid reader code, so that it can slow down on request. Or, add an I2C gate driver (sort of like an I2C mux with only one child bus) in the HDMI transmitter driver and implement the delay there. Then move the monitor to this new gate/mux child bus. Cheers, Peter
