linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/mediatek: fixup cursor moving unsmooth issue
@ 2019-08-30  7:38 Bibby Hsieh
  2019-08-30  7:38 ` [PATCH 1/2] drm/mediatek: Only block updates to CRTCs that have a pending update Bibby Hsieh
  2019-08-30  7:38 ` [PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates Bibby Hsieh
  0 siblings, 2 replies; 5+ messages in thread
From: Bibby Hsieh @ 2019-08-30  7:38 UTC (permalink / raw)
  To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
  Cc: Philipp Zabel, YT Shen, Thierry Reding, CK Hu, linux-arm-kernel,
	tfiga, drinkcat, linux-kernel, Bibby Hsieh

These patches can fixup cursor moving is not smooth when heavy load in
webgl.

Bibby Hsieh (2):
  drm/mediatek: Only block updates to CRTCs that have a pending update
  drm/mediatek: Bypass atomic helpers for cursor updates

 drivers/gpu/drm/mediatek/mtk_drm_crtc.c  |  53 +++++-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h  |   2 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c   | 214 ++++++++++++++++++++---
 drivers/gpu/drm/mediatek/mtk_drm_drv.h   |  15 +-
 drivers/gpu/drm/mediatek/mtk_drm_plane.c |  73 +++++++-
 5 files changed, 330 insertions(+), 27 deletions(-)

-- 
2.18.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] drm/mediatek: Only block updates to CRTCs that have a pending update
  2019-08-30  7:38 [PATCH 0/2] drm/mediatek: fixup cursor moving unsmooth issue Bibby Hsieh
