From patchwork Thu Oct 22 18:09:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 7466531 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 978E0BEEA4 for ; Thu, 22 Oct 2015 18:09:38 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8BF752095F for ; Thu, 22 Oct 2015 18:09:37 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 4F6AB2094C for ; Thu, 22 Oct 2015 18:09:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3FA596E5D0; Thu, 22 Oct 2015 11:09:35 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id BC76F6E5D0 for ; Thu, 22 Oct 2015 11:09:34 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 22 Oct 2015 11:09:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,183,1444719600"; d="scan'208";a="832794261" Received: from dsgordon-linux2.isw.intel.com ([10.102.226.88]) by orsmga002.jf.intel.com with ESMTP; 22 Oct 2015 11:09:12 -0700 From: Dave Gordon To: intel-gfx@lists.freedesktop.org Date: Thu, 22 Oct 2015 19:09:03 +0100 Message-Id: <1445537343-10587-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] 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-Spam-Status: No, score=-4.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 '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), a simpler solution is just to give up trying to track it at all, and always prefix each render batch with the single instruction needed to explicitly set it to the required mode for the current batch. 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. 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 | 2 -- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 32 +++++++++++++----------------- drivers/gpu/drm/i915/intel_lrc.c | 20 ++++++++----------- 4 files changed, 22 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9a15ebe..9ddbb93 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1707,8 +1707,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 d0fa548..769982a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4947,8 +4947,6 @@ i915_gem_load(struct drm_device *dev) i915_gem_idle_work_handler); init_waitqueue_head(&dev_priv->gpu_error.reset_queue); - dev_priv->relative_constants_mode = I915_EXEC_CONSTANTS_REL_GENERAL; - if (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev)) dev_priv->num_fence_regs = 32; else if (INTEL_INFO(dev)->gen >= 4 || IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev)) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 6ed7d63a..07ae12d 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1211,30 +1211,28 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, return -EINVAL; } - if (instp_mode != dev_priv->relative_constants_mode) { - if (INTEL_INFO(dev)->gen < 4) { - DRM_DEBUG("no rel constants on pre-gen4\n"); - return -EINVAL; - } - - if (INTEL_INFO(dev)->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)->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)->gen >= 6) - instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; + if (INTEL_INFO(dev)->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)->gen >= 6) + instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; + break; default: DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode); return -EINVAL; } - if (ring == &dev_priv->ring[RCS] && - instp_mode != dev_priv->relative_constants_mode) { + if (ring == &dev_priv->ring[RCS]) { ret = intel_ring_begin(params->request, 4); if (ret) return ret; @@ -1244,8 +1242,6 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, intel_ring_emit(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) { diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 3265427..8b3fe8f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -889,15 +889,14 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, return -EINVAL; } - if (instp_mode != dev_priv->relative_constants_mode) { - if (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 */ - instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; + if (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 */ + instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; + break; default: DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode); @@ -913,8 +912,7 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, if (ret) return ret; - if (ring == &dev_priv->ring[RCS] && - instp_mode != dev_priv->relative_constants_mode) { + if (ring == &dev_priv->ring[RCS]) { ret = intel_logical_ring_begin(params->request, 4); if (ret) return ret; @@ -924,8 +922,6 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, intel_logical_ring_emit(ringbuf, INSTPM); intel_logical_ring_emit(ringbuf, instp_mask << 16 | instp_mode); intel_logical_ring_advance(ringbuf); - - dev_priv->relative_constants_mode = instp_mode; } exec_start = params->batch_obj_vm_offset +