From patchwork Fri Sep 16 16:33:28 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 12978726 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 39CAEECAAA1 for ; Fri, 16 Sep 2022 16:34:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CF5E510E4CE; Fri, 16 Sep 2022 16:34:14 +0000 (UTC) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7838010E05A; Fri, 16 Sep 2022 16:33:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1663346020; x=1694882020; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=+XPV8HcaE/9L71hkSOiFNYaVu+xuhMgleVwsoSICrYg=; b=ZvZxcVdEmN8yQqPPY4GgbBQ7WA6BuchmR6bGp603/7IAPezrJ4a+KQtN NJ0nTcsfcjBA1B1ROsjX21JPowfMHIVKSoBT1jnMl2IY13JiCAM0hYCqc mRWrehgYzhVDo7AXQRwpygKglg1zqvFSYNTBWSGMiEl2/zYT6zyTRDW9V zge1b+BPlTeOxEAp8O3CGOw2NqcPVNcaN9Dns1zkFd4Qb2af1THbC8IC/ bf19wpJsm0RQCU8fu2R94tB5kNq0u66puDMANW2P6SjFBcH6mwHQ3a8ny cSsJ0F6m5BQYGe/OC1v24CvxwLMF1+vnwXPN0kIDQPycTzldbvf+hfaf9 w==; X-IronPort-AV: E=McAfee;i="6500,9779,10472"; a="278756930" X-IronPort-AV: E=Sophos;i="5.93,320,1654585200"; d="scan'208";a="278756930" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Sep 2022 09:33:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,320,1654585200"; d="scan'208";a="620140528" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.191]) by fmsmga007.fm.intel.com with SMTP; 16 Sep 2022 09:33:36 -0700 Received: by stinkbox (sSMTP sendmail emulation); Fri, 16 Sep 2022 19:33:35 +0300 From: Ville Syrjala To: dri-devel@lists.freedesktop.org Subject: [PATCH 1/4] drm/atomic: Treat a nonblocking commit following a blocking commit as blocking commit Date: Fri, 16 Sep 2022 19:33:28 +0300 Message-Id: <20220916163331.6849-2-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220916163331.6849-1-ville.syrjala@linux.intel.com> References: <20220916163331.6849-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 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: Pekka Paalanen , Daniel Vetter , intel-gfx@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Ville Syrjälä Currently a nonblocking commit will actually block if it is preceded by a blocking commit. It just happens block on the mutex rather than on the completion. I shall call these as not-actually-nonblocking commits. I would like to make blocking commits execute locklessly, just as nonblocking commits already do. The main benefit would that parallel TEST_ONLY commits would not get blocked on the mutexes until the parallel blocking commit is done. To achieve that without a significant change in behaviour for the not-actually-nonblocking commits let's treat them exactly the same as blocking commit, ie. instead of returning -EBUSY they will just block. Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Rob Clark Cc: Simon Ser Cc: Pekka Paalanen Cc: Jonas Ådahl Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic_helper.c | 19 ++++++++++++------- include/drm/drm_atomic.h | 7 +++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ee5fea48b5cb..bff087674cb5 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2109,7 +2109,7 @@ static int stall_checks(struct drm_crtc *crtc, bool nonblock) * Userspace is not allowed to get ahead of the previous * commit with nonblocking ones. */ - if (!completed && nonblock) { + if (!completed && nonblock && commit->nonblock) { spin_unlock(&crtc->commit_lock); drm_dbg_atomic(crtc->dev, "[CRTC:%d:%s] busy with a previous commit\n", @@ -2152,7 +2152,7 @@ static void release_crtc_commit(struct completion *completion) drm_crtc_commit_put(commit); } -static void init_commit(struct drm_crtc_commit *commit, struct drm_crtc *crtc) +static void init_commit(struct drm_crtc_commit *commit, struct drm_crtc *crtc, bool nonblock) { init_completion(&commit->flip_done); init_completion(&commit->hw_done); @@ -2160,10 +2160,11 @@ static void init_commit(struct drm_crtc_commit *commit, struct drm_crtc *crtc) INIT_LIST_HEAD(&commit->commit_entry); kref_init(&commit->ref); commit->crtc = crtc; + commit->nonblock = nonblock; } static struct drm_crtc_commit * -crtc_or_fake_commit(struct drm_atomic_state *state, struct drm_crtc *crtc) +crtc_or_fake_commit(struct drm_atomic_state *state, struct drm_crtc *crtc, bool nonblock) { if (crtc) { struct drm_crtc_state *new_crtc_state; @@ -2178,7 +2179,7 @@ crtc_or_fake_commit(struct drm_atomic_state *state, struct drm_crtc *crtc) if (!state->fake_commit) return NULL; - init_commit(state->fake_commit, NULL); + init_commit(state->fake_commit, NULL, nonblock); } return state->fake_commit; @@ -2250,7 +2251,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, if (!commit) return -ENOMEM; - init_commit(commit, crtc); + init_commit(commit, crtc, nonblock); new_crtc_state->commit = commit; @@ -2299,6 +2300,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, * commit with nonblocking ones. */ if (nonblock && old_conn_state->commit && + old_conn_state->commit->nonblock && !try_wait_for_completion(&old_conn_state->commit->flip_done)) { drm_dbg_atomic(conn->dev, "[CONNECTOR:%d:%s] busy with a previous commit\n", @@ -2308,7 +2310,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, } /* Always track connectors explicitly for e.g. link retraining. */ - commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: old_conn_state->crtc); + commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: old_conn_state->crtc, + nonblock); if (!commit) return -ENOMEM; @@ -2321,6 +2324,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, * commit with nonblocking ones. */ if (nonblock && old_plane_state->commit && + old_plane_state->commit->nonblock && !try_wait_for_completion(&old_plane_state->commit->flip_done)) { drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] busy with a previous commit\n", @@ -2330,7 +2334,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, } /* Always track planes explicitly for async pageflip support. */ - commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: old_plane_state->crtc); + commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: old_plane_state->crtc, + nonblock); if (!commit) return -ENOMEM; diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 10b1990bc1f6..0924c322ddfb 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -155,6 +155,13 @@ struct drm_crtc_commit { * used by the free code to remove the second reference if commit fails. */ bool abort_completion; + + /** + * @nonblock: + * + * Nonblocking commit? + */ + bool nonblock; }; struct __drm_planes_state { From patchwork Fri Sep 16 16:33:29 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 12978725 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 329EBECAAA1 for ; Fri, 16 Sep 2022 16:34:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2083B10E4E0; Fri, 16 Sep 2022 16:34:12 +0000 (UTC) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9766810E21D; Fri, 16 Sep 2022 16:33:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1663346024; x=1694882024; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=mHI24FmnqlV4Cs36rLp+0uhjn0jhUt841zuGdrfWm4Q=; b=Lgnhu97MEntLMLwSTvdtOMsBP8+dY6DmwJ8nnSBiY71rv9N556pu0Rv9 fVJbzDYbRdMkr5QzBfoK++CKCF1M5MvIVRDrhNJVBbmH/WEmi5OXOXFDP axFJpDdk/OfprZLcA20Jubs80YZ87giqETUF46WDtoLleLwzpG7U7zLcP 9xbFRWa/ONkheWew4cNeKB8l5/pV6v2D9v7p5HkCx10ligw8NZP1NvWot yokjvRPYVL3c4gX+2+7VgJforCjW/9JnsBYb9Vm23AOPE6cgLbbTvwBHa j8XK5wVzoufAjqRRB5iQT0LUKiWNaGsCJAQn6DxDxj6Ku5gjCMg/9mtwi A==; X-IronPort-AV: E=McAfee;i="6500,9779,10472"; a="362990794" X-IronPort-AV: E=Sophos;i="5.93,320,1654585200"; d="scan'208";a="362990794" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Sep 2022 09:33:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,320,1654585200"; d="scan'208";a="613308406" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.191]) by orsmga007.jf.intel.com with SMTP; 16 Sep 2022 09:33:40 -0700 Received: by stinkbox (sSMTP sendmail emulation); Fri, 16 Sep 2022 19:33:39 +0300 From: Ville Syrjala To: dri-devel@lists.freedesktop.org Subject: [PATCH 2/4] drm/i915: Don't reuse commit_work for the cleanup Date: Fri, 16 Sep 2022 19:33:29 +0300 Message-Id: <20220916163331.6849-3-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220916163331.6849-1-ville.syrjala@linux.intel.com> References: <20220916163331.6849-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 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: Pekka Paalanen , Daniel Vetter , intel-gfx@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Ville Syrjälä Currently we reuse the commit_work for a later cleanup step. Let's not do that so that atomic ioctl handler won't accidentally wait for the cleanup work when it really wants to just wait on the commit_tail() part. We'll just add another work struct for the cleanup. Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Rob Clark Cc: Simon Ser Cc: Pekka Paalanen Cc: Jonas Ådahl Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 6 +++--- drivers/gpu/drm/i915/display/intel_display_types.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index dd008ba8afe3..cd617046e0ee 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7422,7 +7422,7 @@ static void intel_cleanup_dsbs(struct intel_atomic_state *state) static void intel_atomic_cleanup_work(struct work_struct *work) { struct intel_atomic_state *state = - container_of(work, struct intel_atomic_state, base.commit_work); + container_of(work, struct intel_atomic_state, cleanup_work); struct drm_i915_private *i915 = to_i915(state->base.dev); intel_cleanup_dsbs(state); @@ -7643,8 +7643,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) * schedule point (cond_resched()) here anyway to keep latencies * down. */ - INIT_WORK(&state->base.commit_work, intel_atomic_cleanup_work); - queue_work(system_highpri_wq, &state->base.commit_work); + INIT_WORK(&state->cleanup_work, intel_atomic_cleanup_work); + queue_work(system_highpri_wq, &state->cleanup_work); } static void intel_atomic_commit_work(struct work_struct *work) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 298d00a11f47..971e2b1e1b26 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -655,6 +655,7 @@ struct intel_atomic_state { struct i915_sw_fence commit_ready; + struct work_struct cleanup_work; struct llist_node freed; }; From patchwork Fri Sep 16 16:33:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 12978724 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 8309BECAAD8 for ; Fri, 16 Sep 2022 16:34:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EBA2510E4CD; Fri, 16 Sep 2022 16:34:06 +0000 (UTC) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id 70AE310E4C5; Fri, 16 Sep 2022 16:33:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1663346029; x=1694882029; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=fxFbYNk+mC+hLp92Ar8qnUDygZQHSO3zI9XyyF1M+SU=; b=gTBj8ozCoZHiBpHzCp4iH/MtC4a9MbjOHVvY5e1vYdxaq1ltms4oEiUJ T2bDb3xiXSEldlotDlhT1NxxprK91AI5c8cjfYAhCp824hvELkohnXC5n c+NaZuUrSWqISZcbyiU7L93fKfcx6z5Tv+0fyVigGabLLruVzTYhO7Nti AV9sW2+hY62lkjiOSn5yn7X+MCVEu/6YNJJyX9jphTvb8nh4ahtsUMLc+ zbXIucKV1t3sLIu2fa1If5IJ7J1OtMNhgNVeTT0NOdU02aK1YfVtOaaW+ 62ZBC+ip7CX8haoqcShBdf2prrLj8rYCyO5+FZeu76hwsmTaZWhnoigyW A==; X-IronPort-AV: E=McAfee;i="6500,9779,10472"; a="282056307" X-IronPort-AV: E=Sophos;i="5.93,320,1654585200"; d="scan'208";a="282056307" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Sep 2022 09:33:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,320,1654585200"; d="scan'208";a="680024986" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.191]) by fmsmga008.fm.intel.com with SMTP; 16 Sep 2022 09:33:44 -0700 Received: by stinkbox (sSMTP sendmail emulation); Fri, 16 Sep 2022 19:33:44 +0300 From: Ville Syrjala To: dri-devel@lists.freedesktop.org Subject: [PATCH 3/4] drm/atomic: Allow lockless blocking commits Date: Fri, 16 Sep 2022 19:33:30 +0300 Message-Id: <20220916163331.6849-4-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220916163331.6849-1-ville.syrjala@linux.intel.com> References: <20220916163331.6849-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 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: Pekka Paalanen , Daniel Vetter , intel-gfx@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Ville Syrjälä The easiest way to execute blocking commits locklessly is to just schedule them onto the workqueue excatly as we do for nonblocking commits. And to preserve the blocking behaviour of the ioctl we just flush the work before exiting the kernel. We do need to reorder the state_put() vs drop_locks() of course so we don't flush_work() while still holding the locks. Note that a lot of the current users of drm_atomic_commit() (eg. lot of the atomic helpers) have the ww_ctx stuff outside the drm_atomic_state handling. With that structure we can't actually pull the flush_work() past the drop_locks(). So in order to make those places actually lockless we'll need reverse the layers. That is left for a future excercise and for now we just roll the flush_work() straight into drm_atomic_commit(), leaving the non-flushing version for just the atomic ioctl handler. Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Rob Clark Cc: Simon Ser Cc: Pekka Paalanen Cc: Jonas Ådahl Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic.c | 32 +++++++++++++++++++++++++++++-- drivers/gpu/drm/drm_atomic_uapi.c | 11 ++++++++--- include/drm/drm_atomic.h | 1 + 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index f197f59f6d99..6d728af4e8cf 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1411,7 +1411,7 @@ int drm_atomic_check_only(struct drm_atomic_state *state) EXPORT_SYMBOL(drm_atomic_check_only); /** - * drm_atomic_commit - commit configuration atomically + * drm_atomic_commit_noflush - commit configuration atomically, without waiting for the commit * @state: atomic configuration to check * * Note that this function can return -EDEADLK if the driver needed to acquire @@ -1424,7 +1424,7 @@ EXPORT_SYMBOL(drm_atomic_check_only); * Returns: * 0 on success, negative error code on failure. */ -int drm_atomic_commit(struct drm_atomic_state *state) +int drm_atomic_commit_noflush(struct drm_atomic_state *state) { struct drm_mode_config *config = &state->dev->mode_config; struct drm_printer p = drm_info_printer(state->dev->dev); @@ -1441,6 +1441,34 @@ int drm_atomic_commit(struct drm_atomic_state *state) return config->funcs->atomic_commit(state->dev, state, false); } +EXPORT_SYMBOL(drm_atomic_commit_noflush); + +/** + * drm_atomic_commit - commit configuration atomically, waiting for the commit to finish + * @state: atomic configuration to check + * + * Note that this function can return -EDEADLK if the driver needed to acquire + * more locks but encountered a deadlock. The caller must then do the usual w/w + * backoff dance and restart. All other errors are fatal. + * + * This function will take its own reference on @state. + * Callers should always release their reference with drm_atomic_state_put(). + * + * Returns: + * 0 on success, negative error code on failure. + */ +int drm_atomic_commit(struct drm_atomic_state *state) +{ + int ret; + + ret = drm_atomic_commit_noflush(state); + if (ret) + return ret; + + flush_work(&state->commit_work); + + return 0; +} EXPORT_SYMBOL(drm_atomic_commit); /** diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 79730fa1dd8e..73ec26fe3393 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1290,6 +1290,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, struct drm_atomic_state *state; struct drm_modeset_acquire_ctx ctx; struct drm_out_fence_state *fence_state; + bool flush = false; int ret = 0; unsigned int i, j, num_fences; @@ -1423,7 +1424,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { ret = drm_atomic_nonblocking_commit(state); } else { - ret = drm_atomic_commit(state); + ret = drm_atomic_commit_noflush(state); + flush = ret == 0; } out: @@ -1436,10 +1438,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, goto retry; } - drm_atomic_state_put(state); - drm_modeset_drop_locks(&ctx); drm_modeset_acquire_fini(&ctx); + if (flush) + flush_work(&state->commit_work); + + drm_atomic_state_put(state); + return ret; } diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 0924c322ddfb..d19ce8898bd4 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -740,6 +740,7 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state, struct drm_crtc *crtc); int __must_check drm_atomic_check_only(struct drm_atomic_state *state); +int __must_check drm_atomic_commit_noflush(struct drm_atomic_state *state); int __must_check drm_atomic_commit(struct drm_atomic_state *state); int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); From patchwork Fri Sep 16 16:33:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 12978727 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 103C2ECAAA1 for ; Fri, 16 Sep 2022 16:34:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E3E0E10E4D4; Fri, 16 Sep 2022 16:34:17 +0000 (UTC) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 818B410E4C7; Fri, 16 Sep 2022 16:33:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1663346033; x=1694882033; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=iXmQ3je1QQz+KMzb7zBVEb50RsHdewUaM8xgwvQ8P8s=; b=aaLGUePhYDjsrAe+iJZsF+1NcdaB+qczxoCFvj0l1xeABycm3uH1YwTJ SdPLtlZH46eizcXM29TNLUeIZEs/LYttHgPNST4B4l9c4s35ogawY+cwV FCsJYZcoCJQyjFgQASR7JtdBilj+l8TN6iixAA3XhobHe8/TSxPG0dCiC fDI/rSpixN6I7sCDqXBqOHYUvHklsMfVEXzl2kes+Q/rq0AHO/dFCKtqg msitSVMpz5wkaioYyKbErF4ASWm3TzGnBmU5yb4Snfag9NpwAvltyueXU 06LMfwsDFzXj1C7hiI6UCIWF+qD4v8KfvGRWQ+l1d1JIJqGlWZZy8vUri A==; X-IronPort-AV: E=McAfee;i="6500,9779,10472"; a="286068363" X-IronPort-AV: E=Sophos;i="5.93,320,1654585200"; d="scan'208";a="286068363" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Sep 2022 09:33:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,320,1654585200"; d="scan'208";a="648318623" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.191]) by orsmga008.jf.intel.com with SMTP; 16 Sep 2022 09:33:49 -0700 Received: by stinkbox (sSMTP sendmail emulation); Fri, 16 Sep 2022 19:33:48 +0300 From: Ville Syrjala To: dri-devel@lists.freedesktop.org Subject: [PATCH 4/4] drm/i915: Make blocking commits lockless Date: Fri, 16 Sep 2022 19:33:31 +0300 Message-Id: <20220916163331.6849-5-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220916163331.6849-1-ville.syrjala@linux.intel.com> References: <20220916163331.6849-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 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: Pekka Paalanen , Daniel Vetter , intel-gfx@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Ville Syrjälä Make blocking commits execute locklessly (just as nonblocking commits do) by scheduling them onto the workqueues as well. There will be a later flush_work() done by whoever called the commit hook to make sure the blocking behaviour of the ioctl/etc. is preserved. I also wondered about just dropping the locks straight from the driver, but I guess whoever called us might still depend on having the locks so that seems like a terrible idea. Also calling commit_tail() directly when not holding the locks would then allow eg. two ioctls to execute full modesets in parallel, which we don't want as we haven't fully evaluated all modeset codepaths for concurrency issues. Currently we achieve serial excution with a combination of an ordered workqueue (for nonblocking commits) and reliance on the singular connection_mutex (for blocking commits), and a flush_workqueue() to sync between the two. So by always scheduling everything onto the workqueues we don't have to worry about racing between the lockless direct commit_tail() calls, and we don't need some kind of new atomic hook that would do said call for us after dropping the locks. Also less codepaths in general seems like a beneficial thing. Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Rob Clark Cc: Simon Ser Cc: Pekka Paalanen Cc: Jonas Ådahl Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index cd617046e0ee..18a5f14e7f41 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7771,15 +7771,10 @@ static int intel_atomic_commit(struct drm_device *dev, INIT_WORK(&state->base.commit_work, intel_atomic_commit_work); i915_sw_fence_commit(&state->commit_ready); - if (nonblock && state->modeset) { + if (state->modeset) queue_work(dev_priv->display.wq.modeset, &state->base.commit_work); - } else if (nonblock) { + else queue_work(dev_priv->display.wq.flip, &state->base.commit_work); - } else { - if (state->modeset) - flush_workqueue(dev_priv->display.wq.modeset); - intel_atomic_commit_tail(state); - } return 0; }