@ 2019-08-30  7:38 ` Bibby Hsieh
  2019-08-30  8:35   ` CK Hu
  2019-08-30  7:38 ` [PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates Bibby Hsieh
  1 sibling, 1 reply; 5+ messages in thread
From: Bibby Hsieh @ 2019-08-30  7:38 UTC (permalink / raw)
  To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
  Cc: Philipp Zabel, YT Shen, Thierry Reding, CK Hu, linux-arm-kernel,
	tfiga, drinkcat, linux-kernel, Bibby Hsieh, Daniel Kurtz

Currently we use a single mutex to allow only a single atomic
update at a time. In truth, though, we really only want to
ensure that only a single atomic update is allowed per CRTC.

In other words, for each atomic update, we only block if there
is a pending update for the CRTCs involved, and don't block if
there are only pending updates for other CRTCs.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  14 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 182 +++++++++++++++++++++---
 drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  12 +-
 3 files changed, 184 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index b55970a2869d..7697b40baac0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -5,6 +5,7 @@
 
 #include <asm/barrier.h>
 #include <drm/drmP.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
@@ -45,6 +46,8 @@ struct mtk_drm_crtc {
 	struct mtk_disp_mutex		*mutex;
 	unsigned int			ddp_comp_nr;
 	struct mtk_ddp_comp		**ddp_comp;
+
+	struct drm_crtc_state		*old_crtc_state;
 };
 
 struct mtk_crtc_state {
@@ -343,6 +346,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
 static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
 	struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
 	struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
 	unsigned int i;
@@ -382,6 +386,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
 			}
 		}
 		mtk_crtc->pending_planes = false;
+		mtk_atomic_state_put_queue(atomic_state);
 	}
 }
 
@@ -451,6 +456,7 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
 static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 				      struct drm_crtc_state *old_crtc_state)
 {
+	struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
 	struct mtk_drm_private *priv = crtc->dev->dev_private;
 	unsigned int pending_planes = 0;
@@ -469,8 +475,13 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 			pending_planes |= BIT(i);
 		}
 	}
-	if (pending_planes)
+
+	if (pending_planes) {
 		mtk_crtc->pending_planes = true;
+		drm_atomic_state_get(old_atomic_state);
+		mtk_crtc->old_crtc_state = old_crtc_state;
+	}
+
 	if (crtc->state->color_mgmt_changed)
 		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
 			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
@@ -526,6 +537,7 @@ static int mtk_drm_crtc_init(struct drm_device *drm,
 
 void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *comp)
 {
+
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
 	struct mtk_drm_private *priv = crtc->dev->dev_private;
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index c0928b69dc43..b0308a3a7483 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -31,11 +31,120 @@
 #define DRIVER_MAJOR 1
 #define DRIVER_MINOR 0
 
-static void mtk_atomic_schedule(struct mtk_drm_private *private,
+struct mtk_atomic_state {
+	struct drm_atomic_state base;
+	struct list_head list;
+	struct work_struct work;
+};
+
+static inline struct mtk_atomic_state *to_mtk_state(struct drm_atomic_state *s)
+{
+	return container_of(s, struct mtk_atomic_state, base);
+}
+
+void mtk_atomic_state_put_queue(struct drm_atomic_state *state)
+{
+	struct drm_device *drm = state->dev;
+	struct mtk_drm_private *mtk_drm = drm->dev_private;
+	struct mtk_atomic_state *mtk_state = to_mtk_state(state);
+	unsigned long flags;
+
+	spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+	list_add_tail(&mtk_state->list, &mtk_drm->unreference.list);
+	spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+
+	schedule_work(&mtk_drm->unreference.work);
+}
+
+static uint32_t mtk_atomic_crtc_mask(struct drm_device *drm,
+				     struct drm_atomic_state *state)
+{
+	uint32_t crtc_mask;
+	int i;
+
+	for (i = 0, crtc_mask = 0; i < drm->mode_config.num_crtc; i++) {
+		struct drm_crtc *crtc = state->crtcs[i].ptr;
+
+		if (crtc)
+			crtc_mask |= (1 << drm_crtc_index(crtc));
+	}
+
+	return crtc_mask;
+}
+
+/*
+ * Block until specified crtcs are no longer pending update, and atomically
+ * mark them as pending update.
+ */
+static int mtk_atomic_get_crtcs(struct drm_device *drm,
+				struct drm_atomic_state *state)
+{
+	struct mtk_drm_private *private = drm->dev_private;
+	uint32_t crtc_mask;
+	int ret;
+
+	crtc_mask = mtk_atomic_crtc_mask(drm, state);
+
+	/*
+	 * Wait for all pending updates to complete for the set of crtcs being
+	 * changed in this atomic commit
+	 */
+	spin_lock(&private->commit.crtcs_event.lock);
+	ret = wait_event_interruptible_locked(private->commit.crtcs_event,
+			!(private->commit.crtcs & crtc_mask));
+	if (ret == 0)
+		private->commit.crtcs |= crtc_mask;
+	spin_unlock(&private->commit.crtcs_event.lock);
+
+	return ret;
+}
+
+/*
+ * Mark specified crtcs as no longer pending update.
+ */
+static void mtk_atomic_put_crtcs(struct drm_device *drm,
+				 struct drm_atomic_state *state)
+{
+	struct mtk_drm_private *private = drm->dev_private;
+	uint32_t crtc_mask;
+
+	crtc_mask = mtk_atomic_crtc_mask(drm, state);
+
+	spin_lock(&private->commit.crtcs_event.lock);
+	private->commit.crtcs &= ~crtc_mask;
+	wake_up_all_locked(&private->commit.crtcs_event);
+	spin_unlock(&private->commit.crtcs_event.lock);
+}
+
+static void mtk_unreference_work(struct work_struct *work)
+{
+	struct mtk_drm_private *mtk_drm = container_of(work,
+			struct mtk_drm_private, unreference.work);
+	unsigned long flags;
+	struct mtk_atomic_state *state, *tmp;
+
+	/*
+	 * framebuffers cannot be unreferenced in atomic context.
+	 * Therefore, only hold the spinlock when iterating unreference_list,
+	 * and drop it when doing the unreference.
+	 */
+	spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+	list_for_each_entry_safe(state, tmp, &mtk_drm->unreference.list, list) {
+		list_del(&state->list);
+		spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+		drm_atomic_state_put(&state->base);
+		spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+	}
+	spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+}
+
+
+static void mtk_atomic_schedule(struct drm_device *drm,
 				struct drm_atomic_state *state)
 {
-	private->commit.state = state;
-	schedule_work(&private->commit.work);
+	struct mtk_atomic_state *mtk_state = to_mtk_state(state);
+
+	schedule_work(&mtk_state->work);
 }
 
 static void mtk_atomic_wait_for_fences(struct drm_atomic_state *state)
@@ -48,13 +157,10 @@ static void mtk_atomic_wait_for_fences(struct drm_atomic_state *state)
 		mtk_fb_wait(new_plane_state->fb);
 }
 
-static void mtk_atomic_complete(struct mtk_drm_private *private,
+static void mtk_atomic_complete(struct drm_device *drm,
 				struct drm_atomic_state *state)
 {
-	struct drm_device *drm = private->drm;
-
 	mtk_atomic_wait_for_fences(state);
-
 	/*
 	 * Mediatek drm supports runtime PM, so plane registers cannot be
 	 * written when their crtc is disabled.
@@ -77,53 +183,86 @@ static void mtk_atomic_complete(struct mtk_drm_private *private,
 	drm_atomic_helper_wait_for_vblanks(drm, state);
 
 	drm_atomic_helper_cleanup_planes(drm, state);
+	mtk_atomic_put_crtcs(drm, state);
+
 	drm_atomic_state_put(state);
 }
 
 static void mtk_atomic_work(struct work_struct *work)
 {
-	struct mtk_drm_private *private = container_of(work,
-			struct mtk_drm_private, commit.work);
+	struct mtk_atomic_state *mtk_state = container_of(work,
+			struct mtk_atomic_state, work);
+	struct drm_atomic_state *state = &mtk_state->base;
+	struct drm_device *drm = state->dev;
 
-	mtk_atomic_complete(private, private->commit.state);
+	mtk_atomic_complete(drm, state);
 }
 
 static int mtk_atomic_commit(struct drm_device *drm,
 			     struct drm_atomic_state *state,
 			     bool async)
 {
-	struct mtk_drm_private *private = drm->dev_private;
 	int ret;
 
 	ret = drm_atomic_helper_prepare_planes(drm, state);
 	if (ret)
 		return ret;
 
-	mutex_lock(&private->commit.lock);
-	flush_work(&private->commit.work);
+	ret = mtk_atomic_get_crtcs(drm, state);
+	if (ret) {
+		drm_atomic_helper_cleanup_planes(drm, state);
+		return ret;
+	}
 
 	ret = drm_atomic_helper_swap_state(state, true);
 	if (ret) {
-		mutex_unlock(&private->commit.lock);
 		drm_atomic_helper_cleanup_planes(drm, state);
 		return ret;
 	}
 
 	drm_atomic_state_get(state);
 	if (async)
-		mtk_atomic_schedule(private, state);
+		mtk_atomic_schedule(drm, state);
 	else
-		mtk_atomic_complete(private, state);
-
-	mutex_unlock(&private->commit.lock);
+		mtk_atomic_complete(drm, state);
 
 	return 0;
 }
 
+static struct drm_atomic_state *mtk_drm_atomic_state_alloc(
+		struct drm_device *dev)
+{
+	struct mtk_atomic_state *mtk_state;
+
+	mtk_state = kzalloc(sizeof(*mtk_state), GFP_KERNEL);
+	if (!mtk_state)
+		return NULL;
+
+	if (drm_atomic_state_init(dev, &mtk_state->base) < 0) {
+		kfree(mtk_state);
+		return NULL;
+	}
+
+	INIT_LIST_HEAD(&mtk_state->list);
+	INIT_WORK(&mtk_state->work, mtk_atomic_work);
+
+	return &mtk_state->base;
+}
+
+static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
+{
+	struct mtk_atomic_state *mtk_state = to_mtk_state(state);
+
+	drm_atomic_state_default_release(state);
+	kfree(mtk_state);
+}
+
 static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
 	.fb_create = mtk_drm_mode_fb_create,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = mtk_atomic_commit,
+	.atomic_state_alloc = mtk_drm_atomic_state_alloc,
+	.atomic_state_free = mtk_drm_atomic_state_free
 };
 
 static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
@@ -319,6 +458,11 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 	drm_kms_helper_poll_init(drm);
 	drm_mode_config_reset(drm);
 
+	INIT_WORK(&private->unreference.work, mtk_unreference_work);
+	INIT_LIST_HEAD(&private->unreference.list);
+	spin_lock_init(&private->unreference.lock);
+	init_waitqueue_head(&private->commit.crtcs_event);
+
 	return 0;
 
 err_component_unbind:
@@ -504,8 +648,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
 	if (!private)
 		return -ENOMEM;
 
-	mutex_init(&private->commit.lock);
-	INIT_WORK(&private->commit.work, mtk_atomic_work);
 	private->data = of_device_get_match_data(dev);
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 823ba4081c18..0934f83b860d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -48,12 +48,16 @@ struct mtk_drm_private {
 	const struct mtk_mmsys_driver_data *data;
 
 	struct {
-		struct drm_atomic_state *state;
-		struct work_struct work;
-		struct mutex lock;
+		uint32_t crtcs;
+		wait_queue_head_t crtcs_event;
 	} commit;
 
 	struct drm_atomic_state *suspend_state;
+	struct {
+		struct work_struct	work;
+		struct list_head	list;
+		spinlock_t		lock;
+	} unreference;
 };
 
 extern struct platform_driver mtk_ddp_driver;
@@ -64,4 +68,6 @@ extern struct platform_driver mtk_dpi_driver;
 extern struct platform_driver mtk_dsi_driver;
 extern struct platform_driver mtk_mipi_tx_driver;
 
+void mtk_atomic_state_put_queue(struct drm_atomic_state *state);
+
 #endif /* MTK_DRM_DRV_H */
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates
  2019-08-30  7:38 [PATCH 0/2] drm/mediatek: fixup cursor moving unsmooth issue Bibby Hsieh
  2019-08-30  7:38 ` [PATCH 1/2] drm/mediatek: Only block updates to CRTCs that have a pending update Bibby Hsieh
