[PATCH v2 0/7] drm/bridge: tc358767: fixes and improvements (resend)

[PATCH v2 0/7] drm/bridge: tc358767: fixes and improvements (resend)

[Andrey Gusakov]

This set of patches fixes several issues that was found during testing
tc358767 with desktop DisplayPort displays.

Changes in V2:
	- fixed maximum pixelclock frequency
	- copyright patch added

Andrey Gusakov (7):
  drm/bridge: tc358767: do no fail on hi-res displays
  drm/bridge: tc358767: filter out too high modes
  drm/bridge: tc358767: fix DP0_MISC register set
  drm/bridge: tc358767: fix timing calculations
  drm/bridge: tc358767: fix AUXDATAn registers access
  drm/bridge: tc358767: fix 1-lane behavior
  drm/bridge: tc358767: add copyright lines

 drivers/gpu/drm/bridge/tc358767.c |   75 +++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 33 deletions(-)

-- 
1.7.10.4

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

[PATCH v2 1/7] drm/bridge: tc358767: do no fail on hi-res displays

[Andrey Gusakov]

Do not fail data rates higher than 2.7 and more than 2 lanes.
Try to fall back to 2.7Gbps and 2 lanes.
---
 drivers/gpu/drm/bridge/tc358767.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 8571cfd..d52cd41 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -603,8 +603,15 @@ static int tc_get_display_props(struct tc_data *tc)
 	ret = drm_dp_link_probe(&tc->aux, &tc->link.base);
 	if (ret < 0)
 		goto err_dpcd_read;
-	if ((tc->link.base.rate != 162000) && (tc->link.base.rate != 270000))
-		goto err_dpcd_inval;
+	if ((tc->link.base.rate != 162000) && (tc->link.base.rate != 270000)) {
+		dev_dbg(tc->dev, "Falling to 2.7 Gbps rate\n");
+		tc->link.base.rate = 270000;
+	}
+
+	if (tc->link.base.num_lanes > 2) {
+		dev_dbg(tc->dev, "Falling to 2 lanes\n");
+		tc->link.base.num_lanes = 2;
+	}
 
 	ret = drm_dp_dpcd_readb(&tc->aux, DP_MAX_DOWNSPREAD, tmp);
 	if (ret < 0)
@@ -637,9 +644,6 @@ static int tc_get_display_props(struct tc_data *tc)
 err_dpcd_read:
 	dev_err(tc->dev, "failed to read DPCD: %d\n", ret);
 	return ret;
-err_dpcd_inval:
-	dev_err(tc->dev, "invalid DPCD\n");
-	return -EINVAL;
 }
 
 static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
-- 
1.7.10.4

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

[PATCH v2 2/7] drm/bridge: tc358767: filter out too high modes

[Andrey Gusakov]

Minimum pixel clock period is 6.5 nS for DPI. Do not accept  modes
with lower pixel clock period.
---
 drivers/gpu/drm/bridge/tc358767.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index d52cd41..7f9c2e0 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1103,7 +1103,10 @@ static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
 static int tc_connector_mode_valid(struct drm_connector *connector,
 				   struct drm_display_mode *mode)
 {
-	/* Accept any mode */
+	/* PCLK limitation = 6.5 nS */
+	if (mode->clock > 154000)
+		return MODE_CLOCK_HIGH;
+
 	return MODE_OK;
 }
 
-- 
1.7.10.4

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

[PATCH v2 3/7] drm/bridge: tc358767: fix DP0_MISC register set

[Andrey Gusakov]

Remove shift from TU_SIZE_RECOMMENDED define as it used to
calculate max_tu_symbols.
---
 drivers/gpu/drm/bridge/tc358767.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 7f9c2e0..90d6b49 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -97,7 +97,7 @@
 #define DP0_ACTIVEVAL		0x0650
 #define DP0_SYNCVAL		0x0654
 #define DP0_MISC		0x0658
-#define TU_SIZE_RECOMMENDED		(0x3f << 16) /* LSCLK cycles per TU */
+#define TU_SIZE_RECOMMENDED		(63) /* LSCLK cycles per TU */
 #define BPC_6				(0 << 5)
 #define BPC_8				(1 << 5)
 
@@ -716,7 +716,8 @@ static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
 	 * Must be less than tu_size.
 	 */
 	max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
-	tc_write(DP0_MISC, (max_tu_symbol << 23) | TU_SIZE_RECOMMENDED | BPC_8);
+	tc_write(DP0_MISC, (max_tu_symbol << 23) | (TU_SIZE_RECOMMENDED << 16) |
+			   BPC_8);
 
 	return 0;
 err:
