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 <ma...@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.ha...@intel.com>
> Cc: Conor Dooley <conor...@kernel.org>
> Cc: Daniel Vetter <dan...@ffwll.ch>
> Cc: David Airlie <airl...@gmail.com>
> Cc: Jernej Skrabec <jernej.skra...@gmail.com>
> Cc: Jonas Karlman <jo...@kwiboo.se>
> Cc: Krzysztof Kozlowski <krzk...@kernel.org>
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Cc: Lucas Stach <l.st...@pengutronix.de>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Maxime Ripard <mrip...@kernel.org>
> Cc: Neil Armstrong <neil.armstr...@linaro.org>
> Cc: Rob Herring <r...@kernel.org>
> Cc: Robert Foss <rf...@kernel.org>
> Cc: Thomas Zimmermann <tzimmerm...@suse.de>
> Cc: devicet...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: ker...@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.st...@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) {
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
