drm/msm: rework inactive-work

Re-arrange things a bit so that we can get work requested after a bo fence passes, like pageflip, done before retiring bo's. Without any sort of bo cache in userspace, some games can trigger hundred's of transient bo's, which can cause retire to take a long time (5-10ms). Obviously we want a bo cache.. but this cleanup will make things a bit easier for atomic as well and makes things a bit cleaner. Signed-off-by: Rob Clark <robdclark@gmail.com> Acked-by: David Brown <davidb@codeaurora.org>

drm/msm: rework inactive-work
Re-arrange things a bit so that we can get work requested after a bo fence passes, like pageflip, done before retiring bo's. Without any sort of bo cache in userspace, some games can trigger hundred's of transient bo's, which can cause retire to take a long time (5-10ms). Obviously we want a bo cache.. but this cleanup will make things a bit easier for atomic as well and makes things a bit cleaner. Signed-off-by: Rob Clark <robdclark@gmail.com> Acked-by: David Brown <davidb@codeaurora.org>
Rob Clark
1 parent a862391871
Showing 6 changed files with 71 additions and 33 deletions Side-by-side Diff
drivers/gpu/drm/msm/mdp4/mdp4_crtc.c
drivers/gpu/drm/msm/msm_drv.c
drivers/gpu/drm/msm/msm_drv.h
drivers/gpu/drm/msm/msm_gem.c
drivers/gpu/drm/msm/msm_gem.h
drivers/gpu/drm/msm/msm_gpu.c
@@ -51,7 +51,7 @@
  
 	/* if there is a pending flip, these will be non-null: */
 	struct drm_pending_vblank_event *event;
-	struct work_struct pageflip_work;
+	struct msm_fence_cb pageflip_cb;
  
 	/* the fb that we currently hold a scanout ref to: */
 	struct drm_framebuffer *fb;
  
@@ -132,10 +132,10 @@
 	mdp4_write(mdp4_kms, REG_MDP4_OVERLAY_FLUSH, flush);
 }
  
-static void pageflip_worker(struct work_struct *work)
+static void pageflip_cb(struct msm_fence_cb *cb)
 {
 	struct mdp4_crtc *mdp4_crtc =
-		container_of(work, struct mdp4_crtc, pageflip_work);
+		container_of(cb, struct mdp4_crtc, pageflip_cb);
 	struct drm_crtc *crtc = &mdp4_crtc->base;
  
 	mdp4_plane_set_scanout(mdp4_crtc->plane, crtc->fb);
@@ -397,8 +397,7 @@
 	mdp4_crtc->event = event;
 	update_fb(crtc, true, new_fb);
  
-	return msm_gem_queue_inactive_work(obj,
-			&mdp4_crtc->pageflip_work);
+	return msm_gem_queue_inactive_cb(obj, &mdp4_crtc->pageflip_cb);
 }
  
 static int mdp4_crtc_set_property(struct drm_crtc *crtc,
@@ -702,7 +701,7 @@
 	ret = drm_flip_work_init(&mdp4_crtc->unref_cursor_work, 64,
 			"unref cursor", unref_cursor_worker);
  
-	INIT_WORK(&mdp4_crtc->pageflip_work, pageflip_worker);
+	INIT_FENCE_CB(&mdp4_crtc->pageflip_cb, pageflip_cb);
  
 	drm_crtc_init(dev, crtc, &mdp4_crtc_funcs);
 	drm_crtc_helper_add(crtc, &mdp4_crtc_helper_funcs);
@@ -187,6 +187,7 @@
 	init_waitqueue_head(&priv->fence_event);
  
 	INIT_LIST_HEAD(&priv->inactive_list);
+	INIT_LIST_HEAD(&priv->fence_cbs);
  
 	drm_mode_config_init(dev);
  
  
  
@@ -539,15 +540,36 @@
 	return ret;
 }
  
