From patchwork Mon Feb 13 14:36:49 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robert Bragg X-Patchwork-Id: 9569873 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 DA1F860442 for ; Mon, 13 Feb 2017 14:37:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C03A424B44 for ; Mon, 13 Feb 2017 14:37:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B503E267EC; Mon, 13 Feb 2017 14:37:11 +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 4F93E24B44 for ; Mon, 13 Feb 2017 14:37:11 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7D7BB6E408; Mon, 13 Feb 2017 14:37:10 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wr0-x242.google.com (mail-wr0-x242.google.com [IPv6:2a00:1450:400c:c0c::242]) by gabe.freedesktop.org (Postfix) with ESMTPS id A26416E408 for ; Mon, 13 Feb 2017 14:37:09 +0000 (UTC) Received: by mail-wr0-x242.google.com with SMTP id k90so24360978wrc.3 for ; Mon, 13 Feb 2017 06:37:09 -0800 (PST) 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=WhoZpxVxfULkBy6VGcrOpKks95oTfTi+cdDuF4zBV1A=; b=n1u87/OWogCXWVGTkkZJE63F8WnozboNC5wVzGrt3W/leVc1/DwZ+9FDhqIm5MFNkd xHWgiXGl2YJg9dyvgYqAcACIrofDr3S6LoQ1B7vQY8XI8U7jwu4+W2IT1Ym1GBA1AZhA vmKQRUq7RJU2DWEd94xYgvHExzOPuQp+a5HE0tM/NlxoER6hLxtAd/429w6dZBKE11v6 t2UzUBv4jvRZJajgRNNW+28HNlloRitPI8dCpa4rid1JgS5FmSNuuSLTdpH5SqJMxl0o BiPMnMxbvR06O1GTGBR24MCmyajbmQmko4EMuswFs34BSoyDxnO4Ui7bSt0RTG4XOelp V/0w== 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=WhoZpxVxfULkBy6VGcrOpKks95oTfTi+cdDuF4zBV1A=; b=cqKAZL5hREohpVoxhrWTp1LgoGdGkZZlsyIo4PFAWenAMVQHuuSIUttoQtdWkwK2Et 94Cq59nQ+fuZxwV8WugSuU/vUntz10lil/A5naqZc5OG9xnPRzO0vzFgjEIPHC/y08Hf RqKEIg4NlFci/hC+LhgIAkUpX1w81KbtR92raZHO+OZYM6fpVobPsASbCqBBHCBzVXLI ssmd9etJT0k32iR/PdegvmzO/IY29bq5g5UyA2zng3mDG3lLB4e+Q+acA13bznyuvpMc PvMofn6QH1IDHLq/M17/LC1wF8YriKbFAAv9rh1K5KaCAT7V3BlpDpWkKe2+jrhK7i9L hzsw== X-Gm-Message-State: AMke39nBEw16wCv4J3Iw0WrDfCceFhIG0PHyJkHJMJwprCtNFeY5MP5ER5Lpar0mUrvuBg== X-Received: by 10.223.150.34 with SMTP id b31mr20740893wra.196.1486996628069; Mon, 13 Feb 2017 06:37:08 -0800 (PST) Received: from sixbynine.org (host-78-151-19-238.as13285.net. [78.151.19.238]) by smtp.gmail.com with ESMTPSA id y80sm14238504wrb.12.2017.02.13.06.37.07 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 13 Feb 2017 06:37:07 -0800 (PST) From: Robert Bragg To: intel-gfx@lists.freedesktop.org Date: Mon, 13 Feb 2017 14:36:49 +0000 Message-Id: <20170213143651.4737-4-robert@sixbynine.org> X-Mailer: git-send-email 2.11.1 In-Reply-To: <20170213143651.4737-1-robert@sixbynine.org> References: <20170213143651.4737-1-robert@sixbynine.org> Subject: [Intel-gfx] [PATCH v2 3/5] drm/i915/perf: avoid read back of head register 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 There's no need for the driver to keep reading back the head pointer from hardware since the hardware doesn't update it automatically. This way we can treat any invalid head pointer value as a software/driver bug instead of spurious hardware behaviour. This change is also a small stepping stone towards re-working how the head and tail state is managed as part of an improved workaround for the tail register race condition. Signed-off-by: Robert Bragg Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/i915_drv.h | 11 ++++++++++ drivers/gpu/drm/i915/i915_perf.c | 46 ++++++++++++++++++---------------------- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b5f150b0870d..b990549cfe65 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2428,6 +2428,17 @@ struct drm_i915_private { u8 *vaddr; int format; int format_size; + + /** + * Although we can always read back the head + * pointer register, we prefer to avoid + * trusting the HW state, just to avoid any + * risk that some hardware condition could + * somehow bump the head pointer unpredictably + * and cause us to forward the wrong OA buffer + * data to userspace. + */ + u32 head; } oa_buffer; u32 gen7_latched_oastatus1; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 4bb7333dac45..e85583d0bcff 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -322,9 +322,8 @@ struct perf_open_properties { static bool gen7_oa_buffer_is_empty_fop_unlocked(struct drm_i915_private *dev_priv) { int report_size = dev_priv->perf.oa.oa_buffer.format_size; - u32 oastatus2 = I915_READ(GEN7_OASTATUS2); u32 oastatus1 = I915_READ(GEN7_OASTATUS1); - u32 head = oastatus2 & GEN7_OASTATUS2_HEAD_MASK; + u32 head = dev_priv->perf.oa.oa_buffer.head; u32 tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK; return OA_TAKEN(tail, head) < @@ -458,16 +457,24 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, return -EIO; head = *head_ptr - 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 + * be incremented by multiples of the report size (notably also + * all a power of two). + */ + if (WARN_ONCE(head > OA_BUFFER_SIZE || head % report_size, + "Inconsistent OA buffer head pointer = %u\n", head)) + return -EIO; + tail -= gtt_offset; /* The OA unit is expected to wrap the tail pointer according to the OA - * buffer size and since we should never write a misaligned head - * pointer we don't expect to read one back either... + * buffer size */ - if (tail > OA_BUFFER_SIZE || head > OA_BUFFER_SIZE || - head % report_size) { - DRM_ERROR("Inconsistent OA buffer pointer (head = %u, tail = %u): force restart\n", - head, tail); + if (tail > OA_BUFFER_SIZE) { + DRM_ERROR("Inconsistent OA buffer tail pointer = %u: force restart\n", + 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) & @@ -562,8 +569,6 @@ static int gen7_oa_read(struct i915_perf_stream *stream, size_t *offset) { struct drm_i915_private *dev_priv = stream->dev_priv; - int report_size = dev_priv->perf.oa.oa_buffer.format_size; - u32 oastatus2; u32 oastatus1; u32 head; u32 tail; @@ -572,10 +577,9 @@ static int gen7_oa_read(struct i915_perf_stream *stream, if (WARN_ON(!dev_priv->perf.oa.oa_buffer.vaddr)) return -EIO; - oastatus2 = I915_READ(GEN7_OASTATUS2); oastatus1 = I915_READ(GEN7_OASTATUS1); - head = oastatus2 & GEN7_OASTATUS2_HEAD_MASK; + 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 @@ -616,10 +620,9 @@ static int gen7_oa_read(struct i915_perf_stream *stream, dev_priv->perf.oa.ops.oa_disable(dev_priv); dev_priv->perf.oa.ops.oa_enable(dev_priv); - oastatus2 = I915_READ(GEN7_OASTATUS2); oastatus1 = I915_READ(GEN7_OASTATUS1); - head = oastatus2 & GEN7_OASTATUS2_HEAD_MASK; + head = dev_priv->perf.oa.oa_buffer.head; tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK; } @@ -635,17 +638,6 @@ static int gen7_oa_read(struct i915_perf_stream *stream, ret = gen7_append_oa_reports(stream, buf, count, offset, &head, tail); - /* All the report sizes are a power of two and the - * head should always be incremented by some multiple - * of the report size. - * - * A warning here, but notably if we later read back a - * misaligned pointer we will treat that as a bug since - * it could lead to a buffer overrun. - */ - WARN_ONCE(head & (report_size - 1), - "i915: Writing misaligned OA head pointer"); - /* 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. @@ -653,6 +645,7 @@ static int gen7_oa_read(struct i915_perf_stream *stream, I915_WRITE(GEN7_OASTATUS2, ((head & GEN7_OASTATUS2_HEAD_MASK) | OA_MEM_SELECT_GGTT)); + dev_priv->perf.oa.oa_buffer.head = head; return ret; } @@ -834,7 +827,10 @@ static void gen7_init_oa_buffer(struct drm_i915_private *dev_priv) * before OASTATUS1, but after OASTATUS2 */ I915_WRITE(GEN7_OASTATUS2, gtt_offset | OA_MEM_SELECT_GGTT); /* head */ + dev_priv->perf.oa.oa_buffer.head = gtt_offset; + I915_WRITE(GEN7_OABUFFER, gtt_offset); + I915_WRITE(GEN7_OASTATUS1, gtt_offset | OABUFFER_SIZE_16M); /* tail */ /* On Haswell we have to track which OASTATUS1 flags we've