From patchwork Thu Oct 15 09:15:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Krishna Manikandan X-Patchwork-Id: 11842947 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 671DDC388F2 for ; Sat, 17 Oct 2020 19:05:23 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 20F642073A for ; Sat, 17 Oct 2020 19:05:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 20F642073A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 76CD96E231; Sat, 17 Oct 2020 19:04:50 +0000 (UTC) Received: from alexa-out.qualcomm.com (alexa-out.qualcomm.com [129.46.98.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4E8526E120; Thu, 15 Oct 2020 09:15:34 +0000 (UTC) Received: from ironmsg07-lv.qualcomm.com (HELO ironmsg07-lv.qulacomm.com) ([10.47.202.151]) by alexa-out.qualcomm.com with ESMTP; 15 Oct 2020 02:15:34 -0700 X-QCInternal: smtphost Received: from ironmsg02-blr.qualcomm.com ([10.86.208.131]) by ironmsg07-lv.qulacomm.com with ESMTP/TLS/AES256-SHA; 15 Oct 2020 02:15:32 -0700 X-QCInternal: smtphost Received: from mkrishn-linux.qualcomm.com ([10.204.66.35]) by ironmsg02-blr.qualcomm.com with ESMTP; 15 Oct 2020 14:45:13 +0530 Received: by mkrishn-linux.qualcomm.com (Postfix, from userid 438394) id E4DC5214B6; Thu, 15 Oct 2020 14:45:11 +0530 (IST) From: Krishna Manikandan To: dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, devicetree@vger.kernel.org Subject: [v2] drm/msm: Fix race condition in msm driver with async layer updates Date: Thu, 15 Oct 2020 14:45:10 +0530 Message-Id: <1602753310-22105-1-git-send-email-mkrishn@codeaurora.org> X-Mailer: git-send-email 2.7.4 X-Mailman-Approved-At: Sat, 17 Oct 2020 19:04:47 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Krishna Manikandan , linux-kernel@vger.kernel.org, dianders@chromium.org, seanpaul@chromium.org, kalyan_t@codeaurora.org, hoegsberg@chromium.org MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" When there are back to back commits with async cursor update, there is a case where second commit can program the DPU hw blocks while first didn't complete flushing config to HW. Synchronize the compositions such that second commit waits until first commit flushes the composition. This change also introduces per crtc commit lock, such that commits on different crtcs are not blocked by each other. Changes in v2: - Use an array of mutexes in kms to handle commit lock per crtc. (Rob Clark) Signed-off-by: Krishna Manikandan --- drivers/gpu/drm/msm/msm_atomic.c | 32 +++++++++++++++++++------------- drivers/gpu/drm/msm/msm_kms.h | 6 ++++-- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 561bfa4..f9bd472 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -61,10 +61,10 @@ static void msm_atomic_async_commit(struct msm_kms *kms, int crtc_idx) trace_msm_atomic_async_commit_start(crtc_mask); - mutex_lock(&kms->commit_lock); + mutex_lock(&kms->commit_lock[crtc_idx]); if (!(kms->pending_crtc_mask & crtc_mask)) { - mutex_unlock(&kms->commit_lock); + mutex_unlock(&kms->commit_lock[crtc_idx]); goto out; } @@ -79,7 +79,6 @@ static void msm_atomic_async_commit(struct msm_kms *kms, int crtc_idx) */ trace_msm_atomic_flush_commit(crtc_mask); kms->funcs->flush_commit(kms, crtc_mask); - mutex_unlock(&kms->commit_lock); /* * Wait for flush to complete: @@ -90,9 +89,8 @@ static void msm_atomic_async_commit(struct msm_kms *kms, int crtc_idx) vblank_put(kms, crtc_mask); - mutex_lock(&kms->commit_lock); kms->funcs->complete_commit(kms, crtc_mask); - mutex_unlock(&kms->commit_lock); + mutex_unlock(&kms->commit_lock[crtc_idx]); kms->funcs->disable_commit(kms); out: @@ -171,6 +169,16 @@ static unsigned get_crtc_mask(struct drm_atomic_state *state) return mask; } +static int get_crtc_id(struct msm_kms *kms, unsigned int crtc_mask) +{ + struct drm_crtc *crtc; + + for_each_crtc_mask(kms->dev, crtc, crtc_mask) + return drm_crtc_index(crtc); + + return 0; +} + void msm_atomic_commit_tail(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; @@ -180,6 +188,7 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state) unsigned crtc_mask = get_crtc_mask(state); bool async = kms->funcs->vsync_time && can_do_async(state, &async_crtc); + int crtc_idx = get_crtc_id(kms, crtc_mask); trace_msm_atomic_commit_tail_start(async, crtc_mask); @@ -189,12 +198,11 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state) * Ensure any previous (potentially async) commit has * completed: */ + mutex_lock(&kms->commit_lock[crtc_idx]); trace_msm_atomic_wait_flush_start(crtc_mask); kms->funcs->wait_flush(kms, crtc_mask); trace_msm_atomic_wait_flush_finish(crtc_mask); - mutex_lock(&kms->commit_lock); - /* * Now that there is no in-progress flush, prepare the * current update: @@ -232,8 +240,7 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state) } kms->funcs->disable_commit(kms); - mutex_unlock(&kms->commit_lock); - + mutex_unlock(&kms->commit_lock[crtc_idx]); /* * At this point, from drm core's perspective, we * are done with the atomic update, so we can just @@ -260,8 +267,7 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state) */ trace_msm_atomic_flush_commit(crtc_mask); kms->funcs->flush_commit(kms, crtc_mask); - mutex_unlock(&kms->commit_lock); - + mutex_unlock(&kms->commit_lock[crtc_idx]); /* * Wait for flush to complete: */ @@ -271,9 +277,9 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state) vblank_put(kms, crtc_mask); - mutex_lock(&kms->commit_lock); + mutex_lock(&kms->commit_lock[crtc_idx]); kms->funcs->complete_commit(kms, crtc_mask); - mutex_unlock(&kms->commit_lock); + mutex_unlock(&kms->commit_lock[crtc_idx]); kms->funcs->disable_commit(kms); drm_atomic_helper_commit_hw_done(state); diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 1cbef6b..2049847 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -155,7 +155,7 @@ struct msm_kms { * For async commit, where ->flush_commit() and later happens * from the crtc's pending_timer close to end of the frame: */ - struct mutex commit_lock; + struct mutex commit_lock[MAX_CRTCS]; unsigned pending_crtc_mask; struct msm_pending_timer pending_timers[MAX_CRTCS]; }; @@ -165,7 +165,9 @@ static inline void msm_kms_init(struct msm_kms *kms, { unsigned i; - mutex_init(&kms->commit_lock); + for (i = 0; i < ARRAY_SIZE(kms->commit_lock); i++) + mutex_init(&kms->commit_lock[i]); + kms->funcs = funcs; for (i = 0; i < ARRAY_SIZE(kms->pending_timers); i++)