-/* call under struct_mutex */
+/* called from workqueue */
 void msm_update_fence(struct drm_device *dev, uint32_t fence)
 {
 	struct msm_drm_private *priv = dev->dev_private;
  
-	if (fence > priv->completed_fence) {
-		priv->completed_fence = fence;
-		wake_up_all(&priv->fence_event);
+	mutex_lock(&dev->struct_mutex);
+	priv->completed_fence = max(fence, priv->completed_fence);
+
+	while (!list_empty(&priv->fence_cbs)) {
+		struct msm_fence_cb *cb;
+
+		cb = list_first_entry(&priv->fence_cbs,
+				struct msm_fence_cb, work.entry);
+
+		if (cb->fence > priv->completed_fence)
+			break;
+
+		list_del_init(&cb->work.entry);
+		queue_work(priv->wq, &cb->work);
 	}
+
+	mutex_unlock(&dev->struct_mutex);
+
+	wake_up_all(&priv->fence_event);
+}
+
+void __msm_fence_worker(struct work_struct *work)
+{
+	struct msm_fence_cb *cb = container_of(work, struct msm_fence_cb, work);
+	cb->func(cb);
 }
  
 /*
@@ -73,6 +73,9 @@
  
 	struct workqueue_struct *wq;
  
+	/* callbacks deferred until bo is inactive: */
+	struct list_head fence_cbs;
+
 	/* registered IOMMU domains: */
 	unsigned int num_iommus;
 	struct iommu_domain *iommus[NUM_DOMAINS];
@@ -97,6 +100,20 @@
 	uint32_t pixel_format;
 };
  
+/* callback from wq once fence has passed: */
+struct msm_fence_cb {
+	struct work_struct work;
+	uint32_t fence;
+	void (*func)(struct msm_fence_cb *cb);
+};
+
+void __msm_fence_worker(struct work_struct *work);
+
+#define INIT_FENCE_CB(_cb, _func)  do {                     \
+		INIT_WORK(&(_cb)->work, __msm_fence_worker); \
+		(_cb)->func = _func;                         \
+	} while (0)
+
 /* As there are different display controller blocks depending on the
  * snapdragon version, the kms support is split out and the appropriate
  * implementation is loaded at runtime.  The kms module is responsible
@@ -160,8 +177,8 @@
 void msm_gem_prime_unpin(struct drm_gem_object *obj);
 void *msm_gem_vaddr_locked(struct drm_gem_object *obj);
 void *msm_gem_vaddr(struct drm_gem_object *obj);
-int msm_gem_queue_inactive_work(struct drm_gem_object *obj,
-		struct work_struct *work);
+int msm_gem_queue_inactive_cb(struct drm_gem_object *obj,
+		struct msm_fence_cb *cb);
 void msm_gem_move_to_active(struct drm_gem_object *obj,
 		struct msm_gpu *gpu, bool write, uint32_t fence);
 void msm_gem_move_to_inactive(struct drm_gem_object *obj);
@@ -309,7 +309,17 @@
  
 int msm_gem_get_iova(struct drm_gem_object *obj, int id, uint32_t *iova)
 {
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	int ret;
+
+	/* this is safe right now because we don't unmap until the
+	 * bo is deleted:
+	 */
+	if (msm_obj->domain[id].iova) {
+		*iova = msm_obj->domain[id].iova;
+		return 0;
+	}
+
 	mutex_lock(&obj->dev->struct_mutex);
 	ret = msm_gem_get_iova_locked(obj, id, iova);
 	mutex_unlock(&obj->dev->struct_mutex);
@@ -379,8 +389,11 @@
 	return ret;
 }
  
-int msm_gem_queue_inactive_work(struct drm_gem_object *obj,
-		struct work_struct *work)
+/* setup callback for when bo is no longer busy..
+ * TODO probably want to differentiate read vs write..
+ */
+int msm_gem_queue_inactive_cb(struct drm_gem_object *obj,
+		struct msm_fence_cb *cb)
 {
 	struct drm_device *dev = obj->dev;
 	struct msm_drm_private *priv = dev->dev_private;
  
  
@@ -388,12 +401,13 @@
 	int ret = 0;
  
 	mutex_lock(&dev->struct_mutex);
-	if (!list_empty(&work->entry)) {
+	if (!list_empty(&cb->work.entry)) {
 		ret = -EINVAL;
 	} else if (is_active(msm_obj)) {
-		list_add_tail(&work->entry, &msm_obj->inactive_work);
+		cb->fence = max(msm_obj->read_fence, msm_obj->write_fence);
+		list_add_tail(&cb->work.entry, &priv->fence_cbs);
 	} else {
-		queue_work(priv->wq, work);
+		queue_work(priv->wq, &cb->work);
 	}
 	mutex_unlock(&dev->struct_mutex);
  
@@ -426,16 +440,6 @@
 	msm_obj->write_fence = 0;
 	list_del_init(&msm_obj->mm_list);
 	list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
-
-	while (!list_empty(&msm_obj->inactive_work)) {
-		struct work_struct *work;
-
-		work = list_first_entry(&msm_obj->inactive_work,
-				struct work_struct, entry);
-
-		list_del_init(&work->entry);
-		queue_work(priv->wq, work);
-	}
 }
  
 int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op,
@@ -604,7 +608,6 @@
 	reservation_object_init(msm_obj->resv);
  
 	INIT_LIST_HEAD(&msm_obj->submit_entry);
-	INIT_LIST_HEAD(&msm_obj->inactive_work);
 	list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
  
 	*obj = &msm_obj->base;
@@ -45,9 +45,6 @@
 	 */
 	struct list_head submit_entry;
  
-	/* work defered until bo is inactive: */
-	struct list_head inactive_work;
-
 	struct page **pages;
 	struct sg_table *sgt;
 	void *vaddr;
@@ -268,6 +268,8 @@
 	struct drm_device *dev = gpu->dev;
 	uint32_t fence = gpu->funcs->last_fence(gpu);
  
+	msm_update_fence(gpu->dev, fence);
+
 	mutex_lock(&dev->struct_mutex);
  
 	while (!list_empty(&gpu->active_list)) {
@@ -286,8 +288,6 @@
 			break;
 		}
 	}
-
-	msm_update_fence(gpu->dev, fence);
  
 	mutex_unlock(&dev->struct_mutex);
 }
...	...	@@ -51,7 +51,7 @@
51	51
52	52	/* if there is a pending flip, these will be non-null: */
53	53	struct drm_pending_vblank_event *event;
54		- struct work_struct pageflip_work;
	54	+ struct msm_fence_cb pageflip_cb;
55	55
56	56	/* the fb that we currently hold a scanout ref to: */
57	57	struct drm_framebuffer *fb;
58	58
...	...	@@ -132,10 +132,10 @@
132	132	mdp4_write(mdp4_kms, REG_MDP4_OVERLAY_FLUSH, flush);
133	133	}
134	134
135		-static void pageflip_worker(struct work_struct *work)
	135	+static void pageflip_cb(struct msm_fence_cb *cb)
136	136	{
137	137	struct mdp4_crtc *mdp4_crtc =
138		- container_of(work, struct mdp4_crtc, pageflip_work);
	138	+ container_of(cb, struct mdp4_crtc, pageflip_cb);
139	139	struct drm_crtc *crtc = &mdp4_crtc->base;
140	140
141	141	mdp4_plane_set_scanout(mdp4_crtc->plane, crtc->fb);
...	...	@@ -397,8 +397,7 @@
397	397	mdp4_crtc->event = event;
398	398	update_fb(crtc, true, new_fb);
399	399
400		- return msm_gem_queue_inactive_work(obj,
401		- &mdp4_crtc->pageflip_work);
	400	+ return msm_gem_queue_inactive_cb(obj, &mdp4_crtc->pageflip_cb);
402	401	}
403	402
404	403	static int mdp4_crtc_set_property(struct drm_crtc *crtc,
...	...	@@ -702,7 +701,7 @@
702	701	ret = drm_flip_work_init(&mdp4_crtc->unref_cursor_work, 64,
703	702	"unref cursor", unref_cursor_worker);
704	703
705		- INIT_WORK(&mdp4_crtc->pageflip_work, pageflip_worker);
	704	+ INIT_FENCE_CB(&mdp4_crtc->pageflip_cb, pageflip_cb);
706	705
707	706	drm_crtc_init(dev, crtc, &mdp4_crtc_funcs);
708	707	drm_crtc_helper_add(crtc, &mdp4_crtc_helper_funcs);
...	...	@@ -187,6 +187,7 @@
187	187	init_waitqueue_head(&priv->fence_event);
188	188
189	189	INIT_LIST_HEAD(&priv->inactive_list);
	190	+ INIT_LIST_HEAD(&priv->fence_cbs);
190	191
191	192	drm_mode_config_init(dev);
192	193
193	194
194	195
...	...	@@ -539,15 +540,36 @@
539	540	return ret;
540	541	}
541	542
542		-/* call under struct_mutex */
	543	+/* called from workqueue */
