All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: ti-sn65dsi86: Add retries for link training
@ 2020-10-02 21:03 ` Douglas Anderson
  0 siblings, 0 replies; 7+ messages in thread
From: Douglas Anderson @ 2020-10-02 21:03 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Sam Ravnborg
  Cc: Rob Clark, Bjorn Andersson, Steev Klimaszewski, Douglas Anderson,
	Daniel Vetter, David Airlie, Jernej Skrabec, Jonas Karlman,
	Laurent Pinchart, dri-devel, linux-kernel

On some panels hooked up to the ti-sn65dsi86 bridge chip we found that
link training was failing.  Specifically, we'd see:

  ti_sn65dsi86 2-002d: [drm:ti_sn_bridge_enable] *ERROR* Link training failed, link is off (-5)

The panel was hooked up to a logic analyzer and it was found that, as
part of link training, the bridge chip was writing a 0x1 to DPCD
address 00600h and the panel responded NACK.  As can be seen in header
files, the write of 0x1 to DPCD address 0x600h means we were trying to
write the value DP_SET_POWER_D0 to the register DP_SET_POWER.  The
panel vendor says that a NACK in this case is not unexpected and means
"not ready, try again".

In testing, we found that this panel would respond with a NACK in
about 1/25 times.  Adding the retry logic worked fine and the most
number of tries needed was 3.  Just to be safe, we'll add 10 tries
here and we'll add a little blurb to the logs if we ever need more
than 5.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++++++++++++++--------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index ecdf9b01340f..6e12cda69b54 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -106,6 +106,8 @@
 #define SN_NUM_GPIOS			4
 #define SN_GPIO_PHYSICAL_OFFSET		1
 
+#define SN_LINK_TRAINING_TRIES		10
+
 /**
  * struct ti_sn_bridge - Platform data for ti-sn65dsi86 driver.
  * @dev:          Pointer to our device.
@@ -673,6 +675,7 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
 {
 	unsigned int val;
 	int ret;
+	int i;
 
 	/* set dp clk frequency value */
 	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
@@ -689,19 +692,34 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
 		goto exit;
 	}
 
-	/* Semi auto link training mode */
-	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
-	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
-				       val == ML_TX_MAIN_LINK_OFF ||
-				       val == ML_TX_NORMAL_MODE, 1000,
-				       500 * 1000);
-	if (ret) {
-		*last_err_str = "Training complete polling failed";
-	} else if (val == ML_TX_MAIN_LINK_OFF) {
-		*last_err_str = "Link training failed, link is off";
-		ret = -EIO;
+	/*
+	 * We'll try to link train several times.  As part of link training
+	 * the bridge chip will write DP_SET_POWER_D0 to DP_SET_POWER.  If
+	 * the panel isn't ready quite it might respond NAK here which means
+	 * we need to try again.
+	 */
+	for (i = 0; i < SN_LINK_TRAINING_TRIES; i++) {
+		/* Semi auto link training mode */
+		regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
+		ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
+					val == ML_TX_MAIN_LINK_OFF ||
+					val == ML_TX_NORMAL_MODE, 1000,
+					500 * 1000);
+		if (ret) {
+			*last_err_str = "Training complete polling failed";
+		} else if (val == ML_TX_MAIN_LINK_OFF) {
+			*last_err_str = "Link training failed, link is off";
+			ret = -EIO;
+			continue;
+		}
+
+		break;
 	}
 
+	/* If we saw quite a few retries, add a note about it */
+	if (!ret && i > SN_LINK_TRAINING_TRIES / 2)
+		DRM_DEV_INFO(pdata->dev, "Link training needed %d retries\n", i);
+
 exit:
 	/* Disable the PLL if we failed */
 	if (ret)
-- 
2.28.0.806.g8561365e88-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH] drm/bridge: ti-sn65dsi86: Add retries for link training
@ 2020-10-02 21:03 ` Douglas Anderson
  0 siblings, 0 replies; 7+ messages in thread
From: Douglas Anderson @ 2020-10-02 21:03 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Sam Ravnborg
  Cc: Rob Clark, Jernej Skrabec, Jonas Karlman, David Airlie,
	linux-kernel, dri-devel, Douglas Anderson, Steev Klimaszewski,
	Bjorn Andersson, Laurent Pinchart

On some panels hooked up to the ti-sn65dsi86 bridge chip we found that
link training was failing.  Specifically, we'd see:

  ti_sn65dsi86 2-002d: [drm:ti_sn_bridge_enable] *ERROR* Link training failed, link is off (-5)

The panel was hooked up to a logic analyzer and it was found that, as
part of link training, the bridge chip was writing a 0x1 to DPCD
address 00600h and the panel responded NACK.  As can be seen in header
files, the write of 0x1 to DPCD address 0x600h means we were trying to
write the value DP_SET_POWER_D0 to the register DP_SET_POWER.  The
panel vendor says that a NACK in this case is not unexpected and means
"not ready, try again".

In testing, we found that this panel would respond with a NACK in
about 1/25 times.  Adding the retry logic worked fine and the most
number of tries needed was 3.  Just to be safe, we'll add 10 tries
here and we'll add a little blurb to the logs if we ever need more
than 5.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++++++++++++++--------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index ecdf9b01340f..6e12cda69b54 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -106,6 +106,8 @@
 #define SN_NUM_GPIOS			4
 #define SN_GPIO_PHYSICAL_OFFSET		1
 
+#define SN_LINK_TRAINING_TRIES		10
+
 /**
  * struct ti_sn_bridge - Platform data for ti-sn65dsi86 driver.
  * @dev:          Pointer to our device.
@@ -673,6 +675,7 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
 {
 	unsigned int val;
 	int ret;
+	int i;
 
 	/* set dp clk frequency value */
 	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
@@ -689,19 +692,34 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
 		goto exit;
 	}
 
-	/* Semi auto link training mode */
-	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
-	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
-				       val == ML_TX_MAIN_LINK_OFF ||
-				       val == ML_TX_NORMAL_MODE, 1000,
-				       500 * 1000);
-	if (ret) {
-		*last_err_str = "Training complete polling failed";
-	} else if (val == ML_TX_MAIN_LINK_OFF) {
-		*last_err_str = "Link training failed, link is off";
-		ret = -EIO;
+	/*
+	 * We'll try to link train several times.  As part of link training
+	 * the bridge chip will write DP_SET_POWER_D0 to DP_SET_POWER.  If
+	 * the panel isn't ready quite it might respond NAK here which means
+	 * we need to try again.
+	 */
+	for (i = 0; i < SN_LINK_TRAINING_TRIES; i++) {
+		/* Semi auto link training mode */
+		regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
+		ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
+					val == ML_TX_MAIN_LINK_OFF ||
+					val == ML_TX_NORMAL_MODE, 1000,
+					500 * 1000);
+		if (ret) {
+			*last_err_str = "Training complete polling failed";
+		} else if (val == ML_TX_MAIN_LINK_OFF) {
+			*last_err_str = "Link training failed, link is off";
+			ret = -EIO;
+			continue;
+		}
+
+		break;
 	}
 
+	/* If we saw quite a few retries, add a note about it */
+	if (!ret && i > SN_LINK_TRAINING_TRIES / 2)
+		DRM_DEV_INFO(pdata->dev, "Link training needed %d retries\n", i);
+
 exit:
 	/* Disable the PLL if we failed */
 	if (ret)
-- 
2.28.0.806.g8561365e88-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* RE: [PATCH] drm/bridge: ti-sn65dsi86: Add retries for link training
  2020-10-02 21:03 ` Douglas Anderson
  (?)
@ 2020-10-04  2:35 ` Steev Klimaszewski
  -1 siblings, 0 replies; 7+ messages in thread
From: Steev Klimaszewski @ 2020-10-04  2:35 UTC (permalink / raw)
  To: Douglas Anderson, Andrzej Hajda, Neil Armstrong, Sam Ravnborg
  Cc: Rob Clark, Jernej Skrabec, Jonas Karlman, David Airlie,
	linux-kernel, Douglas Anderson, dri-devel, Bjorn Andersson,
	Laurent Pinchart