-- 
1.7.10.4

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

[PATCH v2 4/7] drm/bridge: tc358767: fix timing calculations

[Andrey Gusakov]

Fields in HTIM01 and HTIM02 regs should be even.
Recomended thresh_dly value is max_tu_symbol.
---
 drivers/gpu/drm/bridge/tc358767.c |   34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 90d6b49..319d582 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -659,6 +659,14 @@ static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
 	int lower_margin = mode->vsync_start - mode->vdisplay;
 	int vsync_len = mode->vsync_end - mode->vsync_start;
 
+	/*
+	 * Recommended maximum number of symbols transferred in a transfer unit:
+	 * DIV_ROUND_UP((input active video bandwidth in bytes) * tu_size,
+	 *              (output active video bandwidth in bytes))
+	 * Must be less than tu_size.
+	 */
+	max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
+
 	dev_dbg(tc->dev, "set mode %dx%d\n",
 		mode->hdisplay, mode->vdisplay);
 	dev_dbg(tc->dev, "H margin %d,%d sync %d\n",
@@ -668,13 +676,18 @@ static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
 	dev_dbg(tc->dev, "total: %dx%d\n", mode->htotal, mode->vtotal);
 
 
-	/* LCD Ctl Frame Size */
-	tc_write(VPCTRL0, (0x40 << 20) /* VSDELAY */ |
+	/*
+	 * LCD Ctl Frame Size
+	 * datasheet is not clear of vsdelay in case of DPI
+	 * assume we do not need any delay when DPI is a source of
+	 * sync signals
+	 */
+	tc_write(VPCTRL0, (0 << 20) /* VSDELAY */ |
 		 OPXLFMT_RGB888 | FRMSYNC_DISABLED | MSF_DISABLED);
-	tc_write(HTIM01, (left_margin << 16) |		/* H back porch */
-			 (hsync_len << 0));		/* Hsync */
-	tc_write(HTIM02, (right_margin << 16) |		/* H front porch */
-			 (mode->hdisplay << 0));	/* width */
+	tc_write(HTIM01, (ALIGN(left_margin, 2) << 16) | /* H back porch */
+			 (ALIGN(hsync_len, 2) << 0));	 /* Hsync */
+	tc_write(HTIM02, (ALIGN(right_margin, 2) << 16) |  /* H front porch */
+			 (ALIGN(mode->hdisplay, 2) << 0)); /* width */
 	tc_write(VTIM01, (upper_margin << 16) |		/* V back porch */
 			 (vsync_len << 0));		/* Vsync */
 	tc_write(VTIM02, (lower_margin << 16) |		/* V front porch */
@@ -693,7 +706,7 @@ static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
 	/* DP Main Stream Attributes */
 	vid_sync_dly = hsync_len + left_margin + mode->hdisplay;
 	tc_write(DP0_VIDSYNCDELAY,
-		 (0x003e << 16) |	/* thresh_dly */
+		 (max_tu_symbol << 16) |	/* thresh_dly */
 		 (vid_sync_dly << 0));
 
 	tc_write(DP0_TOTALVAL, (mode->vtotal << 16) | (mode->htotal));
@@ -709,13 +722,6 @@ static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
 	tc_write(DPIPXLFMT, VS_POL_ACTIVE_LOW | HS_POL_ACTIVE_LOW |
 		 DE_POL_ACTIVE_HIGH | SUB_CFG_TYPE_CONFIG1 | DPI_BPP_RGB888);
 
-	/*
-	 * Recommended maximum number of symbols transferred in a transfer unit:
-	 * DIV_ROUND_UP((input active video bandwidth in bytes) * tu_size,
-	 *              (output active video bandwidth in bytes))
-	 * Must be less than tu_size.
-	 */
-	max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
 	tc_write(DP0_MISC, (max_tu_symbol << 23) | (TU_SIZE_RECOMMENDED << 16) |
 			   BPC_8);
 
-- 
1.7.10.4

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

[PATCH v2 5/7] drm/bridge: tc358767: fix AUXDATAn registers access

[Andrey Gusakov]

First four bytes should go to DP0_AUXWDATA0. Due to bug if
len > 4 first four bytes was writen to DP0_AUXWDATA1 and all
data get shifted by 4 bytes. Fix it.
---
 drivers/gpu/drm/bridge/tc358767.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 319d582..4394131 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -318,7 +318,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
 				tmp = (tmp << 8) | buf[i];
 			i++;
 			if (((i % 4) == 0) || (i == size)) {
-				tc_write(DP0_AUXWDATA(i >> 2), tmp);
+				tc_write(DP0_AUXWDATA((i - 1) >> 2), tmp);
 				tmp = 0;
 			}
 		}
-- 
1.7.10.4

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

[PATCH v2 6/7] drm/bridge: tc358767: fix 1-lane behavior

[Andrey Gusakov]

Use drm_dp_channel_eq_ok helper
---
 drivers/gpu/drm/bridge/tc358767.c |   13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 4394131..1e677be 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -819,8 +819,6 @@ static int tc_main_link_setup(struct tc_data *tc)
 	unsigned int rate;
 	u32 dp_phy_ctrl;
 	int timeout;
-	bool aligned;
-	bool ready;
 	u32 value;
 	int ret;
 	u8 tmp[8];
@@ -965,16 +963,15 @@ static int tc_main_link_setup(struct tc_data *tc)
 		ret = drm_dp_dpcd_read_link_status(aux, tmp + 2);
 		if (ret < 0)
 			goto err_dpcd_read;
-		ready = (tmp[2] == ((DP_CHANNEL_EQ_BITS << 4) | /* Lane1 */
-				     DP_CHANNEL_EQ_BITS));      /* Lane0 */
-		aligned = tmp[4] & DP_INTERLANE_ALIGN_DONE;
-	} while ((--timeout) && !(ready && aligned));
+	} while ((--timeout) &&
+		 !(drm_dp_channel_eq_ok(tmp + 2,  tc->link.base.num_lanes)));
 
 	if (timeout == 0) {
 		/* Read DPCD 0x200-0x201 */
 		ret = drm_dp_dpcd_read(aux, DP_SINK_COUNT, tmp, 2);
 		if (ret < 0)
 			goto err_dpcd_read;
+		dev_err(dev, "channel(s) EQ not ok\n");
 		dev_info(dev, "0x0200 SINK_COUNT: 0x%02x\n", tmp[0]);
 		dev_info(dev, "0x0201 DEVICE_SERVICE_IRQ_VECTOR: 0x%02x\n",
 			 tmp[1]);
@@ -985,10 +982,6 @@ static int tc_main_link_setup(struct tc_data *tc)
 		dev_info(dev, "0x0206 ADJUST_REQUEST_LANE0_1: 0x%02x\n",
 			 tmp[6]);
 
-		if (!ready)
-			dev_err(dev, "Lane0/1 not ready\n");
-		if (!aligned)
-			dev_err(dev, "Lane0/1 not aligned\n");
 		return -EAGAIN;
 	}
 