543	544	void msm_update_fence(struct drm_device *dev, uint32_t fence)
544	545	{
545	546	struct msm_drm_private *priv = dev->dev_private;
546	547
547		- if (fence > priv->completed_fence) {
548		- priv->completed_fence = fence;
549		- wake_up_all(&priv->fence_event);
	548	+ mutex_lock(&dev->struct_mutex);
	549	+ priv->completed_fence = max(fence, priv->completed_fence);
	550	+
	551	+ while (!list_empty(&priv->fence_cbs)) {
	552	+ struct msm_fence_cb *cb;
	553	+
	554	+ cb = list_first_entry(&priv->fence_cbs,
	555	+ struct msm_fence_cb, work.entry);
	556	+
	557	+ if (cb->fence > priv->completed_fence)
	558	+ break;
	559	+
	560	+ list_del_init(&cb->work.entry);
	561	+ queue_work(priv->wq, &cb->work);
550	562	}
	563	+
	564	+ mutex_unlock(&dev->struct_mutex);
	565	+
	566	+ wake_up_all(&priv->fence_event);
	567	+}
	568	+
	569	+void __msm_fence_worker(struct work_struct *work)
	570	+{
	571	+ struct msm_fence_cb *cb = container_of(work, struct msm_fence_cb, work);
	572	+ cb->func(cb);
551	573	}
552	574
553	575	/*
...	...	@@ -73,6 +73,9 @@
73	73
74	74	struct workqueue_struct *wq;
75	75
	76	+ /* callbacks deferred until bo is inactive: */
	77	+ struct list_head fence_cbs;
	78	+
76	79	/* registered IOMMU domains: */
77	80	unsigned int num_iommus;
78	81	struct iommu_domain *iommus[NUM_DOMAINS];
...	...	@@ -97,6 +100,20 @@
97	100	uint32_t pixel_format;
98	101	};
99	102
	103	+/* callback from wq once fence has passed: */
	104	+struct msm_fence_cb {
	105	+ struct work_struct work;
	106	+ uint32_t fence;
	107	+ void (func)(struct msm_fence_cb cb);
	108	+};
	109	+
	110	+void __msm_fence_worker(struct work_struct *work);
	111	+
	112	+#define INIT_FENCE_CB(_cb, _func) do { \
	113	+ INIT_WORK(&(_cb)->work, __msm_fence_worker); \
	114	+ (_cb)->func = _func; \
	115	+ } while (0)
	116	+
100	117	/* As there are different display controller blocks depending on the
101	118	* snapdragon version, the kms support is split out and the appropriate
102	119	* implementation is loaded at runtime. The kms module is responsible
...	...	@@ -160,8 +177,8 @@
160	177	void msm_gem_prime_unpin(struct drm_gem_object *obj);
161	178	void msm_gem_vaddr_locked(struct drm_gem_object obj);
162	179	void msm_gem_vaddr(struct drm_gem_object obj);
163		-int msm_gem_queue_inactive_work(struct drm_gem_object *obj,
164		- struct work_struct *work);
	180	+int msm_gem_queue_inactive_cb(struct drm_gem_object *obj,
	181	+ struct msm_fence_cb *cb);
165	182	void msm_gem_move_to_active(struct drm_gem_object *obj,
166	183	struct msm_gpu *gpu, bool write, uint32_t fence);
167	184	void msm_gem_move_to_inactive(struct drm_gem_object *obj);
...	...	@@ -309,7 +309,17 @@
309	309
310	310	int msm_gem_get_iova(struct drm_gem_object obj, int id, uint32_t iova)
311	311	{
	312	+ struct msm_gem_object *msm_obj = to_msm_bo(obj);
312	313	int ret;
	314	+
	315	+ /* this is safe right now because we don't unmap until the
	316	+ * bo is deleted:
	317	+ */
	318	+ if (msm_obj->domain[id].iova) {
	319	+ *iova = msm_obj->domain[id].iova;
	320	+ return 0;
	321	+ }
	322	+
