From patchwork Mon Apr 18 10:00:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 8869331 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 2CBB59F441 for ; Mon, 18 Apr 2016 10:00:29 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0D33C2013A for ; Mon, 18 Apr 2016 10:00:28 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id DD22E2010B for ; Mon, 18 Apr 2016 10:00:26 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 12E0E6E3B3; Mon, 18 Apr 2016 10:00:23 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTP id D6B926E3B3; Mon, 18 Apr 2016 10:00:21 +0000 (UTC) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 18 Apr 2016 03:00:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,501,1455004800"; d="scan'208";a="947431429" Received: from lcostell-mobl.ger.corp.intel.com (HELO patser.lan) ([10.252.7.207]) by fmsmga001.fm.intel.com with ESMTP; 18 Apr 2016 03:00:20 -0700 Subject: [PATCH v2.1 02/11] drm/i915: Remove stallcheck special handling, v2. To: Ander Conselvan De Oliveira , intel-gfx@lists.freedesktop.org References: <1460539104-15661-1-git-send-email-maarten.lankhorst@linux.intel.com> <1460539104-15661-3-git-send-email-maarten.lankhorst@linux.intel.com> <1460704033.6600.25.camel@gmail.com> <5714714C.3040803@linux.intel.com> <1460966275.3739.18.camel@gmail.com> From: Maarten Lankhorst Message-ID: <5714B033.4080001@linux.intel.com> Date: Mon, 18 Apr 2016 12:00:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1460966275.3739.18.camel@gmail.com> Cc: dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, 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 Both intel_unpin_work.pending and intel_unpin_work.enable_stall_check were used to see if work should be enabled. By only using pending some special cases are gone, and access to unpin_work can be simplified. Use this to only access work members untilintel_mark_page_flip_active is called, or intel_queue_mmio_flip is used. This will prevent use-after-free, and makes it easier to verify accesses. Changes since v1: - Reword commit message. - Do not access unpin_work after intel_mark_page_flip_active. - Add the right memory barriers. Signed-off-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 2d11b4948a74..99a461e34f60 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -582,9 +582,14 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, "No flip due on pipe %c (plane %c)\n", pipe, plane); } else { + u32 pending; u32 addr; - if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) { + pending = atomic_read(&work->pending); + if (pending == INTEL_FLIP_INACTIVE) { + seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n", + pipe, plane); + } else if (pending >= INTEL_FLIP_COMPLETE) { seq_printf(m, "Flip queued on pipe %c (plane %c)\n", pipe, plane); } else { @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) work->flip_queued_vblank, work->flip_ready_vblank, drm_crtc_vblank_count(&crtc->base)); - if (work->enable_stall_check) - seq_puts(m, "Stall check enabled, "); - else - seq_puts(m, "Stall check waiting for page flip ioctl, "); seq_printf(m, "%d prepares\n", atomic_read(&work->pending)); if (INTEL_INFO(dev)->gen >= 4) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a3908dda0ece..8061b329f974 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3879,8 +3879,6 @@ static void page_flip_completed(struct intel_crtc *intel_crtc) struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); struct intel_unpin_work *work = intel_crtc->unpin_work; - /* ensure that the unpin work is consistent wrt ->pending. */ - smp_rmb(); intel_crtc->unpin_work = NULL; if (work->event) @@ -10948,16 +10946,13 @@ static void do_intel_finish_page_flip(struct drm_device *dev, spin_lock_irqsave(&dev->event_lock, flags); work = intel_crtc->unpin_work; - /* Ensure we don't miss a work->pending update ... */ - smp_rmb(); + if (work && atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE) { + /* ensure that the unpin work is consistent wrt ->pending. */ + smp_mb__after_atomic(); - if (work == NULL || atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) { - spin_unlock_irqrestore(&dev->event_lock, flags); - return; + page_flip_completed(intel_crtc); } - page_flip_completed(intel_crtc); - spin_unlock_irqrestore(&dev->event_lock, flags); } @@ -11054,10 +11049,8 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane) static inline void intel_mark_page_flip_active(struct intel_unpin_work *work) { /* Ensure that the work item is consistent when activating it ... */ - smp_wmb(); + smp_mb__before_atomic(); atomic_set(&work->pending, INTEL_FLIP_PENDING); - /* and that it is marked active as soon as the irq could fire. */ - smp_wmb(); } static int intel_gen2_queue_flip(struct drm_device *dev, @@ -11091,7 +11084,6 @@ static int intel_gen2_queue_flip(struct drm_device *dev, intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset); intel_ring_emit(engine, 0); /* aux display base address, unused */ - intel_mark_page_flip_active(intel_crtc->unpin_work); return 0; } @@ -11123,7 +11115,6 @@ static int intel_gen3_queue_flip(struct drm_device *dev, intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset); intel_ring_emit(engine, MI_NOOP); - intel_mark_page_flip_active(intel_crtc->unpin_work); return 0; } @@ -11162,7 +11153,6 @@ static int intel_gen4_queue_flip(struct drm_device *dev, pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff; intel_ring_emit(engine, pf | pipesrc); - intel_mark_page_flip_active(intel_crtc->unpin_work); return 0; } @@ -11198,7 +11188,6 @@ static int intel_gen6_queue_flip(struct drm_device *dev, pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff; intel_ring_emit(engine, pf | pipesrc); - intel_mark_page_flip_active(intel_crtc->unpin_work); return 0; } @@ -11293,7 +11282,6 @@ static int intel_gen7_queue_flip(struct drm_device *dev, intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset); intel_ring_emit(engine, (MI_NOOP)); - intel_mark_page_flip_active(intel_crtc->unpin_work); return 0; } @@ -11420,8 +11408,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip) if (work == NULL) return; - intel_mark_page_flip_active(work); - intel_pipe_update_start(crtc); if (INTEL_INFO(mmio_flip->i915)->gen >= 9) @@ -11431,6 +11417,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip) ilk_do_mmio_flip(crtc, work); intel_pipe_update_end(crtc); + + intel_mark_page_flip_active(work); } static void intel_mmio_flip_work_func(struct work_struct *work) @@ -11497,15 +11485,14 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_unpin_work *work = intel_crtc->unpin_work; u32 addr; + u32 pending; - if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE) - return true; - - if (atomic_read(&work->pending) < INTEL_FLIP_PENDING) - return false; + pending = atomic_read(&work->pending); + /* ensure that the unpin work is consistent wrt ->pending. */ + smp_mb__after_atomic(); - if (!work->enable_stall_check) - return false; + if (pending != INTEL_FLIP_PENDING) + return pending == INTEL_FLIP_COMPLETE; if (work->flip_ready_vblank == 0) { if (work->flip_queued_req && @@ -11681,6 +11668,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, */ if (!mmio_flip) { ret = i915_gem_object_sync(obj, engine, &request); + if (!ret && !request) { + request = i915_gem_request_alloc(engine, NULL); + ret = PTR_ERR_OR_ZERO(request); + } + if (ret) goto cleanup_pending; } @@ -11694,36 +11686,29 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, work->gtt_offset += intel_crtc->dspaddr_offset; if (mmio_flip) { - ret = intel_queue_mmio_flip(dev, crtc, obj); - if (ret) - goto cleanup_unpin; + work->flip_queued_vblank = drm_crtc_vblank_count(crtc); i915_gem_request_assign(&work->flip_queued_req, obj->last_write_req); - } else { - if (!request) { - request = i915_gem_request_alloc(engine, NULL); - if (IS_ERR(request)) { - ret = PTR_ERR(request); - goto cleanup_unpin; - } - } + ret = intel_queue_mmio_flip(dev, crtc, obj); + if (ret) + goto cleanup_unpin; + } else { ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request, page_flip_flags); if (ret) goto cleanup_unpin; i915_gem_request_assign(&work->flip_queued_req, request); - } - if (request) - i915_add_request_no_flush(request); + work->flip_queued_vblank = drm_crtc_vblank_count(crtc); + intel_mark_page_flip_active(work); - work->flip_queued_vblank = drm_crtc_vblank_count(crtc); - work->enable_stall_check = true; + i915_add_request_no_flush(request); + } - i915_gem_track_fb(intel_fb_obj(work->old_fb), obj, + i915_gem_track_fb(intel_fb_obj(old_fb), obj, to_intel_plane(primary)->frontbuffer_bit); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index e0fcfa1683cc..ce2a6d985bbe 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -952,7 +952,6 @@ struct intel_unpin_work { struct drm_i915_gem_request *flip_queued_req; u32 flip_queued_vblank; u32 flip_ready_vblank; - bool enable_stall_check; }; struct intel_load_detect_pipe {