All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: kishon@ti.com, heiko@sntech.de, zyw@rock-chips.com
Cc: groeck@chromium.org, shawnn@chromium.org, dnschneid@chromium.org,
	Douglas Anderson <dianders@chromium.org>,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 2/4] phy: rockchip-typec: Don't set the aux voltage swing to 400 mV
Date: Fri, 22 Sep 2017 09:44:04 -0700	[thread overview]
Message-ID: <20170922164406.27606-3-dianders@chromium.org> (raw)
In-Reply-To: <20170922164406.27606-1-dianders@chromium.org>

On rk3399-gru-kevin there are some cases where we're seeing AUX CH
failures when trying to do DisplayPort over type C.  Problems are
intermittent and don't reproduce all the time.  Problems are often
bursty and failures persist for several seconds before going away.
The failure case I focused on is:
* A particular type C to HDMI adapter.
* One orientation (flip mode) of that adapter.
* Easier to see failures when something is plugged into the _other
  type C port at the same time.
* Problems reproduce on both type C ports (left and right side).

Ironically problems also stop reproducing when I solder wires onto the
AUX CH signals on a port (even if no scope is connected to the
signals).  In this case, problems only stop reproducing on the port
with the wires connected.

>From the above it appears that something about the signaling on the
aux channel is marginal and any slight differences can bring us over
the edge to failure.

It turns out that we can fix our problems by just increasing the
voltage swing of the AUX CH, giving us a bunch of extra margin.  In DP
up to version 1.2 the voltage swing on the aux channel was specced as
.29 V to 1.38 V.  In DP version 1.3 the aux channel voltage was
tightened to be between .29 V and .40 V, but it clarifies that it
really only needs the lower voltage when operating at the highest
speed (HBR3 mode).  So right now we are trying to use a voltage that
technically should be valid for all versions of the spec (including
version 1.3 when transmitting at HBR3).  That would be great to do if
it worked reliably.  ...but it doesn't seem to.

It turns out that if you continue to read through the DP part of the
rk3399 TRM and other parts of the type C PHY spec you'll find out that
while the rk3399 does support DP 1.3, it doesn't support HBR3.  The
docs specifically say "RBR, HBR and HBR2 data rates only".  Thus there
is actually no requirement to support an AUX CH swing of .4 V.

Even if there is no actual requirement to support the tighter voltage
swing, one could possibly argue that we should support it anyway.  The
DP spec clarifies that the lower voltage on the AUX CH will reduce
cross talk in some cases and that seems like it could be beneficial
even at the lower bit rates.  At the moment, though, we are seeing
problems with the AUX CH and not on the other lines.  Also, checking
another known working and similar laptop shows that the other laptop
runs the AUX channel at a higher voltage.

Other notes:
* Looking at measurements done on the AUX CH we weren't actually
  compliant with the DP 1.3 spec anyway.  AUX CH peek-to-peek voltage
  was measured on rk3399-gru-kevin as .466 V which is > .4 V.
* With this new patch the AUX channel isn't actually 1.0 V, but it has
  been confirmed that the signal is better and has more margin.  Eye
  diagram passes.
* If someone were truly an expert in the Type C PHY and in DisplayPort
  signaling they might be able to make things work and keep the
  voltage at < .4 V.  The Type C PHY seems to have a plethora of
  tuning knobs that could almost certainly improve the signal
  integrity.  Some of these things (like enabling tx_fcm_full_margin)
  even seem to fix my problems.  However, lacking expertise I can't
  say whether this is a better or worse solution.  Tightening signals
  to give cleaner waveforms can often have adverse affects, like
  increasing EMI or adding noise to other signals.  I'd rather not
  tune things like this without a healthy application of expertise
  that I don't have.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- Voltage swing patch now patch 2.

