[PATCH 0/7] drm/bridge: tc358767: small fixes

[PATCH 0/7] drm/bridge: tc358767: small fixes

[Tomi Valkeinen]

Hi,

We have TC358867 on our board, which I believe is almost identical to
TC358767. We're using it with a DP connector instead of eDP with a fixed
panel.

I have tested these patches only on TI's 4.14 based kernel, as
unfortunately we don't have all the necessary support in mainline yet.
These patches fix various bugs, but I'm still seeing at least two
issues:

* Sync with some videomodes is not correct, resulting in a jumping and
  skewed display
* Link training fails sometimes

I would appreciate if someone is able to verify these patches with
TC358767.

 Tomi

Tomi Valkeinen (7):
  drm/bridge: tc358767: add bus flags
  drm/bridge: tc358767: add defines for DP1_SRCCTRL & PHY_2LANE
  drm/bridge: tc358767: fix single lane configuration
  drm/bridge: tc358767: fix initial DP0/1_SRCCTRL value
  drm/bridge: tc358767: reject modes which require too much BW
  drm/bridge: tc358767: fix output H/V syncs
  drm/bridge: tc358767: use DP connector if no panel set

 drivers/gpu/drm/bridge/tc358767.c | 46 ++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 10 deletions(-)

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 1/7] drm/bridge: tc358767: add bus flags

[Tomi Valkeinen]

tc358767 driver does not set DRM bus_flags, even if it does configures
the polarity settings into its registers. This means that the DPI source
can't configure the polarities correctly.

Add sync flags accordingly.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 8e28e738cb52..29a7e33e8ae0 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1195,6 +1195,10 @@ static int tc_bridge_attach(struct drm_bridge *bridge)
 
 	drm_display_info_set_bus_formats(&tc->connector.display_info,
 					 &bus_format, 1);
+	tc->connector.display_info.bus_flags =
+		DRM_BUS_FLAG_DE_HIGH |
+		DRM_BUS_FLAG_PIXDATA_NEGEDGE |
+		DRM_BUS_FLAG_SYNC_NEGEDGE;
 	drm_connector_attach_encoder(&tc->connector, tc->bridge.encoder);
 
 	return 0;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 2/7] drm/bridge: tc358767: add defines for DP1_SRCCTRL & PHY_2LANE

[Tomi Valkeinen]

DP1_SRCCTRL register and PHY_2LANE field did not have matching defines.
Add these.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 29a7e33e8ae0..5f0a666db2fd 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -142,6 +142,8 @@
 #define DP0_LTLOOPCTRL		0x06d8
 #define DP0_SNKLTCTRL		0x06e4
 
+#define DP1_SRCCTRL		0x07a0
+
 /* PHY */
 #define DP_PHY_CTRL		0x0800
 #define DP_PHY_RST			BIT(28)  /* DP PHY Global Soft Reset */
@@ -150,6 +152,7 @@
 #define PHY_M1_RST			BIT(12)  /* Reset PHY1 Main Channel */
 #define PHY_RDY				BIT(16)  /* PHY Main Channels Ready */
 #define PHY_M0_RST			BIT(8)   /* Reset PHY0 Main Channel */
+#define PHY_2LANE			BIT(2)   /* PHY Enable 2 lanes */
 #define PHY_A0_EN			BIT(1)   /* PHY Aux Channel0 Enable */
 #define PHY_M0_EN			BIT(0)   /* PHY Main Channel0 Enable */
 
@@ -564,7 +567,7 @@ static int tc_aux_link_setup(struct tc_data *tc)
 	value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2;
 	tc_write(SYS_PLLPARAM, value);
 
-	tc_write(DP_PHY_CTRL, BGREN | PWR_SW_EN | BIT(2) | PHY_A0_EN);
+	tc_write(DP_PHY_CTRL, BGREN | PWR_SW_EN | PHY_2LANE | PHY_A0_EN);
 
 	/*
 	 * Initially PLLs are in bypass. Force PLL parameter update,
@@ -834,7 +837,7 @@ static int tc_main_link_setup(struct tc_data *tc)
 		 DP0_SRCCTRL_LANESKEW | DP0_SRCCTRL_LANES_2 |
 		 DP0_SRCCTRL_BW27 | DP0_SRCCTRL_AUTOCORRECT);
 	/* from excel file - DP1_SrcCtrl */
