| Message ID | 20240625120552.145389-2-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis | expand |
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) { >
On 6/26/24 9:36 AM, Alexander Stein wrote: Hi, sorry for the late reply. >> @@ -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. Fixed in V4, thanks.
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) {
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(-)