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

[Frieder Schrempf]

On 09.12.22 15:49, Marek Vasut wrote:
> On 12/9/22 14:38, Alexander Stein wrote:
>> 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 at 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.
> 
> I don't suppose you can have that capacitor reduced or better yet, some
> external pull up added, can you ?

Actually our HW engineers have implemented a similar RC circuit to
provide a hardware delay for the EN signal. I think this is due to a
design note in the datasheet (see chapter 7.4.1) and therefore it's
probably widely spread.