Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1312098 > unrolled thread

[PATCH v2 0/3] drm/rockchip: fix cursor performance with atomic

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 0/3] drm/rockchip: fix cursor performance with atomic John Keeping <john@metanate.com> - 2016-01-19 11:50 +0100
    [PATCH v2 3/3] drm/rockchip: explain why we can't wait_for_vblanks John Keeping <john@metanate.com> - 2016-01-19 11:50 +0100
    [PATCH v2 2/3] drm/rockchip: don't wait for vblank if fb hasn't changed John Keeping <john@metanate.com> - 2016-01-19 11:50 +0100
    Re: [PATCH v2 0/3] drm/rockchip: fix cursor performance with atomic Mark yao <mark.yao@rock-chips.com> - 2016-01-21 02:00 +0100

#1312098 — [PATCH v2 0/3] drm/rockchip: fix cursor performance with atomic

The first two patches are unchanged since v1 but the comment in the
third has been expanded following Thierry's comments.

John Keeping (3):
  drm/atomic-helper: Export framebuffer_changed()
  drm/rockchip: don't wait for vblank if fb hasn't changed
  drm/rockchip: explain why we can't wait_for_vblanks

 drivers/gpu/drm/drm_atomic_helper.c        | 24 ++++++++++++++++++++----
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 23 +++++++++++++++++++++--
 include/drm/drm_atomic_helper.h            |  4 ++++
 3 files changed, 45 insertions(+), 6 deletions(-)

-- 
2.7.0.226.gfe986fe

[toc] | [next] | [standalone]

#1312099 — [PATCH v2 3/3] drm/rockchip: explain why we can't wait_for_vblanks

Signed-off-by: John Keeping <john@metanate.com>
---
v2:
  - Add more detail of the particular race that could happen if we used
    drm_atomic_helper_wait_for_vblanks().

 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 679d23a..cf0b7bd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -177,6 +177,21 @@ static void rockchip_crtc_wait_for_update(struct drm_crtc *crtc)
 		crtc_funcs->wait_for_update(crtc);
 }
 
+/*
+ * We can't use drm_atomic_helper_wait_for_vblanks() because rk3288 and rk3066
+ * have hardware counters for neither vblanks nor scanlines, which results in
+ * a race where:
+ *				| <-- HW vsync irq and reg take effect
+ *	       plane_commit --> |
+ *	get_vblank and wait --> |
+ *				| <-- handle_vblank, vblank->count + 1
+ *		 cleanup_fb --> |
+ *		iommu crash --> |
+ *				| <-- HW vsync irq and reg take effect
+ *
+ * This function is equivalent but uses rockchip_crtc_wait_for_update() instead
+ * of waiting for vblank_count to change.
+ */
 static void
 rockchip_atomic_wait_for_complete(struct drm_device *dev, struct drm_atomic_state *old_state)
 {
-- 
2.7.0.226.gfe986fe

[toc] | [prev] | [next] | [standalone]

#1312102 — [PATCH v2 2/3] drm/rockchip: don't wait for vblank if fb hasn't changed

As commented in drm_atomic_helper_wait_for_vblanks(), userspace relies
on cursor ioctls being unsynced.  Converting the rockchip driver to
atomic has significantly impacted cursor performance by making every
cursor update wait for vblank.

By skipping the vblank sync when the framebuffer has not changed (as is
done in drm_atomic_helper_wait_for_vblanks()) we can avoid this for the
common case of moving the cursor and only need to delay the cursor ioctl
when the cursor icon changes.

We cannot add the check on legacy_cursor_update since that results in
the cursor bo being unreferenced while the hardware may still be reading
it.  Fully supporting unsynced cursor updates is left for the future
when the atomic helper framework supports async updates.

Signed-off-by: John Keeping <john@metanate.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
Unchanged since v1.

 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index f784488..679d23a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -178,7 +178,7 @@ static void rockchip_crtc_wait_for_update(struct drm_crtc *crtc)
 }
 
 static void
-rockchip_atomic_wait_for_complete(struct drm_atomic_state *old_state)
+rockchip_atomic_wait_for_complete(struct drm_device *dev, struct drm_atomic_state *old_state)
 {
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc *crtc;
@@ -194,6 +194,10 @@ rockchip_atomic_wait_for_complete(struct drm_atomic_state *old_state)
 		if (!crtc->state->active)
 			continue;
 
+		if (!drm_atomic_helper_framebuffer_changed(dev,
+				old_state, crtc))
+			continue;
+
 		ret = drm_crtc_vblank_get(crtc);
 		if (ret != 0)
 			continue;
@@ -241,7 +245,7 @@ rockchip_atomic_commit_complete(struct rockchip_atomic_commit *commit)
 
 	drm_atomic_helper_commit_planes(dev, state, true);
 
-	rockchip_atomic_wait_for_complete(state);
+	rockchip_atomic_wait_for_complete(dev, state);
 
 	drm_atomic_helper_cleanup_planes(dev, state);
 
-- 
2.7.0.226.gfe986fe

[toc] | [prev] | [next] | [standalone]

#1313757

On 2016年01月19日 18:46, John Keeping wrote:
> The first two patches are unchanged since v1 but the comment in the
> third has been expanded following Thierry's comments.
>
> John Keeping (3):
>    drm/atomic-helper: Export framebuffer_changed()
>    drm/rockchip: don't wait for vblank if fb hasn't changed
>    drm/rockchip: explain why we can't wait_for_vblanks
>
>   drivers/gpu/drm/drm_atomic_helper.c        | 24 ++++++++++++++++++++----
>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 23 +++++++++++++++++++++--
>   include/drm/drm_atomic_helper.h            |  4 ++++
>   3 files changed, 45 insertions(+), 6 deletions(-)
>

Hi John

Thanks for your fix, applied these three patches into my drm-next, :-)

-- 
Ｍark Yao

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web