313	323	mutex_lock(&obj->dev->struct_mutex);
314	324	ret = msm_gem_get_iova_locked(obj, id, iova);
315	325	mutex_unlock(&obj->dev->struct_mutex);
...	...	@@ -379,8 +389,11 @@
379	389	return ret;
380	390	}
381	391
382		-int msm_gem_queue_inactive_work(struct drm_gem_object *obj,
383		- struct work_struct *work)
	392	+/* setup callback for when bo is no longer busy..
	393	+ * TODO probably want to differentiate read vs write..
	394	+ */
	395	+int msm_gem_queue_inactive_cb(struct drm_gem_object *obj,
	396	+ struct msm_fence_cb *cb)
384	397	{
385	398	struct drm_device *dev = obj->dev;
386	399	struct msm_drm_private *priv = dev->dev_private;
387	400
388	401
...	...	@@ -388,12 +401,13 @@
388	401	int ret = 0;
389	402
390	403	mutex_lock(&dev->struct_mutex);
391		- if (!list_empty(&work->entry)) {
	404	+ if (!list_empty(&cb->work.entry)) {
392	405	ret = -EINVAL;
393	406	} else if (is_active(msm_obj)) {
394		- list_add_tail(&work->entry, &msm_obj->inactive_work);
	407	+ cb->fence = max(msm_obj->read_fence, msm_obj->write_fence);
	408	+ list_add_tail(&cb->work.entry, &priv->fence_cbs);
395	409	} else {
396		- queue_work(priv->wq, work);
	410	+ queue_work(priv->wq, &cb->work);
397	411	}
398	412	mutex_unlock(&dev->struct_mutex);
399	413
...	...	@@ -426,16 +440,6 @@
426	440	msm_obj->write_fence = 0;
427	441	list_del_init(&msm_obj->mm_list);
428	442	list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
429		-
430		- while (!list_empty(&msm_obj->inactive_work)) {
431		- struct work_struct *work;
432		-
433		- work = list_first_entry(&msm_obj->inactive_work,
434		- struct work_struct, entry);
435		-
436		- list_del_init(&work->entry);
437		- queue_work(priv->wq, work);
438		- }
439	443	}
440	444
441	445	int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op,
...	...	@@ -604,7 +608,6 @@
604	608	reservation_object_init(msm_obj->resv);
605	609
606	610	INIT_LIST_HEAD(&msm_obj->submit_entry);
607		- INIT_LIST_HEAD(&msm_obj->inactive_work);
608	611	list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
609	612
610	613	*obj = &msm_obj->base;
...	...	@@ -45,9 +45,6 @@
45	45	*/
46	46	struct list_head submit_entry;
47	47
48		- /* work defered until bo is inactive: */
49		- struct list_head inactive_work;
50		-
51	48	struct page **pages;
52	49	struct sg_table *sgt;
53	50	void *vaddr;
...	...	@@ -268,6 +268,8 @@
268	268	struct drm_device *dev = gpu->dev;
269	269	uint32_t fence = gpu->funcs->last_fence(gpu);
270	270
	271	+ msm_update_fence(gpu->dev, fence);
	272	+
271	273	mutex_lock(&dev->struct_mutex);
272	274
273	275	while (!list_empty(&gpu->active_list)) {
...	...	@@ -286,8 +288,6 @@
286	288	break;
287	289	}
288	290	}
289		-
290		- msm_update_fence(gpu->dev, fence);
291	291
292	292	mutex_unlock(&dev->struct_mutex);
293	293	}