-	tc_write(0x07a0, 0x00003083);
+	tc_write(DP1_SRCCTRL, 0x00003083);
 
 	rate = clk_get_rate(tc->refclk);
 	switch (rate) {
@@ -855,8 +858,9 @@ static int tc_main_link_setup(struct tc_data *tc)
 	}
 	value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2;
 	tc_write(SYS_PLLPARAM, value);
+
 	/* Setup Main Link */
-	dp_phy_ctrl = BGREN | PWR_SW_EN | BIT(2) | PHY_A0_EN |  PHY_M0_EN;
+	dp_phy_ctrl = BGREN | PWR_SW_EN | PHY_2LANE | PHY_A0_EN |  PHY_M0_EN;
 	tc_write(DP_PHY_CTRL, dp_phy_ctrl);
 	msleep(100);
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 3/7] drm/bridge: tc358767: fix single lane configuration

[Tomi Valkeinen]

PHY_2LANE bit is always set in DP_PHY_CTRL, breaking 1 lane use.

Set PHY_2LANE only when 2 lanes are used.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 5f0a666db2fd..fee53422c31f 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -543,6 +543,7 @@ static int tc_aux_link_setup(struct tc_data *tc)
 	unsigned long rate;
 	u32 value;
 	int ret;
+	u32 dp_phy_ctrl;
 
 	rate = clk_get_rate(tc->refclk);
 	switch (rate) {
@@ -567,7 +568,10 @@ static int tc_aux_link_setup(struct tc_data *tc)
 	value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2;
 	tc_write(SYS_PLLPARAM, value);
 
-	tc_write(DP_PHY_CTRL, BGREN | PWR_SW_EN | PHY_2LANE | PHY_A0_EN);
+	dp_phy_ctrl = BGREN | PWR_SW_EN | PHY_A0_EN;
+	if (tc->link.base.num_lanes == 2)
+		dp_phy_ctrl |= PHY_2LANE;
+	tc_write(DP_PHY_CTRL, dp_phy_ctrl);
 
 	/*
 	 * Initially PLLs are in bypass. Force PLL parameter update,
@@ -860,7 +864,9 @@ static int tc_main_link_setup(struct tc_data *tc)
 	tc_write(SYS_PLLPARAM, value);
 
 	/* Setup Main Link */
-	dp_phy_ctrl = BGREN | PWR_SW_EN | PHY_2LANE | PHY_A0_EN |  PHY_M0_EN;
+	dp_phy_ctrl = BGREN | PWR_SW_EN | PHY_A0_EN | PHY_M0_EN;
+	if (tc->link.base.num_lanes == 2)
+		dp_phy_ctrl |= PHY_2LANE;
 	tc_write(DP_PHY_CTRL, dp_phy_ctrl);
 	msleep(100);
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 4/7] drm/bridge: tc358767: fix initial DP0/1_SRCCTRL value

[Tomi Valkeinen]

Initially DP0_SRCCTRL is set to a static value which includes
DP0_SRCCTRL_LANES_2 and DP0_SRCCTRL_BW27, even when only 1 lane of
1.62Gbps speed is used. DP1_SRCCTRL is configured to a magic number.

This patch changes the configuration as follows:

Configure DP0_SRCCTRL by using tc_srcctrl() which provides the correct
value.

DP1_SRCCTRL needs to bits to be set to the same value as DP0_SRCCTRL:
SSCG and BW27. All other bits can be zero.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index fee53422c31f..ab299f4debfa 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -836,12 +836,11 @@ static int tc_main_link_setup(struct tc_data *tc)
 	if (!tc->mode)
 		return -EINVAL;
 
