[4/8] drm/ast: Pin framebuffer BO during dirty update

Commit Message

Thomas Zimmermann June 4, 2019, 3:41 p.m. UTC

Another explicit lock operation of a GEM VRAM BO is located in AST's
framebuffer update code. Instead of locking the BO, we pin it to wherever
it is.

Signed-off-by: Thomas Zimmermann <[e-mail]>
---
 drivers/gpu/drm/ast/ast_fb.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Gerd Hoffmann June 5, 2019, 9:46 a.m. UTC | #1

> +	/* We pin the BO to system memory so it won't be moved during
> +	 * the update and doesn't waste video ram. If the BO is already
> +	 * located in VRAM, the pin operation will simply increment the
> +	 * pin count.
>  	 */

s/located/pinned/ I guess?

Maybe we should update drm_gem_vram_pin() to skip the
drm_gem_vram_placement() call in case pl_flag is zero, so you can pin
the bo where it happens to be at the moment.

This will avoid moving around the bo for no good reason.  We don't care
where the bo is, we only want make sure it doesn't move while we access
it.

(same goes for the cursor patch, and I guess the mga patches too but I
havn't looked yet).

cheers,
  Gerd

Patch

diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 05f45222b702..7d911391c9cf 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -48,32 +48,32 @@  static void ast_dirty_update(struct ast_fbdev *afbdev,
 			     int x, int y, int width, int height)
 {
 	int i;
-	struct drm_gem_object *obj;
 	struct drm_gem_vram_object *gbo;
 	int src_offset, dst_offset;
 	int bpp = afbdev->afb.base.format->cpp[0];
-	int ret = -EBUSY;
+	int ret;
 	u8 *dst;
 	bool unmap = false;
 	bool store_for_later = false;
 	int x2, y2;
 	unsigned long flags;
 
-	obj = afbdev->afb.obj;
-	gbo = drm_gem_vram_of_gem(obj);
+	gbo = drm_gem_vram_of_gem(afbdev->afb.obj);
 
-	/* Try to lock the BO. If we fail with -EBUSY then
-	 * the BO is being moved and we should store up the
-	 * damage until later.
+	/* We pin the BO to system memory so it won't be moved during
+	 * the update and doesn't waste video ram. If the BO is already
+	 * located in VRAM, the pin operation will simply increment the
+	 * pin count.
 	 */
-	if (drm_can_sleep())
-		ret = drm_gem_vram_lock(gbo, true);
-	if (ret) {
-		if (ret != -EBUSY)
-			return;
-
+	if (drm_can_sleep()) {
+		ret = drm_gem_vram_pin(gbo, TTM_PL_FLAG_SYSTEM);
+		if (ret) {
+			if (ret != -EBUSY)
+				return;
+			store_for_later = true;
+		}
+	} else
 		store_for_later = true;
-	}
 
 	x2 = x + width - 1;
 	y2 = y + height - 1;
@@ -126,7 +126,7 @@  static void ast_dirty_update(struct ast_fbdev *afbdev,
 		drm_gem_vram_kunmap(gbo);
 
 out:
-	drm_gem_vram_unlock(gbo);
+	drm_gem_vram_unpin(gbo);
 }
 
 static void ast_fillrect(struct fb_info *info,