From patchwork Fri Nov 16 13:55:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joonas Lahtinen X-Patchwork-Id: 10686399 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6069014D6 for ; Fri, 16 Nov 2018 13:55:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4F9482CD25 for ; Fri, 16 Nov 2018 13:55:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 437452CD59; Fri, 16 Nov 2018 13:55:20 +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 7DCBD2CD25 for ; Fri, 16 Nov 2018 13:55:19 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C8D0C6E704; Fri, 16 Nov 2018 13:55:17 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id D19AF6E704 for ; Fri, 16 Nov 2018 13:55:16 +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 orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Nov 2018 05:55:16 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,240,1539673200"; d="scan'208";a="96833682" Received: from jlahtine-desk.ger.corp.intel.com ([10.252.4.118]) by FMSMGA003.fm.intel.com with ESMTP; 16 Nov 2018 05:55:14 -0800 From: Joonas Lahtinen To: Intel graphics driver community testing & development Date: Fri, 16 Nov 2018 15:55:09 +0200 Message-Id: <20181116135510.13807-1-joonas.lahtinen@linux.intel.com> X-Mailer: git-send-email 2.17.2 Subject: [Intel-gfx] [PATCH 1/2] Revert "drm/i915/perf: add a parameter to control the size of OA buffer" 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: Matthew Auld MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Userspace portion is still missing. This reverts commit cd956bfcd0f58d20485ac0a785415f7d9327a95f. Cc: Lionel Landwerlin Cc: Matthew Auld Signed-off-by: Joonas Lahtinen Acked-by: Matthew Auld --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_perf.c | 99 +++++++++++--------------------- drivers/gpu/drm/i915/i915_reg.h | 2 - include/uapi/drm/i915_drm.h | 7 --- 4 files changed, 33 insertions(+), 76 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d69b71d368d3..017f851a586a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2004,7 +2004,6 @@ struct drm_i915_private { u32 last_ctx_id; int format; int format_size; - int size_exponent; /** * Locks reads and writes to all head/tail state diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 2c2b63be7a6c..c762418d3b01 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -212,7 +212,13 @@ #include "i915_oa_icl.h" #include "intel_lrc_reg.h" -#define OA_TAKEN(tail, head) (((tail) - (head)) & (dev_priv->perf.oa.oa_buffer.vma->size - 1)) +/* HW requires this to be a power of two, between 128k and 16M, though driver + * is currently generally designed assuming the largest 16M size is used such + * that the overflow cases are unlikely in normal operation. + */ +#define OA_BUFFER_SIZE SZ_16M + +#define OA_TAKEN(tail, head) ((tail - head) & (OA_BUFFER_SIZE - 1)) /** * DOC: OA Tail Pointer Race @@ -356,7 +362,6 @@ struct perf_open_properties { int oa_format; bool oa_periodic; int oa_period_exponent; - u32 oa_buffer_size_exponent; }; static void free_oa_config(struct drm_i915_private *dev_priv, @@ -519,7 +524,7 @@ static bool oa_buffer_check_unlocked(struct drm_i915_private *dev_priv) * could put the tail out of bounds... */ if (hw_tail >= gtt_offset && - hw_tail < (gtt_offset + dev_priv->perf.oa.oa_buffer.vma->size)) { + hw_tail < (gtt_offset + OA_BUFFER_SIZE)) { dev_priv->perf.oa.oa_buffer.tails[!aged_idx].offset = aging_tail = hw_tail; dev_priv->perf.oa.oa_buffer.aging_timestamp = now; @@ -648,7 +653,7 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream, int report_size = dev_priv->perf.oa.oa_buffer.format_size; u8 *oa_buf_base = dev_priv->perf.oa.oa_buffer.vaddr; u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma); - u32 mask = (dev_priv->perf.oa.oa_buffer.vma->size - 1); + u32 mask = (OA_BUFFER_SIZE - 1); size_t start_offset = *offset; unsigned long flags; unsigned int aged_tail_idx; @@ -688,8 +693,8 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream, * only be incremented by multiples of the report size (notably also * all a power of two). */ - if (WARN_ONCE(head > dev_priv->perf.oa.oa_buffer.vma->size || head % report_size || - tail > dev_priv->perf.oa.oa_buffer.vma->size || tail % report_size, + if (WARN_ONCE(head > OA_BUFFER_SIZE || head % report_size || + tail > OA_BUFFER_SIZE || tail % report_size, "Inconsistent OA buffer pointers: head = %u, tail = %u\n", head, tail)) return -EIO; @@ -712,7 +717,7 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream, * here would imply a driver bug that would result * in an overrun. */ - if (WARN_ON((dev_priv->perf.oa.oa_buffer.vma->size - head) < report_size)) { + if (WARN_ON((OA_BUFFER_SIZE - head) < report_size)) { DRM_ERROR("Spurious OA head ptr: non-integral report offset\n"); break; } @@ -871,6 +876,11 @@ static int gen8_oa_read(struct i915_perf_stream *stream, * automatically triggered reports in this condition and so we * have to assume that old reports are now being trampled * over. + * + * Considering how we don't currently give userspace control + * over the OA buffer size and always configure a large 16MB + * buffer, then a buffer overflow does anyway likely indicate + * that something has gone quite badly wrong. */ if (oastatus & GEN8_OASTATUS_OABUFFER_OVERFLOW) { ret = append_oa_status(stream, buf, count, offset, @@ -932,7 +942,7 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, int report_size = dev_priv->perf.oa.oa_buffer.format_size; u8 *oa_buf_base = dev_priv->perf.oa.oa_buffer.vaddr; u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma); - u32 mask = (dev_priv->perf.oa.oa_buffer.vma->size - 1); + u32 mask = (OA_BUFFER_SIZE - 1); size_t start_offset = *offset; unsigned long flags; unsigned int aged_tail_idx; @@ -969,8 +979,8 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, * only be incremented by multiples of the report size (notably also * all a power of two). */ - if (WARN_ONCE(head > dev_priv->perf.oa.oa_buffer.vma->size || head % report_size || - tail > dev_priv->perf.oa.oa_buffer.vma->size || tail % report_size, + if (WARN_ONCE(head > OA_BUFFER_SIZE || head % report_size || + tail > OA_BUFFER_SIZE || tail % report_size, "Inconsistent OA buffer pointers: head = %u, tail = %u\n", head, tail)) return -EIO; @@ -990,7 +1000,7 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, * here would imply a driver bug that would result * in an overrun. */ - if (WARN_ON((dev_priv->perf.oa.oa_buffer.vma->size - head) < report_size)) { + if (WARN_ON((OA_BUFFER_SIZE - head) < report_size)) { DRM_ERROR("Spurious OA head ptr: non-integral report offset\n"); break; } @@ -1385,9 +1395,7 @@ static void gen7_init_oa_buffer(struct drm_i915_private *dev_priv) I915_WRITE(GEN7_OABUFFER, gtt_offset); - I915_WRITE(GEN7_OASTATUS1, gtt_offset | - ((dev_priv->perf.oa.oa_buffer.size_exponent - 17) << - GEN7_OASTATUS1_BUFFER_SIZE_SHIFT)); /* tail */ + I915_WRITE(GEN7_OASTATUS1, gtt_offset | OABUFFER_SIZE_16M); /* tail */ /* Mark that we need updated tail pointers to read from... */ dev_priv->perf.oa.oa_buffer.tails[0].offset = INVALID_TAIL_PTR; @@ -1412,8 +1420,7 @@ static void gen7_init_oa_buffer(struct drm_i915_private *dev_priv) * the assumption that new reports are being written to zeroed * memory... */ - memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, - dev_priv->perf.oa.oa_buffer.vma->size); + memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, OA_BUFFER_SIZE); /* Maybe make ->pollin per-stream state if we support multiple * concurrent streams in the future. @@ -1443,9 +1450,7 @@ static void gen8_init_oa_buffer(struct drm_i915_private *dev_priv) * bit." */ I915_WRITE(GEN8_OABUFFER, gtt_offset | - ((dev_priv->perf.oa.oa_buffer.size_exponent - 17) << - GEN8_OABUFFER_BUFFER_SIZE_SHIFT) | - GEN8_OABUFFER_MEM_SELECT_GGTT); + OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT); I915_WRITE(GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); /* Mark that we need updated tail pointers to read from... */ @@ -1473,8 +1478,7 @@ static void gen8_init_oa_buffer(struct drm_i915_private *dev_priv) * the assumption that new reports are being written to zeroed * memory... */ - memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, - dev_priv->perf.oa.oa_buffer.vma->size); + memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, OA_BUFFER_SIZE); /* * Maybe make ->pollin per-stream state if we support multiple @@ -1483,24 +1487,23 @@ static void gen8_init_oa_buffer(struct drm_i915_private *dev_priv) dev_priv->perf.oa.pollin = false; } -static int alloc_oa_buffer(struct drm_i915_private *dev_priv, int size_exponent) +static int alloc_oa_buffer(struct drm_i915_private *dev_priv) { struct drm_i915_gem_object *bo; struct i915_vma *vma; - size_t size = 1U << size_exponent; int ret; if (WARN_ON(dev_priv->perf.oa.oa_buffer.vma)) return -ENODEV; - if (WARN_ON(size < SZ_128K || size > SZ_16M)) - return -EINVAL; - ret = i915_mutex_lock_interruptible(&dev_priv->drm); if (ret) return ret; - bo = i915_gem_object_create(dev_priv, size); + BUILD_BUG_ON_NOT_POWER_OF_2(OA_BUFFER_SIZE); + BUILD_BUG_ON(OA_BUFFER_SIZE < SZ_128K || OA_BUFFER_SIZE > SZ_16M); + + bo = i915_gem_object_create(dev_priv, OA_BUFFER_SIZE); if (IS_ERR(bo)) { DRM_ERROR("Failed to allocate OA buffer\n"); ret = PTR_ERR(bo); @@ -1518,7 +1521,6 @@ static int alloc_oa_buffer(struct drm_i915_private *dev_priv, int size_exponent) goto err_unref; } dev_priv->perf.oa.oa_buffer.vma = vma; - dev_priv->perf.oa.oa_buffer.size_exponent = size_exponent; dev_priv->perf.oa.oa_buffer.vaddr = i915_gem_object_pin_map(bo, I915_MAP_WB); @@ -1527,10 +1529,9 @@ static int alloc_oa_buffer(struct drm_i915_private *dev_priv, int size_exponent) goto err_unpin; } - DRM_DEBUG_DRIVER("OA Buffer initialized, gtt offset = 0x%x, vaddr = %p, size = %llu\n", + DRM_DEBUG_DRIVER("OA Buffer initialized, gtt offset = 0x%x, vaddr = %p\n", i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma), - dev_priv->perf.oa.oa_buffer.vaddr, - dev_priv->perf.oa.oa_buffer.vma->size); + dev_priv->perf.oa.oa_buffer.vaddr); goto unlock; @@ -2090,7 +2091,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, intel_runtime_pm_get(dev_priv); intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); - ret = alloc_oa_buffer(dev_priv, props->oa_buffer_size_exponent); + ret = alloc_oa_buffer(dev_priv); if (ret) goto err_oa_buf_alloc; @@ -2649,26 +2650,6 @@ static u64 oa_exponent_to_ns(struct drm_i915_private *dev_priv, int exponent) 1000ULL * INTEL_INFO(dev_priv)->cs_timestamp_frequency_khz); } -static int -select_oa_buffer_exponent(struct drm_i915_private *i915, - u64 requested_size) -{ - int order; - - /* - * When no size is specified, use the largest size supported by all - * generations. - */ - if (!requested_size) - return order_base_2(SZ_16M); - - order = order_base_2(clamp_t(u64, requested_size, SZ_128K, SZ_16M)); - if (requested_size != (1UL << order)) - return -EINVAL; - - return order; -} - /** * read_properties_unlocked - validate + copy userspace stream open properties * @dev_priv: i915 device instance @@ -2796,14 +2777,6 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, props->oa_periodic = true; props->oa_period_exponent = value; break; - case DRM_I915_PERF_PROP_OA_BUFFER_SIZE: - ret = select_oa_buffer_exponent(dev_priv, value); - if (ret < 0) { - DRM_DEBUG("OA buffer size invalid %llu\n", value); - return ret; - } - props->oa_buffer_size_exponent = ret; - break; case DRM_I915_PERF_PROP_MAX: MISSING_CASE(id); return -EINVAL; @@ -2812,12 +2785,6 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, uprop += 2; } - /* If no buffer size was requested, select the default one. */ - if (!props->oa_buffer_size_exponent) { - props->oa_buffer_size_exponent = - select_oa_buffer_exponent(dev_priv, 0); - } - return 0; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 94ba86018a4f..edb58af1e903 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -586,14 +586,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define GEN8_OABUFFER_UDW _MMIO(0x23b4) #define GEN8_OABUFFER _MMIO(0x2b14) #define GEN8_OABUFFER_MEM_SELECT_GGTT (1 << 0) /* 0: PPGTT, 1: GGTT */ -#define GEN8_OABUFFER_BUFFER_SIZE_SHIFT 3 #define GEN7_OASTATUS1 _MMIO(0x2364) #define GEN7_OASTATUS1_TAIL_MASK 0xffffffc0 #define GEN7_OASTATUS1_COUNTER_OVERFLOW (1 << 2) #define GEN7_OASTATUS1_OABUFFER_OVERFLOW (1 << 1) #define GEN7_OASTATUS1_REPORT_LOST (1 << 0) -#define GEN7_OASTATUS1_BUFFER_SIZE_SHIFT 3 #define GEN7_OASTATUS2 _MMIO(0x2368) #define GEN7_OASTATUS2_HEAD_MASK 0xffffffc0 diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index e477ef8c644e..298b2e197744 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1540,13 +1540,6 @@ enum drm_i915_perf_property_id { */ DRM_I915_PERF_PROP_OA_EXPONENT, - /** - * Specify a global OA buffer size to be allocated in bytes. The size - * specified must be supported by HW (currently supported sizes are - * powers of 2 ranging from 128Kb to 16Mb). - */ - DRM_I915_PERF_PROP_OA_BUFFER_SIZE, - DRM_I915_PERF_PROP_MAX /* non-ABI */ };