From patchwork Mon Jun 25 09:48:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10485373 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 18481603B5 for ; Mon, 25 Jun 2018 09:48:59 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0D4DA287C5 for ; Mon, 25 Jun 2018 09:48:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F0D89287DA; Mon, 25 Jun 2018 09:48:58 +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=-5.2 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED 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 8659C287C5 for ; Mon, 25 Jun 2018 09:48:58 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4504A89A5C; Mon, 25 Jun 2018 09:48:57 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 31B4A89A4F for ; Mon, 25 Jun 2018 09:48:55 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 12149639-1500050 for multiple; Mon, 25 Jun 2018 10:48:42 +0100 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Mon, 25 Jun 2018 10:48:43 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Mon, 25 Jun 2018 10:48:12 +0100 Message-Id: <20180625094842.8499-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.18.0 X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Subject: [Intel-gfx] [PATCH 01/31] drm/i915: Defer modeset cleanup to a secondary task X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 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 If we avoid cleaning up the old state immediately in intel_atomic_commit_tail() and defer it to a second task, we can avoid taking heavily contended locks when the caller is ready to procede. Subsequent modesets will wait for the cleanup operation (either directly via the ordered modeset wq or indirectly through the atomic helperr) which keeps the number of inflight cleanup tasks in check. As an example, during reset an immediate modeset is performed to disable the displays before the HW is reset, which must avoid struct_mutex to avoid recursion. Moving the cleanup to a separate task, defers acquiring the struct_mutex to after the GPU is running again, allowing it to complete. Even in a few patches time (optimist!) when we no longer require struct_mutex to unpin the framebuffers, it will still be good practice to minimise the number of contention points along reset. The mutex dependency still exists (as one modeset flushes the other), but in the short term it resolves the deadlock for simple reset cases. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3d849ec17f5c..3709fa1b6318 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12554,6 +12554,19 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat finish_wait(&dev_priv->gpu_error.wait_queue, &wait_reset); } +static void intel_atomic_cleanup_work(struct work_struct *work) +{ + struct drm_atomic_state *state = + container_of(work, struct drm_atomic_state, commit_work); + struct drm_i915_private *i915 = to_i915(state->dev); + + drm_atomic_helper_cleanup_planes(&i915->drm, state); + drm_atomic_helper_commit_cleanup_done(state); + drm_atomic_state_put(state); + + intel_atomic_helper_free_state(i915); +} + static void intel_atomic_commit_tail(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; @@ -12714,13 +12727,16 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET); } - drm_atomic_helper_cleanup_planes(dev, state); - - drm_atomic_helper_commit_cleanup_done(state); - - drm_atomic_state_put(state); - - intel_atomic_helper_free_state(dev_priv); + /* + * Defer the cleanup of the old state to a separate worker to not + * impede the current task (userspace for blocking modesets) that + * are executed inline. For out-of-line asynchronous modesets/flips, + * deferring to a new worker seems overkill, but we would place a + * schedule point (cond_resched()) here anyway to keep latencies + * down. + */ + INIT_WORK(&state->commit_work, intel_atomic_cleanup_work); + schedule_work(&state->commit_work); } static void intel_atomic_commit_work(struct work_struct *work)