-	/* from excel file - DP0_SrcCtrl */
-	tc_write(DP0_SRCCTRL, DP0_SRCCTRL_SCRMBLDIS | DP0_SRCCTRL_EN810B |
-		 DP0_SRCCTRL_LANESKEW | DP0_SRCCTRL_LANES_2 |
-		 DP0_SRCCTRL_BW27 | DP0_SRCCTRL_AUTOCORRECT);
-	/* from excel file - DP1_SrcCtrl */
-	tc_write(DP1_SRCCTRL, 0x00003083);
+	tc_write(DP0_SRCCTRL, tc_srcctrl(tc));
+	/* SSCG and BW27 on DP1 must be set to the same as on DP0 */
+	tc_write(DP1_SRCCTRL,
+		 (tc->link.spread ? DP0_SRCCTRL_SSCG : 0) |
+		 ((tc->link.base.rate != 162000) ? DP0_SRCCTRL_BW27 : 0));
 
 	rate = clk_get_rate(tc->refclk);
 	switch (rate) {
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 5/7] drm/bridge: tc358767: reject modes which require too much BW

[Tomi Valkeinen]

The current driver accepts any videomode with pclk < 154MHz. This is not
correct, as with 1 lane and/or 1.62Mbps speed not all videomodes can be
supported.

Add code to reject modes that require more bandwidth that is available.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index ab299f4debfa..b026b5ef7378 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1114,10 +1114,20 @@ static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
 static enum drm_mode_status tc_connector_mode_valid(struct drm_connector *connector,
 				   struct drm_display_mode *mode)
 {
+	struct tc_data *tc = connector_to_tc(connector);
+	u32 req, avail;
+	u32 bits_per_pixel = 24;
+
 	/* DPI interface clock limitation: upto 154 MHz */
 	if (mode->clock > 154000)
 		return MODE_CLOCK_HIGH;
 
+	req = mode->clock * bits_per_pixel / 8 / 1000;
+	avail = tc->link.base.num_lanes * tc->link.base.rate / 1000;
+
+	if (req > avail)
+		return MODE_BAD;
+
 	return MODE_OK;
 }
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 6/7] drm/bridge: tc358767: fix output H/V syncs

[Tomi Valkeinen]

The H and V syncs of the DP output are always set to active high. This
patch fixes the syncs by configuring them according to the videomode.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index b026b5ef7378..be013bd7b30b 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -726,7 +726,9 @@ static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
 
 	tc_write(DP0_ACTIVEVAL, (mode->vdisplay << 16) | (mode->hdisplay));
 
-	tc_write(DP0_SYNCVAL, (vsync_len << 16) | (hsync_len << 0));
+	tc_write(DP0_SYNCVAL, (vsync_len << 16) | (hsync_len << 0) |
+		 ((mode->flags & DRM_MODE_FLAG_NHSYNC) ? (1 << 15) : 0) |
+		 ((mode->flags & DRM_MODE_FLAG_NVSYNC) ? (1 << 31) : 0));
 
 	tc_write(DPIPXLFMT, VS_POL_ACTIVE_LOW | HS_POL_ACTIVE_LOW |
 		 DE_POL_ACTIVE_HIGH | SUB_CFG_TYPE_CONFIG1 | DPI_BPP_RGB888);
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 7/7] drm/bridge: tc358767: use DP connector if no panel set

[Tomi Valkeinen]

tc358767 driver sets the connector type always to eDP.

This patch sets the type to DP if there is no panel defined, which
implies that there's a DP connector on the board.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index be013bd7b30b..db93d927cb9a 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1207,7 +1207,8 @@ static int tc_bridge_attach(struct drm_bridge *bridge)
 	/* Create eDP connector */
 	drm_connector_helper_add(&tc->connector, &tc_connector_helper_funcs);
 	ret = drm_connector_init(drm, &tc->connector, &tc_connector_funcs,
-				 DRM_MODE_CONNECTOR_eDP);
+				 tc->panel ? DRM_MODE_CONNECTOR_eDP :
+				 DRM_MODE_CONNECTOR_DisplayPort);
 	if (ret)
 		return ret;
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

Re: [PATCH 4/7] drm/bridge: tc358767: fix initial DP0/1_SRCCTRL value

[Andrzej Hajda]

On 29.10.2018 12:46, Tomi Valkeinen wrote:
> Initially DP0_SRCCTRL is set to a static value which includes
> DP0_SRCCTRL_LANES_2 and DP0_SRCCTRL_BW27, even when only 1 lane of
> 1.62Gbps speed is used. DP1_SRCCTRL is configured to a magic number.
>
> This patch changes the configuration as follows:
>
> Configure DP0_SRCCTRL by using tc_srcctrl() which provides the correct
> value.
>
> DP1_SRCCTRL needs to bits to be set to the same value as DP0_SRCCTRL:

