[PATCH 0/2] TI SN65DSI83 GPIO enable delay support

[PATCH 0/2] TI SN65DSI83 GPIO enable delay support

[Alexander Stein]

Hi all,

this small series adds a new optional property for specifying a platform
spefic enable delay. The LVDS Bridge requires at least a reset time of
10ms, but this is just the low pulse width. The actual rising time is a
different matter and is highly platform specific. My platform has a rising
time of ~110ms until the SN signal reaches VCC x 0.7 voltage level. Thus
make this time platform configurable.

Alexander Stein (2):
  dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
  drm: bridge: ti-sn65dsi83: Add enable delay support

 .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml   | 4 ++++
 drivers/gpu/drm/bridge/ti-sn65dsi83.c                      | 7 ++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 0/2] TI SN65DSI83 GPIO enable delay support

[Alexander Stein]

Hi all,

this small series adds a new optional property for specifying a platform
spefic enable delay. The LVDS Bridge requires at least a reset time of
10ms, but this is just the low pulse width. The actual rising time is a
different matter and is highly platform specific. My platform has a rising
time of ~110ms until the SN signal reaches VCC x 0.7 voltage level. Thus
make this time platform configurable.

Alexander Stein (2):
  dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
  drm: bridge: ti-sn65dsi83: Add enable delay support

 .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml   | 4 ++++
 drivers/gpu/drm/bridge/ti-sn65dsi83.c                      | 7 ++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.34.1

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

[Alexander Stein]

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
+
   vcc-supply:
     description: A 1.8V power supply (see regulator/regulator.yaml).
 
-- 
2.34.1

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

[Alexander Stein]

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
+
   vcc-supply:
     description: A 1.8V power supply (see regulator/regulator.yaml).
 
-- 
2.34.1

[PATCH 2/2] drm: bridge: ti-sn65dsi83: Add enable delay support

[Alexander Stein]

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.
Fall back to current default if not specified in DT.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 047c14ddbbf11..6510ee384315e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -145,6 +145,7 @@ struct sn65dsi83 {
 	struct drm_bridge		*panel_bridge;
 	struct gpio_desc		*enable_gpio;
 	struct regulator		*vcc;
+	u32				enable_delay;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
 };
@@ -346,7 +347,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 
 	/* Deassert reset */
 	gpiod_set_value_cansleep(ctx->enable_gpio, 1);
-	usleep_range(10000, 11000);
+	fsleep(ctx->enable_delay);
 
 	/* Get the LVDS format from the bridge state. */
 	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
@@ -603,6 +604,10 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 		return dev_err_probe(dev, PTR_ERR(ctx->vcc),
 				     "Failed to get supply 'vcc'\n");
 
+	if (of_property_read_u32(dev->of_node, "ti,enable-delay-us",
+				 &ctx->enable_delay))
+		ctx->enable_delay = 10000;
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH 2/2] drm: bridge: ti-sn65dsi83: Add enable delay support

[Alexander Stein]

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.
Fall back to current default if not specified in DT.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 047c14ddbbf11..6510ee384315e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -145,6 +145,7 @@ struct sn65dsi83 {
 	struct drm_bridge		*panel_bridge;
 	struct gpio_desc		*enable_gpio;
 	struct regulator		*vcc;
+	u32				enable_delay;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
 };
@@ -346,7 +347,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 
 	/* Deassert reset */
 	gpiod_set_value_cansleep(ctx->enable_gpio, 1);
-	usleep_range(10000, 11000);
+	fsleep(ctx->enable_delay);
 
 	/* Get the LVDS format from the bridge state. */
 	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
@@ -603,6 +604,10 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
 		return dev_err_probe(dev, PTR_ERR(ctx->vcc),
 				     "Failed to get supply 'vcc'\n");
 
+	if (of_property_read_u32(dev->of_node, "ti,enable-delay-us",
+				 &ctx->enable_delay))
+		ctx->enable_delay = 10000;
+
 	return 0;
 }
 
