From patchwork Mon Jul 12 16:42:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kai-Heng Feng X-Patchwork-Id: 1504096 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=Nfc/mLaq; dkim-atps=neutral Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4GNqLS6bDKz9sWw; Tue, 13 Jul 2021 02:42:24 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1m2z0f-0000Bn-J2; Mon, 12 Jul 2021 16:42:21 +0000 Received: from smtp-relay-canonical-1.internal ([10.131.114.174] helo=smtp-relay-canonical-1.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1m2z0e-0000BS-27 for kernel-team@lists.ubuntu.com; Mon, 12 Jul 2021 16:42:20 +0000 Received: from localhost (1.general.khfeng.us.vpn [10.172.68.174]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-1.canonical.com (Postfix) with ESMTPSA id D405B40555 for ; Mon, 12 Jul 2021 16:42:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1626108139; bh=XsYhT4Nav4kZm94/m67xLmCwQPg/GB3+A8adCheYPYE=; h=From:To:Subject:Date:Message-Id:MIME-Version:Content-Type; b=Nfc/mLaq+tZf1mUQZOi6q0b2+qdVzYR3m2XF+cVHJcg3F/pRC85ZL+vmxWoIMZL3V ugfQDGJ41sNrfDfhfH87wKMlW8Z2468Sc+k5E9EHUs+EVvO+Lxns6ju/NBhNmuOi4e 4Vh3BYDkibyZmBMaHwARGPvaJbbZTTicIsJ2nqRsUHZ+1vMJ4MwLVW5bWmFVXrtIGh NjJN1XZyPBs9bmHBBBLH1JVgbimUkS12JQXxJp7hD9QLvb4Td2Tw3nUAgBkxqL4iEm ThQFYbAARsUyqQxpXQm91C8OdOLOoxYlVx4vpRZ/ahCbZYKnIG6Cnk0fUlHlC3FlRW 60xcAxhPxg84Q== From: Kai-Heng Feng To: kernel-team@lists.ubuntu.com Subject: [SRU] [OEM-5.10] [PATCH 1/1] drm/i915: Get rid of ibx_irq_pre_postinstall() Date: Tue, 13 Jul 2021 00:42:10 +0800 Message-Id: <20210712164210.1141992-2-kai.heng.feng@canonical.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210712164210.1141992-1-kai.heng.feng@canonical.com> References: <20210712164210.1141992-1-kai.heng.feng@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Ville Syrjälä BugLink: https://bugs.launchpad.net/bugs/1935853 ibx_irq_pre_postinstall() looks totally pointless. We can just init both SDEIMR and SDEIER at the same time before enabling the master interrupt. It's equally racy as the other order due to doing all of this from the postinstall stage with the interrupt handler already in place. That is, safe with MSI but racy with shared legacy interrupts. Fortunately we should have MSI on all ilk+. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20201028213323.5423-20-ville.syrjala@linux.intel.com Reviewed-by: Lucas De Marchi (backported from commit a0a6d8cb552b20dc94d8a40d88126d8548fd7124) Signed-off-by: Kai-Heng Feng --- drivers/gpu/drm/i915/i915_irq.c | 63 +++++++++------------------------ 1 file changed, 16 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 706f73947de68..7df9f06340e0d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2738,24 +2738,6 @@ static void ibx_irq_reset(struct drm_i915_private *dev_priv) I915_WRITE(SERR_INT, 0xffffffff); } -/* - * SDEIER is also touched by the interrupt handler to work around missed PCH - * interrupts. Hence we can't update it after the interrupt handler is enabled - - * instead we unconditionally enable all PCH interrupt sources here, but then - * only unmask them as needed with SDEIMR. - * - * This function needs to be called before interrupts are enabled. - */ -static void ibx_irq_pre_postinstall(struct drm_i915_private *dev_priv) -{ - if (HAS_PCH_NOP(dev_priv)) - return; - - drm_WARN_ON(&dev_priv->drm, I915_READ(SDEIER) != 0); - I915_WRITE(SDEIER, 0xffffffff); - POSTING_READ(SDEIER); -} - static void vlv_display_irq_reset(struct drm_i915_private *dev_priv) { struct intel_uncore *uncore = &dev_priv->uncore; @@ -3284,8 +3266,20 @@ static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv) __bxt_hpd_detection_setup(dev_priv, enabled_irqs); } +/* + * SDEIER is also touched by the interrupt handler to work around missed PCH + * interrupts. Hence we can't update it after the interrupt handler is enabled - + * instead we unconditionally enable all PCH interrupt sources here, but then + * only unmask them as needed with SDEIMR. + * + * Note that we currently do this after installing the interrupt handler, + * but before we enable the master interrupt. That should be sufficient + * to avoid races with the irq handler, assuming we have MSI. Shared legacy + * interrupts could still race. + */ static void ibx_irq_postinstall(struct drm_i915_private *dev_priv) { + struct intel_uncore *uncore = &dev_priv->uncore; u32 mask; if (HAS_PCH_NOP(dev_priv)) @@ -3298,14 +3292,7 @@ static void ibx_irq_postinstall(struct drm_i915_private *dev_priv) else mask = SDE_GMBUS_CPT; - gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR); - I915_WRITE(SDEIMR, ~mask); - - if (HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv) || - HAS_PCH_LPT(dev_priv)) - ibx_hpd_detection_setup(dev_priv); - else - spt_hpd_detection_setup(dev_priv); + GEN3_IRQ_INIT(uncore, SDE, ~mask, 0xffffffff); } static void ilk_irq_postinstall(struct drm_i915_private *dev_priv) @@ -3335,27 +3322,12 @@ static void ilk_irq_postinstall(struct drm_i915_private *dev_priv) dev_priv->irq_mask = ~display_mask; - ibx_irq_pre_postinstall(dev_priv); - - GEN3_IRQ_INIT(uncore, DE, dev_priv->irq_mask, - display_mask | extra_mask); + ibx_irq_postinstall(dev_priv); gen5_gt_irq_postinstall(&dev_priv->gt); - ilk_hpd_detection_setup(dev_priv); - - ibx_irq_postinstall(dev_priv); - - if (IS_IRONLAKE_M(dev_priv)) { - /* Enable PCU event interrupts - * - * spinlocking not required here for correctness since interrupt - * setup is guaranteed to run in single-threaded context. But we - * need it to make the assert_spin_locked happy. */ - spin_lock_irq(&dev_priv->irq_lock); - ilk_enable_display_irq(dev_priv, DE_PCU_EVENT); - spin_unlock_irq(&dev_priv->irq_lock); - } + GEN3_IRQ_INIT(uncore, DE, dev_priv->irq_mask, + display_mask | extra_mask); } void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv) @@ -3491,9 +3463,6 @@ static void gen8_irq_postinstall(struct drm_i915_private *dev_priv) gen8_gt_irq_postinstall(&dev_priv->gt); gen8_de_irq_postinstall(dev_priv); - if (HAS_PCH_SPLIT(dev_priv)) - ibx_irq_postinstall(dev_priv); - gen8_master_intr_enable(dev_priv->uncore.regs); }