From patchwork Fri Aug 26 19:46:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 9301927 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id AACF2601C0 for ; Fri, 26 Aug 2016 19:46:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8E5F0295CD for ; Fri, 26 Aug 2016 19:46:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6BB2B2964F; Fri, 26 Aug 2016 19:46:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C8DAB295CD for ; Fri, 26 Aug 2016 19:46:38 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E166A6E215; Fri, 26 Aug 2016 19:46:37 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id CE10B6E215 for ; Fri, 26 Aug 2016 19:46:36 +0000 (UTC) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP; 26 Aug 2016 12:46:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,582,1464678000"; d="scan'208";a="161386109" Received: from dsgordon-linux2.isw.intel.com ([10.102.226.88]) by fmsmga004.fm.intel.com with ESMTP; 26 Aug 2016 12:46:34 -0700 From: Dave Gordon To: intel-gfx@lists.freedesktop.org Date: Fri, 26 Aug 2016 20:46:25 +0100 Message-Id: <1472240785-18235-1-git-send-email-david.s.gordon@intel.com> X-Mailer: git-send-email 1.9.1 Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ Subject: [Intel-gfx] [PATCH v2] drm/i915: don't track relative-constants-mode X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 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-Virus-Scanned: ClamAV using ClamSMTP 'relative_constants_mode' has always been tracked per-device, but this has actually been wrong ever since hardware contexts were introduced, as the INSTPM register is saved (and automatically restored) as part of the render ring context. The software per-device value could therefore get out of sync with the hardware per-context value. Rather than track this correctly (per-context in LRC modes, per-device in legacy ringbuffer mode), a simpler solution is just to give up trying to track it at all, and always surround each render batch that uses a non-default mode with the instructions to set it before and reset it to default after. Test case (if anyone wants to write it) would be to create two contexts, submit a batch with a non-default mode in the first context, submit a batch with the default mode in the other context, submit another batch in the first context, but this time in default mode. Without this patch, the driver will fail to insert the instructions to reset INSTPM into the first context's ringbuffer. v2: toggle mode on and off for non-default batches, rather than setting it explicitly for every batch, Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92448 Signed-off-by: Dave Gordon --- drivers/gpu/drm/i915/i915_drv.h | 2 - drivers/gpu/drm/i915/i915_gem.c | 3 -- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 70 ++++++++++++++++++------------ 3 files changed, 43 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9fdaf23..7938da0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1728,8 +1728,6 @@ struct drm_i915_private { const struct intel_device_info info; - int relative_constants_mode; - void __iomem *regs; struct intel_uncore uncore; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index d37b441..8c06778 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4615,9 +4615,6 @@ int i915_gem_init(struct drm_device *dev) i915_gem_idle_work_handler); init_waitqueue_head(&dev_priv->gpu_error.wait_queue); init_waitqueue_head(&dev_priv->gpu_error.reset_queue); - - dev_priv->relative_constants_mode = I915_EXEC_CONSTANTS_REL_GENERAL; - init_waitqueue_head(&dev_priv->pending_flip_queue); dev_priv->mm.interruptible = true; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 601156c..21578a5 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1432,6 +1432,25 @@ static void eb_export_fence(struct drm_i915_gem_object *obj, return vma; } +static inline int +emit_instpm(struct drm_i915_gem_request *request, u32 mask, int mode) +{ + struct intel_ring *ring = request->ring; + int ret; + + ret = intel_ring_begin(request, 4); + if (ret) + return ret; + + intel_ring_emit(ring, MI_NOOP); + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); + intel_ring_emit_reg(ring, INSTPM); + intel_ring_emit(ring, mask << 16 | mode); + intel_ring_advance(ring); + + return 0; +} + static int execbuf_submit(struct i915_execbuffer_params *params, struct drm_i915_gem_execbuffer2 *args, @@ -1462,43 +1481,32 @@ static void eb_export_fence(struct drm_i915_gem_object *obj, return -EINVAL; } - if (instp_mode != dev_priv->relative_constants_mode) { - if (INTEL_INFO(dev_priv)->gen < 4) { - DRM_DEBUG("no rel constants on pre-gen4\n"); - return -EINVAL; - } - - if (INTEL_INFO(dev_priv)->gen > 5 && - instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) { - DRM_DEBUG("rel surface constants mode invalid on gen5+\n"); - return -EINVAL; - } + if (INTEL_INFO(dev_priv)->gen < 4) { + DRM_DEBUG("no rel constants on pre-gen4\n"); + return -EINVAL; + } - /* The HW changed the meaning on this bit on gen6 */ - if (INTEL_INFO(dev_priv)->gen >= 6) - instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; + if (INTEL_INFO(dev_priv)->gen > 5 && + instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) { + DRM_DEBUG("rel surface constants mode invalid on gen5+\n"); + return -EINVAL; } + + /* The HW changed the meaning on this bit on gen6 */ + if (INTEL_INFO(dev_priv)->gen >= 6) + instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; + break; default: DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode); return -EINVAL; } - if (params->engine->id == RCS && - instp_mode != dev_priv->relative_constants_mode) { - struct intel_ring *ring = params->request->ring; - - ret = intel_ring_begin(params->request, 4); + if (instp_mode != I915_EXEC_CONSTANTS_REL_GENERAL) { + /* Set non-default value of INSTPM */ + ret = emit_instpm(params->request, instp_mask, instp_mode); if (ret) return ret; - - intel_ring_emit(ring, MI_NOOP); - intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); - intel_ring_emit_reg(ring, INSTPM); - intel_ring_emit(ring, instp_mask << 16 | instp_mode); - intel_ring_advance(ring); - - dev_priv->relative_constants_mode = instp_mode; } if (args->flags & I915_EXEC_GEN7_SOL_RESET) { @@ -1520,6 +1528,14 @@ static void eb_export_fence(struct drm_i915_gem_object *obj, if (ret) return ret; + if (instp_mode != I915_EXEC_CONSTANTS_REL_GENERAL) { + /* Restore default value of INSTPM */ + ret = emit_instpm(params->request, instp_mask, + I915_EXEC_CONSTANTS_REL_GENERAL); + if (ret) + return ret; + } + trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags); i915_gem_execbuffer_move_to_active(vmas, params->request);