[-- Attachment #1: Type: text/html, Size: 10176 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi86: Add retries for link training
  2020-10-02 21:03 ` Douglas Anderson
@ 2020-10-04 15:45   ` Steev Klimaszewski
  -1 siblings, 0 replies; 7+ messages in thread
From: Steev Klimaszewski @ 2020-10-04 15:45 UTC (permalink / raw)
  To: Douglas Anderson, Andrzej Hajda, Neil Armstrong, Sam Ravnborg
  Cc: Rob Clark, Bjorn Andersson, Daniel Vetter, David Airlie,
	Jernej Skrabec, Jonas Karlman, Laurent Pinchart, dri-devel,
	linux-kernel


On 10/2/20 4:03 PM, Douglas Anderson wrote:
> On some panels hooked up to the ti-sn65dsi86 bridge chip we found that
> link training was failing.  Specifically, we'd see:
>
>   ti_sn65dsi86 2-002d: [drm:ti_sn_bridge_enable] *ERROR* Link training failed, link is off (-5)
>
> The panel was hooked up to a logic analyzer and it was found that, as
> part of link training, the bridge chip was writing a 0x1 to DPCD
> address 00600h and the panel responded NACK.  As can be seen in header
> files, the write of 0x1 to DPCD address 0x600h means we were trying to
> write the value DP_SET_POWER_D0 to the register DP_SET_POWER.  The
> panel vendor says that a NACK in this case is not unexpected and means
> "not ready, try again".
>
> In testing, we found that this panel would respond with a NACK in
> about 1/25 times.  Adding the retry logic worked fine and the most
> number of tries needed was 3.  Just to be safe, we'll add 10 tries
> here and we'll add a little blurb to the logs if we ever need more
> than 5.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index ecdf9b01340f..6e12cda69b54 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -106,6 +106,8 @@
>  #define SN_NUM_GPIOS			4
>  #define SN_GPIO_PHYSICAL_OFFSET		1
>  
> +#define SN_LINK_TRAINING_TRIES		10
> +
>  /**
>   * struct ti_sn_bridge - Platform data for ti-sn65dsi86 driver.
>   * @dev:          Pointer to our device.
> @@ -673,6 +675,7 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
>  {
>  	unsigned int val;
>  	int ret;
> +	int i;
>  
>  	/* set dp clk frequency value */
>  	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
> @@ -689,19 +692,34 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
>  		goto exit;
>  	}
>  
> -	/* Semi auto link training mode */
> -	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> -	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> -				       val == ML_TX_MAIN_LINK_OFF ||
> -				       val == ML_TX_NORMAL_MODE, 1000,
> -				       500 * 1000);
> -	if (ret) {
> -		*last_err_str = "Training complete polling failed";
> -	} else if (val == ML_TX_MAIN_LINK_OFF) {
> -		*last_err_str = "Link training failed, link is off";
> -		ret = -EIO;
> +	/*
> +	 * We'll try to link train several times.  As part of link training
> +	 * the bridge chip will write DP_SET_POWER_D0 to DP_SET_POWER.  If
> +	 * the panel isn't ready quite it might respond NAK here which means
> +	 * we need to try again.
> +	 */
> +	for (i = 0; i < SN_LINK_TRAINING_TRIES; i++) {
> +		/* Semi auto link training mode */
> +		regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> +		ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> +					val == ML_TX_MAIN_LINK_OFF ||
> +					val == ML_TX_NORMAL_MODE, 1000,
> +					500 * 1000);
> +		if (ret) {
> +			*last_err_str = "Training complete polling failed";
> +		} else if (val == ML_TX_MAIN_LINK_OFF) {
> +			*last_err_str = "Link training failed, link is off";
> +			ret = -EIO;
> +			continue;
> +		}
> +
> +		break;
>  	}
>  
> +	/* If we saw quite a few retries, add a note about it */
> +	if (!ret && i > SN_LINK_TRAINING_TRIES / 2)
> +		DRM_DEV_INFO(pdata->dev, "Link training needed %d retries\n", i);
> +
>  exit:
>  	/* Disable the PLL if we failed */
>  	if (ret)


Apologies for the previous HTML email, I was trying a new mail client
and... will not be switching to it.

Anyway.. again, this time in text..


Tested on the Lenovo C630, and haven’t seen the message, although I
hadn’t seen the described issue before either.

Tested-By: Steev Klimaszewski <steev@kali.org>