s/to/two/


> SSCG and BW27. All other bits can be zero.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index fee53422c31f..ab299f4debfa 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -836,12 +836,11 @@ static int tc_main_link_setup(struct tc_data *tc)
>  	if (!tc->mode)
>  		return -EINVAL;
>  
> -	/* from excel file - DP0_SrcCtrl */
> -	tc_write(DP0_SRCCTRL, DP0_SRCCTRL_SCRMBLDIS | DP0_SRCCTRL_EN810B |
> -		 DP0_SRCCTRL_LANESKEW | DP0_SRCCTRL_LANES_2 |
> -		 DP0_SRCCTRL_BW27 | DP0_SRCCTRL_AUTOCORRECT);
> -	/* from excel file - DP1_SrcCtrl */
> -	tc_write(DP1_SRCCTRL, 0x00003083);
> +	tc_write(DP0_SRCCTRL, tc_srcctrl(tc));
> +	/* SSCG and BW27 on DP1 must be set to the same as on DP0 */
> +	tc_write(DP1_SRCCTRL,
> +		 (tc->link.spread ? DP0_SRCCTRL_SSCG : 0) |
> +		 ((tc->link.base.rate != 162000) ? DP0_SRCCTRL_BW27 : 0));


Wouldn't  be better then to modify tc_srcctrl to support DP0 and DP1:

    tc_write(DP0_SRCCTRL, tc_srcctrl(tc, 0));

    tc_write(DP1_SRCCTRL, tc_srcctrl(tc, 1));

Just suggestion.

With or without this change:

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

 --
Regards
Andrzej


Regards

Andrzej



>  
>  	rate = clk_get_rate(tc->refclk);
>  	switch (rate) {


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

Re: [PATCH 6/7] drm/bridge: tc358767: fix output H/V syncs

[Andrzej Hajda]

On 29.10.2018 12:47, Tomi Valkeinen wrote:
> The H and V syncs of the DP output are always set to active high. This
> patch fixes the syncs by configuring them according to the videomode.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index b026b5ef7378..be013bd7b30b 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -726,7 +726,9 @@ static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
>  
>  	tc_write(DP0_ACTIVEVAL, (mode->vdisplay << 16) | (mode->hdisplay));
>  
> -	tc_write(DP0_SYNCVAL, (vsync_len << 16) | (hsync_len << 0));
> +	tc_write(DP0_SYNCVAL, (vsync_len << 16) | (hsync_len << 0) |
> +		 ((mode->flags & DRM_MODE_FLAG_NHSYNC) ? (1 << 15) : 0) |
> +		 ((mode->flags & DRM_MODE_FLAG_NVSYNC) ? (1 << 31) : 0));


Defines for sync flags would be better.

Anyway:

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

 --
Regards
Andrzej


>  
>  	tc_write(DPIPXLFMT, VS_POL_ACTIVE_LOW | HS_POL_ACTIVE_LOW |
>  		 DE_POL_ACTIVE_HIGH | SUB_CFG_TYPE_CONFIG1 | DPI_BPP_RGB888);


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

Re: [PATCH 7/7] drm/bridge: tc358767: use DP connector if no panel set

[Andrzej Hajda]

On 29.10.2018 12:47, Tomi Valkeinen wrote:
> tc358767 driver sets the connector type always to eDP.
>
> This patch sets the type to DP if there is no panel defined, which
> implies that there's a DP connector on the board.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index be013bd7b30b..db93d927cb9a 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1207,7 +1207,8 @@ static int tc_bridge_attach(struct drm_bridge *bridge)
>  	/* Create eDP connector */
>  	drm_connector_helper_add(&tc->connector, &tc_connector_helper_funcs);
>  	ret = drm_connector_init(drm, &tc->connector, &tc_connector_funcs,
> -				 DRM_MODE_CONNECTOR_eDP);
> +				 tc->panel ? DRM_MODE_CONNECTOR_eDP :
> +				 DRM_MODE_CONNECTOR_DisplayPort);
>  	if (ret)
>  		return ret;
>  

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 5/7] drm/bridge: tc358767: reject modes which require too much BW

