From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 7/7] drm/i915: wait for actual vblank, not just 20ms Date: Wed, 18 Aug 2010 14:05:17 -0700 Message-ID: <20100818140517.46538871@virtuousgeek.org> References: <1282158036-1671-1-git-send-email-jbarnes@virtuousgeek.org> <1282158036-1671-8-git-send-email-jbarnes@virtuousgeek.org> <20100818204850.GD4929@melanie.nicotinebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cpoproxy1-pub.bluehost.com (cpoproxy1-pub.bluehost.com [69.89.21.11]) by gabe.freedesktop.org (Postfix) with SMTP id D7AFC9E83E for ; Wed, 18 Aug 2010 14:05:20 -0700 (PDT) In-Reply-To: <20100818204850.GD4929@melanie.nicotinebsd.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Owain Ainsworth Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, 18 Aug 2010 21:48:50 +0100 Owain Ainsworth wrote: > On Wed, Aug 18, 2010 at 12:00:36PM -0700, Jesse Barnes wrote: > > Waiting for a hard coded 20ms isn't always enough to make sure a vblank > > period has actually occurred, so add code to make sure we really have > > passed through a vblank period (or that the pipe is off when disabling). > > > > This prevents problems with mode setting and link training, and seems to > > fix a bug like https://bugs.freedesktop.org/show_bug.cgi?id=29278, but > > on an HP 8440p instead. Hopefully also fixes > > https://bugs.freedesktop.org/show_bug.cgi?id=29141. > > > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > drivers/gpu/drm/i915/intel_crt.c | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 75 ++++++++++++++++++++++++++-------- > > drivers/gpu/drm/i915/intel_dp.c | 3 +- > > drivers/gpu/drm/i915/intel_drv.h | 3 +- > > drivers/gpu/drm/i915/intel_sdvo.c | 3 +- > > drivers/gpu/drm/i915/intel_tv.c | 9 ++-- > > 7 files changed, 71 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index cf41c67..822b21c 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -2080,6 +2080,7 @@ > > #define PIPE_DITHER_TYPE_ST01 (1 << 2) > > /* Pipe A */ > > #define PIPEADSL 0x70000 > > +#define DSL_LINEMASK 0x00000fff > > #define PIPEACONF 0x70008 > > #define PIPEACONF_ENABLE (1<<31) > > #define PIPEACONF_DISABLE 0 > > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c > > index ee0732b..4a2f593 100644 > > --- a/drivers/gpu/drm/i915/intel_crt.c > > +++ b/drivers/gpu/drm/i915/intel_crt.c > > @@ -331,7 +331,7 @@ intel_crt_load_detect(struct drm_crtc *crtc, struct intel_encoder *intel_encoder > > I915_WRITE(pipeconf_reg, pipeconf | PIPECONF_FORCE_BORDER); > > /* Wait for next Vblank to substitue > > * border color for Color info */ > > - intel_wait_for_vblank(dev); > > + intel_wait_for_vblank(dev, pipe); > > st00 = I915_READ8(VGA_MSR_WRITE); > > status = ((st00 & (1 << 4)) != 0) ? > > connector_status_connected : > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 928bcc2..27e60b8 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -972,11 +972,57 @@ intel_find_pll_g4x_dp(const intel_limit_t *limit, struct drm_crtc *crtc, > > return true; > > } > > > > -void > > -intel_wait_for_vblank(struct drm_device *dev) > > +/** > > + * intel_wait_for_vblank - wait for vblank on a given pipe > > + * @dev: drm device > > + * @pipe: pipe to wait for > > + * > > + * Wait for vblank to occur on a given pipe. Needed for various bits of > > + * mode setting code. > > + */ > > +void intel_wait_for_vblank(struct drm_device *dev, int pipe) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + int pipestat_reg = (pipe == 0 ? PIPEASTAT : PIPEBSTAT); > > + unsigned long timeout = jiffies + msecs_to_jiffies(100); > > + > > + /* Wait for vblank interrupt bit to set */ > > + while (!(I915_READ(pipestat_reg) & PIPE_VBLANK_INTERRUPT_STATUS) && > > + time_after(timeout, jiffies)) > > + mdelay(1); > > Why not actually go to sleep and let the interrupt wake you up? let the > machine do something else in the meantime. Yeah, I thought that might be nice, but I was worried about locking and the KDB paths... -- Jesse Barnes, Intel Open Source Technology Center