-- 
1.7.10.4

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

[PATCH v2 7/7] drm/bridge: tc358767: add copyright lines

[Andrey Gusakov]

Add copyright lines for Zodiac who paid for driver development.

Signed-off-by: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
---
 drivers/gpu/drm/bridge/tc358767.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 1e677be..d98d717 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -6,6 +6,8 @@
  *
  * Copyright (C) 2016 Pengutronix, Philipp Zabel <p.zabel@pengutronix.de>
  *
+ * Copyright (C) 2016 Zodiac Inflight Innovations
+ *
  * Initially based on: drivers/gpu/drm/i2c/tda998x_drv.c
  *
  * Copyright (C) 2012 Texas Instruments
-- 
1.7.10.4

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

Re: [PATCH v2 1/7] drm/bridge: tc358767: do no fail on hi-res displays

[Andrzej Hajda]

On 03.11.2017 18:01, Andrey Gusakov wrote:
> Do not fail data rates higher than 2.7 and more than 2 lanes.
> Try to fall back to 2.7Gbps and 2 lanes.
> ---
>  drivers/gpu/drm/bridge/tc358767.c |   14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 8571cfd..d52cd41 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -603,8 +603,15 @@ static int tc_get_display_props(struct tc_data *tc)
>  	ret = drm_dp_link_probe(&tc->aux, &tc->link.base);
>  	if (ret < 0)
>  		goto err_dpcd_read;
> -	if ((tc->link.base.rate != 162000) && (tc->link.base.rate != 270000))
> -		goto err_dpcd_inval;
> +	if ((tc->link.base.rate != 162000) && (tc->link.base.rate != 270000)) {
> +		dev_dbg(tc->dev, "Falling to 2.7 Gbps rate\n");
> +		tc->link.base.rate = 270000;
> +	}
> +
> +	if (tc->link.base.num_lanes > 2) {
> +		dev_dbg(tc->dev, "Falling to 2 lanes\n");
> +		tc->link.base.num_lanes = 2;
> +	}
>  
>  	ret = drm_dp_dpcd_readb(&tc->aux, DP_MAX_DOWNSPREAD, tmp);
>  	if (ret < 0)
> @@ -637,9 +644,6 @@ static int tc_get_display_props(struct tc_data *tc)
>  err_dpcd_read:
>  	dev_err(tc->dev, "failed to read DPCD: %d\n", ret);
>  	return ret;
> -err_dpcd_inval:
> -	dev_err(tc->dev, "invalid DPCD\n");
> -	return -EINVAL;
>  }
>  
>  static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)

It looks like tc_get_display_props does more than its name suggests, ie
it gets display caps and matches with source caps, but this is little
bit different issue.

So:

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej

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

Re: [PATCH v2 2/7] drm/bridge: tc358767: filter out too high modes

[Andrzej Hajda]

On 03.11.2017 18:01, Andrey Gusakov wrote:
> Minimum pixel clock period is 6.5 nS for DPI. Do not accept  modes
> with lower pixel clock period.
> ---
>  drivers/gpu/drm/bridge/tc358767.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index d52cd41..7f9c2e0 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1103,7 +1103,10 @@ static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
>  static int tc_connector_mode_valid(struct drm_connector *connector,
>  				   struct drm_display_mode *mode)
>  {
> -	/* Accept any mode */
> +	/* PCLK limitation = 6.5 nS */
> +	if (mode->clock > 154000)
> +		return MODE_CLOCK_HIGH;
> +

From Datasheet I've found, and your response in v1 it looks like
Datasheet defines the limit as "154 MPs (MPixel per sec)",
so let's remove 6.5nS from comment and description (if there will be
next patchset iteration, or quick fix).

This is not blocking issue so:
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej

>  	return MODE_OK;
>  }
>  


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

Re: [PATCH v2 0/7] drm/bridge: tc358767: fixes and improvements (resend)

[Andrzej Hajda]

On 03.11.2017 18:01, Andrey Gusakov wrote:
> This set of patches fixes several issues that was found during testing
> tc358767 with desktop DisplayPort displays.
>
> Changes in V2:
> 	- fixed maximum pixelclock frequency
> 	- copyright patch added
>
> Andrey Gusakov (7):
>   drm/bridge: tc358767: do no fail on hi-res displays
>   drm/bridge: tc358767: filter out too high modes
>   drm/bridge: tc358767: fix DP0_MISC register set
>   drm/bridge: tc358767: fix timing calculations
>   drm/bridge: tc358767: fix AUXDATAn registers access
>   drm/bridge: tc358767: fix 1-lane behavior
>   drm/bridge: tc358767: add copyright lines
>
>  drivers/gpu/drm/bridge/tc358767.c |   75 +++++++++++++++++++++----------------
>  1 file changed, 42 insertions(+), 33 deletions(-)
>
v1 of the patchset was already reviewed, there are no Acks in this
patchset and one comment was not addressed( VSDELAY removal in patch 4).

Beside this the patchset looks OK, it probably just requires Ack for
copyright patch and could be merged.


Regards

Andrzej


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

Re: [PATCH v2 0/7] drm/bridge: tc358767: fixes and improvements (resend)

[Archit Taneja]

On 11/06/2017 01:28 PM, Andrzej Hajda wrote:
> On 03.11.2017 18:01, Andrey Gusakov wrote:
>> This set of patches fixes several issues that was found during testing
>> tc358767 with desktop DisplayPort displays.
>>
>> Changes in V2:
>> 	- fixed maximum pixelclock frequency
>> 	- copyright patch added
>>
>> Andrey Gusakov (7):
>>    drm/bridge: tc358767: do no fail on hi-res displays
>>    drm/bridge: tc358767: filter out too high modes
>>    drm/bridge: tc358767: fix DP0_MISC register set
>>    drm/bridge: tc358767: fix timing calculations
>>    drm/bridge: tc358767: fix AUXDATAn registers access
>>    drm/bridge: tc358767: fix 1-lane behavior
>>    drm/bridge: tc358767: add copyright lines
>>
>>   drivers/gpu/drm/bridge/tc358767.c |   75 +++++++++++++++++++++----------------
>>   1 file changed, 42 insertions(+), 33 deletions(-)
>>
> v1 of the patchset was already reviewed, there are no Acks in this
> patchset and one comment was not addressed( VSDELAY removal in patch 4).
> 
> Beside this the patchset looks OK, it probably just requires Ack for
> copyright patch and could be merged.

Also, all the patches apart from 7/7 don't have your "Signed-off-by" in the
commit message, could you please add those, and the "Reviewed-by"s

Thanks,
Archit

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

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel