From patchwork Thu Oct 19 11:16:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 10016619 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 46976602C8 for ; Thu, 19 Oct 2017 12:34:36 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4917F28BFD for ; Thu, 19 Oct 2017 12:34:36 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3E0D928C12; Thu, 19 Oct 2017 12:34:36 +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_ADSP_CUSTOM_MED, DKIM_SIGNED,FREEMAIL_FROM,RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=unavailable 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 BED4628BFD for ; Thu, 19 Oct 2017 12:34:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 906016EAA9; Thu, 19 Oct 2017 12:34:05 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id B7BF66EA83; Thu, 19 Oct 2017 11:16:31 +0000 (UTC) Received: by mail-wm0-x241.google.com with SMTP id i124so15170153wmf.3; Thu, 19 Oct 2017 04:16:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=sKSwwLdUHopZrN8AFXnAc6LMJmtCTSnXFxS+5Y8nBcg=; b=eoUU3EC74sq3baafMFZMPS7MUO1OVM1pBTmLSijFGdf/EkZF9dQxmBnlZdjGpHqrcP 8KvK9z+bSLIK1fgeSpXScSnwM1A0osdlEHQR6IpyvoAPmCsK5BL2D1oTZm4pHYWfwlkh HuCP/6g0UbQJJ4rKxrPe3p/bLtW5fopm8qI/CaxTI52X86W679K32NrHzkdKCDld1i0Y d3k9yW7q7aH5XcJHqJX6zh66YQP93UwJizNSDXuSrth8C2g0tb9D/2U4I3mZnz8OUmBs obkL6f0nlckc8w0aC6E0JeFOr8d3iWS04GSNdJXBV6LjNB/OEqVqoAO6YwNoJMbmkGzS UCcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=sKSwwLdUHopZrN8AFXnAc6LMJmtCTSnXFxS+5Y8nBcg=; b=px/cw7Izg/2biaYfD5Do72eUM4mW+qg4eREhZb4dCF31mQGTdTeWdNPix2RglQPR36 ELmT5ilRu1P5sGfV6CJ+c5hE7WrSBEhFynQnUThOSii7/Qe5v2B7/yVh0AsiK3x/DpEl KJzs8VrN+tzgd2xZ+fiX/2QIdHoABI/Nj7eQBBeg7EoB+jejdeuPlRkDr4/H/30RHepV UboTGiXyEMQLUOw2EjAsQnlcEzxh3rIh9iWbhgFVPisVZRvwi2wAxKd7p8Op9iUEtV9j VgCWKZw/HQJeaI7b17B6FrfZVOb6BALrSVjvhTfhDf5YWk/q+P7On1R7T3zsUn5Vdqz6 WW9w== X-Gm-Message-State: AMCzsaV94kYmXgP1I3dxYeWJZU7jV9N257fbRzqS1/l+XuU6lWne4Jq5 iGxbGJ+4IVTq9skfRjla8nM= X-Google-Smtp-Source: ABhQp+SrJ88stFHEokybe+9QDSOj5aGwHgIw7P8JW1FJJDMszDmxB3OkYl4QNiSHrGF9TRnQzZAmrQ== X-Received: by 10.28.157.139 with SMTP id g133mr1342752wme.102.1508411790335; Thu, 19 Oct 2017 04:16:30 -0700 (PDT) Received: from shalem.localdomain.com (546A5441.cm-12-3b.dynamic.ziggo.nl. [84.106.84.65]) by smtp.gmail.com with ESMTPSA id h8sm1379762wme.30.2017.10.19.04.16.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 19 Oct 2017 04:16:29 -0700 (PDT) From: Hans de Goede X-Google-Original-From: Hans de Goede To: Daniel Vetter , Jani Nikula , =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= , Imre Deak , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" Subject: [PATCH v5 2/2] drm/i915: Acquire PUNIT->PMIC bus for intel_uncore_forcewake_reset() Date: Thu, 19 Oct 2017 13:16:20 +0200 Message-Id: <20171019111620.26761-3-hdegoede@redhat.com> X-Mailer: git-send-email 2.14.2 In-Reply-To: <20171019111620.26761-1-hdegoede@redhat.com> References: <20171019111620.26761-1-hdegoede@redhat.com> X-Mailman-Approved-At: Thu, 19 Oct 2017 12:33:49 +0000 Cc: Hans de Goede , intel-gfx , x86@kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP intel_uncore_forcewake_reset() does forcewake puts and gets as such we need to make sure that no-one tries to access the PUNIT->PMIC bus (on systems where this bus is shared) while it runs, otherwise bad things happen. Normally this is taken care of by the i915_pmic_bus_access_notifier() which does an intel_uncore_forcewake_get(FORCEWAKE_ALL) when some other driver tries to access the PMIC bus, so that later forcewake gets are no-ops (for the duration of the bus access). But intel_uncore_forcewake_reset gets called in 3 cases: 1) Before registering the pmic_bus_access_notifier 2) After unregistering the pmic_bus_access_notifier 3) To reset forcewake state on a GPU reset In all 3 cases the i915_pmic_bus_access_notifier() protection is insufficient. This commit fixes this race by calling iosf_mbi_punit_acquire() before calling intel_uncore_forcewake_reset(). In the case where it is called directly after unregistering the pmic_bus_access_notifier, we need to hold the punit-lock over both calls to avoid a race where intel_uncore_fw_release_timer() may execute between the 2 calls. Signed-off-by: Hans de Goede Reviewed-by: Imre Deak --- Changes in v2: -Rebase on current (July 6th 2017) drm-next Changes in v3: -Keep punit acquired / locked over the unregister + forcewake_reset call combo to avoid a race hitting between the 2 calls -This requires modifying iosf_mbi_unregister_pmic_bus_access_notifier to not take the lock itself, since we are the only users this is done in this same commit Changes in v4: -Fix missing rename in doc-comment -Add and use iosf_mbi_assert_punit_acquired() helper -Add missing acquire surrounding intel_uncore_forcewake_reset() inside intel_uncore_check_forcewake_domains() -Add Imre's Reviewed-by Changes in v5: -Separate out arch/x86 iosf_mbi changes into a separate patch --- drivers/gpu/drm/i915/intel_uncore.c | 17 +++++++++++++---- drivers/gpu/drm/i915/selftests/intel_uncore.c | 3 +++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 8c2ce81f01c2..0da81faf3981 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -229,6 +229,7 @@ intel_uncore_fw_release_timer(struct hrtimer *timer) return HRTIMER_NORESTART; } +/* Note callers must have acquired the PUNIT->PMIC bus, before calling this. */ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, bool restore) { @@ -237,6 +238,8 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, int retry_count = 100; enum forcewake_domains fw, active_domains; + iosf_mbi_assert_punit_acquired(); + /* Hold uncore.lock across reset to prevent any register access * with forcewake not set correctly. Wait until all pending * timers are run before holding. @@ -416,14 +419,18 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, GT_FIFO_CTL_RC6_POLICY_STALL); } + iosf_mbi_punit_acquire(); intel_uncore_forcewake_reset(dev_priv, restore_forcewake); + iosf_mbi_punit_release(); } void intel_uncore_suspend(struct drm_i915_private *dev_priv) { - iosf_mbi_unregister_pmic_bus_access_notifier( + iosf_mbi_punit_acquire(); + iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( &dev_priv->uncore.pmic_bus_access_nb); intel_uncore_forcewake_reset(dev_priv, false); + iosf_mbi_punit_release(); } void intel_uncore_resume_early(struct drm_i915_private *dev_priv) @@ -1315,12 +1322,14 @@ void intel_uncore_init(struct drm_i915_private *dev_priv) void intel_uncore_fini(struct drm_i915_private *dev_priv) { - iosf_mbi_unregister_pmic_bus_access_notifier( - &dev_priv->uncore.pmic_bus_access_nb); - /* Paranoia: make sure we have disabled everything before we exit. */ intel_uncore_sanitize(dev_priv); + + iosf_mbi_punit_acquire(); + iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( + &dev_priv->uncore.pmic_bus_access_nb); intel_uncore_forcewake_reset(dev_priv, false); + iosf_mbi_punit_release(); } static const struct reg_whitelist { diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c index 3cac22eb47ce..733d87fe7737 100644 --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c @@ -148,7 +148,10 @@ static int intel_uncore_check_forcewake_domains(struct drm_i915_private *dev_pri for_each_set_bit(offset, valid, FW_RANGE) { i915_reg_t reg = { offset }; + iosf_mbi_punit_acquire(); intel_uncore_forcewake_reset(dev_priv, false); + iosf_mbi_punit_release(); + check_for_unclaimed_mmio(dev_priv); (void)I915_READ(reg);