From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCHv5] drm/i915: use waitqueue in wait for vblank Date: Wed, 21 May 2014 11:40:21 +0300 Message-ID: <20140521084021.GL27580@intel.com> References: <1400657998-10086-1-git-send-email-arun.r.murthy@intel.com> <1400657998-10086-2-git-send-email-arun.r.murthy@intel.com> <20140521075404.GB3409@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id C76DA6E9E6 for ; Wed, 21 May 2014 01:40:40 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140521075404.GB3409@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Arun R Murthy , daniel.vetter@ffwll.ch, jani.nikula@linux.intel.com, intel-gfx@lists.freedesktop.org, airlied@linux.ie List-Id: intel-gfx@lists.freedesktop.org On Wed, May 21, 2014 at 08:54:04AM +0100, Chris Wilson wrote: > On Wed, May 21, 2014 at 01:09:58PM +0530, Arun R Murthy wrote: > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i91= 5_irq.c > > index 56edff3..f97f0fe 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -1513,8 +1513,11 @@ static irqreturn_t valleyview_irq_handler(int ir= q, void *arg) > > spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > > = > > for_each_pipe(pipe) { > > - if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS) > > + if (pipe_stats[pipe] & > > + PIPE_START_VBLANK_INTERRUPT_STATUS) { > > drm_handle_vblank(dev, pipe); > > + wake_up_interruptible(&dev_priv->wait_vblank); > = > We now have intel_handle_vblank() so these chunks can be simplified. > = > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i91= 5/intel_display.c > > index 4d4a0d9..e1eb564 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -757,12 +757,14 @@ enum transcoder intel_pipe_to_cpu_transcoder(stru= ct drm_i915_private *dev_priv, > > static void g4x_wait_for_vblank(struct drm_device *dev, int pipe) > > { > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > - u32 frame, frame_reg =3D PIPE_FRMCOUNT_GM45(pipe); > > + u32 vblank_cnt; > > = > > - frame =3D I915_READ(frame_reg); > > + vblank_cnt =3D drm_vblank_count(dev, pipe); > > = > > - if (wait_for(I915_READ_NOTRACE(frame_reg) !=3D frame, 50)) > > - DRM_DEBUG_KMS("vblank wait timed out\n"); > > + /* TODO: get the vblank time dynamically or from platform data */ > > + wait_event_interruptible_timeout(dev_priv->wait_vblank, > > + (vblank_cnt !=3D drm_vblank_count(dev, pipe)), > > + msecs_to_jiffies(16)); > = > Keep the ultimate timeout at 50 until you have evidence you can reduce > it. And then it should be 2x vrefresh interval to be safe. However, you > are likely still hitting the timeout as the vblank irq is not guaranteed > to be enabled here. How safe calling drm_vblank_get() is during modeset > I defer to Ville since he has just taken a pass over the whole mess. The plan is to make drm_vblank_get() work until drm_vblank_off() has been called. And when enabling, drm_vblank_get() will succeed only after drm_vblank_on() has been called. The place where those should end up is at the start/end of intel_crtc_{disable,enable}_planes(). So you have access to vblank irqs while planes are getting enabled/disabled, but no further since we can't guarantee their function once we start shutting off pipes/ports/etc. -- = Ville Syrj=E4l=E4 Intel OTC