Delay between stop condition and start condition

Delay between stop condition and start condition

[Fabrizio Castro]

[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]

Hello Wolfram,

While working out what's needed to add HDMI support to the iwg23s from iWave I have stumbled across a problem with the HDMI transmitter (SiI9022ACNU). Such an HDMI transmitter has a DDC pass through mode that allows the SoC to talk directly to the monitor (to allow the SoC to read the EDID back from the monitor, for example). While in this working mode, if the SoC generates a start condition too close to the previous stop condition, then the monitor will miss the start condition, alongside a clock cycle. The consequences of this may be catastrophic, as you can imagine. I have attached a picture of a trace grabbed with my logic analyser, where SDA and SDL are related to the I2C bus between the SoC and the HDMI transmitter, while DDCT_SDA and DDCT_SCL (digital and analogic traces) are related to the I2C bus connecting the HDMI transmitter to the monitor.

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?

Thanks,
Fab



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

[-- Attachment #2: bug.png --]
[-- Type: image/png, Size: 65781 bytes --]

Re: Delay between stop condition and start condition

[Wolfram Sang]

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?

Thanks,

   Wolfram

RE: Delay between stop condition and start condition

[Fabrizio Castro]

Hello Wolfram,

Thank you so much for your feedback!

> Subject: Re: Delay between stop condition and start condition
>
> 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?

My understanding is that when this HDMI transmitter is configured in pass
through mode then a bigger 'tbuf' is required.
The I2C spec (v2.1) says that in "standard mode" tbuf>=4.7us" and in
fast-mode "tbuf>=1.3us", in this particular case the 20us of bus free time
between the STOP and START conditions you find in the trace are not enough.
This device seems to require an out of spec solution (an hack..), what's the
most acceptable solution from an upstreaming perspective? Other platforms
may want to use the same HDMI transmitter in their solutions and may
stumble across the same problem if the platform is quick enough.
I may just put this delay in the driver and call it a day by wrapping
writes/reads/modifies and stuff? What do you think?

Thanks,
Fab

>
> Thanks,
>
>    Wolfram



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

Re: Delay between stop condition and start condition

[Peter Rosin]

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

Re: Delay between stop condition and start condition

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]


> 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.

That would actually be my preferred solution. Because it describes the
HW setup best. It is the passthrough creating the problem, so it should
be fixed in its driver. It probably could be a generic driver, or?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Delay between stop condition and start condition

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 170 bytes --]


> be fixed in its driver. It probably could be a generic driver, or?

Wishful thinking. Setting the passthrough mode is probably not default,
so it is device specific.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Delay between stop condition and start condition

[Geert Uytterhoeven]

Hi Wolfram,

On Wed, Oct 17, 2018 at 2:21 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > be fixed in its driver. It probably could be a generic driver, or?
>
> Wishful thinking. Setting the passthrough mode is probably not default,
> so it is device specific.

According to the block diagram in
https://www.semiconductorstore.com/pdf/newsite/siliconimage/SiI9022a_pb.pdf,
it has "Registers & Config Logic", and a separate internal "I²C Master".

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: Delay between stop condition and start condition

[Fabrizio Castro]

> Subject: Re: Delay between stop condition and start condition
>
>
> > be fixed in its driver. It probably could be a generic driver, or?
>
> Wishful thinking. Setting the passthrough mode is probably not default,
> so it is device specific.

The passthrough mode is not default, it gets activated by the driver so that
drm_get_edid can then fetch the EDID. One other nasty thing is that to end the conversation
with the monitor you are supposed to write 0x00 to register 0x1a of the HDMI transmitter,
which means the SoC puts address 0x39 on the bus, but the HDMI transmitter lets that
through, the monitor NACKs the address (because its address is 0x50), and from that
moment on the control is back with the HDMI transmitter.
Unfortunately I don't have any other slave on the same bus, but I wonder what happens
if someone else tries to use the same bus while the pass through mode is operating...




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

Re: Delay between stop condition and start condition

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]


> The passthrough mode is not default, it gets activated by the driver so that
> drm_get_edid can then fetch the EDID. One other nasty thing is that to end the conversation
> with the monitor you are supposed to write 0x00 to register 0x1a of the HDMI transmitter,
> which means the SoC puts address 0x39 on the bus, but the HDMI transmitter lets that
> through, the monitor NACKs the address (because its address is 0x50), and from that
> moment on the control is back with the HDMI transmitter.
> Unfortunately I don't have any other slave on the same bus, but I wonder what happens
> if someone else tries to use the same bus while the pass through mode is operating...

Wouldn't all this really speak for an i2c gate? Do all enablement stuff
in select(), all disablement stuff in deselect(), and make sure
deselect() is called after every transfer. That would mean the
passthrough is only active when the EDID eeprom is accessed. Would that
work? Given the above, it looks quite sane to do it like that in order
to avoid side-effects of the open passthrough.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: Delay between stop condition and start condition

[Fabrizio Castro]

> Subject: Re: Delay between stop condition and start condition
>
>
> > The passthrough mode is not default, it gets activated by the driver so that
> > drm_get_edid can then fetch the EDID. One other nasty thing is that to end the conversation
> > with the monitor you are supposed to write 0x00 to register 0x1a of the HDMI transmitter,
> > which means the SoC puts address 0x39 on the bus, but the HDMI transmitter lets that
> > through, the monitor NACKs the address (because its address is 0x50), and from that
> > moment on the control is back with the HDMI transmitter.
> > Unfortunately I don't have any other slave on the same bus, but I wonder what happens
> > if someone else tries to use the same bus while the pass through mode is operating...
>
> Wouldn't all this really speak for an i2c gate? Do all enablement stuff
> in select(), all disablement stuff in deselect(), and make sure
> deselect() is called after every transfer. That would mean the
> passthrough is only active when the EDID eeprom is accessed. Would that
> work? Given the above, it looks quite sane to do it like that in order
> to avoid side-effects of the open passthrough.

It sounds like an i2c gate could fit the purpose. I'll give it a shot!

Thank you All!

Fab




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

Re: Delay between stop condition and start condition

[Peter Rosin]

On 2018-10-17 14:16, Wolfram Sang wrote:
> 
>> 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.
> 
> That would actually be my preferred solution. Because it describes the
> HW setup best. It is the passthrough creating the problem, so it should
> be fixed in its driver. It probably could be a generic driver, or?

Don't know about the possibility of a generic driver, but one thing to
look out for is that if the "gate" is left open at all times, *other*
xfers on the bus might not have the required delay between stop and
start, which might lead to the monitor (or other clients on the other
side of the HDMI transmitter) seeing potentially nasty things on the
distorted bus...

Cheers,
Peter