@ 2019-08-30  7:38 ` Bibby Hsieh
  2019-08-31  0:04   ` CK Hu
  1 sibling, 1 reply; 5+ messages in thread
From: Bibby Hsieh @ 2019-08-30  7:38 UTC (permalink / raw)
  To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
  Cc: Philipp Zabel, YT Shen, Thierry Reding, CK Hu, linux-arm-kernel,
	tfiga, drinkcat, linux-kernel, Bibby Hsieh, Daniel Kurtz

Moving the driver to atomic helpers regressed cursor responsiveness,
because cursor updates need their own atomic commits, which have to be
serialized with other commits, that might include fence waits. To avoid
this, in certain conditions, we can bypass the atomic helpers for legacy
cursor update IOCTLs. Currently the conditions are:
 - no asynchronous mode setting commit pending,
 - no asynchronous commit that updates the cursor plane is pending.
With the above two conditions met, we know that the manual cursor state
update will not conflict with any scheduled update.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")

Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c  | 41 ++++++++++++-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h  |  2 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c   | 34 ++++++++++-
 drivers/gpu/drm/mediatek/mtk_drm_drv.h   |  3 +
 drivers/gpu/drm/mediatek/mtk_drm_plane.c | 73 +++++++++++++++++++++++-
 5 files changed, 148 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 7697b40baac0..092e502ed27b 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -40,7 +40,7 @@ struct mtk_drm_crtc {
 	struct drm_plane		*planes;
 	unsigned int			layer_nr;
 	bool				pending_planes;
-
+	bool                            cursor_update;
 	void __iomem			*config_regs;
 	const struct mtk_mmsys_reg_data *mmsys_reg_data;
 	struct mtk_disp_mutex		*mutex;
@@ -386,8 +386,45 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
 			}
 		}
 		mtk_crtc->pending_planes = false;
