diff mbox series

drm/bridge: tc358767: fix poll timeouts

Message ID 20191209082707.24531-1-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show
Series drm/bridge: tc358767: fix poll timeouts | expand

Commit Message

Tomi Valkeinen Dec. 9, 2019, 8:27 a.m. UTC
Link training fails with:

  Link training timeout waiting for LT_LOOPDONE!
  main link enable error: -110

This is caused by too tight timeouts, which were changed recently in
aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()").

With a quick glance, the commit does not change the timeouts. However,
the method of delaying/sleeping is different, and as the timeout in the
previous implementation was not explicit, the new version in practice
has much tighter timeout.

The same change was made to other parts in the driver, but the link
training timeout is the only one I have seen causing issues.
Nevertheless, 1 us sleep is not very sane, and the timeouts look pretty
tight, so lets fix all the timeouts.

One exception was the aux busy poll, where the poll sleep was much
longer than necessary (or optimal).

I measured the times on my setup, and now the sleep times are set to
such values that they result in multiple loops, but not too many (say,
5-10 loops). The timeouts were all increased to 100ms, which should be
more than enough for all of these, but in case of bad errors, shouldn't
stop the driver as multi-second timeouts could do.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Fixes: aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()")
---
 drivers/gpu/drm/bridge/tc358767.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Andrey Smirnov Dec. 9, 2019, 2:45 p.m. UTC | #1
+ Chris Healy

On Mon, Dec 9, 2019 at 12:27 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> Link training fails with:
>
>   Link training timeout waiting for LT_LOOPDONE!
>   main link enable error: -110
>
> This is caused by too tight timeouts, which were changed recently in
> aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()").
>
> With a quick glance, the commit does not change the timeouts. However,
> the method of delaying/sleeping is different, and as the timeout in the
> previous implementation was not explicit, the new version in practice
> has much tighter timeout.
>
> The same change was made to other parts in the driver, but the link
> training timeout is the only one I have seen causing issues.
> Nevertheless, 1 us sleep is not very sane, and the timeouts look pretty
> tight, so lets fix all the timeouts.
>
> One exception was the aux busy poll, where the poll sleep was much
> longer than necessary (or optimal).
>
> I measured the times on my setup, and now the sleep times are set to
> such values that they result in multiple loops, but not too many (say,
> 5-10 loops). The timeouts were all increased to 100ms, which should be
> more than enough for all of these, but in case of bad errors, shouldn't
> stop the driver as multi-second timeouts could do.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Fixes: aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()")

Tested on RDU2 with TC358767/eDP panel, doesn't seem to break anything
there, so:

Tested-by: Andrey Smirnov <andrew.smirnov@gmail.com>

