Re: [Intel-gfx] [PATCH 37/40] drm/i915: Fix eDP link training when switching pipes
On Mon, Jun 30, 2014 at 02:52:12PM -0700, Jesse Barnes wrote:
On Sat, 28 Jun 2014 02:04:28 +0300
ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
When switching from one pipe to another, the power sequencer of the new
pipe seems to need a bit of kicking to lock into the port. Even the vdd
force bit doesn't work before the power sequencer has been sufficiently
kicked, so this must be done even before any AUX transactions.
This sequence has been found to do the trick:
1) enable port with idle pattern
2) enable the power sequencer
3) proceed with link training
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 65ab54c..07b0320 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2010,6 +2010,37 @@ static void chv_post_disable_dp(struct intel_encoder
*encoder)
mutex_unlock(dev_priv-dpio_lock);
}
+static void intel_edp_init_train(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!is_edp(intel_dp))
+ return;
This changes the order of events as observed by the sink, so I really
wonder why this is edp specific?
We do have bug reports about external DP monitors not waking up from the
sink_dpms call properly ...
-Daniel
+
+ /*
+* Need to enable the port with idle pattern to allow the power
+* sequencer to lock into the port. Otherwise the power sequencer
+* (including vdd force bit!) doesn't work on this port.
+*/
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-DP |= DP_PORT_EN;
+
+ if (IS_CHERRYVIEW(dev))
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK_CHV;
+ else
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK;
+ intel_dp-DP |= DP_LINK_TRAIN_PAT_IDLE;
+
+ I915_WRITE(intel_dp-output_reg, intel_dp-DP);
+ POSTING_READ(intel_dp-output_reg);
+ }
+
+ intel_edp_panel_on(intel_dp);
+ edp_panel_vdd_off(intel_dp, true);
+}
+
static void intel_enable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
@@ -2021,10 +2052,9 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
return;
intel_edp_panel_vdd_on(intel_dp);
+ intel_edp_init_train(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
- intel_edp_panel_on(intel_dp);
- edp_panel_vdd_off(intel_dp, true);
intel_dp_complete_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
}
Yeah I think this matches the doc too. I never pushed this change
because I could never find anything that it actually fixed.
I guess you have something now though!
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 37/40] drm/i915: Fix eDP link training when switching pipes
On Tue, Jul 29, 2014 at 08:06:57PM +0200, Daniel Vetter wrote:
On Mon, Jun 30, 2014 at 02:52:12PM -0700, Jesse Barnes wrote:
On Sat, 28 Jun 2014 02:04:28 +0300
ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
When switching from one pipe to another, the power sequencer of the new
pipe seems to need a bit of kicking to lock into the port. Even the vdd
force bit doesn't work before the power sequencer has been sufficiently
kicked, so this must be done even before any AUX transactions.
This sequence has been found to do the trick:
1) enable port with idle pattern
2) enable the power sequencer
3) proceed with link training
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 65ab54c..07b0320 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2010,6 +2010,37 @@ static void chv_post_disable_dp(struct
intel_encoder *encoder)
mutex_unlock(dev_priv-dpio_lock);
}
+static void intel_edp_init_train(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!is_edp(intel_dp))
+ return;
This changes the order of events as observed by the sink, so I really
wonder why this is edp specific?
It's not really. I need to kick the power sequencer for regular DP ports
too (in a later patch), and recently I started to wonder if we also need
it for HDMI ports but I didn't test that theory yet.
Based on my observations there are several problems intermingled here:
1. the power sequencer prevents the port from starting up until the
power up sequence has finished
2. vdd force bit doesn't work until the power sequencer has finished
3. the power sequencer won't finish the power up sequence unless idle
pattern is used
So the fix is to enable the port with idle pattern and enable the power
sequencer even before doing any aux transactions (including the sink
dpms write).
Once the power sequencer has finished powering up on to the port once.
the vdd force bit will keep working on the port even if the port and
power sequencer are later disabled. Also iirc the power sequencer will
no longer prevent the port from starting up even if the power sequencer
is left disabled when re-enabling the port later. But the same problem
will reappear when we change the pipe-port mapping, and then we need
to kick the power sequencer again.
We do have bug reports about external DP monitors not waking up from the
sink_dpms call properly ...
On vlv or something else? I'm not quite sure if the same problems would
be possible on other platforms since they only have one power sequencer.
But maybe that too locks into the port and would need a similar kick.
But IIRC on PCH platforms the spec says that we must enable the port
with training pattern 1. So the use of idle pattern would at least go
against the spec. Which is why I left that part as vlv/chv specific.
-Daniel
+
+ /*
+ * Need to enable the port with idle pattern to allow the power
+ * sequencer to lock into the port. Otherwise the power sequencer
+ * (including vdd force bit!) doesn't work on this port.
+ */
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-DP |= DP_PORT_EN;
+
+ if (IS_CHERRYVIEW(dev))
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK_CHV;
+ else
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK;
+ intel_dp-DP |= DP_LINK_TRAIN_PAT_IDLE;
+
+ I915_WRITE(intel_dp-output_reg, intel_dp-DP);
+ POSTING_READ(intel_dp-output_reg);
+ }
+
+ intel_edp_panel_on(intel_dp);
+ edp_panel_vdd_off(intel_dp, true);
+}
+
static void intel_enable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
@@ -2021,10 +2052,9 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
return;
intel_edp_panel_vdd_on(intel_dp);
+ intel_edp_init_train(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
- intel_edp_panel_on(intel_dp);
- edp_panel_vdd_off(intel_dp, true);
intel_dp_complete_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
}
Yeah I think this matches the doc too. I never pushed this change
because I could never find anything that it actually fixed.
I guess you have something now though!
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
Re: [Intel-gfx] [PATCH 37/40] drm/i915: Fix eDP link training when switching pipes
On Tue, Jul 29, 2014 at 10:18:46PM +0300, Ville Syrjälä wrote:
On Tue, Jul 29, 2014 at 08:06:57PM +0200, Daniel Vetter wrote:
On Mon, Jun 30, 2014 at 02:52:12PM -0700, Jesse Barnes wrote:
On Sat, 28 Jun 2014 02:04:28 +0300
ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
When switching from one pipe to another, the power sequencer of the new
pipe seems to need a bit of kicking to lock into the port. Even the vdd
force bit doesn't work before the power sequencer has been sufficiently
kicked, so this must be done even before any AUX transactions.
This sequence has been found to do the trick:
1) enable port with idle pattern
2) enable the power sequencer
3) proceed with link training
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 65ab54c..07b0320 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2010,6 +2010,37 @@ static void chv_post_disable_dp(struct
intel_encoder *encoder)
mutex_unlock(dev_priv-dpio_lock);
}
+static void intel_edp_init_train(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port =
dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!is_edp(intel_dp))
+ return;
This changes the order of events as observed by the sink, so I really
wonder why this is edp specific?
It's not really. I need to kick the power sequencer for regular DP ports
too (in a later patch), and recently I started to wonder if we also need
it for HDMI ports but I didn't test that theory yet.
Based on my observations there are several problems intermingled here:
1. the power sequencer prevents the port from starting up until the
power up sequence has finished
2. vdd force bit doesn't work until the power sequencer has finished
3. the power sequencer won't finish the power up sequence unless idle
pattern is used
So the fix is to enable the port with idle pattern and enable the power
sequencer even before doing any aux transactions (including the sink
dpms write).
Once the power sequencer has finished powering up on to the port once.
the vdd force bit will keep working on the port even if the port and
power sequencer are later disabled. Also iirc the power sequencer will
no longer prevent the port from starting up even if the power sequencer
is left disabled when re-enabling the port later. But the same problem
will reappear when we change the pipe-port mapping, and then we need
to kick the power sequencer again.
We do have bug reports about external DP monitors not waking up from the
sink_dpms call properly ...
On vlv or something else? I'm not quite sure if the same problems would
be possible on other platforms since they only have one power sequencer.
But maybe that too locks into the port and would need a similar kick.
But IIRC on PCH platforms the spec says that we must enable the port
with training pattern 1. So the use of idle pattern would at least go
against the spec. Which is why I left that part as vlv/chv specific.
Hm ... tricky. And especially since it seems to be required only once. I'm
just concerned about the slight behavioural difference between the general
dp link enabling where we do the sink dpms aux transaction first, then
start with link training. But on vlv/chv here we first kick the port a bit
and enable the idle pattern. If now a few displays don't like this or
require this we have a problem.
In general I really want us to try as hard as possible to have 0
differences in sink-visible behaviour. DP is simply too fickle imo. But if
there's nothing we can do I guess good explanations in commit message and
comments is all I can ask for. I.e. something like the above should go
into the commit message and maybe we should make the comment a bit more
dangerously-sounding.
-Daniel
-Daniel
+
+ /*
+* Need to enable the port with idle pattern to allow the power
+* sequencer to lock into the port. Otherwise the power
sequencer
+* (including vdd force bit!) doesn't work on this port.
+*/
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-DP |= DP_PORT_EN;
+
+ if (IS_CHERRYVIEW(dev))
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK_CHV;
+ else
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK;
+ intel_dp-DP |= DP_LINK_TRAIN_PAT_IDLE;
+
+
Re: [Intel-gfx] [PATCH 37/40] drm/i915: Fix eDP link training when switching pipes
On Sat, 28 Jun 2014 02:04:28 +0300
ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
When switching from one pipe to another, the power sequencer of the new
pipe seems to need a bit of kicking to lock into the port. Even the vdd
force bit doesn't work before the power sequencer has been sufficiently
kicked, so this must be done even before any AUX transactions.
This sequence has been found to do the trick:
1) enable port with idle pattern
2) enable the power sequencer
3) proceed with link training
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 65ab54c..07b0320 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2010,6 +2010,37 @@ static void chv_post_disable_dp(struct intel_encoder
*encoder)
mutex_unlock(dev_priv-dpio_lock);
}
+static void intel_edp_init_train(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!is_edp(intel_dp))
+ return;
+
+ /*
+ * Need to enable the port with idle pattern to allow the power
+ * sequencer to lock into the port. Otherwise the power sequencer
+ * (including vdd force bit!) doesn't work on this port.
+ */
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-DP |= DP_PORT_EN;
+
+ if (IS_CHERRYVIEW(dev))
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK_CHV;
+ else
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK;
+ intel_dp-DP |= DP_LINK_TRAIN_PAT_IDLE;
+
+ I915_WRITE(intel_dp-output_reg, intel_dp-DP);
+ POSTING_READ(intel_dp-output_reg);
+ }
+
+ intel_edp_panel_on(intel_dp);
+ edp_panel_vdd_off(intel_dp, true);
+}
+
static void intel_enable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
@@ -2021,10 +2052,9 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
return;
intel_edp_panel_vdd_on(intel_dp);
+ intel_edp_init_train(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
- intel_edp_panel_on(intel_dp);
- edp_panel_vdd_off(intel_dp, true);
intel_dp_complete_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
}
Yeah I think this matches the doc too. I never pushed this change
because I could never find anything that it actually fixed.
I guess you have something now though!
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 37/40] drm/i915: Fix eDP link training when switching pipes
From: Ville Syrjälä ville.syrj...@linux.intel.com
When switching from one pipe to another, the power sequencer of the new
pipe seems to need a bit of kicking to lock into the port. Even the vdd
force bit doesn't work before the power sequencer has been sufficiently
kicked, so this must be done even before any AUX transactions.
This sequence has been found to do the trick:
1) enable port with idle pattern
2) enable the power sequencer
3) proceed with link training
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 65ab54c..07b0320 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2010,6 +2010,37 @@ static void chv_post_disable_dp(struct intel_encoder
*encoder)
mutex_unlock(dev_priv-dpio_lock);
}
+static void intel_edp_init_train(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!is_edp(intel_dp))
+ return;
+
+ /*
+* Need to enable the port with idle pattern to allow the power
+* sequencer to lock into the port. Otherwise the power sequencer
+* (including vdd force bit!) doesn't work on this port.
+*/
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-DP |= DP_PORT_EN;
+
+ if (IS_CHERRYVIEW(dev))
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK_CHV;
+ else
+ intel_dp-DP = ~DP_LINK_TRAIN_MASK;
+ intel_dp-DP |= DP_LINK_TRAIN_PAT_IDLE;
+
+ I915_WRITE(intel_dp-output_reg, intel_dp-DP);
+ POSTING_READ(intel_dp-output_reg);
+ }
+
+ intel_edp_panel_on(intel_dp);
+ edp_panel_vdd_off(intel_dp, true);
+}
+
static void intel_enable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
@@ -2021,10 +2052,9 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
return;
intel_edp_panel_vdd_on(intel_dp);
+ intel_edp_init_train(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
- intel_edp_panel_on(intel_dp);
- edp_panel_vdd_off(intel_dp, true);
intel_dp_complete_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
}
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
