Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Am Freitag, 9. Dezember 2022, 13:43:02 CET schrieb Marek Vasut:
> On 12/9/22 13:21, Alexander Stein wrote:
> > Hi Marek,
> > 
> > Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
> >> On 12/9/22 10:36, Alexander Stein wrote:
> >>> Hello Krzysztof,
> >> 
> >> Hi,
> >> 
> >>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
> >>>> On 09/12/2022 09:54, Alexander Stein wrote:
> >>>>> Hello Krzysztof,
> >>>>> 
> >>>>> thanks for the fast feedback.
> >>>>> 
> >>>>> Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof 
Kozlowski:
> >>>>>> On 09/12/2022 09:33, Alexander Stein wrote:
> >>>>>>> It takes some time until the enable GPIO has settled when turning
> >>>>>>> on.
> >>>>>>> This delay is platform specific and may be caused by e.g. voltage
> >>>>>>> shifts, capacitors etc.
> >>>>>>> 
> >>>>>>> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> >>>>>>> ---
> >>>>>>> 
> >>>>>>>    .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml      | 4
> >>>>>>>    ++++
> >>>>>>>    1 file changed, 4 insertions(+)
> >>>>>>> 
> >>>>>>> diff --git
> >>>>>>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> >>>>>>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> >>>>>>> index 48a97bb3e2e0d..3f50d497cf8ac 100644
> >>>>>>> ---
> >>>>>>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> >>>>>>> +++
> >>>>>>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> >>>>>>> 
> >>>>>>> @@ -32,6 +32,10 @@ properties:
> >>>>>>>        maxItems: 1
> >>>>>>>        description: GPIO specifier for bridge_en pin (active high).
> >>>>>>> 
> >>>>>>> +  ti,enable-delay-us:
> >>>>>>> +    default: 10000
> >>>>>>> +    description: Enable time delay for enable-gpios
> >>>>>> 
> >>>>>> Aren't you now mixing two separate delays? One for entire block on (I
> >>>>>> would assume mostly fixed delay) and one depending on regulators
> >>>>>> (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss
> >>>>>> the
> >>>>>> second delays in your power supply? If so, the first one might be
> >>>>>> fixed
> >>>>>> and hard-coded in the driver?
> >>>>> 
> >>>>> Apparently there are two different delays: reset time (t_reset) of
> >>>>> 10ms
> >>>>> as
> >>>>> specified by datasheet. This is already ensured by a following delay
> >>>>> after
> >>>>> requesting enable_gpio as low and switching the GPIO to low in disable
> >>>>> path.
> >>>>> 
> >>>>> When enabling this GPIO it takes some time until it is valid on the
> >>>>> chip,
> >>>>> this is what this series is about. It's highly platform specific.
> >>>>> 
> >>>>> Unfortunately this is completely unrelated to the vcc-supply
> >>>>> regulator.
> >>>>> This one has to be enabled before the enable GPIO can be enabled. So
> >>>>> there is no regulator-ramp-delay.
> >>>> 
> >>>> Your driver does one after another - regulator followed immediately by
> >>>> gpio - so this as well can be a delay from regulator (maybe not ramp
> >>>> but
> >>>> enable delay).
> >> 
> >> The chip has two separate input pins:
> >> 
> >> VCC -- power supply that's regulator
> >> EN -- reset line, that's GPIO
> >> 
> >> Alexander is talking about EN line here.
> >> 
> >>> But this will introduce a section which must not be interrupted or
> >>> delayed.
> >>> This is impossible as the enable gpio is attached to an i2c expander in
> >>> my
> >>> case.
> >>> 
> >>> Given the following time chart:
> >>>    vcc                  set             EN
> >>> 
> >>> enable               GPIO             PAD
> >>> 
> >>>     |                    |<-- t_raise -->|
> >>>     | 
> >>>     | <-- t_vcc_gpio --> |               |
> >>>     | <--        t_enable_delay      --> |
> >>> 
> >>> t_raise is the time from changing the GPIO output at the expander until
> >>> voltage on the EN (input) pad from the bridge has reached high voltage
> >>> level. This is an electrical characteristic I can not change and have to
> >>> take into account.
> >>> t_vcc_gpio is the time from enabling supply voltage to enabling the
> >>> bridge
> >>> (removing from reset). Minimum t_vcc_gpio is something which can be
> >>> addressed by the regulator and is no problem so far. But there is no
> >>> upper bound to it.
> >> 
> >> What exactly is your EN signal rise time (should be ns or so)? Can you
> >> look at that with a scope , maybe even with relation to the VCC regulator
> >> ?
> > 
> > I checked EN rise time using a scope, it's ~110ms. I not an expert in
> > hardware but on the mainboard there is some capacitor attached to this
> > line, which increased the time, independent from the internal pull-up.
> 
> This does seem like a hardware bug right there, can you double-check
> this with the hardware engineer ?

Yep, checked with hardware engineer. An 470nF is attached, together with an 
open drain output and only the internal pull-up. So yes ~113ms rising time 
until 0.7 x VCC.

Best regards,
Alexander

> I would expect the capacitor to charge quickly when you flip the I2C
> expander output HIGH, unless the I2C expander output is open drain, at
> which point the transistor in the output is closed when the output is
> set to HIGH and the capacitor is charging over the DSI83 EN pullup ,
> which might be slow.