> ---
>  drivers/gpu/drm/bridge/tc358767.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 8a8d605021f0..0454675a44cb 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -294,7 +294,7 @@ static inline int tc_poll_timeout(struct tc_data *tc, unsigned int addr,
>
>  static int tc_aux_wait_busy(struct tc_data *tc)
>  {
> -       return tc_poll_timeout(tc, DP0_AUXSTATUS, AUX_BUSY, 0, 1000, 100000);
> +       return tc_poll_timeout(tc, DP0_AUXSTATUS, AUX_BUSY, 0, 100, 100000);
>  }
>
>  static int tc_aux_write_data(struct tc_data *tc, const void *data,
> @@ -637,7 +637,7 @@ static int tc_aux_link_setup(struct tc_data *tc)
>         if (ret)
>                 goto err;
>
> -       ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 1, 1000);
> +       ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 100, 100000);
>         if (ret == -ETIMEDOUT) {
>                 dev_err(tc->dev, "Timeout waiting for PHY to become ready");
>                 return ret;
> @@ -861,7 +861,7 @@ static int tc_wait_link_training(struct tc_data *tc)
>         int ret;
>
>         ret = tc_poll_timeout(tc, DP0_LTSTAT, LT_LOOPDONE,
> -                             LT_LOOPDONE, 1, 1000);
> +                             LT_LOOPDONE, 500, 100000);
>         if (ret) {
>                 dev_err(tc->dev, "Link training timeout waiting for LT_LOOPDONE!\n");
>                 return ret;
> @@ -934,7 +934,7 @@ static int tc_main_link_enable(struct tc_data *tc)
>         dp_phy_ctrl &= ~(DP_PHY_RST | PHY_M1_RST | PHY_M0_RST);
>         ret = regmap_write(tc->regmap, DP_PHY_CTRL, dp_phy_ctrl);
>
> -       ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 1, 1000);
> +       ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 500, 100000);
>         if (ret) {
>                 dev_err(dev, "timeout waiting for phy become ready");
>                 return ret;
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
Tomi Valkeinen Jan. 13, 2020, 1:31 p.m. UTC | #2
Hi Andrzej,

On 09/12/2019 16:45, Andrey Smirnov wrote:
> + Chris Healy
> 
> On Mon, Dec 9, 2019 at 12:27 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>> Link training fails with:
>>
>>    Link training timeout waiting for LT_LOOPDONE!
>>    main link enable error: -110
>>
>> This is caused by too tight timeouts, which were changed recently in
>> aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()").
>>
>> With a quick glance, the commit does not change the timeouts. However,
>> the method of delaying/sleeping is different, and as the timeout in the
>> previous implementation was not explicit, the new version in practice
>> has much tighter timeout.
>>
>> The same change was made to other parts in the driver, but the link
>> training timeout is the only one I have seen causing issues.
>> Nevertheless, 1 us sleep is not very sane, and the timeouts look pretty
>> tight, so lets fix all the timeouts.
>>
>> One exception was the aux busy poll, where the poll sleep was much
>> longer than necessary (or optimal).
>>
>> I measured the times on my setup, and now the sleep times are set to
>> such values that they result in multiple loops, but not too many (say,
>> 5-10 loops). The timeouts were all increased to 100ms, which should be
>> more than enough for all of these, but in case of bad errors, shouldn't
>> stop the driver as multi-second timeouts could do.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Fixes: aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()")
> 
> Tested on RDU2 with TC358767/eDP panel, doesn't seem to break anything
> there, so:
> 
> Tested-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Andrzej, can you pick this up for -fixes?

  Tomi
Neil Armstrong Feb. 10, 2020, 9:18 a.m. UTC | #3
On 13/01/2020 14:31, Tomi Valkeinen wrote:
> Hi Andrzej,
> 
> On 09/12/2019 16:45, Andrey Smirnov wrote:
>> + Chris Healy
>>
>> On Mon, Dec 9, 2019 at 12:27 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>
>>> Link training fails with:
>>>
>>>    Link training timeout waiting for LT_LOOPDONE!
>>>    main link enable error: -110
>>>
>>> This is caused by too tight timeouts, which were changed recently in
>>> aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()").
>>>
>>> With a quick glance, the commit does not change the timeouts. However,
>>> the method of delaying/sleeping is different, and as the timeout in the
>>> previous implementation was not explicit, the new version in practice
>>> has much tighter timeout.
>>>
>>> The same change was made to other parts in the driver, but the link
>>> training timeout is the only one I have seen causing issues.
>>> Nevertheless, 1 us sleep is not very sane, and the timeouts look pretty
>>> tight, so lets fix all the timeouts.
>>>
>>> One exception was the aux busy poll, where the poll sleep was much
>>> longer than necessary (or optimal).
>>>
>>> I measured the times on my setup, and now the sleep times are set to
>>> such values that they result in multiple loops, but not too many (say,
>>> 5-10 loops). The timeouts were all increased to 100ms, which should be
>>> more than enough for all of these, but in case of bad errors, shouldn't
>>> stop the driver as multi-second timeouts could do.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> Fixes: aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()")
>>
>> Tested on RDU2 with TC358767/eDP panel, doesn't seem to break anything
>> there, so:
>>
>> Tested-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> 
> Andrzej, can you pick this up for -fixes?
> 
>  Tomi
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Applied to drm-misc-fixes

Neil
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 8a8d605021f0..0454675a44cb 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -294,7 +294,7 @@  static inline int tc_poll_timeout(struct tc_data *tc, unsigned int addr,
 
 static int tc_aux_wait_busy(struct tc_data *tc)
 {
-	return tc_poll_timeout(tc, DP0_AUXSTATUS, AUX_BUSY, 0, 1000, 100000);
+	return tc_poll_timeout(tc, DP0_AUXSTATUS, AUX_BUSY, 0, 100, 100000);
 }
 
 static int tc_aux_write_data(struct tc_data *tc, const void *data,
@@ -637,7 +637,7 @@  static int tc_aux_link_setup(struct tc_data *tc)
 	if (ret)
 		goto err;
 
-	ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 1, 1000);
+	ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 100, 100000);
 	if (ret == -ETIMEDOUT) {
 		dev_err(tc->dev, "Timeout waiting for PHY to become ready");
 		return ret;
@@ -861,7 +861,7 @@  static int tc_wait_link_training(struct tc_data *tc)
 	int ret;
 
 	ret = tc_poll_timeout(tc, DP0_LTSTAT, LT_LOOPDONE,
-			      LT_LOOPDONE, 1, 1000);
+			      LT_LOOPDONE, 500, 100000);
 	if (ret) {
 		dev_err(tc->dev, "Link training timeout waiting for LT_LOOPDONE!\n");
 		return ret;
@@ -934,7 +934,7 @@  static int tc_main_link_enable(struct tc_data *tc)
 	dp_phy_ctrl &= ~(DP_PHY_RST | PHY_M1_RST | PHY_M0_RST);
 	ret = regmap_write(tc->regmap, DP_PHY_CTRL, dp_phy_ctrl);
 
-	ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 1, 1000);
+	ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 500, 100000);
 	if (ret) {
 		dev_err(dev, "timeout waiting for phy become ready");
 		return ret;