Changes in v2:
- Voltage swing patch new for v2.

 drivers/phy/rockchip/phy-rockchip-typec.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index 342a77733207..b25c00432f9b 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -543,10 +543,11 @@ static void tcphy_dp_aux_calibration(struct rockchip_typec_phy *tcphy)
 	writel(0, tcphy->base + TX_ANA_CTRL_REG_5);
 
 	/*
-	 * Controls low_power_swing_en, set the voltage swing of the driver
-	 * to 400mv. The values	below are peak to peak (differential) values.
+	 * Controls low_power_swing_en, don't set the voltage swing of the
+	 * driver to 400mv. The values below are peak to peak (differential)
+	 * values.
 	 */
-	writel(4, tcphy->base + TXDA_COEFF_CALC_CTRL);
+	writel(0, tcphy->base + TXDA_COEFF_CALC_CTRL);
 	writel(0, tcphy->base + TXDA_CYA_AUXDA_CYA);
 
 	/* Controls tx_high_z_tm_en */
-- 
2.14.1.821.g8fa685d3b7-goog

WARNING: multiple messages have this Message-ID (diff)
From: Douglas Anderson <dianders@chromium.org>
To: kishon@ti.com, heiko@sntech.de, zyw@rock-chips.com
Cc: groeck@chromium.org, shawnn@chromium.org, dnschneid@chromium.org,
	Douglas Anderson <dianders@chromium.org>,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 2/4] phy: rockchip-typec: Don't set the aux voltage swing to 400 mV
Date: Fri, 22 Sep 2017 09:44:04 -0700	[thread overview]
Message-ID: <20170922164406.27606-3-dianders@chromium.org> (raw)
In-Reply-To: <20170922164406.27606-1-dianders@chromium.org>

On rk3399-gru-kevin there are some cases where we're seeing AUX CH
failures when trying to do DisplayPort over type C.  Problems are
intermittent and don't reproduce all the time.  Problems are often
bursty and failures persist for several seconds before going away.
The failure case I focused on is:
* A particular type C to HDMI adapter.
* One orientation (flip mode) of that adapter.
* Easier to see failures when something is plugged into the _other
  type C port at the same time.
* Problems reproduce on both type C ports (left and right side).

Ironically problems also stop reproducing when I solder wires onto the
AUX CH signals on a port (even if no scope is connected to the
signals).  In this case, problems only stop reproducing on the port
with the wires connected.

From the above it appears that something about the signaling on the
aux channel is marginal and any slight differences can bring us over
the edge to failure.

It turns out that we can fix our problems by just increasing the
voltage swing of the AUX CH, giving us a bunch of extra margin.  In DP
up to version 1.2 the voltage swing on the aux channel was specced as
.29 V to 1.38 V.  In DP version 1.3 the aux channel voltage was
tightened to be between .29 V and .40 V, but it clarifies that it
really only needs the lower voltage when operating at the highest
speed (HBR3 mode).  So right now we are trying to use a voltage that
technically should be valid for all versions of the spec (including
version 1.3 when transmitting at HBR3).  That would be great to do if
it worked reliably.  ...but it doesn't seem to.

It turns out that if you continue to read through the DP part of the
rk3399 TRM and other parts of the type C PHY spec you'll find out that
while the rk3399 does support DP 1.3, it doesn't support HBR3.  The
docs specifically say "RBR, HBR and HBR2 data rates only".  Thus there
is actually no requirement to support an AUX CH swing of .4 V.

Even if there is no actual requirement to support the tighter voltage
swing, one could possibly argue that we should support it anyway.  The
DP spec clarifies that the lower voltage on the AUX CH will reduce
cross talk in some cases and that seems like it could be beneficial
even at the lower bit rates.  At the moment, though, we are seeing
problems with the AUX CH and not on the other lines.  Also, checking
another known working and similar laptop shows that the other laptop
runs the AUX channel at a higher voltage.

Other notes:
* Looking at measurements done on the AUX CH we weren't actually
  compliant with the DP 1.3 spec anyway.  AUX CH peek-to-peek voltage
  was measured on rk3399-gru-kevin as .466 V which is > .4 V.
* With this new patch the AUX channel isn't actually 1.0 V, but it has
  been confirmed that the signal is better and has more margin.  Eye
  diagram passes.