-- 
2.34.1

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

[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?


Best regards,
Krzysztof

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

[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?


Best regards,
Krzysztof

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

[Alexander Stein]

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.

Best regards,
Alexander

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

[Alexander Stein]

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.

Best regards,
Alexander

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

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


Best regards,
Krzysztof

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

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


Best regards,
Krzysztof

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

[Alexander Stein]

Hello Krzysztof,

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

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.

If I understand you correctly, you want to specify t_enable_delay in a 
regulator property. This only works if you can upper bound t_vcc_gpio which is 
not possible due to e.g. scheduling and i2c bus contention.

IMHO that's why there needs to be an configurable delay in the bridge driver.

Best regards,
Alexander

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

[Alexander Stein]

Hello Krzysztof,

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

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.

If I understand you correctly, you want to specify t_enable_delay in a 
regulator property. This only works if you can upper bound t_vcc_gpio which is 
not possible due to e.g. scheduling and i2c bus contention.

IMHO that's why there needs to be an configurable delay in the bridge driver.

Best regards,
Alexander

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

[Krzysztof Kozlowski]

On 09/12/2022 10:36, Alexander Stein wrote:
> Hello Krzysztof,
> 
> 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).
> 
> 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.
> 
> If I understand you correctly, you want to specify t_enable_delay in a 
> regulator property. This only works if you can upper bound t_vcc_gpio which is 
> not possible due to e.g. scheduling and i2c bus contention.
> 
> IMHO that's why there needs to be an configurable delay in the bridge driver.

What I am saying that you might be here mixing two separate delays:
regulator enable and/or ramp delay (which more or less matches your
t_vcc_gpio) and t_raise. I don't know about which board we talk, but the
mainline users of this bridge do not have even regulator supply,
therefore its enable time might be not factored.

Why this all raising questions? Because only your t_vcc_gpio should be
board dependent, right? Your bridge has fixed internal delays - from
datasheet: ten, tdis and treset. Nothing in your device is board
specific, thus I assume any enable delay is coming from power supply.

Probably experiment to prove it would be to keep power supply enabled
always and check on the scope of EN pin.

Anyway, even if this is variable delay on your EN input pin, it is still
input to the device and based on your time-diagram it is not a property
of the device. Property of the device could be:

EN-pad goes high <--------------> output pins stable

which is either:
1. ten already described in datasheet,
2. not the case here.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 09/12/2022 10:36, Alexander Stein wrote:
> Hello Krzysztof,
> 
> 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).
> 
> 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.
> 
> If I understand you correctly, you want to specify t_enable_delay in a 
> regulator property. This only works if you can upper bound t_vcc_gpio which is 
> not possible due to e.g. scheduling and i2c bus contention.
> 
> IMHO that's why there needs to be an configurable delay in the bridge driver.

What I am saying that you might be here mixing two separate delays:
regulator enable and/or ramp delay (which more or less matches your
t_vcc_gpio) and t_raise. I don't know about which board we talk, but the
mainline users of this bridge do not have even regulator supply,
therefore its enable time might be not factored.

Why this all raising questions? Because only your t_vcc_gpio should be
board dependent, right? Your bridge has fixed internal delays - from
datasheet: ten, tdis and treset. Nothing in your device is board
specific, thus I assume any enable delay is coming from power supply.

Probably experiment to prove it would be to keep power supply enabled
always and check on the scope of EN pin.

Anyway, even if this is variable delay on your EN input pin, it is still
input to the device and based on your time-diagram it is not a property
of the device. Property of the device could be:

EN-pad goes high <--------------> output pins stable

