From patchwork Wed Apr 11 15:46:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Lis, Tomasz" X-Patchwork-Id: 10335733 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 90D7560134 for ; Wed, 11 Apr 2018 15:46:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 81A2F2675C for ; Wed, 11 Apr 2018 15:46:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 75DF2286DD; Wed, 11 Apr 2018 15:46:28 +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=-5.2 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, 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 BA1EA2675C for ; Wed, 11 Apr 2018 15:46:26 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1E1016E6B8; Wed, 11 Apr 2018 15:46:26 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id E47516E6B8 for ; Wed, 11 Apr 2018 15:46:24 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Apr 2018 08:46:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,436,1517904000"; d="scan'208";a="41223705" Received: from szara.igk.intel.com ([172.28.178.192]) by FMSMGA003.fm.intel.com with ESMTP; 11 Apr 2018 08:46:22 -0700 From: Tomasz Lis To: intel-gfx@lists.freedesktop.org Date: Wed, 11 Apr 2018 17:46:21 +0200 Message-Id: <1523461581-19652-1-git-send-email-tomasz.lis@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1521463055-5325-2-git-send-email-tomasz.lis@intel.com> References: <1521463055-5325-2-git-send-email-tomasz.lis@intel.com> Subject: [Intel-gfx] [PATCH v2] drm/i915: Add Exec param to control data port coherency. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: bartosz.dunajski@intel.com MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP The patch adds a parameter to control the data port coherency functionality on a per-exec call basis. When data port coherency flag value is different than what it was in previous call for the context, a command to switch data port coherency state is added before the buffer to be executed. Rationale: The OpenCL driver develpers requested a functionality to control cache coherency at data port level. Keeping the coherency at that level is disabled by default due to its performance costs. OpenCL driver is planning to enable it for a small subset of submissions, when such functionality is required. Below are answers to basic question explaining background of the functionality and reasoning for the proposed implementation: 1. Why do we need a coherency enable/disable switch for memory that is shared between CPU and GEN (GPU)? Memory coherency between CPU and GEN, while being a great feature that enables CL_MEM_SVM_FINE_GRAIN_BUFFER OCL capability on Intel GEN architecture, adds overhead related to tracking (snooping) memory inside different cache units (L1$, L2$, L3$, LLC$, etc.). At the same time, minority of modern OCL applications actually use CL_MEM_SVM_FINE_GRAIN_BUFFER (and hence require memory coherency between CPU and GPU). The goal of coherency enable/disable switch is to remove overhead of memory coherency when memory coherency is not needed. 2. Why do we need a global coherency switch? In order to support I/O commands from within EUs (Execution Units), Intel GEN ISA (GEN Instruction Set Assembly) contains dedicated "send" instructions. These send instructions provide several addressing models. One of these addressing models (named "stateless") provides most flexible I/O using plain virtual addresses (as opposed to buffer_handle+offset models). This "stateless" model is similar to regular memory load/store operations available on typical CPUs. Since this model provides I/O using arbitrary virtual addresses, it enables algorithmic designs that are based on pointer-to-pointer (e.g. buffer of pointers) concepts. For instance, it allows creating tree-like data structures such as: ________________ | NODE1 | | uint64_t data | +----------------| | NODE* | NODE*| +--------+-------+ / \ ________________/ \________________ | NODE2 | | NODE3 | | uint64_t data | | uint64_t data | +----------------| +----------------| | NODE* | NODE*| | NODE* | NODE*| +--------+-------+ +--------+-------+ Please note that pointers inside such structures can point to memory locations in different OCL allocations - e.g. NODE1 and NODE2 can reside in one OCL allocation while NODE3 resides in a completely separate OCL allocation. Additionally, such pointers can be shared with CPU (i.e. using SVM - Shared Virtual Memory feature). Using pointers from different allocations doesn't affect the stateless addressing model which even allows scattered reading from different allocations at the same time (i.e. by utilizing SIMD-nature of send instructions). When it comes to coherency programming, send instructions in stateless model can be encoded (at ISA level) to either use or disable coherency. However, for generic OCL applications (such as example with tree-like data structure), OCL compiler is not able to determine origin of memory pointed to by an arbitrary pointer - i.e. is not able to track given pointer back to a specific allocation. As such, it's not able to decide whether coherency is needed or not for specific pointer (or for specific I/O instruction). As a result, compiler encodes all stateless sends as coherent (doing otherwise would lead to functional issues resulting from data corruption). Please note that it would be possible to workaround this (e.g. based on allocations map and pointer bounds checking prior to each I/O instruction) but the performance cost of such workaround would be many times greater than the cost of keeping coherency always enabled. As such, enabling/disabling memory coherency at GEN ISA level is not feasible and alternative method is needed. Such alternative solution is to have a global coherency switch that allows disabling coherency for single (though entire) GPU submission. This is beneficial because this way we: * can enable (and pay for) coherency only in submissions that actually need coherency (submissions that use CL_MEM_SVM_FINE_GRAIN_BUFFER resources) * don't care about coherency at GEN ISA granularity (no performance impact) 3. Will coherency switch be used frequently? There are scenarios that will require frequent toggling of the coherency switch. E.g. an application has two OCL compute kernels: kern_master and kern_worker. kern_master uses, concurrently with CPU, some fine grain SVM resources (CL_MEM_SVM_FINE_GRAIN_BUFFER). These resources contain descriptors of computational work that needs to be executed. kern_master analyzes incoming work descriptors and populates a plain OCL buffer (non-fine-grain) with payload for kern_worker. Once kern_master is done, kern_worker kicks-in and processes the payload that kern_master produced. These two kernels work in a loop, one after another. Since only kern_master requires coherency, kern_worker should not be forced to pay for it. This means that we need to have the ability to toggle coherency switch on or off per each GPU submission: (ENABLE COHERENCY) kern_master -> (DISABLE COHERENCY)kern_worker -> (ENABLE COHERENCY) kern_master -> (DISABLE COHERENCY)kern_worker -> ... 4. Why the execlist flag approach was chosen? There are two other ways of providing the functionality to UMDs, besides the execlist flag: a) Chicken bit register whitelisting. This approach would allow adding the functionality without any change to KMDs interface. Also, it has been determined that whitelisting is safe for gen10 and gen11. The issue is with gen9, where hardware whitelisting cannot be used, and OCL driver needs support for it. A workaround there would be to use command parser, which verifies buffers before execution. But such parsing comes at a considerable performance cost. b) Providing the flag as context IOCTL setting. The data port coherency switch could be implemented as a context parameter, which would schedule submission of a buffer to switch the coherency flag. That is an elegant solution with bounds the flag to context, which matches the hardware placement of the feature. This solution was not accepted because of OCL driver performance concerns. The OCL driver is constructed with emphasis on creating small, but very frequent submissions. With such architecture, adding IOCTL setparam call before submission has considerable impact on the performance. v2: Fixed compilation warning. Bspec: 11419 Signed-off-by: Tomasz Lis --- drivers/gpu/drm/i915/i915_drv.c | 3 ++ drivers/gpu/drm/i915/i915_gem_context.h | 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 18 ++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 53 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lrc.h | 3 ++ include/uapi/drm/i915_drm.h | 12 ++++++- 6 files changed, 89 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f770be1..19493b0 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -436,6 +436,9 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data, case I915_PARAM_CS_TIMESTAMP_FREQUENCY: value = 1000 * INTEL_INFO(dev_priv)->cs_timestamp_frequency_khz; break; + case I915_PARAM_HAS_EXEC_DATA_PORT_COHERENCY: + value = (INTEL_GEN(dev_priv) >= 9); + break; default: DRM_DEBUG("Unknown parameter %d\n", param->param); return -EINVAL; diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 7854262..00aa309 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -118,6 +118,7 @@ struct i915_gem_context { #define CONTEXT_BANNABLE 3 #define CONTEXT_BANNED 4 #define CONTEXT_FORCE_SINGLE_SUBMISSION 5 +#define CONTEXT_DATA_PORT_COHERENT 6 /** * @hw_id: - unique identifier for the context diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index c74f5df..ada376c 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2274,6 +2274,18 @@ i915_gem_do_execbuffer(struct drm_device *dev, eb.batch_flags |= I915_DISPATCH_RS; } + if (args->flags & I915_EXEC_DATA_PORT_COHERENT) { + if (INTEL_GEN(eb.i915) < 9) { + DRM_DEBUG("Data Port Coherency is only allowed for Gen9 and above\n"); + return -EINVAL; + } + if (eb.engine->class != RENDER_CLASS) { + DRM_DEBUG("Data Port Coherency is not available on %s\n", + eb.engine->name); + return -EINVAL; + } + } + if (args->flags & I915_EXEC_FENCE_IN) { in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2)); if (!in_fence) @@ -2400,6 +2412,12 @@ i915_gem_do_execbuffer(struct drm_device *dev, goto err_batch_unpin; } + /* Emit the switch of data port coherency state if needed */ + err = intel_lr_context_modify_data_port_coherency(eb.request, + (args->flags & I915_EXEC_DATA_PORT_COHERENT) != 0); + if (GEM_WARN_ON(err)) + DRM_DEBUG("Data Port Coherency toggle failed, keeping old setting.\n"); + if (in_fence) { err = i915_request_await_dma_fence(eb.request, in_fence); if (err < 0) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 02b25bf..b25df7e 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -255,6 +255,59 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx, ce->lrc_desc = desc; } +static int emit_set_data_port_coherency(struct i915_request *req, bool enable) +{ + u32 *cs; + i915_reg_t reg; + + GEM_BUG_ON(req->engine->class != RENDER_CLASS); + GEM_BUG_ON(INTEL_GEN(req->i915) < 9); + + cs = intel_ring_begin(req, 4); + if (IS_ERR(cs)) + return PTR_ERR(cs); + + if (INTEL_GEN(req->i915) >= 10) + reg = CNL_HDC_CHICKEN0; + else + reg = HDC_CHICKEN0; + + *cs++ = MI_LOAD_REGISTER_IMM(1); + *cs++ = i915_mmio_reg_offset(reg); + /* Enabling coherency means disabling the bit which forces it off */ + if (enable) + *cs++ = _MASKED_BIT_DISABLE(HDC_FORCE_NON_COHERENT); + else + *cs++ = _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT); + *cs++ = MI_NOOP; + + intel_ring_advance(req, cs); + + return 0; +} + +int +intel_lr_context_modify_data_port_coherency(struct i915_request *req, + bool enable) +{ + struct i915_gem_context *ctx = req->ctx; + int ret; + + if (test_bit(CONTEXT_DATA_PORT_COHERENT, &ctx->flags) == enable) + return 0; + + ret = emit_set_data_port_coherency(req, enable); + + if (!ret) { + if (enable) + __set_bit(CONTEXT_DATA_PORT_COHERENT, &ctx->flags); + else + __clear_bit(CONTEXT_DATA_PORT_COHERENT, &ctx->flags); + } + + return ret; +} + static struct i915_priolist * lookup_priolist(struct intel_engine_cs *engine, struct i915_priotree *pt, diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 59d7b86..c46b239 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -111,4 +111,7 @@ intel_lr_context_descriptor(struct i915_gem_context *ctx, return ctx->engine[engine->id].lrc_desc; } +int intel_lr_context_modify_data_port_coherency(struct i915_request *req, + bool enable); + #endif /* _INTEL_LRC_H_ */ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 7f5634c..0f52793 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -529,6 +529,11 @@ typedef struct drm_i915_irq_wait { */ #define I915_PARAM_CS_TIMESTAMP_FREQUENCY 51 +/* Query whether DRM_I915_GEM_EXECBUFFER2 supports the ability to switch + * Data Cache access into Data Port Coherency mode. + */ +#define I915_PARAM_HAS_EXEC_DATA_PORT_COHERENCY 52 + typedef struct drm_i915_getparam { __s32 param; /* @@ -1048,7 +1053,12 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_FENCE_ARRAY (1<<19) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_ARRAY<<1)) +/* Data Port Coherency capability will be switched before an exec call + * which has this flag different than previous call for the context. + */ +#define I915_EXEC_DATA_PORT_COHERENT (1<<20) + +#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_DATA_PORT_COHERENT<<1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \