diff mbox series

[09/14] drm/ast: Keep cursor HW BOs mapped

Message ID 20200623081901.10667-10-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/ast: Managed modesetting | expand

Commit Message

Thomas Zimmermann June 23, 2020, 8:18 a.m. UTC 
Updating the image in a cursor's HW BO requires a mapping of the BO's
buffer in the kernel's address space. Cursor image updates can happen
frequently and create CPU overhead.

As cursor HW BOs are small and never move, they are now map exactly
once during the initialization and the mapping is used throughout the
driver's lifetime.

This change also removes a possible source of failures from
ast_cursor_show(). As the helper does not establish mappings, it cannot
fail. As a result, the cursor plane's atomic-update helper does not
call any failable interfaces. All failures are detected before trying
to update the cursor plane.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_cursor.c | 44 +++++++++++++-------------------
 drivers/gpu/drm/ast/ast_drv.h    |  5 ++--
 2 files changed, 21 insertions(+), 28 deletions(-)

Comments

kernel test robot June 24, 2020, 7:58 a.m. UTC | #1 
Hi Thomas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20200623]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.8-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-ast-Managed-modesetting/20200623-162238
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
:::::: branch date: 19 hours ago
:::::: commit date: 19 hours ago
config: x86_64-randconfig-s021-20200623 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=x86_64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/gpu/drm/ast/ast_cursor.c:250:26: sparse: sparse: duplicate [noderef]
>> drivers/gpu/drm/ast/ast_cursor.c:250:26: sparse: sparse: multiple address space given: __iomem & __iomem

# https://github.com/0day-ci/linux/commit/f438d6b57853cfafd5057aef14de32086b93e43d
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout f438d6b57853cfafd5057aef14de32086b93e43d
vim +250 drivers/gpu/drm/ast/ast_cursor.c

233fef619e5ac2 Thomas Zimmermann 2020-06-23  245  
f438d6b57853cf Thomas Zimmermann 2020-06-23  246  void ast_cursor_show(struct ast_private *ast, int x, int y,
233fef619e5ac2 Thomas Zimmermann 2020-06-23  247  		     unsigned int offset_x, unsigned int offset_y)
30686a542ba2b5 Thomas Zimmermann 2020-06-23  248  {
233fef619e5ac2 Thomas Zimmermann 2020-06-23  249  	u8 x_offset, y_offset;
f438d6b57853cf Thomas Zimmermann 2020-06-23 @250  	u8 __iomem *dst, __iomem *sig;
30686a542ba2b5 Thomas Zimmermann 2020-06-23  251  	u8 jreg;
30686a542ba2b5 Thomas Zimmermann 2020-06-23  252  
f438d6b57853cf Thomas Zimmermann 2020-06-23  253  	dst = ast->cursor.vaddr[ast->cursor.next_index];
30686a542ba2b5 Thomas Zimmermann 2020-06-23  254  
30686a542ba2b5 Thomas Zimmermann 2020-06-23  255  	sig = dst + AST_HWC_SIZE;
30686a542ba2b5 Thomas Zimmermann 2020-06-23  256  	writel(x, sig + AST_HWC_SIGNATURE_X);
30686a542ba2b5 Thomas Zimmermann 2020-06-23  257  	writel(y, sig + AST_HWC_SIGNATURE_Y);
30686a542ba2b5 Thomas Zimmermann 2020-06-23  258  
30686a542ba2b5 Thomas Zimmermann 2020-06-23  259  	if (x < 0) {
233fef619e5ac2 Thomas Zimmermann 2020-06-23  260  		x_offset = (-x) + offset_x;
30686a542ba2b5 Thomas Zimmermann 2020-06-23  261  		x = 0;
233fef619e5ac2 Thomas Zimmermann 2020-06-23  262  	} else {
233fef619e5ac2 Thomas Zimmermann 2020-06-23  263  		x_offset = offset_x;
30686a542ba2b5 Thomas Zimmermann 2020-06-23  264  	}
30686a542ba2b5 Thomas Zimmermann 2020-06-23  265  	if (y < 0) {
233fef619e5ac2 Thomas Zimmermann 2020-06-23  266  		y_offset = (-y) + offset_y;
30686a542ba2b5 Thomas Zimmermann 2020-06-23  267  		y = 0;
233fef619e5ac2 Thomas Zimmermann 2020-06-23  268  	} else {
233fef619e5ac2 Thomas Zimmermann 2020-06-23  269  		y_offset = offset_y;
30686a542ba2b5 Thomas Zimmermann 2020-06-23  270  	}
233fef619e5ac2 Thomas Zimmermann 2020-06-23  271  
233fef619e5ac2 Thomas Zimmermann 2020-06-23  272  	ast_cursor_set_location(ast, x, y, x_offset, y_offset);
30686a542ba2b5 Thomas Zimmermann 2020-06-23  273  
30686a542ba2b5 Thomas Zimmermann 2020-06-23  274  	/* dummy write to fire HWC */
30686a542ba2b5 Thomas Zimmermann 2020-06-23  275  	jreg = 0x02 |
30686a542ba2b5 Thomas Zimmermann 2020-06-23  276  	       0x01; /* enable ARGB4444 cursor */
30686a542ba2b5 Thomas Zimmermann 2020-06-23  277  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
30686a542ba2b5 Thomas Zimmermann 2020-06-23  278  }
10536b22de4fbf Thomas Zimmermann 2020-06-23  279  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 5421241015d6..35680402e410 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -39,6 +39,7 @@  int ast_cursor_init(struct ast_private *ast)
 	struct drm_device *dev = ast->dev;
 	size_t size, i;
 	struct drm_gem_vram_object *gbo;
