diff mbox series

[v3,2/2] drm/bridge: tc358767: Add configurable default preemphasis

Message ID 20240625120552.145389-2-marex@denx.de (mailing list archive)
State New
Headers show
Series [v3,1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis | expand

Commit Message

Marek Vasut June 25, 2024, 12:05 p.m. UTC
Make the default DP port preemphasis configurable via new DT property
"toshiba,pre-emphasis". This is useful in case the DP link properties
are known and starting link training from preemphasis setting of 0 dB
is not useful. The preemphasis can be set separately for both DP lanes
in range 0=0dB, 1=3.5dB, 2=6dB .

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: kernel@dh-electronics.com
---
V2: - Parse toshiba,pre-emphasis property out of an endpoint of port 2 (the DP port)
V3: - No change
---
 drivers/gpu/drm/bridge/tc358767.c | 45 ++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 7 deletions(-)

Comments

Alexander Stein June 26, 2024, 7:36 a.m. UTC | #1
Hi Marek,

thanks for patch.

Am Dienstag, 25. Juni 2024, 14:05:15 CEST schrieb Marek Vasut:
> Make the default DP port preemphasis configurable via new DT property
> "toshiba,pre-emphasis". This is useful in case the DP link properties
> are known and starting link training from preemphasis setting of 0 dB
> is not useful. The preemphasis can be set separately for both DP lanes
> in range 0=0dB, 1=3.5dB, 2=6dB .
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: kernel@dh-electronics.com
> ---
> V2: - Parse toshiba,pre-emphasis property out of an endpoint of port 2 (the DP port)
> V3: - No change
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 45 ++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index dde1b2734c98a..257fe15080099 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -241,6 +241,10 @@
>  
>  /* Link Training */
>  #define DP0_SRCCTRL		0x06a0
> +#define DP0_SRCCTRL_PRE1		GENMASK(29, 28)
> +#define DP0_SRCCTRL_SWG1		GENMASK(25, 24)
> +#define DP0_SRCCTRL_PRE0		GENMASK(21, 20)
> +#define DP0_SRCCTRL_SWG0		GENMASK(17, 16)
>  #define DP0_SRCCTRL_SCRMBLDIS		BIT(13)
>  #define DP0_SRCCTRL_EN810B		BIT(12)
>  #define DP0_SRCCTRL_NOTP		(0 << 8)
> @@ -278,6 +282,8 @@
>  #define AUDIFDATA6		0x0720	/* DP0 Audio Info Frame Bytes 27 to 24 */
>  
>  #define DP1_SRCCTRL		0x07a0	/* DP1 Control Register */
> +#define DP1_SRCCTRL_PRE			GENMASK(21, 20)
> +#define DP1_SRCCTRL_SWG			GENMASK(17, 16)
>  
>  /* PHY */
>  #define DP_PHY_CTRL		0x0800
> @@ -369,6 +375,7 @@ struct tc_data {
>  
>  	u32			rev;
>  	u8			assr;
> +	u8			pre_emphasis[2];
>  
>  	struct gpio_desc	*sd_gpio;
>  	struct gpio_desc	*reset_gpio;
> @@ -1090,13 +1097,17 @@ static int tc_main_link_enable(struct tc_data *tc)
>  			return ret;
>  	}
>  
> -	ret = regmap_write(tc->regmap, DP0_SRCCTRL, tc_srcctrl(tc));
> +	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
> +			   tc_srcctrl(tc) |
> +			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
> +			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
>  	if (ret)
>  		return ret;
>  	/* SSCG and BW27 on DP1 must be set to the same as on DP0 */
>  	ret = regmap_write(tc->regmap, DP1_SRCCTRL,
>  		 (tc->link.spread ? DP0_SRCCTRL_SSCG : 0) |
> -		 ((tc->link.rate != 162000) ? DP0_SRCCTRL_BW27 : 0));
> +		 ((tc->link.rate != 162000) ? DP0_SRCCTRL_BW27 : 0) |
> +		 FIELD_PREP(DP1_SRCCTRL_PRE, tc->pre_emphasis[1]));
>  	if (ret)
>  		return ret;
>  
> @@ -1188,8 +1199,10 @@ static int tc_main_link_enable(struct tc_data *tc)
>  		goto err_dpcd_write;
>  
>  	/* Reset voltage-swing & pre-emphasis */
> -	tmp[0] = tmp[1] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
> -			  DP_TRAIN_PRE_EMPH_LEVEL_0;
> +	tmp[0] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
> +		 FIELD_PREP(DP_TRAIN_PRE_EMPHASIS_MASK, tc->pre_emphasis[0]);
> +	tmp[1] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
> +		 FIELD_PREP(DP_TRAIN_PRE_EMPHASIS_MASK, tc->pre_emphasis[1]);
>  	ret = drm_dp_dpcd_write(aux, DP_TRAINING_LANE0_SET, tmp, 2);
>  	if (ret < 0)
>  		goto err_dpcd_write;
> @@ -1213,7 +1226,9 @@ static int tc_main_link_enable(struct tc_data *tc)
>  	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
>  			   tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
>  			   DP0_SRCCTRL_AUTOCORRECT |
> -			   DP0_SRCCTRL_TP1);
> +			   DP0_SRCCTRL_TP1 |
> +			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
> +			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
>  	if (ret)
>  		return ret;
>  
> @@ -1248,7 +1263,9 @@ static int tc_main_link_enable(struct tc_data *tc)
>  	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
>  			   tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
>  			   DP0_SRCCTRL_AUTOCORRECT |
> -			   DP0_SRCCTRL_TP2);
> +			   DP0_SRCCTRL_TP2 |
> +			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
> +			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
>  	if (ret)
>  		return ret;
>  
> @@ -1274,7 +1291,9 @@ static int tc_main_link_enable(struct tc_data *tc)
>  
>  	/* Clear Training Pattern, set AutoCorrect Mode = 1 */
>  	ret = regmap_write(tc->regmap, DP0_SRCCTRL, tc_srcctrl(tc) |
> -			   DP0_SRCCTRL_AUTOCORRECT);
> +			   DP0_SRCCTRL_AUTOCORRECT |
> +			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
> +			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
>  	if (ret)
>  		return ret;
>  
> @@ -2435,6 +2454,18 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc)
>  			return -EINVAL;
>  		}
>  		mode |= BIT(endpoint.port);
> +		if (endpoint.port != 2)
> +			continue;
> +

