All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	hoegsberg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org
Subject: [PATCH v4 8/9] drm/msm: Remove msm_commit/worker, use atomic helper commit
Date: Wed,  4 Apr 2018 12:40:47 -0400	[thread overview]
Message-ID: <20180404164048.42126-9-seanpaul@chromium.org> (raw)
In-Reply-To: <20180404164048.42126-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Moving further towards switching fully to the the atomic helpers, this
patch removes the hand-rolled worker nonblock commit code and uses the
atomic helpers commit_work model.

Changes in v2:
- Remove commit_destroy()
- Shuffle order of commit_tail calls to further serialize commits
- Use stall in swap_state to avoid abandoned events on disable
Changes in v3:
- Rebased on Archit's private_obj set
Changes in v4:
- None

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/msm_atomic.c | 153 +++++++++----------------------
 drivers/gpu/drm/msm/msm_drv.c    |   1 -
 drivers/gpu/drm/msm/msm_drv.h    |   4 -
 3 files changed, 42 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 94f9c3e0e7bf..95c7868445dd 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -21,66 +21,6 @@
 #include "msm_gem.h"
 #include "msm_fence.h"
 
-struct msm_commit {
-	struct drm_device *dev;
-	struct drm_atomic_state *state;
-	struct work_struct work;
-	uint32_t crtc_mask;
-};
-
-static void commit_worker(struct work_struct *work);
-
-/* block until specified crtcs are no longer pending update, and
- * atomically mark them as pending update
- */
-static int start_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
-	int ret;
-
-	spin_lock(&priv->pending_crtcs_event.lock);
-	ret = wait_event_interruptible_locked(priv->pending_crtcs_event,
-			!(priv->pending_crtcs & crtc_mask));
-	if (ret == 0) {
-		DBG("start: %08x", crtc_mask);
-		priv->pending_crtcs |= crtc_mask;
-	}
-	spin_unlock(&priv->pending_crtcs_event.lock);
-
-	return ret;
-}
-
-/* clear specified crtcs (no longer pending update)
- */
-static void end_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
-	spin_lock(&priv->pending_crtcs_event.lock);
-	DBG("end: %08x", crtc_mask);
-	priv->pending_crtcs &= ~crtc_mask;
-	wake_up_all_locked(&priv->pending_crtcs_event);
-	spin_unlock(&priv->pending_crtcs_event.lock);
-}
-
-static struct msm_commit *commit_init(struct drm_atomic_state *state)
-{
-	struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL);
-
-	if (!c)
-		return NULL;
-
-	c->dev = state->dev;
-	c->state = state;
-
-	INIT_WORK(&c->work, commit_worker);
-
-	return c;
-}
-
-static void commit_destroy(struct msm_commit *c)
-{
-	end_atomic(c->dev->dev_private, c->crtc_mask);
-	kfree(c);
-}
-
 static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
 		struct drm_atomic_state *old_state)
 {
@@ -148,31 +88,37 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state)
 
 	msm_atomic_wait_for_commit_done(dev, state);
 
-	drm_atomic_helper_cleanup_planes(dev, state);
-
 	kms->funcs->complete_commit(kms, state);
+
+	drm_atomic_helper_wait_for_vblanks(dev, state);
+
+	drm_atomic_helper_commit_hw_done(state);
+
+	drm_atomic_helper_cleanup_planes(dev, state);
 }
 
 /* The (potentially) asynchronous part of the commit.  At this point
  * nothing can fail short of armageddon.
  */
-static void complete_commit(struct msm_commit *c)
+static void commit_tail(struct drm_atomic_state *state)
 {
-	struct drm_atomic_state *state = c->state;
-	struct drm_device *dev = state->dev;
+	drm_atomic_helper_wait_for_fences(state->dev, state, false);
 
-	drm_atomic_helper_wait_for_fences(dev, state, false);
+	drm_atomic_helper_wait_for_dependencies(state);
 
 	msm_atomic_commit_tail(state);
 
-	drm_atomic_state_put(state);
+	drm_atomic_helper_commit_cleanup_done(state);
 
-	commit_destroy(c);
+	drm_atomic_state_put(state);
 }
 
-static void commit_worker(struct work_struct *work)
+static void commit_work(struct work_struct *work)
 {
-	complete_commit(container_of(work, struct msm_commit, work));
+	struct drm_atomic_state *state = container_of(work,
+						      struct drm_atomic_state,
+						      commit_work);
+	commit_tail(state);
 }
 
 /**
@@ -191,17 +137,12 @@ int msm_atomic_commit(struct drm_device *dev,
 		struct drm_atomic_state *state, bool nonblock)
 {
 	struct msm_drm_private *priv = dev->dev_private;
-	struct msm_commit *c;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
 	struct drm_plane *plane;
 	struct drm_plane_state *old_plane_state, *new_plane_state;
 	int i, ret;
 
-	ret = drm_atomic_helper_prepare_planes(dev, state);
-	if (ret)
-		return ret;
-
 	/*
 	 * Note that plane->atomic_async_check() should fail if we need
 	 * to re-assign hwpipe or anything that touches global atomic
@@ -209,45 +150,39 @@ int msm_atomic_commit(struct drm_device *dev,
 	 * cases.
 	 */
 	if (state->async_update) {
+		ret = drm_atomic_helper_prepare_planes(dev, state);
+		if (ret)
+			return ret;
+
 		drm_atomic_helper_async_commit(dev, state);
 		drm_atomic_helper_cleanup_planes(dev, state);
 		return 0;
 	}
 
