From patchwork Sat Oct 26 08:27:42 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mario Kleiner X-Patchwork-Id: 3098011 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1B0B09F372 for ; Sat, 26 Oct 2013 10:07:27 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 25CD820256 for ; Sat, 26 Oct 2013 10:07:26 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 3488120251 for ; Sat, 26 Oct 2013 10:07:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DA75AE681E for ; Sat, 26 Oct 2013 03:07:24 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from xmailer.gwdg.de (xmailer.gwdg.de [134.76.10.29]) by gabe.freedesktop.org (Postfix) with ESMTP id 70B90E6748; Sat, 26 Oct 2013 02:53:52 -0700 (PDT) Received: from smtp-out.tuebingen.mpg.de ([192.124.26.249] helo=tuebingen.mpg.de) by mailer.gwdg.de with esmtp (Exim 4.80) (envelope-from ) id 1VZzEZ-0005wa-Hv; Sat, 26 Oct 2013 10:28:35 +0200 Received: from fir.kyb.local (account mario.kleiner@tuebingen.mpg.de [10.38.132.250] verified) by tuebingen.mpg.de (CommuniGate Pro SMTP 5.4.2) with ESMTPA id 25873539; Sat, 26 Oct 2013 10:28:35 +0200 From: Mario Kleiner To: dri-devel@lists.freedesktop.org, airlied@gmail.com Date: Sat, 26 Oct 2013 10:27:42 +0200 Message-Id: <1382776062-3770-5-git-send-email-mario.kleiner.de@gmail.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1382776062-3770-1-git-send-email-mario.kleiner.de@gmail.com> References: <1382776062-3770-1-git-send-email-mario.kleiner.de@gmail.com> X-Virus-Scanned: (clean) by exiscan+sophie Cc: alexdeucher@gmail.com, intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 4/4] drm/intel: Push get_scanout_position() timestamping into kms driver. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Move the ktime_get() clock readouts and potential preempt_disable() calls from drm core into kms driver to make it compatible with the api changes in the drm core. The intel-kms driver needs to take the uncore.lock inside i915_get_crtc_scanoutpos() and intel_pipe_in_vblank(). This is incompatible with the preempt_disable() on a PREEMPT_RT patched kernel, as regular spin locks must not be taken within a preempt_disable'd section. Lock contention on the uncore.lock also introduced too much uncertainty in vblank timestamps. Push the ktime_get() timestamping for scanoutpos queries and potential preempt_disable_rt() into i915_get_crtc_scanoutpos(), so these problems can be avoided: 1. First lock the uncore.lock (might sleep on a PREEMPT_RT kernel). 2. preempt_disable_rt() (will be added by the rt-linux folks). 3. ktime_get() a timestamp before scanout pos query. 4. Do all mmio reads as fast as possible without grabbing any new locks! 5. ktime_get() a post-query timestamp. 6. preempt_enable_rt() 7. Unlock the uncore.lock. This reduces timestamp uncertainty on a low-end HP Atom Mini netbook with Intel GMA-950 nicely: Before: 3-8 usecs with spikes > 20 usecs, triggering query retries. After : Typically 1 usec (98% of all samples), occassionally 2 usecs (2% of all samples), with maximum of 3 usecs (a handful). Signed-off-by: Mario Kleiner --- drivers/gpu/drm/i915/i915_irq.c | 53 +++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 156a1a4..a3e41d3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -599,35 +599,40 @@ static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe) return I915_READ(reg); } -static bool intel_pipe_in_vblank(struct drm_device *dev, enum pipe pipe) +/* raw reads, only for fast reads of display block, no need for forcewake etc. */ +#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__)) +#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs + (reg__)) + +static bool intel_pipe_in_vblank_locked(struct drm_device *dev, enum pipe pipe) { struct drm_i915_private *dev_priv = dev->dev_private; uint32_t status; + int reg; if (IS_VALLEYVIEW(dev)) { status = pipe == PIPE_A ? I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT : I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT; - return I915_READ(VLV_ISR) & status; + reg = VLV_ISR; } else if (IS_GEN2(dev)) { status = pipe == PIPE_A ? I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT : I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT; - return I915_READ16(ISR) & status; + reg = ISR; } else if (INTEL_INFO(dev)->gen < 5) { status = pipe == PIPE_A ? I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT : I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT; - return I915_READ(ISR) & status; + reg = ISR; } else if (INTEL_INFO(dev)->gen < 7) { status = pipe == PIPE_A ? DE_PIPEA_VBLANK : DE_PIPEB_VBLANK; - return I915_READ(DEISR) & status; + reg = DEISR; } else { switch (pipe) { default: @@ -642,12 +647,17 @@ static bool intel_pipe_in_vblank(struct drm_device *dev, enum pipe pipe) break; } - return I915_READ(DEISR) & status; + reg = DEISR; } + + if (IS_GEN2(dev)) + return __raw_i915_read16(dev_priv, reg) & status; + else + return __raw_i915_read32(dev_priv, reg) & status; } static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, - int *vpos, int *hpos) + int *vpos, int *hpos, ktime_t *stime, ktime_t *etime) { struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; @@ -657,6 +667,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, int vbl_start, vbl_end, htotal, vtotal; bool in_vbl = true; int ret = 0; + unsigned long irqflags; if (!intel_crtc->active) { DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled " @@ -671,14 +682,26 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; + /* Lock uncore.lock, as we will do multiple timing critical raw + * register reads, potentially with preemption disabled, so the + * following code must not block on uncore.lock. + */ + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); + + /* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */ + + /* Get optional system timestamp before query. */ + if (stime) + *stime = ktime_get(); + if (IS_GEN2(dev) || IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) { /* No obvious pixelcount register. Only query vertical * scanout position from Display scan line register. */ if (IS_GEN2(dev)) - position = I915_READ(PIPEDSL(pipe)) & DSL_LINEMASK_GEN2; + position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN2; else - position = I915_READ(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3; + position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN3; /* * The scanline counter increments at the leading edge @@ -687,7 +710,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, * to get a more accurate picture whether we're in vblank * or not. */ - in_vbl = intel_pipe_in_vblank(dev, pipe); + in_vbl = intel_pipe_in_vblank_locked(dev, pipe); if ((in_vbl && position == vbl_start - 1) || (!in_vbl && position == vbl_end - 1)) position = (position + 1) % vtotal; @@ -696,7 +719,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, * We can split this into vertical and horizontal * scanout position. */ - position = (I915_READ(PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT; + position = (__raw_i915_read32(dev_priv, PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT; /* convert to pixel counts */ vbl_start *= htotal; @@ -704,6 +727,14 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, vtotal *= htotal; } + /* Get optional system timestamp after query. */ + if (etime) + *etime = ktime_get(); + + /* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */ + + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); + in_vbl = position >= vbl_start && position < vbl_end; /*