[Andrzej Hajda]

On 29.10.2018 12:46, Tomi Valkeinen wrote:
> The current driver accepts any videomode with pclk < 154MHz. This is not
> correct, as with 1 lane and/or 1.62Mbps speed not all videomodes can be
> supported.
>
> Add code to reject modes that require more bandwidth that is available.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

> ---
>  drivers/gpu/drm/bridge/tc358767.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index ab299f4debfa..b026b5ef7378 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1114,10 +1114,20 @@ static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
>  static enum drm_mode_status tc_connector_mode_valid(struct drm_connector *connector,
>  				   struct drm_display_mode *mode)
>  {
> +	struct tc_data *tc = connector_to_tc(connector);
> +	u32 req, avail;
> +	u32 bits_per_pixel = 24;
> +
>  	/* DPI interface clock limitation: upto 154 MHz */
>  	if (mode->clock > 154000)
>  		return MODE_CLOCK_HIGH;
>  
> +	req = mode->clock * bits_per_pixel / 8 / 1000;
> +	avail = tc->link.base.num_lanes * tc->link.base.rate / 1000;


You can remove "/ 1000" from both lines.

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

 --
Regards
Andrzej



> +
> +	if (req > avail)
> +		return MODE_BAD;
> +
>  	return MODE_OK;
>  }
>  


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

Re: [PATCH 3/7] drm/bridge: tc358767: fix single lane configuration

[Andrzej Hajda]

On 29.10.2018 12:46, Tomi Valkeinen wrote:
> PHY_2LANE bit is always set in DP_PHY_CTRL, breaking 1 lane use.
>
> Set PHY_2LANE only when 2 lanes are used.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
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 2/7] drm/bridge: tc358767: add defines for DP1_SRCCTRL & PHY_2LANE

[Andrzej Hajda]

On 29.10.2018 12:46, Tomi Valkeinen wrote:
> DP1_SRCCTRL register and PHY_2LANE field did not have matching defines.
> Add these.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
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 1/7] drm/bridge: tc358767: add bus flags

[Andrzej Hajda]

On 29.10.2018 12:46, Tomi Valkeinen wrote:
> tc358767 driver does not set DRM bus_flags, even if it does configures
> the polarity settings into its registers. This means that the DPI source
> can't configure the polarities correctly.
>
> Add sync flags accordingly.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
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 4/7] drm/bridge: tc358767: fix initial DP0/1_SRCCTRL value

[Tomi Valkeinen]

On 19/11/18 09:29, Andrzej Hajda wrote:
> On 29.10.2018 12:46, Tomi Valkeinen wrote:
>> Initially DP0_SRCCTRL is set to a static value which includes
>> DP0_SRCCTRL_LANES_2 and DP0_SRCCTRL_BW27, even when only 1 lane of
>> 1.62Gbps speed is used. DP1_SRCCTRL is configured to a magic number.
>>
>> This patch changes the configuration as follows:
>>
>> Configure DP0_SRCCTRL by using tc_srcctrl() which provides the correct
>> value.
>>
>> DP1_SRCCTRL needs to bits to be set to the same value as DP0_SRCCTRL:
> 
> s/to/two/
> 
> 
>> SSCG and BW27. All other bits can be zero.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/bridge/tc358767.c | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index fee53422c31f..ab299f4debfa 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -836,12 +836,11 @@ static int tc_main_link_setup(struct tc_data *tc)
>>  	if (!tc->mode)
>>  		return -EINVAL;
>>  
>> -	/* from excel file - DP0_SrcCtrl */
>> -	tc_write(DP0_SRCCTRL, DP0_SRCCTRL_SCRMBLDIS | DP0_SRCCTRL_EN810B |
>> -		 DP0_SRCCTRL_LANESKEW | DP0_SRCCTRL_LANES_2 |
>> -		 DP0_SRCCTRL_BW27 | DP0_SRCCTRL_AUTOCORRECT);
>> -	/* from excel file - DP1_SrcCtrl */
>> -	tc_write(DP1_SRCCTRL, 0x00003083);
>> +	tc_write(DP0_SRCCTRL, tc_srcctrl(tc));
>> +	/* SSCG and BW27 on DP1 must be set to the same as on DP0 */
>> +	tc_write(DP1_SRCCTRL,
>> +		 (tc->link.spread ? DP0_SRCCTRL_SSCG : 0) |
>> +		 ((tc->link.base.rate != 162000) ? DP0_SRCCTRL_BW27 : 0));
> 
> 
> Wouldn't  be better then to modify tc_srcctrl to support DP0 and DP1:
> 
>     tc_write(DP0_SRCCTRL, tc_srcctrl(tc, 0));
> 
>     tc_write(DP1_SRCCTRL, tc_srcctrl(tc, 1));