^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi86: Add retries for link training
@ 2020-10-04 15:45   ` Steev Klimaszewski
  0 siblings, 0 replies; 7+ messages in thread
From: Steev Klimaszewski @ 2020-10-04 15:45 UTC (permalink / raw)
  To: Douglas Anderson, Andrzej Hajda, Neil Armstrong, Sam Ravnborg
  Cc: Rob Clark, Jernej Skrabec, Jonas Karlman, David Airlie,
	linux-kernel, dri-devel, Bjorn Andersson, Laurent Pinchart


On 10/2/20 4:03 PM, Douglas Anderson wrote:
> On some panels hooked up to the ti-sn65dsi86 bridge chip we found that
> link training was failing.  Specifically, we'd see:
>
>   ti_sn65dsi86 2-002d: [drm:ti_sn_bridge_enable] *ERROR* Link training failed, link is off (-5)
>
> The panel was hooked up to a logic analyzer and it was found that, as
> part of link training, the bridge chip was writing a 0x1 to DPCD
> address 00600h and the panel responded NACK.  As can be seen in header
> files, the write of 0x1 to DPCD address 0x600h means we were trying to
> write the value DP_SET_POWER_D0 to the register DP_SET_POWER.  The
> panel vendor says that a NACK in this case is not unexpected and means
> "not ready, try again".
>
> In testing, we found that this panel would respond with a NACK in
> about 1/25 times.  Adding the retry logic worked fine and the most
> number of tries needed was 3.  Just to be safe, we'll add 10 tries
> here and we'll add a little blurb to the logs if we ever need more
> than 5.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index ecdf9b01340f..6e12cda69b54 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -106,6 +106,8 @@
>  #define SN_NUM_GPIOS			4
>  #define SN_GPIO_PHYSICAL_OFFSET		1
>  
> +#define SN_LINK_TRAINING_TRIES		10
> +
>  /**
>   * struct ti_sn_bridge - Platform data for ti-sn65dsi86 driver.
>   * @dev:          Pointer to our device.
> @@ -673,6 +675,7 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
>  {
>  	unsigned int val;
>  	int ret;
> +	int i;
>  
>  	/* set dp clk frequency value */
>  	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
> @@ -689,19 +692,34 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
>  		goto exit;
>  	}
>  
> -	/* Semi auto link training mode */
> -	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> -	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> -				       val == ML_TX_MAIN_LINK_OFF ||
> -				       val == ML_TX_NORMAL_MODE, 1000,
> -				       500 * 1000);
> -	if (ret) {
> -		*last_err_str = "Training complete polling failed";
> -	} else if (val == ML_TX_MAIN_LINK_OFF) {
> -		*last_err_str = "Link training failed, link is off";
> -		ret = -EIO;
> +	/*
> +	 * We'll try to link train several times.  As part of link training
> +	 * the bridge chip will write DP_SET_POWER_D0 to DP_SET_POWER.  If
> +	 * the panel isn't ready quite it might respond NAK here which means
> +	 * we need to try again.
> +	 */
> +	for (i = 0; i < SN_LINK_TRAINING_TRIES; i++) {
> +		/* Semi auto link training mode */
> +		regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> +		ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> +					val == ML_TX_MAIN_LINK_OFF ||
> +					val == ML_TX_NORMAL_MODE, 1000,
> +					500 * 1000);
> +		if (ret) {
> +			*last_err_str = "Training complete polling failed";
> +		} else if (val == ML_TX_MAIN_LINK_OFF) {
> +			*last_err_str = "Link training failed, link is off";
> +			ret = -EIO;
> +			continue;
> +		}
> +
> +		break;
>  	}
>  
> +	/* If we saw quite a few retries, add a note about it */
> +	if (!ret && i > SN_LINK_TRAINING_TRIES / 2)
> +		DRM_DEV_INFO(pdata->dev, "Link training needed %d retries\n", i);
> +
>  exit:
>  	/* Disable the PLL if we failed */
>  	if (ret)


Apologies for the previous HTML email, I was trying a new mail client
and... will not be switching to it.

Anyway.. again, this time in text..


Tested on the Lenovo C630, and haven’t seen the message, although I
hadn’t seen the described issue before either.

Tested-By: Steev Klimaszewski <steev@kali.org>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi86: Add retries for link training
  2020-10-02 21:03 ` Douglas Anderson
@ 2020-10-16 18:36   ` Sam Ravnborg
  -1 siblings, 0 replies; 7+ messages in thread
From: Sam Ravnborg @ 2020-10-16 18:36 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Andrzej Hajda, Neil Armstrong, Rob Clark, Jernej Skrabec,
	Jonas Karlman, David Airlie, linux-kernel, dri-devel,
	Steev Klimaszewski, Bjorn Andersson, Laurent Pinchart

Hi Douglas.

On Fri, Oct 02, 2020 at 02:03:51PM -0700, Douglas Anderson wrote:
> On some panels hooked up to the ti-sn65dsi86 bridge chip we found that
> link training was failing.  Specifically, we'd see:
> 
>   ti_sn65dsi86 2-002d: [drm:ti_sn_bridge_enable] *ERROR* Link training failed, link is off (-5)
> 
> The panel was hooked up to a logic analyzer and it was found that, as
> part of link training, the bridge chip was writing a 0x1 to DPCD
> address 00600h and the panel responded NACK.  As can be seen in header
> files, the write of 0x1 to DPCD address 0x600h means we were trying to
> write the value DP_SET_POWER_D0 to the register DP_SET_POWER.  The
> panel vendor says that a NACK in this case is not unexpected and means
> "not ready, try again".
> 
> In testing, we found that this panel would respond with a NACK in
> about 1/25 times.  Adding the retry logic worked fine and the most
> number of tries needed was 3.  Just to be safe, we'll add 10 tries
> here and we'll add a little blurb to the logs if we ever need more
> than 5.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

I have picked this patch up and applied to drm-misc-next now.

	Sam

> ---
> 
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index ecdf9b01340f..6e12cda69b54 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -106,6 +106,8 @@
>  #define SN_NUM_GPIOS			4
>  #define SN_GPIO_PHYSICAL_OFFSET		1
>  
> +#define SN_LINK_TRAINING_TRIES		10
> +
>  /**
>   * struct ti_sn_bridge - Platform data for ti-sn65dsi86 driver.
>   * @dev:          Pointer to our device.
> @@ -673,6 +675,7 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
>  {
>  	unsigned int val;
>  	int ret;
> +	int i;
>  
>  	/* set dp clk frequency value */
>  	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
> @@ -689,19 +692,34 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
>  		goto exit;
>  	}
>  
> -	/* Semi auto link training mode */
> -	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> -	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> -				       val == ML_TX_MAIN_LINK_OFF ||
> -				       val == ML_TX_NORMAL_MODE, 1000,
> -				       500 * 1000);
> -	if (ret) {
> -		*last_err_str = "Training complete polling failed";
> -	} else if (val == ML_TX_MAIN_LINK_OFF) {
> -		*last_err_str = "Link training failed, link is off";
> -		ret = -EIO;
> +	/*
> +	 * We'll try to link train several times.  As part of link training
> +	 * the bridge chip will write DP_SET_POWER_D0 to DP_SET_POWER.  If
> +	 * the panel isn't ready quite it might respond NAK here which means
> +	 * we need to try again.
> +	 */
> +	for (i = 0; i < SN_LINK_TRAINING_TRIES; i++) {
> +		/* Semi auto link training mode */
> +		regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> +		ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> +					val == ML_TX_MAIN_LINK_OFF ||
> +					val == ML_TX_NORMAL_MODE, 1000,
> +					500 * 1000);
> +		if (ret) {
> +			*last_err_str = "Training complete polling failed";
> +		} else if (val == ML_TX_MAIN_LINK_OFF) {
> +			*last_err_str = "Link training failed, link is off";
> +			ret = -EIO;
> +			continue;
> +		}
> +
> +		break;
>  	}
>  
> +	/* If we saw quite a few retries, add a note about it */
> +	if (!ret && i > SN_LINK_TRAINING_TRIES / 2)
> +		DRM_DEV_INFO(pdata->dev, "Link training needed %d retries\n", i);
> +
>  exit:
>  	/* Disable the PLL if we failed */
>  	if (ret)
> -- 
> 2.28.0.806.g8561365e88-goog
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi86: Add retries for link training
@ 2020-10-16 18:36   ` Sam Ravnborg
  0 siblings, 0 replies; 7+ messages in thread
From: Sam Ravnborg @ 2020-10-16 18:36 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Rob Clark, Jernej Skrabec, Jonas Karlman, David Airlie,
	Neil Armstrong, linux-kernel, dri-devel, Bjorn Andersson,
	Andrzej Hajda, Steev Klimaszewski, Laurent Pinchart