* If someone were truly an expert in the Type C PHY and in DisplayPort
  signaling they might be able to make things work and keep the
  voltage at < .4 V.  The Type C PHY seems to have a plethora of
  tuning knobs that could almost certainly improve the signal
  integrity.  Some of these things (like enabling tx_fcm_full_margin)
  even seem to fix my problems.  However, lacking expertise I can't
  say whether this is a better or worse solution.  Tightening signals
  to give cleaner waveforms can often have adverse affects, like
  increasing EMI or adding noise to other signals.  I'd rather not
  tune things like this without a healthy application of expertise
  that I don't have.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- Voltage swing patch now patch 2.

Changes in v2:
- Voltage swing patch new for v2.

 drivers/phy/rockchip/phy-rockchip-typec.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index 342a77733207..b25c00432f9b 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -543,10 +543,11 @@ static void tcphy_dp_aux_calibration(struct rockchip_typec_phy *tcphy)
 	writel(0, tcphy->base + TX_ANA_CTRL_REG_5);
 
 	/*
-	 * Controls low_power_swing_en, set the voltage swing of the driver
-	 * to 400mv. The values	below are peak to peak (differential) values.
+	 * Controls low_power_swing_en, don't set the voltage swing of the
+	 * driver to 400mv. The values below are peak to peak (differential)
+	 * values.
 	 */
-	writel(4, tcphy->base + TXDA_COEFF_CALC_CTRL);
+	writel(0, tcphy->base + TXDA_COEFF_CALC_CTRL);
 	writel(0, tcphy->base + TXDA_CYA_AUXDA_CYA);
 
 	/* Controls tx_high_z_tm_en */
-- 
2.14.1.821.g8fa685d3b7-goog

WARNING: multiple messages have this Message-ID (diff)
From: dianders@chromium.org (Douglas Anderson)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 2/4] phy: rockchip-typec: Don't set the aux voltage swing to 400 mV
Date: Fri, 22 Sep 2017 09:44:04 -0700	[thread overview]
Message-ID: <20170922164406.27606-3-dianders@chromium.org> (raw)
In-Reply-To: <20170922164406.27606-1-dianders@chromium.org>

On rk3399-gru-kevin there are some cases where we're seeing AUX CH
failures when trying to do DisplayPort over type C.  Problems are
intermittent and don't reproduce all the time.  Problems are often
bursty and failures persist for several seconds before going away.
The failure case I focused on is:
* A particular type C to HDMI adapter.
* One orientation (flip mode) of that adapter.
* Easier to see failures when something is plugged into the _other
  type C port at the same time.
* Problems reproduce on both type C ports (left and right side).

Ironically problems also stop reproducing when I solder wires onto the
AUX CH signals on a port (even if no scope is connected to the
signals).  In this case, problems only stop reproducing on the port
with the wires connected.

>From the above it appears that something about the signaling on the
aux channel is marginal and any slight differences can bring us over
the edge to failure.

It turns out that we can fix our problems by just increasing the
voltage swing of the AUX CH, giving us a bunch of extra margin.  In DP
up to version 1.2 the voltage swing on the aux channel was specced as
.29 V to 1.38 V.  In DP version 1.3 the aux channel voltage was
tightened to be between .29 V and .40 V, but it clarifies that it
really only needs the lower voltage when operating at the highest
speed (HBR3 mode).  So right now we are trying to use a voltage that
technically should be valid for all versions of the spec (including
version 1.3 when transmitting at HBR3).  That would be great to do if
it worked reliably.  ...but it doesn't seem to.

It turns out that if you continue to read through the DP part of the
rk3399 TRM and other parts of the type C PHY spec you'll find out that
while the rk3399 does support DP 1.3, it doesn't support HBR3.  The
docs specifically say "RBR, HBR and HBR2 data rates only".  Thus there
is actually no requirement to support an AUX CH swing of .4 V.