In a normal case I would agree. But on this IC, there's only a single
DisplayPort output, DP0. However, for some reason, DP1_SRCCTRL also
needs to be configured (and the register is not even in the docs) for
DP0 to work. Possibly there are IC versions with two DP ports, but I
don't know how DP1_SRCCTRL needs to be configured on those.

So I'd rather keep the normal configuration (for DP0 with tc_srcctrl)
separate from setting these magical bits in DP1_SRCCTRL needed to make
the IC work.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/7] drm/bridge: tc358767: reject modes which require too much BW

[Tomi Valkeinen]

On 19/11/18 09:36, Andrzej Hajda wrote:
> On 29.10.2018 12:46, Tomi Valkeinen wrote:
>> The current driver accepts any videomode with pclk < 154MHz. This is not
>> correct, as with 1 lane and/or 1.62Mbps speed not all videomodes can be
>> supported.
>>
>> Add code to reject modes that require more bandwidth that is available.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
>> ---
>>  drivers/gpu/drm/bridge/tc358767.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index ab299f4debfa..b026b5ef7378 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -1114,10 +1114,20 @@ static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
>>  static enum drm_mode_status tc_connector_mode_valid(struct drm_connector *connector,
>>  				   struct drm_display_mode *mode)
>>  {
>> +	struct tc_data *tc = connector_to_tc(connector);
>> +	u32 req, avail;
>> +	u32 bits_per_pixel = 24;
>> +
>>  	/* DPI interface clock limitation: upto 154 MHz */
>>  	if (mode->clock > 154000)
>>  		return MODE_CLOCK_HIGH;
>>  
>> +	req = mode->clock * bits_per_pixel / 8 / 1000;
>> +	avail = tc->link.base.num_lanes * tc->link.base.rate / 1000;
> 
> 
> You can remove "/ 1000" from both lines.
> 
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

Indeed, thanks!

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm/bridge: tc358767: fix output H/V syncs

[Tomi Valkeinen]

On 19/11/18 09:33, Andrzej Hajda wrote:
> On 29.10.2018 12:47, Tomi Valkeinen wrote:
>> The H and V syncs of the DP output are always set to active high. This
>> patch fixes the syncs by configuring them according to the videomode.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/bridge/tc358767.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index b026b5ef7378..be013bd7b30b 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -726,7 +726,9 @@ static int tc_set_video_mode(struct tc_data *tc, struct drm_display_mode *mode)
>>  
>>  	tc_write(DP0_ACTIVEVAL, (mode->vdisplay << 16) | (mode->hdisplay));
>>  
>> -	tc_write(DP0_SYNCVAL, (vsync_len << 16) | (hsync_len << 0));
>> +	tc_write(DP0_SYNCVAL, (vsync_len << 16) | (hsync_len << 0) |
>> +		 ((mode->flags & DRM_MODE_FLAG_NHSYNC) ? (1 << 15) : 0) |
>> +		 ((mode->flags & DRM_MODE_FLAG_NVSYNC) ? (1 << 31) : 0));
> 
> 
> Defines for sync flags would be better.

Sure. I'll update the above to:

+       tc_write(DP0_SYNCVAL, (vsync_len << 16) | (hsync_len << 0) |
+                ((mode->flags & DRM_MODE_FLAG_NHSYNC) ? SYNCVAL_HS_POL_ACTIVE_LOW : 0) |
+                ((mode->flags & DRM_MODE_FLAG_NVSYNC) ? SYNCVAL_VS_POL_ACTIVE_LOW : 0));

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel