From patchwork Wed Apr 12 15:55:45 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robert Bragg X-Patchwork-Id: 9677675 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 A86A360383 for ; Wed, 12 Apr 2017 15:56:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9B3B62861B for ; Wed, 12 Apr 2017 15:56:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 901F42862B; Wed, 12 Apr 2017 15:56: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=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID 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 066282861B for ; Wed, 12 Apr 2017 15:56:19 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 88CCD89308; Wed, 12 Apr 2017 15:56:19 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-pg0-x244.google.com (mail-pg0-x244.google.com [IPv6:2607:f8b0:400e:c05::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5F8CF89306 for ; Wed, 12 Apr 2017 15:56:18 +0000 (UTC) Received: by mail-pg0-x244.google.com with SMTP id 34so3293182pgx.3 for ; Wed, 12 Apr 2017 08:56:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=PG5mi1ed2aD2e7OjO1yAxkp/PTtNzzs4NZkop+QRrIU=; b=XJOfzcrJrAaPgdxi7uZQo9uqHkc8NBz4rHzqllZH8NIUj0NB3i1dRY9YbCyzlY9B0n sEAT/HS2J4eiCgfQSL+8WRdlnGU5mgwV3yIyoQeerg5fXvXwOx43xl65ZXk8jQ4rVp2o 1Hr5C9zDcJnvT+95vSGl5ObtBsbAE65kb0CieKD5JTd7Fh8jeeZ9s8e+1FLUk2Wii+Fe gYxWJImFCK85hf4aSvDP8zrq3DNGR4p5cDJk19RZtuA4DW/oPnWfmct8hSKlyRe8/U/O 8O3jjTj/ndyxHWehjyX/hnKEA8b0L7JPfQLF97MuYvTBYw3PxYvCkFlyGGHWcVW4gGvf 0VQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=PG5mi1ed2aD2e7OjO1yAxkp/PTtNzzs4NZkop+QRrIU=; b=Kasj84uPAFwWwGp35KAI1JZvYmp5wao0YFKTuBuh9b1jT6ptuNUM5fIBVGbNsWP0pd 0c0KT6g0Y6IjO3L8gPsWWd6bvnz+Fg/Lc+l5Op+0cqYEOuWylEoXAL9qKrJv50+8HVyz TRxBAajF6E0NC/Y/HpWN4D8UOaf3k2LFfzcDYv7WPEc+O/OY3FaZen9SPi8SUE5LuCWx qz+KCRXLAjP9pLt/Iv4dSQbRHiJHXTiGqXxHw9UYetyG5BEjbaURhyd0FhcpeJ52LllX cJk2XqTqyN5krLKQtx0xXPS6HwUX0Kmp+L1z2r9ZuENN/aMRunW++adUstJ9J4ZA+oeP 4+pQ== X-Gm-Message-State: AFeK/H2zQDt9kScd2W+BZ8Qy/kWGIl2/PB0cizxAJGbgbdn1JfqyGi8Ef/2YkTja7HEOZw== X-Received: by 10.84.212.130 with SMTP id e2mr85304066pli.140.1492012577770; Wed, 12 Apr 2017 08:56:17 -0700 (PDT) Received: from sixbynine.org (fmdmzpr01-ext.fm.intel.com. [192.55.54.36]) by smtp.gmail.com with ESMTPSA id v143sm37453283pgb.57.2017.04.12.08.56.16 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 12 Apr 2017 08:56:17 -0700 (PDT) From: Robert Bragg To: intel-gfx@lists.freedesktop.org Date: Wed, 12 Apr 2017 16:55:45 +0100 Message-Id: <20170412155556.6602-5-robert@sixbynine.org> X-Mailer: git-send-email 2.12.0 In-Reply-To: <20170412155556.6602-1-robert@sixbynine.org> References: <20170412155556.6602-1-robert@sixbynine.org> Subject: [Intel-gfx] [PATCH v4 04/15] drm/i915/perf: no head/tail ref in gen7_oa_read 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 This avoids redundantly passing an (inout) head and tail pointer to gen7_append_oa_reports() from gen7_oa_read which doesn't need to reference either itself. Moving the head/tail reads and writes into gen7_append_oa_reports should have no functional effect except to avoid some redundant head pointer writes in cases where nothing was copied to userspace. This is a stepping stone towards updating how the head and tail pointer state is managed to improve the workaround for the OA unit's tail pointer race. It reduces the number of places we need to read/write the head and tail pointers. Signed-off-by: Robert Bragg Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/i915_perf.c | 51 +++++++++++++++------------------------- 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index f47d1cc2144b..83dc67a635fb 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -420,8 +420,6 @@ static int append_oa_sample(struct i915_perf_stream *stream, * @buf: destination buffer given by userspace * @count: the number of bytes userspace wants to read * @offset: (inout): the current position for writing into @buf - * @head_ptr: (inout): the current oa buffer cpu read position - * @tail: the current oa buffer gpu write position * * Notably any error condition resulting in a short read (-%ENOSPC or * -%EFAULT) will be returned even though one or more records may @@ -439,9 +437,7 @@ static int append_oa_sample(struct i915_perf_stream *stream, static int gen7_append_oa_reports(struct i915_perf_stream *stream, char __user *buf, size_t count, - size_t *offset, - u32 *head_ptr, - u32 tail) + size_t *offset) { struct drm_i915_private *dev_priv = stream->dev_priv; int report_size = dev_priv->perf.oa.oa_buffer.format_size; @@ -449,14 +445,15 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, int tail_margin = dev_priv->perf.oa.tail_margin; u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma); u32 mask = (OA_BUFFER_SIZE - 1); - u32 head; + size_t start_offset = *offset; + u32 head, oastatus1, tail; u32 taken; int ret = 0; if (WARN_ON(!stream->enabled)) return -EIO; - head = *head_ptr - gtt_offset; + head = dev_priv->perf.oa.oa_buffer.head - gtt_offset; /* An out of bounds or misaligned head pointer implies a driver bug * since we are in full control of head pointer which should only @@ -467,7 +464,8 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, "Inconsistent OA buffer head pointer = %u\n", head)) return -EIO; - tail -= gtt_offset; + oastatus1 = I915_READ(GEN7_OASTATUS1); + tail = (oastatus1 & GEN7_OASTATUS1_TAIL_MASK) - gtt_offset; /* The OA unit is expected to wrap the tail pointer according to the OA * buffer size @@ -477,8 +475,6 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, tail); dev_priv->perf.oa.ops.oa_disable(dev_priv); dev_priv->perf.oa.ops.oa_enable(dev_priv); - *head_ptr = I915_READ(GEN7_OASTATUS2) & - GEN7_OASTATUS2_HEAD_MASK; return -EIO; } @@ -542,7 +538,18 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, report32[0] = 0; } - *head_ptr = gtt_offset + head; + + if (start_offset != *offset) { + /* We removed the gtt_offset for the copy loop above, indexing + * relative to oa_buf_base so put back here... + */ + head += gtt_offset; + + I915_WRITE(GEN7_OASTATUS2, + ((head & GEN7_OASTATUS2_HEAD_MASK) | + OA_MEM_SELECT_GGTT)); + dev_priv->perf.oa.oa_buffer.head = head; + } return ret; } @@ -570,8 +577,6 @@ static int gen7_oa_read(struct i915_perf_stream *stream, { struct drm_i915_private *dev_priv = stream->dev_priv; u32 oastatus1; - u32 head; - u32 tail; int ret; if (WARN_ON(!dev_priv->perf.oa.oa_buffer.vaddr)) @@ -579,9 +584,6 @@ static int gen7_oa_read(struct i915_perf_stream *stream, oastatus1 = I915_READ(GEN7_OASTATUS1); - head = dev_priv->perf.oa.oa_buffer.head; - tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK; - /* XXX: On Haswell we don't have a safe way to clear oastatus1 * bits while the OA unit is enabled (while the tail pointer * may be updated asynchronously) so we ignore status bits @@ -621,9 +623,6 @@ static int gen7_oa_read(struct i915_perf_stream *stream, dev_priv->perf.oa.ops.oa_enable(dev_priv); oastatus1 = I915_READ(GEN7_OASTATUS1); - - head = dev_priv->perf.oa.oa_buffer.head; - tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK; } if (unlikely(oastatus1 & GEN7_OASTATUS1_REPORT_LOST)) { @@ -635,19 +634,7 @@ static int gen7_oa_read(struct i915_perf_stream *stream, GEN7_OASTATUS1_REPORT_LOST; } - ret = gen7_append_oa_reports(stream, buf, count, offset, - &head, tail); - - /* Note: we update the head pointer here even if an error - * was returned since the error may represent a short read - * where some some reports were successfully copied. - */ - I915_WRITE(GEN7_OASTATUS2, - ((head & GEN7_OASTATUS2_HEAD_MASK) | - OA_MEM_SELECT_GGTT)); - dev_priv->perf.oa.oa_buffer.head = head; - - return ret; + return gen7_append_oa_reports(stream, buf, count, offset); } /**