-		mtk_atomic_state_put_queue(atomic_state);
+		if (!mtk_crtc->cursor_update)
+			mtk_atomic_state_put_queue(atomic_state);
+		mtk_crtc->cursor_update = false;
+	}
+}
+
+void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
+				struct drm_plane_state *plane_state)
+{
+	struct mtk_drm_private *priv = crtc->dev->dev_private;
+	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	const struct drm_plane_helper_funcs *plane_helper_funcs =
+			plane->helper_private;
+	int i;
+
+	if (!mtk_crtc->enabled)
+		return;
+
+	mutex_lock(&priv->hw_lock);
+	plane_helper_funcs->atomic_update(plane, plane_state);
+	for (i = 0; i < mtk_crtc->layer_nr; i++) {
+		struct drm_plane *plane = &mtk_crtc->planes[i];
+		struct mtk_plane_state *plane_state;
+
+		plane_state = to_mtk_plane_state(plane->state);
+		if (plane_state->pending.dirty) {
+			plane_state->pending.config = true;
+			plane_state->pending.dirty = false;
+		}
+	}
+	mtk_crtc->pending_planes = true;
+	mtk_crtc->cursor_update = true;
+
+	if (priv->data->shadow_register) {
+		mtk_disp_mutex_acquire(mtk_crtc->mutex);
+		mtk_crtc_ddp_config(crtc);
+		mtk_disp_mutex_release(mtk_crtc->mutex);
 	}
+	mutex_unlock(&priv->hw_lock);
 }
 
 static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
index fcc134eb00c9..46e903be68ec 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
@@ -19,5 +19,7 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *comp);
 int mtk_drm_crtc_create(struct drm_device *drm_dev,
 			const enum mtk_ddp_comp_id *path,
 			unsigned int path_len);
+void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
+				struct drm_plane_state *plane_state);
 
 #endif /* MTK_DRM_CRTC_H */
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index b0308a3a7483..ca754b954c7c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -80,11 +80,36 @@ static int mtk_atomic_get_crtcs(struct drm_device *drm,
 				struct drm_atomic_state *state)
 {
 	struct mtk_drm_private *private = drm->dev_private;
-	uint32_t crtc_mask;
+	uint32_t crtc_mask, needs_modeset, has_cursor_plane;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int i;
 	int ret;
 
 	crtc_mask = mtk_atomic_crtc_mask(drm, state);
 
+	/*
+	 * Allow cursor updates unless there is a pending modeset or cursor
+	 * plane update.
+	 */
+	needs_modeset = 0;
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+		if (crtc && drm_atomic_crtc_needs_modeset(crtc_state)) {
+			needs_modeset |= (1 << drm_crtc_index(crtc));
+			break;
+		}
+	}
+
+	has_cursor_plane = 0;
+	for_each_new_plane_in_state(state, plane, plane_state, i) {
+		if (crtc && plane->crtc && plane == plane->crtc->cursor) {
+			has_cursor_plane |= (1 << drm_crtc_index(crtc));
+			break;
+		}
+	}
+
 	/*
 	 * Wait for all pending updates to complete for the set of crtcs being
 	 * changed in this atomic commit
@@ -94,6 +119,8 @@ static int mtk_atomic_get_crtcs(struct drm_device *drm,
 			!(private->commit.crtcs & crtc_mask));
 	if (ret == 0)
 		private->commit.crtcs |= crtc_mask;
+
+	private->commit.flush_for_cursor |= needs_modeset | has_cursor_plane;
 	spin_unlock(&private->commit.crtcs_event.lock);
 
 	return ret;
@@ -112,6 +139,7 @@ static void mtk_atomic_put_crtcs(struct drm_device *drm,
 
 	spin_lock(&private->commit.crtcs_event.lock);
 	private->commit.crtcs &= ~crtc_mask;
+	private->commit.flush_for_cursor &= ~crtc_mask;
 	wake_up_all_locked(&private->commit.crtcs_event);
 	spin_unlock(&private->commit.crtcs_event.lock);
 }
@@ -160,6 +188,7 @@ static void mtk_atomic_wait_for_fences(struct drm_atomic_state *state)
 static void mtk_atomic_complete(struct drm_device *drm,
 				struct drm_atomic_state *state)
 {
+	struct mtk_drm_private *private = drm->dev_private;
 	mtk_atomic_wait_for_fences(state);
 	/*
 	 * Mediatek drm supports runtime PM, so plane registers cannot be
@@ -175,10 +204,12 @@ static void mtk_atomic_complete(struct drm_device *drm,
 	 *
 	 * See the kerneldoc entries for these three functions for more details.
 	 */
+	mutex_lock(&private->hw_lock);
 	drm_atomic_helper_commit_modeset_disables(drm, state);
 	drm_atomic_helper_commit_modeset_enables(drm, state);
 	drm_atomic_helper_commit_planes(drm, state,
 					DRM_PLANE_COMMIT_ACTIVE_ONLY);
+	mutex_unlock(&private->hw_lock);
 
 	drm_atomic_helper_wait_for_vblanks(drm, state);
 
@@ -462,6 +493,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 	INIT_LIST_HEAD(&private->unreference.list);
 	spin_lock_init(&private->unreference.lock);
 	init_waitqueue_head(&private->commit.crtcs_event);