+	void __iomem *vaddr;
 	int ret;
 
 	size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
@@ -55,8 +56,16 @@  int ast_cursor_init(struct ast_private *ast)
 			drm_gem_vram_put(gbo);
 			goto err_drm_gem_vram_put;
 		}
+		vaddr = drm_gem_vram_vmap(gbo);
+		if (IS_ERR(vaddr)) {
+			ret = PTR_ERR(vaddr);
+			drm_gem_vram_unpin(gbo);
+			drm_gem_vram_put(gbo);
+			goto err_drm_gem_vram_put;
+		}
 
 		ast->cursor.gbo[i] = gbo;
+		ast->cursor.vaddr[i] = vaddr;
 	}
 
 	return 0;
@@ -65,9 +74,11 @@  int ast_cursor_init(struct ast_private *ast)
 	while (i) {
 		--i;
 		gbo = ast->cursor.gbo[i];
+		drm_gem_vram_vunmap(gbo, ast->cursor.vaddr[i]);
 		drm_gem_vram_unpin(gbo);
 		drm_gem_vram_put(gbo);
 		ast->cursor.gbo[i] = NULL;
+		ast->cursor.vaddr[i] = NULL;
 	}
 	return ret;
 }
@@ -79,6 +90,7 @@  void ast_cursor_fini(struct ast_private *ast)
 
 	for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
 		gbo = ast->cursor.gbo[i];
+		drm_gem_vram_vunmap(gbo, ast->cursor.vaddr[i]);
 		drm_gem_vram_unpin(gbo);
 		drm_gem_vram_put(gbo);
 	}
@@ -154,7 +166,7 @@  int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb)
 	struct drm_gem_vram_object *gbo;
 	int ret;
 	void *src;
-	void *dst;
+	void __iomem *dst;
 
 	if (drm_WARN_ON_ONCE(dev, fb->width > AST_MAX_HWC_WIDTH) ||
 	    drm_WARN_ON_ONCE(dev, fb->height > AST_MAX_HWC_HEIGHT))
@@ -171,28 +183,16 @@  int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb)
 		goto err_drm_gem_vram_unpin;
 	}
 
-	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
-	if (IS_ERR(dst)) {
-		ret = PTR_ERR(dst);
-		goto err_drm_gem_vram_vunmap_src;
-	}
+	dst = ast->cursor.vaddr[ast->cursor.next_index];
 
 	/* do data transfer to cursor BO */
 	update_cursor_image(dst, src, fb->width, fb->height);
 
-	/*
-	 * Always unmap buffers here. Destination buffers are
-	 * perma-pinned while the driver is active. We're only
-	 * changing ref-counters here.
-	 */
-	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
 	drm_gem_vram_vunmap(gbo, src);
 	drm_gem_vram_unpin(gbo);
 
 	return 0;
 
-err_drm_gem_vram_vunmap_src:
-	drm_gem_vram_vunmap(gbo, src);
 err_drm_gem_vram_unpin:
 	drm_gem_vram_unpin(gbo);
 	return ret;
@@ -243,18 +243,14 @@  static void ast_cursor_set_location(struct ast_private *ast, u16 x, u16 y,
 	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc7, y1);
 }
 
-int ast_cursor_show(struct ast_private *ast, int x, int y,
-		    unsigned int offset_x, unsigned int offset_y)
+void ast_cursor_show(struct ast_private *ast, int x, int y,
+		     unsigned int offset_x, unsigned int offset_y)
 {
-	struct drm_gem_vram_object *gbo;
 	u8 x_offset, y_offset;
-	u8 *dst, *sig;
+	u8 __iomem *dst, __iomem *sig;
 	u8 jreg;
 
-	gbo = ast->cursor.gbo[ast->cursor.next_index];
-	dst = drm_gem_vram_vmap(gbo);
-	if (IS_ERR(dst))
-		return PTR_ERR(dst);
+	dst = ast->cursor.vaddr[ast->cursor.next_index];
 
 	sig = dst + AST_HWC_SIZE;
 	writel(x, sig + AST_HWC_SIGNATURE_X);
@@ -279,10 +275,6 @@  int ast_cursor_show(struct ast_private *ast, int x, int y,
 	jreg = 0x02 |
 	       0x01; /* enable ARGB4444 cursor */
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
-
-	drm_gem_vram_vunmap(gbo, dst);
-
-	return 0;
 }
 
 void ast_cursor_hide(struct ast_private *ast)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 92af0637ac48..f465e0c0984b 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -116,6 +116,7 @@  struct ast_private {
 
 	struct {
 		struct drm_gem_vram_object *gbo[AST_DEFAULT_HWC_NUM];
+		void __iomem *vaddr[AST_DEFAULT_HWC_NUM];
 		unsigned int next_index;
 	} cursor;
 
@@ -319,8 +320,8 @@  int ast_cursor_init(struct ast_private *ast);
 void ast_cursor_fini(struct ast_private *ast);
 int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb);
 void ast_cursor_page_flip(struct ast_private *ast);
-int ast_cursor_show(struct ast_private *ast, int x, int y,
-		    unsigned int offset_x, unsigned int offset_y);
+void ast_cursor_show(struct ast_private *ast, int x, int y,
+		     unsigned int offset_x, unsigned int offset_y);
 void ast_cursor_hide(struct ast_private *ast);
 
 #endif