From patchwork Thu Jun 26 17:24:13 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Harrison X-Patchwork-Id: 4429041 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 A13C39F319 for ; Thu, 26 Jun 2014 17:25:43 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D577E20377 for ; Thu, 26 Jun 2014 17:25:39 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 073DA20270 for ; Thu, 26 Jun 2014 17:25:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 52A626E1DB; Thu, 26 Jun 2014 10:25:38 -0700 (PDT) X-Original-To: Intel-GFX@lists.freedesktop.org Delivered-To: Intel-GFX@lists.freedesktop.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 8F2F86E1DB for ; Thu, 26 Jun 2014 10:25:33 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 26 Jun 2014 10:25:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.01,554,1400050800"; d="scan'208";a="561434726" Received: from johnharr-linux.iwi.intel.com ([172.28.253.52]) by fmsmga002.fm.intel.com with ESMTP; 26 Jun 2014 10:25:19 -0700 From: John.C.Harrison@Intel.com To: Intel-GFX@lists.freedesktop.org Date: Thu, 26 Jun 2014 18:24:13 +0100 Message-Id: <1403803475-16337-23-git-send-email-John.C.Harrison@Intel.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1403803475-16337-1-git-send-email-John.C.Harrison@Intel.com> References: <1403803475-16337-1-git-send-email-John.C.Harrison@Intel.com> Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ Subject: [Intel-gfx] [RFC 22/44] drm/i915: Ensure OLS & PLR are always in sync X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_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 From: John Harrison The new seqno alloction code pre-allocates a 'lazy' request structure and then tries to allocate the 'lazy' seqno. The seqno allocation can potential wrap around zero and when doing so, tries to idle the ring by waiting for all oustanding work to complete. With a scheduler in place, this can mean first submitting extra work to the ring. However, at this point in time, the lazy request is valid but the lazy seqno is not. Some existing code was getting confused by this state and Bad Things would happen. The safest solution is to still allocate the lazy request in advance (to avoid having to roll back in an out of memory sitation) but to save the pointer in a local variable rather than immediately updating the lazy pointer. Only after a valid seqno has been acquired is the lazy request pointer actually updated. This guarantees that both lazy values are either invalid or both valid. There can no longer be an inconsistent state. --- drivers/gpu/drm/i915/intel_ringbuffer.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 737c41b..1ef0cbd 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1665,20 +1665,31 @@ int intel_ring_idle(struct intel_engine_cs *ring) int intel_ring_alloc_seqno(struct intel_engine_cs *ring) { - if (ring->outstanding_lazy_seqno) + int ret; + struct drm_i915_gem_request *request; + + /* NB: Some code seems to test the OLS and other code tests the PLR. + * Therefore it is only safe if the two are kept in step. */ + + if (ring->outstanding_lazy_seqno) { + BUG_ON(ring->preallocated_lazy_request == NULL); return 0; + } - if (ring->preallocated_lazy_request == NULL) { - struct drm_i915_gem_request *request; + BUG_ON(ring->preallocated_lazy_request != NULL); - request = kmalloc(sizeof(*request), GFP_KERNEL); - if (request == NULL) - return -ENOMEM; + request = kmalloc(sizeof(*request), GFP_KERNEL); + if (request == NULL) + return -ENOMEM; - ring->preallocated_lazy_request = request; + ret = i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno); + if (ret) { + kfree(request); + return ret; } - return i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_seqno); + ring->preallocated_lazy_request = request; + return 0; } static int __intel_ring_prepare(struct intel_engine_cs *ring,