+	mutex_init(&private->hw_lock);
 
 	return 0;
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 0934f83b860d..4a4c989803d9 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -50,6 +50,7 @@ struct mtk_drm_private {
 	struct {
 		uint32_t crtcs;
 		wait_queue_head_t crtcs_event;
+		uint32_t flush_for_cursor;
 	} commit;
 
 	struct drm_atomic_state *suspend_state;
@@ -58,6 +59,8 @@ struct mtk_drm_private {
 		struct list_head	list;
 		spinlock_t		lock;
 	} unreference;
+
+	struct mutex hw_lock;
 };
 
 extern struct platform_driver mtk_ddp_driver;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index f2ef83aed6f9..59dbdaf07425 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -7,6 +7,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_atomic_uapi.h>
 #include <drm/drm_plane_helper.h>
 
 #include "mtk_drm_crtc.h"
@@ -69,8 +70,71 @@ static void mtk_drm_plane_destroy_state(struct drm_plane *plane,
 	kfree(to_mtk_plane_state(state));
 }
 
+static int mtk_plane_cursor_update(struct drm_plane *plane,
+				   struct drm_crtc *crtc,
+				   struct drm_framebuffer *fb,
+				   int crtc_x, int crtc_y,
+				   unsigned int crtc_w, unsigned int crtc_h,
+				   uint32_t src_x, uint32_t src_y,
+				   uint32_t src_w, uint32_t src_h)
+{
+	struct drm_plane_state *plane_state;
+	const struct drm_plane_helper_funcs *plane_helper_funcs =
+			plane->helper_private;
+	int ret;
+
+	plane_state = plane->funcs->atomic_duplicate_state(plane);
+
+	plane_state->crtc_x = crtc_x;
+	plane_state->crtc_y = crtc_y;
+	plane_state->crtc_h = crtc_h;
+	plane_state->crtc_w = crtc_w;
+	plane_state->src_x = src_x;
+	plane_state->src_y = src_y;
+	plane_state->src_h = src_h;
+	plane_state->src_w = src_w;
+
+	drm_atomic_set_fb_for_plane(plane_state, fb);
+
+	ret = plane_helper_funcs->atomic_check(plane, plane_state);
+	if (ret)
+		goto err_destroy;
+
+	swap(plane_state, plane->state);
+
+	mtk_drm_crtc_cursor_update(crtc, plane, plane_state);
+
+err_destroy:
+	plane->funcs->atomic_destroy_state(plane, plane_state);
+	return ret;
+}
+
+static int mtk_plane_update(struct drm_plane *plane,
+			    struct drm_crtc *crtc,
+			    struct drm_framebuffer *fb,
+			    int crtc_x, int crtc_y,
+			    unsigned int crtc_w, unsigned int crtc_h,
+			    uint32_t src_x, uint32_t src_y,
+			    uint32_t src_w, uint32_t src_h,
+			    struct drm_modeset_acquire_ctx *ctx)
+{
+	struct mtk_drm_private *private = plane->dev->dev_private;
+	uint32_t crtc_mask = (1 << drm_crtc_index(crtc));
+
+	if (crtc && plane == crtc->cursor &&
+	    plane->state->crtc == crtc &&
+	    !(private->commit.flush_for_cursor & crtc_mask))
+		return mtk_plane_cursor_update(plane, crtc, fb,
+				crtc_x, crtc_y, crtc_w, crtc_h,
+				src_x, src_y, src_w, src_h);
+
+	return drm_atomic_helper_update_plane(plane, crtc, fb,
+					      crtc_x, crtc_y, crtc_w, crtc_h,
+					      src_x, src_y, src_w, src_h, ctx);
+}
+
 static const struct drm_plane_funcs mtk_plane_funcs = {
-	.update_plane = drm_atomic_helper_update_plane,
+	.update_plane = mtk_plane_update,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = drm_plane_cleanup,
 	.reset = mtk_plane_reset,
@@ -90,7 +154,12 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
 	if (!state->crtc)
 		return 0;
 
-	crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
+	if (state->state)
+		crtc_state = drm_atomic_get_existing_crtc_state(state->state,
+								state->crtc);
+	else /* Special case for asynchronous cursor updates. */
+		crtc_state = state->crtc->state;
+
 	if (IS_ERR(crtc_state))
 		return PTR_ERR(crtc_state);
 
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] drm/mediatek: Only block updates to CRTCs that have a pending update
  2019-08-30  7:38 ` [PATCH 1/2] drm/mediatek: Only block updates to CRTCs that have a pending update Bibby Hsieh
@ 2019-08-30  8:35   ` CK Hu
  0 siblings, 0 replies; 5+ messages in thread
From: CK Hu @ 2019-08-30  8:35 UTC (permalink / raw)
  To: Bibby Hsieh
  Cc: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
	linux-mediatek, Philipp Zabel, YT Shen, Thierry Reding,
	linux-arm-kernel, tfiga, drinkcat, linux-kernel, Daniel Kurtz

Hi, Bibby:

On Fri, 2019-08-30 at 15:38 +0800, Bibby Hsieh wrote:
> Currently we use a single mutex to allow only a single atomic
> update at a time. In truth, though, we really only want to
> ensure that only a single atomic update is allowed per CRTC.
> 
> In other words, for each atomic update, we only block if there
> is a pending update for the CRTCs involved, and don't block if
> there are only pending updates for other CRTCs.

I don't know why this patch is so complicated. The original problem is
that one mutex for whole drm would block different crtc. So I think each
crtc has its own mutex would solve this problem and we need not the
event waiting. Do I miss something?

Regards,
CK