Mh, I know currently there are not other port-specific properties. But
maybe it's easier to read if 'if (endpoint.port == 2) {' is used.

But either way, this looks good.
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> +		of_property_read_u8_array(node, "toshiba,pre-emphasis",
> +					  tc->pre_emphasis,
> +					  ARRAY_SIZE(tc->pre_emphasis));
> +
> +		if (tc->pre_emphasis[0] < 0 || tc->pre_emphasis[0] > 2 ||
> +		    tc->pre_emphasis[1] < 0 || tc->pre_emphasis[1] > 2) {
> +			dev_err(dev, "Incorrect Pre-Emphasis setting, use either 0=0dB 1=3.5dB 2=6dB\n");
> +			return -EINVAL;
> +		}
>  	}
>  
>  	if (mode == mode_dpi_to_edp || mode == mode_dpi_to_dp) {
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index dde1b2734c98a..257fe15080099 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -241,6 +241,10 @@ 
 
 /* Link Training */
 #define DP0_SRCCTRL		0x06a0
+#define DP0_SRCCTRL_PRE1		GENMASK(29, 28)
+#define DP0_SRCCTRL_SWG1		GENMASK(25, 24)
+#define DP0_SRCCTRL_PRE0		GENMASK(21, 20)
+#define DP0_SRCCTRL_SWG0		GENMASK(17, 16)
 #define DP0_SRCCTRL_SCRMBLDIS		BIT(13)
 #define DP0_SRCCTRL_EN810B		BIT(12)
 #define DP0_SRCCTRL_NOTP		(0 << 8)
@@ -278,6 +282,8 @@ 
 #define AUDIFDATA6		0x0720	/* DP0 Audio Info Frame Bytes 27 to 24 */
 
 #define DP1_SRCCTRL		0x07a0	/* DP1 Control Register */
+#define DP1_SRCCTRL_PRE			GENMASK(21, 20)
+#define DP1_SRCCTRL_SWG			GENMASK(17, 16)
 
 /* PHY */
 #define DP_PHY_CTRL		0x0800
@@ -369,6 +375,7 @@  struct tc_data {
 
 	u32			rev;
 	u8			assr;
+	u8			pre_emphasis[2];
 
 	struct gpio_desc	*sd_gpio;
 	struct gpio_desc	*reset_gpio;
@@ -1090,13 +1097,17 @@  static int tc_main_link_enable(struct tc_data *tc)
 			return ret;
 	}
 
-	ret = regmap_write(tc->regmap, DP0_SRCCTRL, tc_srcctrl(tc));
+	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
+			   tc_srcctrl(tc) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 	/* SSCG and BW27 on DP1 must be set to the same as on DP0 */
 	ret = regmap_write(tc->regmap, DP1_SRCCTRL,
 		 (tc->link.spread ? DP0_SRCCTRL_SSCG : 0) |
-		 ((tc->link.rate != 162000) ? DP0_SRCCTRL_BW27 : 0));
+		 ((tc->link.rate != 162000) ? DP0_SRCCTRL_BW27 : 0) |
+		 FIELD_PREP(DP1_SRCCTRL_PRE, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 
@@ -1188,8 +1199,10 @@  static int tc_main_link_enable(struct tc_data *tc)
 		goto err_dpcd_write;
 
 	/* Reset voltage-swing & pre-emphasis */
-	tmp[0] = tmp[1] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
-			  DP_TRAIN_PRE_EMPH_LEVEL_0;
+	tmp[0] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
+		 FIELD_PREP(DP_TRAIN_PRE_EMPHASIS_MASK, tc->pre_emphasis[0]);
+	tmp[1] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
+		 FIELD_PREP(DP_TRAIN_PRE_EMPHASIS_MASK, tc->pre_emphasis[1]);
 	ret = drm_dp_dpcd_write(aux, DP_TRAINING_LANE0_SET, tmp, 2);
 	if (ret < 0)
 		goto err_dpcd_write;
@@ -1213,7 +1226,9 @@  static int tc_main_link_enable(struct tc_data *tc)
 	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
 			   tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
 			   DP0_SRCCTRL_AUTOCORRECT |
-			   DP0_SRCCTRL_TP1);
+			   DP0_SRCCTRL_TP1 |
+			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 
@@ -1248,7 +1263,9 @@  static int tc_main_link_enable(struct tc_data *tc)
 	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
 			   tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
 			   DP0_SRCCTRL_AUTOCORRECT |
-			   DP0_SRCCTRL_TP2);
+			   DP0_SRCCTRL_TP2 |
+			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 
@@ -1274,7 +1291,9 @@  static int tc_main_link_enable(struct tc_data *tc)
 
 	/* Clear Training Pattern, set AutoCorrect Mode = 1 */
 	ret = regmap_write(tc->regmap, DP0_SRCCTRL, tc_srcctrl(tc) |
-			   DP0_SRCCTRL_AUTOCORRECT);
+			   DP0_SRCCTRL_AUTOCORRECT |
+			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 
@@ -2435,6 +2454,18 @@  static int tc_probe_bridge_endpoint(struct tc_data *tc)
 			return -EINVAL;
 		}
 		mode |= BIT(endpoint.port);
+		if (endpoint.port != 2)
+			continue;
+
+		of_property_read_u8_array(node, "toshiba,pre-emphasis",
+					  tc->pre_emphasis,
+					  ARRAY_SIZE(tc->pre_emphasis));
+
+		if (tc->pre_emphasis[0] < 0 || tc->pre_emphasis[0] > 2 ||
+		    tc->pre_emphasis[1] < 0 || tc->pre_emphasis[1] > 2) {
+			dev_err(dev, "Incorrect Pre-Emphasis setting, use either 0=0dB 1=3.5dB 2=6dB\n");
+			return -EINVAL;
+		}
 	}
 
 	if (mode == mode_dpi_to_edp || mode == mode_dpi_to_dp) {