which is either:
1. ten already described in datasheet,
2. not the case here.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 09/12/2022 10:50, Krzysztof Kozlowski wrote:
> On 09/12/2022 10:36, Alexander Stein wrote:
>> Hello Krzysztof,
>>
>> 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).
>>
>> 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.
>>
>> If I understand you correctly, you want to specify t_enable_delay in a 
>> regulator property. This only works if you can upper bound t_vcc_gpio which is 
>> not possible due to e.g. scheduling and i2c bus contention.
>>
>> IMHO that's why there needs to be an configurable delay in the bridge driver.
> 
> What I am saying that you might be here mixing two separate delays:
> regulator enable and/or ramp delay (which more or less matches your
> t_vcc_gpio) and t_raise. I don't know about which board we talk, but the
> mainline users of this bridge do not have even regulator supply,
> therefore its enable time might be not factored.
> 
> Why this all raising questions? Because only your t_vcc_gpio should be
> board dependent, right? Your bridge has fixed internal delays - from
> datasheet: ten, tdis and treset. Nothing in your device is board
> specific, thus I assume any enable delay is coming from power supply.
> 
> Probably experiment to prove it would be to keep power supply enabled
> always and check on the scope of EN pin.
> 
> Anyway, even if this is variable delay on your EN input pin, it is still
> input to the device and based on your time-diagram it is not a property
> of the device. Property of the device could be:
> 
> EN-pad goes high <--------------> output pins stable
> 
> which is either:
> 1. ten already described in datasheet,
> 2. not the case here.


Ah, I forgot, otherwise this is a generic property of every system not
specific to your bridge. Thus it should be modeled somewhere else or in
in generic way.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 09/12/2022 10:50, Krzysztof Kozlowski wrote:
> On 09/12/2022 10:36, Alexander Stein wrote:
>> Hello Krzysztof,
>>
>> 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).
>>
>> 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.
>>
>> If I understand you correctly, you want to specify t_enable_delay in a 
>> regulator property. This only works if you can upper bound t_vcc_gpio which is 
>> not possible due to e.g. scheduling and i2c bus contention.
>>
>> IMHO that's why there needs to be an configurable delay in the bridge driver.
> 
> What I am saying that you might be here mixing two separate delays:
> regulator enable and/or ramp delay (which more or less matches your
> t_vcc_gpio) and t_raise. I don't know about which board we talk, but the
> mainline users of this bridge do not have even regulator supply,
> therefore its enable time might be not factored.
> 
> Why this all raising questions? Because only your t_vcc_gpio should be
> board dependent, right? Your bridge has fixed internal delays - from
> datasheet: ten, tdis and treset. Nothing in your device is board
> specific, thus I assume any enable delay is coming from power supply.
> 
> Probably experiment to prove it would be to keep power supply enabled
> always and check on the scope of EN pin.
> 
> Anyway, even if this is variable delay on your EN input pin, it is still
> input to the device and based on your time-diagram it is not a property
> of the device. Property of the device could be:
> 
> EN-pad goes high <--------------> output pins stable
> 
> which is either:
> 1. ten already described in datasheet,
> 2. not the case here.


Ah, I forgot, otherwise this is a generic property of every system not
specific to your bridge. Thus it should be modeled somewhere else or in
in generic way.

Best regards,
Krzysztof

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

