[Intel-gfx] [PATCH 7/7] drm/i915: wait for actual vblank, not just 20ms

[Andrew Lutomirski]

On Wed, Aug 18, 2010 at 5:05 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> On Wed, 18 Aug 2010 21:48:50 +0100
> Owain Ainsworth <zerooa at googlemail.com> 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 <jbarnes at virtuousgeek.org>
>> > ---
>> >  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...

msleep at least?  that's a lot of latency on single-processor otherwise.


>
> --
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>