Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp10359172imu; Wed, 5 Dec 2018 22:48:40 -0800 (PST) X-Google-Smtp-Source: AFSGD/WrHPcMfj51PnKIcXub9SyUFQduOTWiHORa2p+habzj0E6AmU/aUib5aveFpiD8t/rmNNCn X-Received: by 2002:a62:59c9:: with SMTP id k70mr27271251pfj.243.1544078920464; Wed, 05 Dec 2018 22:48:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1544078920; cv=none; d=google.com; s=arc-20160816; b=p26DP1+JoNak2eImnHWoVD88JEuuMZ51Jm0ZxV4SFVm1wdhTIve5TOnPSCpiGjt4vO l5VqPj//b8SBTYxPLlafj6yxj6oEdU934OBruuAOJ12vCNQc5hvEOC7UTOSugMNKFehS AhqSxbCTMh6kfXxrFaQtDpVgDwONQIMZEgMzdjwX1sMJhruBTJUpSq93AdU0vyzWAoE7 cKLiUZamCbWO9rVsdkCgDB+sMpeiMLpaZJXL9nbFPgtO1EElmIoqUshNCUBw2XBcwCZV ODwesBmR3xIVNmMKXNVmeH+k1aRRaGIpnl2qUcUNyd4k0nJ0KwpRhyMdLjMlvWeScMLx waEA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dmarc-filter:dkim-signature:dkim-signature; bh=1qgkRxoroE0Z6BgRqbD5stqVgZ1BuvTHE3FCxPVhkN0=; b=ogmLRG43NsaWCay7I7GMqaw77KWoPP7wfdridh6cuzr6ya7QJc1xvrOCGVJ5ngCL1D lqVtaGXcAARqon/HYArHRIAQCpn2G2vDkCr8vRpHhR30pRODMEAnWPFmnL+RWPnZZmFX 8YXKUIcr4W4UJt/bpCbcgAp1WWwDqks4qxf+8p9Cp0VTMuUaTQBoh7XNlDbGNWce2mju REfqprvqvcOctPEK7SipNPzFPOtG2AVRXvbXTkcES59fi91RR4Qo7LUiNmQC09H++khp T846kgp/OVDdw1eMxM9k8kUJC0DEXAwyYHuWB++R2rb6ijj/YLH9UTX6X2eqeqpsAJ3u lmfA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@codeaurora.org header.s=default header.b=OTRez8PA; dkim=pass header.i=@codeaurora.org header.s=default header.b=JDtkz9f2; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 41si22392556plf.347.2018.12.05.22.48.24; Wed, 05 Dec 2018 22:48:40 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@codeaurora.org header.s=default header.b=OTRez8PA; dkim=pass header.i=@codeaurora.org header.s=default header.b=JDtkz9f2; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729104AbeLFGrU (ORCPT + 99 others); Thu, 6 Dec 2018 01:47:20 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:49136 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728791AbeLFGrU (ORCPT ); Thu, 6 Dec 2018 01:47:20 -0500 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 2A4AF60C65; Thu, 6 Dec 2018 06:47:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1544078838; bh=GO7TKZdEH2RWA9o4HpJ11VVGNuLJzeBhjyry+gYpF7U=; h=From:To:Cc:Subject:Date:From; b=OTRez8PAa7QordOAvxWlaGoJgNX7AEVk9Hkb+x4KEBE87vjcf8aJxu/WmgKip72Kj hbYf8HCG/c3YBJSSbtwxOSfvNT/6nsOqeiKDHBX4PLmFSLnKf0fHVOTsgw1KLpm2mc czOG8auLsR/QzYw/ECV5yrO2WuJBJzYt4l3S3Ivo= X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on pdx-caf-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.7 required=2.0 tests=ALL_TRUSTED,BAYES_00, DKIM_INVALID,DKIM_SIGNED autolearn=no autolearn_force=no version=3.4.0 Received: from jshekhar-linux.qualcomm.com (blr-c-bdr-fw-01_globalnat_allzones-outside.qualcomm.com [103.229.19.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: jshekhar@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 5898561374; Thu, 6 Dec 2018 06:47:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1544078830; bh=GO7TKZdEH2RWA9o4HpJ11VVGNuLJzeBhjyry+gYpF7U=; h=From:To:Cc:Subject:Date:From; b=JDtkz9f2ZxmtACiuhlYk8DGdHXORsQ6mtbpDaQvWiqR1h4m9WUEQOuruKDQt9K051 SxEmtLkdZAgkdRXeQp4OGwm7Hh1WoubR/ZqRzOXvAaPtpaBn8U/On6aYSlfNFFmOtb rw9RbllzO/o6WWXWCboImBWhM0T/DDrLIwqcwDqA= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 5898561374 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=jshekhar@codeaurora.org From: Jayant Shekhar To: dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, devicetree@vger.kernel.org Cc: Jayant Shekhar , linux-kernel@vger.kernel.org, robdclark@gmail.com, seanpaul@chromium.org, hoegsberg@chromium.org, abhinavk@codeaurora.org, jsanka@codeaurora.org, chandanu@codeaurora.org, nganji@codeaurora.org Subject: [DPU PATCH] drm/msm/dpu: Fix vblank refcount mismatch Date: Thu, 6 Dec 2018 12:16:53 +0530 Message-Id: <1544078813-10945-1-git-send-email-jshekhar@codeaurora.org> X-Mailer: git-send-email 1.9.1 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org _dpu_crtc_vblank_enable_no_lock releases crtc_lock as its needed for power handle operations. This opens up a window where in a thread running dpu_crtc_disable and a thread running dpu_crtc_vblank can race in using dpu_crtc->enabled. dpu_crtc_disable will change the state, where as dpu_crtc_vblank use the variable. The fix is to cache the crtc enabled state while holding the lock and use it as a gate in calling _dpu_crtc_vblank_enable_no_lock. This issue was introduced with the commit cf871c48 (drm/msm/dpu: Remove suspend state tracking from crtc). Below are stack traces of thread 1 and thread 2 in good case and bad case: Bad case: ------------- Thread 1 dpu_encoder_phys_vid_control_vblank_irq+0xd0/0x170 dpu_encoder_register_vblank_callback+0xb8/0x100 _dpu_crtc_vblank_enable_no_lock+0x240/0x288 dpu_crtc_disable+0xc4/0x288 drm_atomic_helper_commit_modeset_disables+0x19c/0x350 msm_atomic_commit_tail+0x48/0x144 commit_tail+0x44/0x70 drm_atomic_helper_commit+0xf0/0xf8 drm_atomic_commit+0x40/0x4c drm_mode_atomic_ioctl+0x374/0x90c drm_ioctl_kernel+0xac/0xec drm_ioctl+0x218/0x384 drm_compat_ioctl+0xd8/0xe8 Thread 2: dpu_encoder_phys_vid_control_vblank_irq+0x74/0x170 dpu_encoder_register_vblank_callback+0xb8/0x100 _dpu_crtc_vblank_enable_no_lock+0x240/0x288 dpu_crtc_vblank+0xa8/0x118 dpu_kms_disable_vblank+0x20/0x2c vblank_ctrl_worker+0xa0/0xe0 kthread_worker_fn+0xe4/0x1a4 kthread+0x11c/0x12c ret_from_fork+0x10/0x18 Good case: -------------- Thread 1: dpu_encoder_phys_vid_control_vblank_irq+0xd0/0x170 dpu_encoder_phys_vid_irq_control+0xc8/0x110 _dpu_encoder_irq_control+0x48/0xa0 _dpu_encoder_resource_control_helper+0xb4/0x10c dpu_encoder_resource_control+0x4e0/0x664 dpu_encoder_virt_enable+0xb8/0x120 dpu_kms_encoder_enable+0x34/0xcc drm_atomic_helper_commit_modeset_enables+0x120/0x1b8 msm_atomic_commit_tail+0x5c/0x144 commit_tail+0x44/0x70 drm_atomic_helper_commit+0xf0/0xf8 drm_atomic_commit+0x40/0x4c drm_mode_atomic_ioctl+0x374/0x90c Thread 2: dpu_crtc_vblank+0xc8/0x118 dpu_kms_disable_vblank+0x20/0x2c vblank_ctrl_worker+0xa0/0xe0 kthread_worker_fn+0xe4/0x1a4 kthread+0x11c/0x12c Signed-off-by: Jayant Shekhar --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 630cbaa..e81ad8c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -877,6 +877,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc) struct drm_encoder *encoder; struct msm_drm_private *priv; unsigned long flags; + bool crtc_en; if (!crtc || !crtc->dev || !crtc->dev->dev_private || !crtc->state) { DPU_ERROR("invalid crtc\n"); @@ -901,11 +902,21 @@ static void dpu_crtc_disable(struct drm_crtc *crtc) atomic_read(&dpu_crtc->frame_pending)); trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc); - if (dpu_crtc->enabled && dpu_crtc->vblank_requested) { - _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false); - } + + /* + * Cache vblank enabled before calling _dpu_crtc_vblank_enable_no_lock, + * because we release crtc_lock inside and acquire it back. While lock + * is released, there are cases where dpu_crtc_vblank comes in between + * while disable is going on. dpu_crtc_vblank further calls + * _dpu_crtc_vblank_enable_no_lock which tries vblank disable again + * resulting in refcount mismatch. + */ + crtc_en = dpu_crtc->enabled; dpu_crtc->enabled = false; + if (crtc_en && dpu_crtc->vblank_requested) + _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false); + if (atomic_read(&dpu_crtc->frame_pending)) { trace_dpu_crtc_disable_frame_pending(DRMID(crtc), atomic_read(&dpu_crtc->frame_pending)); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project