[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 ?

The DSI84 EN pin already has a built-in pullup per DSI84 datasheet (see 
Table 5-1. Pin Functions), so that should make the signal rise fast, 
certainly not for seconds.

[...]

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

[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 ?

The DSI84 EN pin already has a built-in pullup per DSI84 datasheet (see 
Table 5-1. Pin Functions), so that should make the signal rise fast, 
certainly not for seconds.

[...]

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

[Krzysztof Kozlowski]

On 09/12/2022 13:02, Marek Vasut wrote:
> 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.

I know, but mainline boards seems to be missing power supply, so that
raises questions.

Whether voltage on some input pin reaches specified level is hardly a
problem of only this bridge. OTOH, datasheet describes another delay
which is specific to the chip and which is fixed - t_en/ten/Ten.


Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 09/12/2022 13:02, Marek Vasut wrote:
> 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.

I know, but mainline boards seems to be missing power supply, so that
raises questions.

Whether voltage on some input pin reaches specified level is hardly a
problem of only this bridge. OTOH, datasheet describes another delay
which is specific to the chip and which is fixed - t_en/ten/Ten.


Best regards,
Krzysztof

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

[Alexander Stein]

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.

> The DSI84 EN pin already has a built-in pullup per DSI84 datasheet (see
> Table 5-1. Pin Functions), so that should make the signal rise fast,
> certainly not for seconds.

Here it is >100ms, so the current waiting time is far too less. This results 
in errors regarding PLL lock failure.

Best regards,
Alexander

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

[Alexander Stein]

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.

> The DSI84 EN pin already has a built-in pullup per DSI84 datasheet (see
> Table 5-1. Pin Functions), so that should make the signal rise fast,
> certainly not for seconds.

Here it is >100ms, so the current waiting time is far too less. This results 
in errors regarding PLL lock failure.

Best regards,
Alexander

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

[Alexander Stein]

Hi,

Am Freitag, 9. Dezember 2022, 13:10:00 CET schrieb Krzysztof Kozlowski:
> On 09/12/2022 13:02, Marek Vasut wrote:
> > 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.
> 
> I know, but mainline boards seems to be missing power supply, so that
> raises questions.
> 
> Whether voltage on some input pin reaches specified level is hardly a
> problem of only this bridge. OTOH, datasheet describes another delay
> which is specific to the chip and which is fixed - t_en/ten/Ten.

The mainline board doesn't have a bridge node yet, as this issue has not been 
solved. In my tree this looks like this:

dsi_lvds_bridge: bridge@2d {
    compatible = "ti,sn65dsi83";
    reg = <0x2d>;
    enable-gpios = <&expander0 6 GPIO_ACTIVE_HIGH>;
    vcc-supply = <&reg_sn65dsi83_1v8>;
    ti,enable-delay-us = <120000>;
    status = "disabled";

    ports {
[...]
    };
};

So there is a regulator providing VCC. Once stable enable-gpio is rising EN 
pin. The rising time is what is board specific, but completely independent 
from VCC supply. But as Krzysztof noticed this is independent from this bridge 
driver and can occur everywhere where a GPIO switch/enable is involved.
I have an idea which I'll post on linux-gpio.

Regards,
Alexander

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

[Alexander Stein]

Hi,

Am Freitag, 9. Dezember 2022, 13:10:00 CET schrieb Krzysztof Kozlowski:
> On 09/12/2022 13:02, Marek Vasut wrote:
> > 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.
> 
> I know, but mainline boards seems to be missing power supply, so that
> raises questions.
> 
> Whether voltage on some input pin reaches specified level is hardly a
> problem of only this bridge. OTOH, datasheet describes another delay
> which is specific to the chip and which is fixed - t_en/ten/Ten.

The mainline board doesn't have a bridge node yet, as this issue has not been 
solved. In my tree this looks like this:

dsi_lvds_bridge: bridge@2d {
    compatible = "ti,sn65dsi83";
    reg = <0x2d>;
    enable-gpios = <&expander0 6 GPIO_ACTIVE_HIGH>;
    vcc-supply = <&reg_sn65dsi83_1v8>;
    ti,enable-delay-us = <120000>;
    status = "disabled";

    ports {
[...]
    };
};

So there is a regulator providing VCC. Once stable enable-gpio is rising EN 
pin. The rising time is what is board specific, but completely independent 
from VCC supply. But as Krzysztof noticed this is independent from this bridge 
driver and can occur everywhere where a GPIO switch/enable is involved.
I have an idea which I'll post on linux-gpio.

Regards,
Alexander

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

[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 ?

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.

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

[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 ?

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.

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

[Alexander Stein]

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.

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

[Alexander Stein]

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.

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

[Marek Vasut]

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@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 ?

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

[Marek Vasut]

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@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 ?

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

[Laurent Pinchart]

Hi Alexander,

On Fri, Dec 09, 2022 at 01:21:36PM +0100, Alexander Stein wrote:
> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
> > On 12/9/22 10:36, Alexander Stein wrote:
> > > Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
> > >> On 09/12/2022 09:54, Alexander Stein wrote:
> > >>> 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 is board-specific, and not a property of the SND65DSI83. If the
same circuit was attached to any control input of any chip, you would
need to modify the corresponding driver in a similar way. I don't think
this is right.

How about adding ramp-up (and ramp-down I suppose) delay DT properties
to the GPIO provider instead ? This wouldn't scale very well if all GPIO
providers had to be patched, but with some luck it may be possible to
handle that in the GPIO core ?

Another option would be to add a "GPIO delay" node in DT, between the
GPIO provider and consumer. It could be handled with a small driver that
forwards the GPIO calls with a delay.

> > The DSI84 EN pin already has a built-in pullup per DSI84 datasheet (see
> > Table 5-1. Pin Functions), so that should make the signal rise fast,
> > certainly not for seconds.
> 
> Here it is >100ms, so the current waiting time is far too less. This results 
> in errors regarding PLL lock failure.

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Alexander,

On Fri, Dec 09, 2022 at 01:21:36PM +0100, Alexander Stein wrote:
> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
> > On 12/9/22 10:36, Alexander Stein wrote:
> > > Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
> > >> On 09/12/2022 09:54, Alexander Stein wrote:
> > >>> 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 is board-specific, and not a property of the SND65DSI83. If the
same circuit was attached to any control input of any chip, you would
need to modify the corresponding driver in a similar way. I don't think
this is right.

How about adding ramp-up (and ramp-down I suppose) delay DT properties
to the GPIO provider instead ? This wouldn't scale very well if all GPIO
providers had to be patched, but with some luck it may be possible to
handle that in the GPIO core ?

Another option would be to add a "GPIO delay" node in DT, between the
GPIO provider and consumer. It could be handled with a small driver that
forwards the GPIO calls with a delay.

> > The DSI84 EN pin already has a built-in pullup per DSI84 datasheet (see
> > Table 5-1. Pin Functions), so that should make the signal rise fast,
> > certainly not for seconds.
> 
> Here it is >100ms, so the current waiting time is far too less. This results 
> in errors regarding PLL lock failure.

-- 
Regards,

Laurent Pinchart

Re: [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@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.

Re: [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@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.

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

[Krzysztof Kozlowski]

On 12/12/2022 10:09, Frieder Schrempf wrote:
>>>> 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.

If I read section 7.4.1 correctly, it would be enough to just add delay
Ten=1ms instead of the capacitor, right? And that would be
device-specific. But if one chooses the capacitor solution, it becomes
now board specific.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 12/12/2022 10:09, Frieder Schrempf wrote:
>>>> 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.

If I read section 7.4.1 correctly, it would be enough to just add delay
Ten=1ms instead of the capacitor, right? And that would be
device-specific. But if one chooses the capacitor solution, it becomes
now board specific.

Best regards,
Krzysztof

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

[Laurent Pinchart]

On Mon, Dec 12, 2022 at 10:09:45AM +0100, Frieder Schrempf wrote:
> 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:
> >>>> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
> >>>>> On 12/9/22 10:36, Alexander Stein wrote:
> >>>>>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
> >>>>>>> On 09/12/2022 09:54, Alexander Stein wrote:
> >>>>>>>> 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 (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.

RC delay circuits are very common when tying a control signal to a power
rail. I'm surprise to see it recommended (with such a large time
constant) when the EN signal is actively controlled. It makes sense if
the SN65DSI83 supply comes up before the GPIO can be actively driven low
(for instance if the supply isn't manually controllable but tied to an
always-on power rail), in other cases it's quite counter-productive (I
really hope the EN input has a Schmitt trigger).

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

On Mon, Dec 12, 2022 at 10:09:45AM +0100, Frieder Schrempf wrote:
> 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:
> >>>> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
> >>>>> On 12/9/22 10:36, Alexander Stein wrote:
> >>>>>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
> >>>>>>> On 09/12/2022 09:54, Alexander Stein wrote:
> >>>>>>>> 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 (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.

RC delay circuits are very common when tying a control signal to a power
rail. I'm surprise to see it recommended (with such a large time
constant) when the EN signal is actively controlled. It makes sense if
the SN65DSI83 supply comes up before the GPIO can be actively driven low
(for instance if the supply isn't manually controllable but tied to an
always-on power rail), in other cases it's quite counter-productive (I
really hope the EN input has a Schmitt trigger).

-- 
Regards,

Laurent Pinchart

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

[Frieder Schrempf]

On 12.12.22 10:32, Laurent Pinchart wrote:
> On Mon, Dec 12, 2022 at 10:09:45AM +0100, Frieder Schrempf wrote:
>> 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:
>>>>>> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
>>>>>>> On 12/9/22 10:36, Alexander Stein wrote:
>>>>>>>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
>>>>>>>>> On 09/12/2022 09:54, Alexander Stein wrote:
>>>>>>>>>> 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 (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.
> 
> RC delay circuits are very common when tying a control signal to a power
> rail. I'm surprise to see it recommended (with such a large time
> constant) when the EN signal is actively controlled. It makes sense if
> the SN65DSI83 supply comes up before the GPIO can be actively driven low
> (for instance if the supply isn't manually controllable but tied to an
> always-on power rail), in other cases it's quite counter-productive (I
> really hope the EN input has a Schmitt trigger).

On our hardware there's also a pulldown resistor parallel to the
capacitor which makes sure EN is low from the beginning even if the GPIO
is not driven yet.

I don't really understand what the capacitor should be good for, or why
TI recommends it. It looks counter-productive to me, too.

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

[Frieder Schrempf]

On 12.12.22 10:32, Laurent Pinchart wrote:
> On Mon, Dec 12, 2022 at 10:09:45AM +0100, Frieder Schrempf wrote:
>> 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:
>>>>>> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
>>>>>>> On 12/9/22 10:36, Alexander Stein wrote:
>>>>>>>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
>>>>>>>>> On 09/12/2022 09:54, Alexander Stein wrote:
>>>>>>>>>> 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 (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.
> 
> RC delay circuits are very common when tying a control signal to a power
> rail. I'm surprise to see it recommended (with such a large time
> constant) when the EN signal is actively controlled. It makes sense if
> the SN65DSI83 supply comes up before the GPIO can be actively driven low
> (for instance if the supply isn't manually controllable but tied to an
> always-on power rail), in other cases it's quite counter-productive (I
> really hope the EN input has a Schmitt trigger).

On our hardware there's also a pulldown resistor parallel to the
capacitor which makes sure EN is low from the beginning even if the GPIO
is not driven yet.

I don't really understand what the capacitor should be good for, or why
TI recommends it. It looks counter-productive to me, too.

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

[Laurent Pinchart]

On Mon, Dec 12, 2022 at 12:49:26PM +0100, Frieder Schrempf wrote:
> On 12.12.22 10:32, Laurent Pinchart wrote:
> > On Mon, Dec 12, 2022 at 10:09:45AM +0100, Frieder Schrempf wrote:
> >> 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:
> >>>>>> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
> >>>>>>> On 12/9/22 10:36, Alexander Stein wrote:
> >>>>>>>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
> >>>>>>>>> On 09/12/2022 09:54, Alexander Stein wrote:
> >>>>>>>>>> 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 (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.
> > 
> > RC delay circuits are very common when tying a control signal to a power
> > rail. I'm surprise to see it recommended (with such a large time
> > constant) when the EN signal is actively controlled. It makes sense if
> > the SN65DSI83 supply comes up before the GPIO can be actively driven low
> > (for instance if the supply isn't manually controllable but tied to an
> > always-on power rail), in other cases it's quite counter-productive (I
> > really hope the EN input has a Schmitt trigger).
> 
> On our hardware there's also a pulldown resistor parallel to the
> capacitor which makes sure EN is low from the beginning even if the GPIO
> is not driven yet.
> 
> I don't really understand what the capacitor should be good for, or why
> TI recommends it. It looks counter-productive to me, too.

There's an internal pull-up to VCC, so in case the GPIO is not driven,
the capacitor ensures that the EN voltage rises slowly enough to match
the required timings. If you ensure it gets driven, there should be no
concern.

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

On Mon, Dec 12, 2022 at 12:49:26PM +0100, Frieder Schrempf wrote:
> On 12.12.22 10:32, Laurent Pinchart wrote:
> > On Mon, Dec 12, 2022 at 10:09:45AM +0100, Frieder Schrempf wrote:
> >> 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:
> >>>>>> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
> >>>>>>> On 12/9/22 10:36, Alexander Stein wrote:
> >>>>>>>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
> >>>>>>>>> On 09/12/2022 09:54, Alexander Stein wrote:
> >>>>>>>>>> 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 (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.
> > 
> > RC delay circuits are very common when tying a control signal to a power
> > rail. I'm surprise to see it recommended (with such a large time
> > constant) when the EN signal is actively controlled. It makes sense if
> > the SN65DSI83 supply comes up before the GPIO can be actively driven low
> > (for instance if the supply isn't manually controllable but tied to an
> > always-on power rail), in other cases it's quite counter-productive (I
> > really hope the EN input has a Schmitt trigger).
> 
> On our hardware there's also a pulldown resistor parallel to the
> capacitor which makes sure EN is low from the beginning even if the GPIO
> is not driven yet.
> 
> I don't really understand what the capacitor should be good for, or why
> TI recommends it. It looks counter-productive to me, too.