Hi Douglas.

On Fri, Oct 02, 2020 at 02:03:51PM -0700, Douglas Anderson wrote:
> On some panels hooked up to the ti-sn65dsi86 bridge chip we found that
> link training was failing.  Specifically, we'd see:
> 
>   ti_sn65dsi86 2-002d: [drm:ti_sn_bridge_enable] *ERROR* Link training failed, link is off (-5)
> 
> The panel was hooked up to a logic analyzer and it was found that, as
> part of link training, the bridge chip was writing a 0x1 to DPCD
> address 00600h and the panel responded NACK.  As can be seen in header
> files, the write of 0x1 to DPCD address 0x600h means we were trying to
> write the value DP_SET_POWER_D0 to the register DP_SET_POWER.  The
> panel vendor says that a NACK in this case is not unexpected and means
> "not ready, try again".
> 
> In testing, we found that this panel would respond with a NACK in
> about 1/25 times.  Adding the retry logic worked fine and the most
> number of tries needed was 3.  Just to be safe, we'll add 10 tries
> here and we'll add a little blurb to the logs if we ever need more
> than 5.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

I have picked this patch up and applied to drm-misc-next now.

	Sam

> ---
> 
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index ecdf9b01340f..6e12cda69b54 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -106,6 +106,8 @@
>  #define SN_NUM_GPIOS			4
>  #define SN_GPIO_PHYSICAL_OFFSET		1
>  
> +#define SN_LINK_TRAINING_TRIES		10
> +
>  /**
>   * struct ti_sn_bridge - Platform data for ti-sn65dsi86 driver.
>   * @dev:          Pointer to our device.
> @@ -673,6 +675,7 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
>  {
>  	unsigned int val;
>  	int ret;
> +	int i;
>  
>  	/* set dp clk frequency value */
>  	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
> @@ -689,19 +692,34 @@ static int ti_sn_link_training(struct ti_sn_bridge *pdata, int dp_rate_idx,
>  		goto exit;
>  	}
>  
> -	/* Semi auto link training mode */
> -	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> -	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> -				       val == ML_TX_MAIN_LINK_OFF ||
> -				       val == ML_TX_NORMAL_MODE, 1000,
> -				       500 * 1000);
> -	if (ret) {
> -		*last_err_str = "Training complete polling failed";
> -	} else if (val == ML_TX_MAIN_LINK_OFF) {
> -		*last_err_str = "Link training failed, link is off";
> -		ret = -EIO;
> +	/*
> +	 * We'll try to link train several times.  As part of link training
> +	 * the bridge chip will write DP_SET_POWER_D0 to DP_SET_POWER.  If
> +	 * the panel isn't ready quite it might respond NAK here which means
> +	 * we need to try again.
> +	 */
> +	for (i = 0; i < SN_LINK_TRAINING_TRIES; i++) {
> +		/* Semi auto link training mode */
> +		regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> +		ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> +					val == ML_TX_MAIN_LINK_OFF ||
> +					val == ML_TX_NORMAL_MODE, 1000,
> +					500 * 1000);
> +		if (ret) {
> +			*last_err_str = "Training complete polling failed";
> +		} else if (val == ML_TX_MAIN_LINK_OFF) {
> +			*last_err_str = "Link training failed, link is off";
> +			ret = -EIO;
> +			continue;
> +		}
> +
> +		break;
>  	}
>  
> +	/* If we saw quite a few retries, add a note about it */
> +	if (!ret && i > SN_LINK_TRAINING_TRIES / 2)
> +		DRM_DEV_INFO(pdata->dev, "Link training needed %d retries\n", i);
> +
>  exit:
>  	/* Disable the PLL if we failed */
>  	if (ret)
> -- 
> 2.28.0.806.g8561365e88-goog
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-10-16 18:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-02 21:03 [PATCH] drm/bridge: ti-sn65dsi86: Add retries for link training Douglas Anderson
2020-10-02 21:03 ` Douglas Anderson
2020-10-04  2:35 ` Steev Klimaszewski
2020-10-04 15:45 ` Steev Klimaszewski
2020-10-04 15:45   ` Steev Klimaszewski
2020-10-16 18:36 ` Sam Ravnborg
2020-10-16 18:36   ` Sam Ravnborg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.