From patchwork Fri Dec 28 02:30:47 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Gnoutcheff X-Patchwork-Id: 209023 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 9DCE62C0086 for ; Wed, 2 Jan 2013 23:01:13 +1100 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TqN0I-00029a-EG; Wed, 02 Jan 2013 12:01:02 +0000 Received: from mail-vb0-f41.google.com ([209.85.212.41]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1ToPj3-0007az-80 for kernel-team@lists.ubuntu.com; Fri, 28 Dec 2012 02:31:09 +0000 Received: by mail-vb0-f41.google.com with SMTP id l22so10483703vbn.28 for ; Thu, 27 Dec 2012 18:31:08 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:sender:from:to:subject:date:message-id:x-mailer :x-gm-message-state; bh=C82jFZYue/MJAbaoMZEc+oydm1W4cMb0U56IbhEkzPE=; b=I9FoAUrwCHV/BCneFSaeh3gtAGhXFu6a9spFFpSjWcwdL9pH2JGS6PTYGaMV1GXwbj HQGKnAIOrVo503DfnLprNUfv6f3tsTv0514NvA70ksligdTb2RO+7XI8DTxbT8ZoQPjA XgBkZy50zQZQ7NOd28k7t+dlbHy99h0qea3gZsrsScu8agmZMmouutDzgoN0q+mCwDNE +cN50YnqGmbNM9tY1/lyGqSpyLK1aA12Su62+REYkOv8RqJzz1uTtxerrB9W4mesx/uS 0A9+Qzly04WKicMt7nHSYdZfMiGzc4Sad1wmzF7kj5NvhqA4mqsrRMfEk/aTS87wbtom VoJA== X-Received: by 10.52.156.67 with SMTP id wc3mr43282821vdb.19.1356661868236; Thu, 27 Dec 2012 18:31:08 -0800 (PST) Received: from localhost.localdomain (ool-44c7d8b9.dyn.optonline.net. [68.199.216.185]) by mx.google.com with ESMTPS id b10sm28329632vdk.15.2012.12.27.18.31.06 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 27 Dec 2012 18:31:07 -0800 (PST) From: Daniel Gnoutcheff To: kernel-team@lists.ubuntu.com Subject: [PATCH] drm/i915: Flush outstanding unpin tasks before pageflipping Date: Thu, 27 Dec 2012 21:30:47 -0500 Message-Id: <1356661847-4840-1-git-send-email-daniel@gnoutcheff.name> X-Mailer: git-send-email 1.7.10.4 X-Gm-Message-State: ALoCoQkdSufowv1K8foUeHGJvzKTImwux1iYL5VofDX+KMmaJf5Um28Mck6SIXncaEKzxgCZmrbM X-Mailman-Approved-At: Wed, 02 Jan 2013 12:00:07 +0000 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com From: Chris Wilson commit b4a98e57fc27854b5938fc8b08b68e5e68b91e1f upstream If we accumulate unpin tasks because we are pageflipping faster than the system can schedule its workers, we can effectively create a pin-leak. The solution taken here is to limit the number of unpin tasks we have per-crtc and to flush those outstanding tasks if we accumulate too many. This should prevent any jitter in the normal case, and also prevent the hang if we should run too fast. Note: It is important that we switch from the system workqueue to our own dev_priv->wq since all work items on that queue are guaranteed to only need the dev->struct_mutex and not any modeset resources. For otherwise if we have a work item ahead in the queue which needs the modeset lock (like the output detect work used by both polling or hpd), this work and so the unpin work will never execute since the pageflip code already holds that lock. Unfortunately there's no lockdep support for this scenario in the workqueue code. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=46991 Reported-and-tested-by: Tvrtko Ursulin Signed-off-by: Chris Wilson [danvet: Added note about workqueu deadlock.] Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=56337 Signed-off-by: Daniel Vetter Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58732 Tested-by: Daniel Gnoutcheff [dg: Adjusted context, removed a whitespace change] Launchpad: https://bugs.launchpad.net/linux/+bug/1094173 --- This patch is intended for the 3.5.7.y extended longterm queue. It has had its context adjusted so it will apply cleanly on 3.5.7.2. The same patch (with different context) has been submitted upstream for the 3.7.y queue. Thanks, HTH! drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++----- drivers/gpu/drm/i915/intel_drv.h | 4 +++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 552a229..27edd55 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5872,14 +5872,19 @@ static void intel_unpin_work_fn(struct work_struct *__work) { struct intel_unpin_work *work = container_of(__work, struct intel_unpin_work, work); + struct drm_device *dev = work->crtc->dev; - mutex_lock(&work->dev->struct_mutex); + mutex_lock(&dev->struct_mutex); intel_unpin_fb_obj(work->old_fb_obj); drm_gem_object_unreference(&work->pending_flip_obj->base); drm_gem_object_unreference(&work->old_fb_obj->base); - intel_update_fbc(work->dev); - mutex_unlock(&work->dev->struct_mutex); + intel_update_fbc(dev); + mutex_unlock(&dev->struct_mutex); + + BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0); + atomic_dec(&to_intel_crtc(work->crtc)->unpin_work_count); + kfree(work); } @@ -5950,7 +5955,8 @@ static void do_intel_finish_page_flip(struct drm_device *dev, &obj->pending_flip.counter); wake_up(&dev_priv->pending_flip_queue); - schedule_work(&work->work); + + queue_work(dev_priv->wq, &work->work); trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj); } @@ -6244,7 +6250,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, return -ENOMEM; work->event = event; - work->dev = crtc->dev; + work->crtc = crtc; intel_fb = to_intel_framebuffer(crtc->fb); work->old_fb_obj = intel_fb->obj; INIT_WORK(&work->work, intel_unpin_work_fn); @@ -6269,6 +6275,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, intel_fb = to_intel_framebuffer(fb); obj = intel_fb->obj; + if (atomic_read(&intel_crtc->unpin_work_count) >= 2) + flush_workqueue(dev_priv->wq); + mutex_lock(&dev->struct_mutex); /* Reference the objects for the scheduled work. */ @@ -6285,6 +6294,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, * the flip occurs and the object is no longer visible. */ atomic_add(1 << intel_crtc->plane, &work->old_fb_obj->pending_flip); + atomic_inc(&intel_crtc->unpin_work_count); ret = dev_priv->display.queue_flip(dev, crtc, fb, obj); if (ret) @@ -6299,6 +6309,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, return 0; cleanup_pending: + atomic_dec(&intel_crtc->unpin_work_count); atomic_sub(1 << intel_crtc->plane, &work->old_fb_obj->pending_flip); drm_gem_object_unreference(&work->old_fb_obj->base); drm_gem_object_unreference(&obj->base); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 9040191..39c7ea8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -176,6 +176,8 @@ struct intel_crtc { struct intel_unpin_work *unpin_work; int fdi_lanes; + atomic_t unpin_work_count; + struct drm_i915_gem_object *cursor_bo; uint32_t cursor_addr; int16_t cursor_x, cursor_y; @@ -319,7 +321,7 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane) struct intel_unpin_work { struct work_struct work; - struct drm_device *dev; + struct drm_crtc *crtc; struct drm_i915_gem_object *old_fb_obj; struct drm_i915_gem_object *pending_flip_obj; struct drm_pending_vblank_event *event;