Even if there is no actual requirement to support the tighter voltage
swing, one could possibly argue that we should support it anyway.  The
DP spec clarifies that the lower voltage on the AUX CH will reduce
cross talk in some cases and that seems like it could be beneficial
even at the lower bit rates.  At the moment, though, we are seeing
problems with the AUX CH and not on the other lines.  Also, checking
another known working and similar laptop shows that the other laptop
runs the AUX channel at a higher voltage.

Other notes:
* Looking at measurements done on the AUX CH we weren't actually
  compliant with the DP 1.3 spec anyway.  AUX CH peek-to-peek voltage
  was measured on rk3399-gru-kevin as .466 V which is > .4 V.
* With this new patch the AUX channel isn't actually 1.0 V, but it has
  been confirmed that the signal is better and has more margin.  Eye
  diagram passes.
* If someone were truly an expert in the Type C PHY and in DisplayPort
  signaling they might be able to make things work and keep the
  voltage at < .4 V.  The Type C PHY seems to have a plethora of
  tuning knobs that could almost certainly improve the signal
  integrity.  Some of these things (like enabling tx_fcm_full_margin)
  even seem to fix my problems.  However, lacking expertise I can't
  say whether this is a better or worse solution.  Tightening signals
  to give cleaner waveforms can often have adverse affects, like
  increasing EMI or adding noise to other signals.  I'd rather not
  tune things like this without a healthy application of expertise
  that I don't have.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- Voltage swing patch now patch 2.

Changes in v2:
- Voltage swing patch new for v2.

 drivers/phy/rockchip/phy-rockchip-typec.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index 342a77733207..b25c00432f9b 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -543,10 +543,11 @@ static void tcphy_dp_aux_calibration(struct rockchip_typec_phy *tcphy)
 	writel(0, tcphy->base + TX_ANA_CTRL_REG_5);
 
 	/*
-	 * Controls low_power_swing_en, set the voltage swing of the driver
-	 * to 400mv. The values	below are peak to peak (differential) values.
+	 * Controls low_power_swing_en, don't set the voltage swing of the
+	 * driver to 400mv. The values below are peak to peak (differential)
+	 * values.
 	 */
-	writel(4, tcphy->base + TXDA_COEFF_CALC_CTRL);
+	writel(0, tcphy->base + TXDA_COEFF_CALC_CTRL);
 	writel(0, tcphy->base + TXDA_CYA_AUXDA_CYA);
 
 	/* Controls tx_high_z_tm_en */
-- 
2.14.1.821.g8fa685d3b7-goog

  parent reply	other threads:[~2017-09-22 16:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-22 16:44 [PATCH v3 0/4] phy: rockchip-typec: Set "flip" properly; some cleanups; fix swing Douglas Anderson
2017-09-22 16:44 ` Douglas Anderson
2017-09-22 16:44 ` [PATCH v3 1/4] phy: rockchip-typec: Set the AUX channel flip state earlier Douglas Anderson
2017-09-22 16:44   ` Douglas Anderson
2017-09-22 16:44   ` Douglas Anderson
2017-09-22 16:44 ` Douglas Anderson [this message]
2017-09-22 16:44   ` [PATCH v3 2/4] phy: rockchip-typec: Don't set the aux voltage swing to 400 mV Douglas Anderson
2017-09-22 16:44   ` Douglas Anderson
2017-09-22 16:44 ` [PATCH v3 3/4] phy: rockchip-typec: Avoid magic numbers + add delays in aux calib Douglas Anderson
2017-09-22 16:44   ` Douglas Anderson
2017-09-22 16:44 ` [PATCH v3 4/4] phy: rockchip-typec: Do the calibration more correctly Douglas Anderson
2017-09-22 16:44   ` Douglas Anderson
2017-10-18 11:19   ` Kishon Vijay Abraham I
2017-10-18 11:19     ` Kishon Vijay Abraham I
2017-10-18 11:19     ` Kishon Vijay Abraham I

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170922164406.27606-3-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=dnschneid@chromium.org \
    --cc=groeck@chromium.org \
    --cc=heiko@sntech.de \
    --cc=kishon@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=shawnn@chromium.org \
    --cc=zyw@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.