There's an internal pull-up to VCC, so in case the GPIO is not driven,
the capacitor ensures that the EN voltage rises slowly enough to match
the required timings. If you ensure it gets driven, there should be no
concern.

-- 
Regards,

Laurent Pinchart

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

[Frieder Schrempf]

On 12.12.22 10:23, Krzysztof Kozlowski wrote:
> On 12/12/2022 10:09, Frieder Schrempf wrote:
>>>>> 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.
> 
> If I read section 7.4.1 correctly, it would be enough to just add delay
> Ten=1ms instead of the capacitor, right? And that would be
> device-specific. But if one chooses the capacitor solution, it becomes
> now board specific.

Yes, seems like that's the case.

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

[Frieder Schrempf]

On 12.12.22 10:23, Krzysztof Kozlowski wrote:
> On 12/12/2022 10:09, Frieder Schrempf wrote:
>>>>> 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.
> 
> If I read section 7.4.1 correctly, it would be enough to just add delay
> Ten=1ms instead of the capacitor, right? And that would be
> device-specific. But if one chooses the capacitor solution, it becomes
> now board specific.

Yes, seems like that's the case.

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

[Marek Vasut]

On 12/12/22 13:29, Frieder Schrempf wrote:
> On 12.12.22 10:23, Krzysztof Kozlowski wrote:
>> On 12/12/2022 10:09, Frieder Schrempf wrote:
>>>>>> 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.
>>
>> If I read section 7.4.1 correctly, it would be enough to just add delay
>> Ten=1ms instead of the capacitor, right? And that would be
>> device-specific. But if one chooses the capacitor solution, it becomes
>> now board specific.
> 
> Yes, seems like that's the case.

Can you still fix the board instead ? It would even save you on BOM.

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

[Marek Vasut]

On 12/12/22 13:29, Frieder Schrempf wrote:
> On 12.12.22 10:23, Krzysztof Kozlowski wrote:
>> On 12/12/2022 10:09, Frieder Schrempf wrote:
>>>>>> 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.
>>
>> If I read section 7.4.1 correctly, it would be enough to just add delay
>> Ten=1ms instead of the capacitor, right? And that would be
>> device-specific. But if one chooses the capacitor solution, it becomes
>> now board specific.
> 
> Yes, seems like that's the case.

Can you still fix the board instead ? It would even save you on BOM.