-	c = commit_init(state);
-	if (!c) {
-		ret = -ENOMEM;
-		goto error;
-	}
+	ret = drm_atomic_helper_setup_commit(state, nonblock);
+	if (ret)
+		return ret;
 
-	/*
-	 * Figure out what crtcs we have:
-	 */
-	for_each_new_crtc_in_state(state, crtc, crtc_state, i)
-		c->crtc_mask |= drm_crtc_mask(crtc);
+	INIT_WORK(&state->commit_work, commit_work);
 
-	/*
-	 * Figure out what fence to wait for:
-	 */
-	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
-		if ((new_plane_state->fb != old_plane_state->fb) && new_plane_state->fb) {
-			struct drm_gem_object *obj = msm_framebuffer_bo(new_plane_state->fb, 0);
-			struct msm_gem_object *msm_obj = to_msm_bo(obj);
-			struct dma_fence *fence = reservation_object_get_excl_rcu(msm_obj->resv);
-
-			drm_atomic_set_fence_for_plane(new_plane_state, fence);
-		}
+	ret = drm_atomic_helper_prepare_planes(dev, state);
+	if (ret)
+		return ret;
+
+	if (!nonblock) {
+		ret = drm_atomic_helper_wait_for_fences(dev, state, true);
+		if (ret)
+			goto error;
 	}
 
 	/*
-	 * Wait for pending updates on any of the same crtc's and then
-	 * mark our set of crtc's as busy:
+	 * This is the point of no return - everything below never fails except
+	 * when the hw goes bonghits. Which means we can commit the new state on
+	 * the software side now.
+	 *
+	 * swap driver private state while still holding state_lock
 	 */
-	ret = start_atomic(dev->dev_private, c->crtc_mask);
-	if (ret)
-		goto err_free;
-
-	BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
+	BUG_ON(drm_atomic_helper_swap_state(state, true) < 0);
 
 	/*
 	 * This is the point of no return - everything below never fails except
@@ -272,17 +207,13 @@ int msm_atomic_commit(struct drm_device *dev,
 	 */
 
 	drm_atomic_state_get(state);
-	if (nonblock) {
-		queue_work(priv->atomic_wq, &c->work);
-		return 0;
-	}
-
-	complete_commit(c);
+	if (nonblock)
+		queue_work(system_unbound_wq, &state->commit_work);
+	else
+		commit_tail(state);
 
 	return 0;
 
-err_free:
-	kfree(c);
 error:
 	drm_atomic_helper_cleanup_planes(dev, state);
 	return ret;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 1c89195da4ff..9cec74c79aa2 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -381,7 +381,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
 
 	priv->wq = alloc_ordered_workqueue("msm", 0);
 	priv->atomic_wq = alloc_ordered_workqueue("msm:atomic", 0);
-	init_waitqueue_head(&priv->pending_crtcs_event);
 
 	INIT_LIST_HEAD(&priv->inactive_list);
 	INIT_LIST_HEAD(&priv->vblank_ctrl.event_list);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 98e82230b904..2b2688896295 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -117,10 +117,6 @@ struct msm_drm_private {
 	struct workqueue_struct *wq;
 	struct workqueue_struct *atomic_wq;
 
-	/* crtcs pending async atomic updates: */
-	uint32_t pending_crtcs;
-	wait_queue_head_t pending_crtcs_event;
-
 	unsigned int num_planes;
 	struct drm_plane *planes[16];
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

  parent reply	other threads:[~2018-04-04 16:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-04 16:40 [PATCH v4 0/9] drm/msm: Switch to atomic helpers Sean Paul
     [not found] ` <20180404164048.42126-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-04-04 16:40   ` [PATCH v4 1/9] drm/msm/mdp5: Add global state as a private atomic object Sean Paul
2018-04-04 16:40   ` [PATCH v4 2/9] drm/msm/mdp5: Use the new private_obj state Sean Paul
2018-04-04 16:40   ` [PATCH v4 3/9] drm/msm: Don't subclass drm_atomic_state anymore Sean Paul
2018-04-04 16:40   ` [PATCH v4 4/9] drm/msm: Refactor complete_commit() to look more the helpers Sean Paul
2018-04-04 16:40   ` [PATCH v4 5/9] drm/msm: Move implicit sync handling to prepare_fb Sean Paul
2018-04-04 16:40   ` [PATCH v4 6/9] drm/msm: Mark the crtc->state->event consumed Sean Paul
2018-04-04 16:40   ` [PATCH v4 7/9] drm/msm: Issue queued events when disabling crtc Sean Paul
2018-04-04 16:40   ` Sean Paul [this message]
2018-04-04 16:40   ` [PATCH v4 9/9] drm/msm: Switch to atomic_helper_commit() Sean Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180404164048.42126-9-seanpaul@chromium.org \
    --to=seanpaul-f7+t8e8rja9g9huczpvpmw@public.gmane.org \
    --cc=abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=hoegsberg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.