> 
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> 
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  14 +-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 182 +++++++++++++++++++++---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  12 +-
>  3 files changed, 184 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index b55970a2869d..7697b40baac0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -5,6 +5,7 @@
>  
>  #include <asm/barrier.h>
>  #include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_probe_helper.h>
> @@ -45,6 +46,8 @@ struct mtk_drm_crtc {
>  	struct mtk_disp_mutex		*mutex;
>  	unsigned int			ddp_comp_nr;
>  	struct mtk_ddp_comp		**ddp_comp;
> +
> +	struct drm_crtc_state		*old_crtc_state;
>  };
>  
>  struct mtk_crtc_state {
> @@ -343,6 +346,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
>  static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
>  {
>  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> +	struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
>  	struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
>  	struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
>  	unsigned int i;
> @@ -382,6 +386,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
>  			}
>  		}
>  		mtk_crtc->pending_planes = false;
> +		mtk_atomic_state_put_queue(atomic_state);
>  	}
>  }
>  
> @@ -451,6 +456,7 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
>  static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
>  				      struct drm_crtc_state *old_crtc_state)
>  {
> +	struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
>  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
>  	struct mtk_drm_private *priv = crtc->dev->dev_private;
>  	unsigned int pending_planes = 0;
> @@ -469,8 +475,13 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
>  			pending_planes |= BIT(i);
>  		}
>  	}
> -	if (pending_planes)
> +
> +	if (pending_planes) {
>  		mtk_crtc->pending_planes = true;
> +		drm_atomic_state_get(old_atomic_state);
> +		mtk_crtc->old_crtc_state = old_crtc_state;
> +	}
> +
>  	if (crtc->state->color_mgmt_changed)
>  		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
>  			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> @@ -526,6 +537,7 @@ static int mtk_drm_crtc_init(struct drm_device *drm,
>  
>  void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *comp)
>  {
> +
>  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
>  	struct mtk_drm_private *priv = crtc->dev->dev_private;
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index c0928b69dc43..b0308a3a7483 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -31,11 +31,120 @@
>  #define DRIVER_MAJOR 1
>  #define DRIVER_MINOR 0
>  
> -static void mtk_atomic_schedule(struct mtk_drm_private *private,
> +struct mtk_atomic_state {
> +	struct drm_atomic_state base;
> +	struct list_head list;
> +	struct work_struct work;
> +};
> +
> +static inline struct mtk_atomic_state *to_mtk_state(struct drm_atomic_state *s)
> +{
> +	return container_of(s, struct mtk_atomic_state, base);
> +}
> +
> +void mtk_atomic_state_put_queue(struct drm_atomic_state *state)
> +{
> +	struct drm_device *drm = state->dev;
> +	struct mtk_drm_private *mtk_drm = drm->dev_private;
> +	struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> +	list_add_tail(&mtk_state->list, &mtk_drm->unreference.list);
> +	spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> +
> +	schedule_work(&mtk_drm->unreference.work);
> +}
> +
> +static uint32_t mtk_atomic_crtc_mask(struct drm_device *drm,
> +				     struct drm_atomic_state *state)
> +{
> +	uint32_t crtc_mask;
> +	int i;
> +
> +	for (i = 0, crtc_mask = 0; i < drm->mode_config.num_crtc; i++) {
> +		struct drm_crtc *crtc = state->crtcs[i].ptr;
> +
> +		if (crtc)
> +			crtc_mask |= (1 << drm_crtc_index(crtc));
> +	}
> +
> +	return crtc_mask;
> +}
> +
> +/*
> + * Block until specified crtcs are no longer pending update, and atomically
> + * mark them as pending update.
> + */
> +static int mtk_atomic_get_crtcs(struct drm_device *drm,
> +				struct drm_atomic_state *state)
> +{
> +	struct mtk_drm_private *private = drm->dev_private;
> +	uint32_t crtc_mask;
> +	int ret;
> +
> +	crtc_mask = mtk_atomic_crtc_mask(drm, state);
> +
> +	/*
> +	 * Wait for all pending updates to complete for the set of crtcs being
> +	 * changed in this atomic commit
> +	 */
> +	spin_lock(&private->commit.crtcs_event.lock);
> +	ret = wait_event_interruptible_locked(private->commit.crtcs_event,
> +			!(private->commit.crtcs & crtc_mask));
> +	if (ret == 0)
> +		private->commit.crtcs |= crtc_mask;
> +	spin_unlock(&private->commit.crtcs_event.lock);
> +
> +	return ret;
> +}
> +
> +/*
> + * Mark specified crtcs as no longer pending update.
> + */
> +static void mtk_atomic_put_crtcs(struct drm_device *drm,
> +				 struct drm_atomic_state *state)
> +{
> +	struct mtk_drm_private *private = drm->dev_private;
> +	uint32_t crtc_mask;
> +
> +	crtc_mask = mtk_atomic_crtc_mask(drm, state);
> +
> +	spin_lock(&private->commit.crtcs_event.lock);
> +	private->commit.crtcs &= ~crtc_mask;
> +	wake_up_all_locked(&private->commit.crtcs_event);
> +	spin_unlock(&private->commit.crtcs_event.lock);
> +}
> +
> +static void mtk_unreference_work(struct work_struct *work)
> +{
> +	struct mtk_drm_private *mtk_drm = container_of(work,
> +			struct mtk_drm_private, unreference.work);
> +	unsigned long flags;
> +	struct mtk_atomic_state *state, *tmp;
> +
> +	/*
> +	 * framebuffers cannot be unreferenced in atomic context.
> +	 * Therefore, only hold the spinlock when iterating unreference_list,
> +	 * and drop it when doing the unreference.
> +	 */
> +	spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> +	list_for_each_entry_safe(state, tmp, &mtk_drm->unreference.list, list) {
> +		list_del(&state->list);
> +		spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> +		drm_atomic_state_put(&state->base);
> +		spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> +	}
> +	spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> +}
> +
> +
> +static void mtk_atomic_schedule(struct drm_device *drm,
>  				struct drm_atomic_state *state)
>  {
> -	private->commit.state = state;
> -	schedule_work(&private->commit.work);
> +	struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> +
> +	schedule_work(&mtk_state->work);
>  }
>  
>  static void mtk_atomic_wait_for_fences(struct drm_atomic_state *state)
> @@ -48,13 +157,10 @@ static void mtk_atomic_wait_for_fences(struct drm_atomic_state *state)
>  		mtk_fb_wait(new_plane_state->fb);
>  }
>  
> -static void mtk_atomic_complete(struct mtk_drm_private *private,
> +static void mtk_atomic_complete(struct drm_device *drm,
>  				struct drm_atomic_state *state)
>  {
> -	struct drm_device *drm = private->drm;
> -
>  	mtk_atomic_wait_for_fences(state);
> -
>  	/*
>  	 * Mediatek drm supports runtime PM, so plane registers cannot be
>  	 * written when their crtc is disabled.
> @@ -77,53 +183,86 @@ static void mtk_atomic_complete(struct mtk_drm_private *private,
>  	drm_atomic_helper_wait_for_vblanks(drm, state);
>  
>  	drm_atomic_helper_cleanup_planes(drm, state);
> +	mtk_atomic_put_crtcs(drm, state);
> +
>  	drm_atomic_state_put(state);
>  }
>  
>  static void mtk_atomic_work(struct work_struct *work)
>  {
> -	struct mtk_drm_private *private = container_of(work,
> -			struct mtk_drm_private, commit.work);
> +	struct mtk_atomic_state *mtk_state = container_of(work,
> +			struct mtk_atomic_state, work);
> +	struct drm_atomic_state *state = &mtk_state->base;
> +	struct drm_device *drm = state->dev;
>  
> -	mtk_atomic_complete(private, private->commit.state);
> +	mtk_atomic_complete(drm, state);
>  }
>  
>  static int mtk_atomic_commit(struct drm_device *drm,
>  			     struct drm_atomic_state *state,
>  			     bool async)
>  {
> -	struct mtk_drm_private *private = drm->dev_private;
>  	int ret;
>  
>  	ret = drm_atomic_helper_prepare_planes(drm, state);
>  	if (ret)
>  		return ret;
>  
> -	mutex_lock(&private->commit.lock);
> -	flush_work(&private->commit.work);
> +	ret = mtk_atomic_get_crtcs(drm, state);
> +	if (ret) {
> +		drm_atomic_helper_cleanup_planes(drm, state);
> +		return ret;
> +	}
>  
>  	ret = drm_atomic_helper_swap_state(state, true);
>  	if (ret) {
> -		mutex_unlock(&private->commit.lock);
>  		drm_atomic_helper_cleanup_planes(drm, state);
>  		return ret;
>  	}
>  
>  	drm_atomic_state_get(state);
>  	if (async)
> -		mtk_atomic_schedule(private, state);
> +		mtk_atomic_schedule(drm, state);
>  	else
> -		mtk_atomic_complete(private, state);
> -
> -	mutex_unlock(&private->commit.lock);
> +		mtk_atomic_complete(drm, state);
>  
>  	return 0;
>  }
>  
> +static struct drm_atomic_state *mtk_drm_atomic_state_alloc(
> +		struct drm_device *dev)
> +{
> +	struct mtk_atomic_state *mtk_state;
> +
> +	mtk_state = kzalloc(sizeof(*mtk_state), GFP_KERNEL);
> +	if (!mtk_state)
> +		return NULL;
> +
> +	if (drm_atomic_state_init(dev, &mtk_state->base) < 0) {
> +		kfree(mtk_state);
> +		return NULL;
> +	}
> +
> +	INIT_LIST_HEAD(&mtk_state->list);
> +	INIT_WORK(&mtk_state->work, mtk_atomic_work);
> +
> +	return &mtk_state->base;
> +}
> +
> +static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
> +{
> +	struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> +
> +	drm_atomic_state_default_release(state);
> +	kfree(mtk_state);
> +}
> +
>  static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
>  	.fb_create = mtk_drm_mode_fb_create,
>  	.atomic_check = drm_atomic_helper_check,
>  	.atomic_commit = mtk_atomic_commit,
> +	.atomic_state_alloc = mtk_drm_atomic_state_alloc,
> +	.atomic_state_free = mtk_drm_atomic_state_free
>  };
>  
>  static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
> @@ -319,6 +458,11 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>  	drm_kms_helper_poll_init(drm);
>  	drm_mode_config_reset(drm);
>  
> +	INIT_WORK(&private->unreference.work, mtk_unreference_work);
> +	INIT_LIST_HEAD(&private->unreference.list);
> +	spin_lock_init(&private->unreference.lock);
> +	init_waitqueue_head(&private->commit.crtcs_event);
> +
>  	return 0;
>  
>  err_component_unbind:
> @@ -504,8 +648,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
>  	if (!private)
>  		return -ENOMEM;
>  
> -	mutex_init(&private->commit.lock);
> -	INIT_WORK(&private->commit.work, mtk_atomic_work);
>  	private->data = of_device_get_match_data(dev);
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 823ba4081c18..0934f83b860d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -48,12 +48,16 @@ struct mtk_drm_private {
>  	const struct mtk_mmsys_driver_data *data;
>  
>  	struct {
> -		struct drm_atomic_state *state;
> -		struct work_struct work;
> -		struct mutex lock;
> +		uint32_t crtcs;
> +		wait_queue_head_t crtcs_event;
>  	} commit;
>  
>  	struct drm_atomic_state *suspend_state;
> +	struct {
> +		struct work_struct	work;
> +		struct list_head	list;
> +		spinlock_t		lock;
> +	} unreference;
>  };
>  
>  extern struct platform_driver mtk_ddp_driver;
> @@ -64,4 +68,6 @@ extern struct platform_driver mtk_dpi_driver;
>  extern struct platform_driver mtk_dsi_driver;
>  extern struct platform_driver mtk_mipi_tx_driver;
>  
> +void mtk_atomic_state_put_queue(struct drm_atomic_state *state);
> +
>  #endif /* MTK_DRM_DRV_H */



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates
  2019-08-30  7:38 ` [PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates Bibby Hsieh
@ 2019-08-31  0:04   ` CK Hu
  0 siblings, 0 replies; 5+ messages in thread
From: CK Hu @ 2019-08-31  0:04 UTC (permalink / raw)
  To: Bibby Hsieh
  Cc: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
	linux-mediatek, Philipp Zabel, YT Shen, Thierry Reding,
	linux-arm-kernel, tfiga, drinkcat, linux-kernel, Daniel Kurtz

Hi, Bibby:

On Fri, 2019-08-30 at 15:38 +0800, Bibby Hsieh wrote:
> Moving the driver to atomic helpers regressed cursor responsiveness,
> because cursor updates need their own atomic commits, which have to be
> serialized with other commits, that might include fence waits. To avoid
> this, in certain conditions, we can bypass the atomic helpers for legacy
> cursor update IOCTLs. Currently the conditions are:
>  - no asynchronous mode setting commit pending,
>  - no asynchronous commit that updates the cursor plane is pending.
> With the above two conditions met, we know that the manual cursor state
> update will not conflict with any scheduled update.
> 
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> 
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c  | 41 ++++++++++++-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h  |  2 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c   | 34 ++++++++++-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h   |  3 +
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c | 73 +++++++++++++++++++++++-
>  5 files changed, 148 insertions(+), 5 deletions(-)
> 

[snip]

> +
> +static int mtk_plane_update(struct drm_plane *plane,
> +			    struct drm_crtc *crtc,
> +			    struct drm_framebuffer *fb,
> +			    int crtc_x, int crtc_y,
> +			    unsigned int crtc_w, unsigned int crtc_h,
> +			    uint32_t src_x, uint32_t src_y,
> +			    uint32_t src_w, uint32_t src_h,
> +			    struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct mtk_drm_private *private = plane->dev->dev_private;
> +	uint32_t crtc_mask = (1 << drm_crtc_index(crtc));
> +
> +	if (crtc && plane == crtc->cursor &&
> +	    plane->state->crtc == crtc &&
> +	    !(private->commit.flush_for_cursor & crtc_mask))
> +		return mtk_plane_cursor_update(plane, crtc, fb,
> +				crtc_x, crtc_y, crtc_w, crtc_h,
> +				src_x, src_y, src_w, src_h);
> +
> +	return drm_atomic_helper_update_plane(plane, crtc, fb,
> +					      crtc_x, crtc_y, crtc_w, crtc_h,
> +					      src_x, src_y, src_w, src_h, ctx);
> +}
> +
>  static const struct drm_plane_funcs mtk_plane_funcs = {
> -	.update_plane = drm_atomic_helper_update_plane,
> +	.update_plane = mtk_plane_update,

I think drm core has already process cursor async problem. In [1], you
could search 'legacy_cursor_update' and it need driver to implement
atomic_async_check() and atomic_async_update() callback function. You
could refer to [2] for the implementation. 


[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_atomic_helper.c?h=v5.3-rc6
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c?h=v5.3-rc6#n955

Regards,
CK

>  	.disable_plane = drm_atomic_helper_disable_plane,
>  	.destroy = drm_plane_cleanup,
>  	.reset = mtk_plane_reset,
> @@ -90,7 +154,12 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
>  	if (!state->crtc)
>  		return 0;
>  
> -	crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
> +	if (state->state)
> +		crtc_state = drm_atomic_get_existing_crtc_state(state->state,
> +								state->crtc);
> +	else /* Special case for asynchronous cursor updates. */
> +		crtc_state = state->crtc->state;
> +
>  	if (IS_ERR(crtc_state))
>  		return PTR_ERR(crtc_state);
>  



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-08-31  0:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30  7:38 [PATCH 0/2] drm/mediatek: fixup cursor moving unsmooth issue Bibby Hsieh
2019-08-30  7:38 ` [PATCH 1/2] drm/mediatek: Only block updates to CRTCs that have a pending update Bibby Hsieh
2019-08-30  8:35   ` CK Hu
2019-08-30  7:38 ` [PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates Bibby Hsieh
2019-08-31  0:04   ` CK Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).