While the display port is in training mode, vblank interrupts don't occur. Because we have to wait for the display port output to turn on before starting the training sequence, enable the output in 'normal' mode so that we can tell when a vblank has occurred, then start the training sequence.
Signed-off-by: Keith Packard keithp@keithp.com --- drivers/gpu/drm/i915/intel_dp.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1a51ee0..9ab8708 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1138,18 +1138,14 @@ static bool intel_dp_set_link_train(struct intel_dp *intel_dp, uint32_t dp_reg_value, uint8_t dp_train_pat, - uint8_t train_set[4], - bool first) + uint8_t train_set[4]) { struct drm_device *dev = intel_dp->base.enc.dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.enc.crtc); int ret;
I915_WRITE(intel_dp->output_reg, dp_reg_value); POSTING_READ(intel_dp->output_reg); - if (first) - intel_wait_for_vblank(dev, intel_crtc->pipe);
intel_dp_aux_native_write_1(intel_dp, DP_TRAINING_PATTERN_SET, @@ -1174,10 +1170,15 @@ intel_dp_link_train(struct intel_dp *intel_dp) uint8_t voltage; bool clock_recovery = false; bool channel_eq = false; - bool first = true; int tries; u32 reg; uint32_t DP = intel_dp->DP; + struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.enc.crtc); + + /* Enable output, wait for it to become active */ + I915_WRITE(intel_dp->output_reg, intel_dp->DP); + POSTING_READ(intel_dp->output_reg); + intel_wait_for_vblank(dev, intel_crtc->pipe);
/* Write the link configuration data */ intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, @@ -1210,9 +1211,8 @@ intel_dp_link_train(struct intel_dp *intel_dp) reg = DP | DP_LINK_TRAIN_PAT_1;
if (!intel_dp_set_link_train(intel_dp, reg, - DP_TRAINING_PATTERN_1, train_set, first)) + DP_TRAINING_PATTERN_1, train_set)) break; - first = false; /* Set training pattern 1 */
udelay(100); @@ -1266,8 +1266,7 @@ intel_dp_link_train(struct intel_dp *intel_dp)
/* channel eq pattern */ if (!intel_dp_set_link_train(intel_dp, reg, - DP_TRAINING_PATTERN_2, train_set, - false)) + DP_TRAINING_PATTERN_2, train_set)) break;
udelay(400);
Instead of waiting for the display line value to settle, we can simply wait for the pipe configuration register 'state' bit to turn off.
Contrarywise, disabling the plane will not cause the display line value to stop changing, so instead we wait for the vblank interrupt bit to get set. And, we only do this when we're not about to wait for the pipe to turn off.
Signed-off-by: Keith Packard keithp@keithp.com --- drivers/gpu/drm/i915/intel_display.c | 62 +++++++++++++++++++++------------- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 38 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b5bf51a..9792285 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1013,8 +1013,8 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe) DRM_DEBUG_KMS("vblank wait timed out\n"); }
-/** - * intel_wait_for_vblank_off - wait for vblank after disabling a pipe +/* + * intel_wait_for_pipe_off - wait for pipe to turn off * @dev: drm device * @pipe: pipe to wait for * @@ -1022,25 +1022,39 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe) * spinning on the vblank interrupt status bit, since we won't actually * see an interrupt when the pipe is disabled. * - * So this function waits for the display line value to settle (it - * usually ends up stopping at the start of the next frame). + * On Gen4 and above: + * wait for the pipe register state bit to turn off + * + * Otherwise: + * wait for the display line value to settle (it usually + * ends up stopping at the start of the next frame). + * */ -void intel_wait_for_vblank_off(struct drm_device *dev, int pipe) +static void intel_wait_for_pipe_off(struct drm_device *dev, int pipe) { struct drm_i915_private *dev_priv = dev->dev_private; - int pipedsl_reg = (pipe == 0 ? PIPEADSL : PIPEBDSL); - unsigned long timeout = jiffies + msecs_to_jiffies(100); - u32 last_line; - - /* Wait for the display line to settle */ - do { - last_line = I915_READ(pipedsl_reg) & DSL_LINEMASK; - mdelay(5); - } while (((I915_READ(pipedsl_reg) & DSL_LINEMASK) != last_line) && - time_after(timeout, jiffies)); - - if (time_after(jiffies, timeout)) - DRM_DEBUG_KMS("vblank wait timed out\n"); + + if (INTEL_INFO(dev)->gen >= 4) { + int pipeconf_reg = (pipe == 0 ? PIPEACONF : PIPEBCONF); + + /* Wait for the Pipe State to go off */ + if (wait_for((I915_READ(pipeconf_reg) & I965_PIPECONF_ACTIVE) == 0, + 100, 0)) + DRM_DEBUG_KMS("pipe_off wait timed out\n"); + } else { + u32 last_line; + int pipedsl_reg = (pipe == 0 ? PIPEADSL : PIPEBDSL); + unsigned long timeout = jiffies + msecs_to_jiffies(100); + + /* Wait for the display line to settle */ + do { + last_line = I915_READ(pipedsl_reg) & DSL_LINEMASK; + mdelay(5); + } while (((I915_READ(pipedsl_reg) & DSL_LINEMASK) != last_line) && + time_after(timeout, jiffies)); + if (time_after(jiffies, timeout)) + DRM_DEBUG_KMS("pipe_off wait timed out\n"); + } }
/* Parameters have changed, update FBC info */ @@ -2328,13 +2342,13 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode) I915_READ(dspbase_reg); }
- /* Wait for vblank for the disable to take effect */ - intel_wait_for_vblank_off(dev, pipe); - /* Don't disable pipe A or pipe A PLLs if needed */ if (pipeconf_reg == PIPEACONF && - (dev_priv->quirks & QUIRK_PIPEA_FORCE)) + (dev_priv->quirks & QUIRK_PIPEA_FORCE)) { + /* Wait for vblank for the disable to take effect */ + intel_wait_for_vblank(dev, pipe); goto skip_pipe_off; + }
/* Next, disable display pipes */ temp = I915_READ(pipeconf_reg); @@ -2343,8 +2357,8 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode) I915_READ(pipeconf_reg); }
- /* Wait for vblank for the disable to take effect. */ - intel_wait_for_vblank_off(dev, pipe); + /* Wait for the pipe to turn off */ + intel_wait_for_pipe_off(dev, pipe);
temp = I915_READ(dpll_reg); if ((temp & DPLL_VCO_ENABLE) != 0) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ad312ca..8828b3a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -229,7 +229,6 @@ extern struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, struct drm_crtc *crtc); int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, struct drm_file *file_priv); -extern void intel_wait_for_vblank_off(struct drm_device *dev, int pipe); extern void intel_wait_for_vblank(struct drm_device *dev, int pipe); extern struct drm_crtc *intel_get_crtc_from_pipe(struct drm_device *dev, int pipe); extern struct drm_crtc *intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
On Sun, 3 Oct 2010 00:33:06 -0700, Keith Packard keithp@keithp.com wrote:
Instead of waiting for the display line value to settle, we can simply wait for the pipe configuration register 'state' bit to turn off.
Thanks Keith, series applied to -fixes. -Chris
On Sun, 3 Oct 2010 00:33:06 -0700 Keith Packard keithp@keithp.com wrote:
Instead of waiting for the display line value to settle, we can simply wait for the pipe configuration register 'state' bit to turn off.
Contrarywise, disabling the plane will not cause the display line value to stop changing, so instead we wait for the vblank interrupt bit to get set. And, we only do this when we're not about to wait for the pipe to turn off.
Signed-off-by: Keith Packard keithp@keithp.com
Do these fixes help with the DP issues you've been seeing, Keith? Seems like the first one shouldn't change behavior since we ought to time out on waiting on vblank in that case, and the timeout is the same as the msleep we used to use.
The second one looks like a good change, but really the pipe off change is separate from the plane disable bug fix.
On Sun, 3 Oct 2010 08:10:48 -0700, Jesse Barnes jbarnes@virtuousgeek.org wrote:
Do these fixes help with the DP issues you've been seeing, Keith? Seems like the first one shouldn't change behavior since we ought to time out on waiting on vblank in that case, and the timeout is the same as the msleep we used to use.
The first one changes when the monitor sees the training message -- before the change, the training message would get sent before waiting for the vblank, and could potentially mess up the monitor synchronization with signals.
I tested this by turning an external DP monitor on/off repeatedly without X running. Before the patch, the monitor would fail to sync once in a while. With the patch, I haven't seen it fail. That's not to say that it has actually fixed anything, just that it seems better.
The best feature of the patch is that it shortens the time it takes to light up a DP pipe -- the code was always hitting the timeout instead of seeing a vblank signal, so we'd get a 50ms delay instead of a couple of ms.
The second one looks like a good change, but really the pipe off change is separate from the plane disable bug fix.
Yeah, yeah, I know. I should have split the patch into two pieces...
With these two patches in place, I'm not getting any timeouts while waiting for vblank, which seems like a useful result, and should make mode setting a tiny bit faster as well.
I've got a couple more changes to work on today:
1) re-train the monitor when it gets unplugged and then plugged back in. Right now, if you kick the cable out, you're stuck fumbling around in the dark trying to run 'xrandr' again.
2) send hotplug notification through the X server, at least for the 'reliable' hotplug signals. Right now, when you run 'xrandr' after changing connections, gnome sees the connection status change event and 'does stuff', which frequently collides with the 'xrandr' command you're running. This is very confusing to users.
"Keith Packard" keithp@keithp.com wrote:
On Sun, 3 Oct 2010 08:10:48 -0700, Jesse Barnes jbarnes@virtuousgeek.org wrote:
Do these fixes help with the DP issues you've been seeing, Keith? Seems like the first one shouldn't change behavior since we ought to time out on waiting on vblank in that case, and the timeout is the same as the msleep we used to use.
The first one changes when the monitor sees the training message -- before the change, the training message would get sent before waiting for the vblank, and could potentially mess up the monitor synchronization with signals.
I tested this by turning an external DP monitor on/off repeatedly without X running. Before the patch, the monitor would fail to sync once in a while. With the patch, I haven't seen it fail. That's not to say that it has actually fixed anything, just that it seems better.
The best feature of the patch is that it shortens the time it takes to light up a DP pipe -- the code was always hitting the timeout instead of seeing a vblank signal, so we'd get a 50ms delay instead of a couple of ms.
Yeah, definitely an improvement. I'll test myself when I get home (tested on my PCH eDP already, unfortunately didn't help).
The second one looks like a good change, but really the pipe off change is separate from the plane disable bug fix.
Yeah, yeah, I know. I should have split the patch into two pieces...
With these two patches in place, I'm not getting any timeouts while waiting for vblank, which seems like a useful result, and should make mode setting a tiny bit faster as well.
I've got a couple more changes to work on today:
- re-train the monitor when it gets unplugged and then plugged back in. Right now, if you kick the cable out, you're stuck fumbling around in the dark trying to run 'xrandr' again.
Cool. Making use of our hotplug interrupts and never polling should be a goal; it looked like there were some other aux commands we could add as well. I'm meeting with the SV guys on Wed. To go over our eDP stuff, hopefully we can run the above through their equipment too and make sure we have all the timings etc correct.
- send hotplug notification through the X server, at least for the 'reliable' hotplug signals. Right now, when you run 'xrandr' after changing connections, gnome sees the connection status change event and 'does stuff', which frequently collides with the 'xrandr' command you're running. This is very confusing to users.
Great, yeah we shouldn't be sending events in response to our normal status ioctls, that sounds broken. On 9xx and above we should be able to do reliable hotplug for everything (though at a power cost for TV and maybe VGA).
mode setting a tiny bit faster as well.
I've got a couple more changes to work on today:
1) re-train the monitor when it gets unplugged and then plugged back in. Right now, if you kick the cable out, you're stuck fumbling around in the dark trying to run 'xrandr' again.
don't you do this already? both radeon/nouveau handle DP replug fine, I thought Intel would have been where I stole the code from.
Dave.
"Dave Airlie" airlied@gmail.com wrote:
mode setting a tiny bit faster as well.
I've got a couple more changes to work on today:
1) re-train the monitor when it gets unplugged and then plugged back in. Right now, if you kick the cable out, you're stuck fumbling around in the dark trying to run 'xrandr' again.
don't you do this already? both radeon/nouveau handle DP replug fine, I thought Intel would have been where I stole the code from.
There is some code to handle the interrupts, but I'm not sure if it's wired up. I think we rely on polling right now.
On Mon, 4 Oct 2010 06:33:01 +1000, Dave Airlie airlied@gmail.com wrote:
don't you do this already? both radeon/nouveau handle DP replug fine, I thought Intel would have been where I stole the code from.
There was a one-line bug. Patch already posted.
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Dave Airlie -
Jesse Barnes -
Keith Packard