(was: drm/ast/mgag200: Place cursor BOs at VRAM high-end)
This patchset cleans up the memory management of HW cursors in ast. It further moves the allocated cursor BOs to the of the video RAM to reduce memory fragmentation.
The ast driver manages cursor memory in a dedicated GEM VRAM buffer object. It uses a double-buffering scheme of alternating between offsets within the GEM BO. The code is convoluted and can lead to memory fragmentation if the BO is stored the middle of VRAM. This is especially a problem as ast devices only have a small amount of video memory (e.g., 8 MiB).
With this patchset, the cursor handling in ast is first split up into separate functions for copying cursor images, managing buffer objects, setting scanout addresses, and moving and hiding the cursor. Furthermore, the driver dedicates a few KiB at the high end of the device's video memory to storing the cursor's buffer objects. This prevents memory fragmentation.
The patchset has been tested on ast hardware.
v3: * split-off ast patches into separate series * move around ast_{show,hide}_cursor in a separate patch * fix space-before-tab error near AST_HWC_SIGNATURE_CHECKSUM v2: * remove VRAM buffers in favor of GEM BOs * manage BO placement with pin flag
Thomas Zimmermann (5): drm/ast: Don't call ast_show_cursor() from ast_cursor_move() drm/ast: Move ast_{show,hide}_cursor() within source file drm/ast: Move cursor update code to ast_show_cursor() drm/ast: Move cursor offset swapping into ast_show_cursor() drm/ast: Allocate cursor BOs at high end of video memory
drivers/gpu/drm/ast/ast_drv.h | 43 +++--- drivers/gpu/drm/ast/ast_mode.c | 235 +++++++++++++++++++-------------- 2 files changed, 158 insertions(+), 120 deletions(-)
-- 2.23.0
By putting cursor BOs at the high end of the video memory, we can avoid memory fragmentation. Starting at the low end, contiguous video memory is available for framebuffers.
The patch also simplifies the buffer swapping by splitting struct ast_private.cursor_cache BO into two separate boffer objects. Cursor images alternate between these buffers instead of offsets within cursor_cache.
v3: * fixes space-before-tab error near AST_HWC_SIGNATURE_CHECKSUM
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_drv.h | 43 +++++++++------- drivers/gpu/drm/ast/ast_mode.c | 91 ++++++++++++++++++---------------- 2 files changed, 73 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index 244cc7c382af..ff161bd622f3 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -82,6 +82,25 @@ enum ast_tx_chip { #define AST_DRAM_4Gx16 7 #define AST_DRAM_8Gx16 8
+ +#define AST_MAX_HWC_WIDTH 64 +#define AST_MAX_HWC_HEIGHT 64 + +#define AST_HWC_SIZE (AST_MAX_HWC_WIDTH * AST_MAX_HWC_HEIGHT * 2) +#define AST_HWC_SIGNATURE_SIZE 32 + +#define AST_DEFAULT_HWC_NUM 2 + +/* define for signature structure */ +#define AST_HWC_SIGNATURE_CHECKSUM 0x00 +#define AST_HWC_SIGNATURE_SizeX 0x04 +#define AST_HWC_SIGNATURE_SizeY 0x08 +#define AST_HWC_SIGNATURE_X 0x0C +#define AST_HWC_SIGNATURE_Y 0x10 +#define AST_HWC_SIGNATURE_HOTSPOTX 0x14 +#define AST_HWC_SIGNATURE_HOTSPOTY 0x18 + + struct ast_private { struct drm_device *dev;
@@ -97,8 +116,11 @@ struct ast_private {
int fb_mtrr;
- struct drm_gem_object *cursor_cache; - int next_cursor; + struct { + struct drm_gem_vram_object *gbo[AST_DEFAULT_HWC_NUM]; + unsigned int next_index; + } cursor; + bool support_wide_screen; enum { ast_use_p2a, @@ -199,23 +221,6 @@ static inline void ast_open_key(struct ast_private *ast)
#define AST_VIDMEM_DEFAULT_SIZE AST_VIDMEM_SIZE_8M
-#define AST_MAX_HWC_WIDTH 64 -#define AST_MAX_HWC_HEIGHT 64 - -#define AST_HWC_SIZE (AST_MAX_HWC_WIDTH*AST_MAX_HWC_HEIGHT*2) -#define AST_HWC_SIGNATURE_SIZE 32 - -#define AST_DEFAULT_HWC_NUM 2 -/* define for signature structure */ -#define AST_HWC_SIGNATURE_CHECKSUM 0x00 -#define AST_HWC_SIGNATURE_SizeX 0x04 -#define AST_HWC_SIGNATURE_SizeY 0x08 -#define AST_HWC_SIGNATURE_X 0x0C -#define AST_HWC_SIGNATURE_Y 0x10 -#define AST_HWC_SIGNATURE_HOTSPOTX 0x14 -#define AST_HWC_SIGNATURE_HOTSPOTY 0x18 - - struct ast_i2c_chan { struct i2c_adapter adapter; struct drm_device *dev; diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 6a517ffb1c5c..b13eaa2619ab 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -883,50 +883,53 @@ static int ast_connector_init(struct drm_device *dev) static int ast_cursor_init(struct drm_device *dev) { struct ast_private *ast = dev->dev_private; - int size; - int ret; - struct drm_gem_object *obj; + size_t size, i; struct drm_gem_vram_object *gbo; - s64 gpu_addr; - void *base; + int ret;
- size = (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE) * AST_DEFAULT_HWC_NUM; + size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
- ret = ast_gem_create(dev, size, true, &obj); - if (ret) - return ret; - gbo = drm_gem_vram_of_gem(obj); - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM); - if (ret) - goto fail; - gpu_addr = drm_gem_vram_offset(gbo); - if (gpu_addr < 0) { - drm_gem_vram_unpin(gbo); - ret = (int)gpu_addr; - goto fail; - } + for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) { + gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, + size, 0, false); + if (IS_ERR(gbo)) { + ret = PTR_ERR(gbo); + goto err_drm_gem_vram_put; + } + ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM | + DRM_GEM_VRAM_PL_FLAG_TOPDOWN); + if (ret) { + drm_gem_vram_put(gbo); + goto err_drm_gem_vram_put; + }
- /* kmap the object */ - base = drm_gem_vram_kmap(gbo, true, NULL); - if (IS_ERR(base)) { - ret = PTR_ERR(base); - goto fail; + ast->cursor.gbo[i] = gbo; }
- ast->cursor_cache = obj; return 0; -fail: + +err_drm_gem_vram_put: + while (i) { + --i; + gbo = ast->cursor.gbo[i]; + drm_gem_vram_unpin(gbo); + drm_gem_vram_put(gbo); + ast->cursor.gbo[i] = NULL; + } return ret; }
static void ast_cursor_fini(struct drm_device *dev) { struct ast_private *ast = dev->dev_private; - struct drm_gem_vram_object *gbo = - drm_gem_vram_of_gem(ast->cursor_cache); - drm_gem_vram_kunmap(gbo); - drm_gem_vram_unpin(gbo); - drm_gem_object_put_unlocked(ast->cursor_cache); + size_t i; + struct drm_gem_vram_object *gbo; + + for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) { + gbo = ast->cursor.gbo[i]; + drm_gem_vram_unpin(gbo); + drm_gem_vram_put(gbo); + } }
int ast_mode_init(struct drm_device *dev) @@ -1156,12 +1159,12 @@ static int ast_show_cursor(struct drm_crtc *crtc, void *src, struct ast_private *ast = crtc->dev->dev_private; struct ast_crtc *ast_crtc = to_ast_crtc(crtc); struct drm_gem_vram_object *gbo; - u8 *dst, *dst_next; + void *dst; s64 off; int ret; u8 jreg;
- gbo = drm_gem_vram_of_gem(ast->cursor_cache); + gbo = ast->cursor.gbo[ast->cursor.next_index]; dst = drm_gem_vram_vmap(gbo); if (IS_ERR(dst)) return PTR_ERR(dst); @@ -1171,10 +1174,7 @@ static int ast_show_cursor(struct drm_crtc *crtc, void *src, goto err_drm_gem_vram_vunmap; }
- dst_next = dst + (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE) * - ast->next_cursor; - - ret = ast_cursor_update(dst_next, src, width, height); + ret = ast_cursor_update(dst, src, width, height); if (ret) goto err_drm_gem_vram_vunmap; ast_cursor_set_base(ast, off); @@ -1187,7 +1187,8 @@ static int ast_show_cursor(struct drm_crtc *crtc, void *src, jreg |= 1; ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
- ast->next_cursor = (ast->next_cursor + 1) % AST_DEFAULT_HWC_NUM; + ++ast->cursor.next_index; + ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
drm_gem_vram_vunmap(gbo, dst);
@@ -1257,13 +1258,17 @@ static int ast_cursor_move(struct drm_crtc *crtc, { struct ast_crtc *ast_crtc = to_ast_crtc(crtc); struct ast_private *ast = crtc->dev->dev_private; + struct drm_gem_vram_object *gbo; int x_offset, y_offset; - u8 *sig; + u8 *dst, *sig; u8 jreg;
- sig = drm_gem_vram_kmap(drm_gem_vram_of_gem(ast->cursor_cache), - false, NULL); - sig += (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE)*ast->next_cursor + AST_HWC_SIZE; + gbo = ast->cursor.gbo[ast->cursor.next_index]; + dst = drm_gem_vram_vmap(gbo); + if (IS_ERR(dst)) + return PTR_ERR(dst); + + sig = dst + AST_HWC_SIZE; writel(x, sig + AST_HWC_SIGNATURE_X); writel(y, sig + AST_HWC_SIGNATURE_Y);
@@ -1290,5 +1295,7 @@ static int ast_cursor_move(struct drm_crtc *crtc, 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; }
Separating the cursor's move() function from the show() function in preparation of further rework of the cursor update code.
'Showing' the cursor from within the move() function is required to update the cursor position.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_mode.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 6caa6ebfeaa8..a4cbf2d5ee0a 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1236,6 +1236,7 @@ static int ast_cursor_move(struct drm_crtc *crtc, struct ast_private *ast = crtc->dev->dev_private; int x_offset, y_offset; u8 *sig; + u8 jreg;
sig = drm_gem_vram_kmap(drm_gem_vram_of_gem(ast->cursor_cache), false, NULL); @@ -1262,7 +1263,9 @@ static int ast_cursor_move(struct drm_crtc *crtc, ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc7, ((y >> 8) & 0x07));
/* dummy write to fire HWC */ - ast_show_cursor(crtc); + jreg = 0x02 | + 0x01; /* enable ARGB4444 cursor */ + ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
return 0; }
This patch only moves around code for easier review of later patches. No functional cahnges are made.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_mode.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index a4cbf2d5ee0a..5a9e6a87ea5b 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1064,23 +1064,6 @@ static void ast_i2c_destroy(struct ast_i2c_chan *i2c) kfree(i2c); }
-static void ast_show_cursor(struct drm_crtc *crtc) -{ - struct ast_private *ast = crtc->dev->dev_private; - u8 jreg; - - jreg = 0x2; - /* enable ARGB cursor */ - jreg |= 1; - ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg); -} - -static void ast_hide_cursor(struct drm_crtc *crtc) -{ - struct ast_private *ast = crtc->dev->dev_private; - ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00); -} - static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height) { union { @@ -1137,6 +1120,23 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height) return csum; }
+static void ast_show_cursor(struct drm_crtc *crtc) +{ + struct ast_private *ast = crtc->dev->dev_private; + u8 jreg; + + jreg = 0x2; + /* enable ARGB cursor */ + jreg |= 1; + ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg); +} + +static void ast_hide_cursor(struct drm_crtc *crtc) +{ + struct ast_private *ast = crtc->dev->dev_private; + ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00); +} + static int ast_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, uint32_t handle,
A call to ast's show-cursor function now receives the cursor image and updates the buffer. The change splits off image update and base-address update into separate functions.
v3: * move ast_{show,hide}_cursor() in a previous patch
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_mode.c | 88 ++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 5a9e6a87ea5b..1294f0612fd5 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1120,20 +1120,69 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height) return csum; }
-static void ast_show_cursor(struct drm_crtc *crtc) +static int ast_cursor_update(void *dst, void *src, unsigned int width, + unsigned int height) +{ + u32 csum; + + /* do data transfer to cursor cache */ + csum = copy_cursor_image(src, dst, width, height); + + /* write checksum + signature */ + dst += AST_HWC_SIZE; + writel(csum, dst); + writel(width, dst + AST_HWC_SIGNATURE_SizeX); + writel(height, dst + AST_HWC_SIGNATURE_SizeY); + writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX); + writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY); + + return 0; +} + +static void ast_cursor_set_base(struct ast_private *ast, u64 address) +{ + u8 addr0 = (address >> 3) & 0xff; + u8 addr1 = (address >> 11) & 0xff; + u8 addr2 = (address >> 19) & 0xff; + + ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc8, addr0); + ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc9, addr1); + ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xca, addr2); +} + +static int ast_show_cursor(struct drm_crtc *crtc, void *dst, void *src, + unsigned int width, unsigned int height, + u64 dst_gpu) { struct ast_private *ast = crtc->dev->dev_private; + struct ast_crtc *ast_crtc = to_ast_crtc(crtc); + int ret; u8 jreg;
+ dst += (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE)*ast->next_cursor; + + ret = ast_cursor_update(dst, src, width, height); + if (ret) + return ret; + ast_cursor_set_base(ast, dst_gpu); + + ast->next_cursor = (ast->next_cursor + 1) % AST_DEFAULT_HWC_NUM; + + ast_crtc->offset_x = AST_MAX_HWC_WIDTH - width; + ast_crtc->offset_y = AST_MAX_HWC_WIDTH - height; + jreg = 0x2; /* enable ARGB cursor */ jreg |= 1; ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg); + + return 0; }
static void ast_hide_cursor(struct drm_crtc *crtc) { struct ast_private *ast = crtc->dev->dev_private; + ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00); }
@@ -1144,12 +1193,9 @@ static int ast_cursor_set(struct drm_crtc *crtc, uint32_t height) { struct ast_private *ast = crtc->dev->dev_private; - struct ast_crtc *ast_crtc = to_ast_crtc(crtc); struct drm_gem_object *obj; struct drm_gem_vram_object *gbo; s64 dst_gpu; - u64 gpu_addr; - u32 csum; int ret; u8 *src, *dst;
@@ -1185,37 +1231,9 @@ static int ast_cursor_set(struct drm_crtc *crtc, goto err_drm_gem_vram_vunmap; }
- dst += (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE)*ast->next_cursor; - - /* do data transfer to cursor cache */ - csum = copy_cursor_image(src, dst, width, height); - - /* write checksum + signature */ - { - struct drm_gem_vram_object *dst_gbo = - drm_gem_vram_of_gem(ast->cursor_cache); - u8 *dst = drm_gem_vram_kmap(dst_gbo, false, NULL); - dst += (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE)*ast->next_cursor + AST_HWC_SIZE; - writel(csum, dst); - writel(width, dst + AST_HWC_SIGNATURE_SizeX); - writel(height, dst + AST_HWC_SIGNATURE_SizeY); - writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX); - writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY); - - /* set pattern offset */ - gpu_addr = (u64)dst_gpu; - gpu_addr += (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE)*ast->next_cursor; - gpu_addr >>= 3; - ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc8, gpu_addr & 0xff); - ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc9, (gpu_addr >> 8) & 0xff); - ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xca, (gpu_addr >> 16) & 0xff); - } - ast_crtc->offset_x = AST_MAX_HWC_WIDTH - width; - ast_crtc->offset_y = AST_MAX_HWC_WIDTH - height; - - ast->next_cursor = (ast->next_cursor + 1) % AST_DEFAULT_HWC_NUM; - - ast_show_cursor(crtc); + ret = ast_show_cursor(crtc, dst, src, width, height, dst_gpu); + if (ret) + goto err_drm_gem_vram_kunmap;
drm_gem_vram_vunmap(gbo, src); drm_gem_object_put_unlocked(obj);
Selecting the correct offset for the new cursor image is not relevant outside of ast_show_cursor(). Let the function do the work.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_mode.c | 57 ++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 1294f0612fd5..6a517ffb1c5c 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1150,23 +1150,34 @@ static void ast_cursor_set_base(struct ast_private *ast, u64 address) ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xca, addr2); }
-static int ast_show_cursor(struct drm_crtc *crtc, void *dst, void *src, - unsigned int width, unsigned int height, - u64 dst_gpu) +static int ast_show_cursor(struct drm_crtc *crtc, void *src, + unsigned int width, unsigned int height) { struct ast_private *ast = crtc->dev->dev_private; struct ast_crtc *ast_crtc = to_ast_crtc(crtc); + struct drm_gem_vram_object *gbo; + u8 *dst, *dst_next; + s64 off; int ret; u8 jreg;
- dst += (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE)*ast->next_cursor; + gbo = drm_gem_vram_of_gem(ast->cursor_cache); + dst = drm_gem_vram_vmap(gbo); + if (IS_ERR(dst)) + return PTR_ERR(dst); + off = drm_gem_vram_offset(gbo); + if (off < 0) { + ret = (int)off; + goto err_drm_gem_vram_vunmap; + }
- ret = ast_cursor_update(dst, src, width, height); - if (ret) - return ret; - ast_cursor_set_base(ast, dst_gpu); + dst_next = dst + (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE) * + ast->next_cursor;
- ast->next_cursor = (ast->next_cursor + 1) % AST_DEFAULT_HWC_NUM; + ret = ast_cursor_update(dst_next, src, width, height); + if (ret) + goto err_drm_gem_vram_vunmap; + ast_cursor_set_base(ast, off);
ast_crtc->offset_x = AST_MAX_HWC_WIDTH - width; ast_crtc->offset_y = AST_MAX_HWC_WIDTH - height; @@ -1176,7 +1187,15 @@ static int ast_show_cursor(struct drm_crtc *crtc, void *dst, void *src, jreg |= 1; ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
+ ast->next_cursor = (ast->next_cursor + 1) % AST_DEFAULT_HWC_NUM; + + drm_gem_vram_vunmap(gbo, dst); + return 0; + +err_drm_gem_vram_vunmap: + drm_gem_vram_vunmap(gbo, dst); + return ret; }
static void ast_hide_cursor(struct drm_crtc *crtc) @@ -1192,12 +1211,10 @@ static int ast_cursor_set(struct drm_crtc *crtc, uint32_t width, uint32_t height) { - struct ast_private *ast = crtc->dev->dev_private; struct drm_gem_object *obj; struct drm_gem_vram_object *gbo; - s64 dst_gpu; + u8 *src; int ret; - u8 *src, *dst;
if (!handle) { ast_hide_cursor(crtc); @@ -1219,21 +1236,9 @@ static int ast_cursor_set(struct drm_crtc *crtc, goto err_drm_gem_object_put_unlocked; }
- dst = drm_gem_vram_kmap(drm_gem_vram_of_gem(ast->cursor_cache), - false, NULL); - if (IS_ERR(dst)) { - ret = PTR_ERR(dst); - goto err_drm_gem_vram_vunmap; - } - dst_gpu = drm_gem_vram_offset(drm_gem_vram_of_gem(ast->cursor_cache)); - if (dst_gpu < 0) { - ret = (int)dst_gpu; - goto err_drm_gem_vram_vunmap; - } - - ret = ast_show_cursor(crtc, dst, src, width, height, dst_gpu); + ret = ast_show_cursor(crtc, src, width, height); if (ret) - goto err_drm_gem_vram_kunmap; + goto err_drm_gem_vram_vunmap;
drm_gem_vram_vunmap(gbo, src); drm_gem_object_put_unlocked(obj);
On Fri, Sep 27, 2019 at 11:03:04AM +0200, Thomas Zimmermann wrote:
(was: drm/ast/mgag200: Place cursor BOs at VRAM high-end)
This patchset cleans up the memory management of HW cursors in ast. It further moves the allocated cursor BOs to the of the video RAM to reduce memory fragmentation.
The ast driver manages cursor memory in a dedicated GEM VRAM buffer object. It uses a double-buffering scheme of alternating between offsets within the GEM BO. The code is convoluted and can lead to memory fragmentation if the BO is stored the middle of VRAM. This is especially a problem as ast devices only have a small amount of video memory (e.g., 8 MiB).
With this patchset, the cursor handling in ast is first split up into separate functions for copying cursor images, managing buffer objects, setting scanout addresses, and moving and hiding the cursor. Furthermore, the driver dedicates a few KiB at the high end of the device's video memory to storing the cursor's buffer objects. This prevents memory fragmentation.
The patchset has been tested on ast hardware.
v3:
- split-off ast patches into separate series
- move around ast_{show,hide}_cursor in a separate patch
- fix space-before-tab error near AST_HWC_SIGNATURE_CHECKSUM
v2:
- remove VRAM buffers in favor of GEM BOs
- manage BO placement with pin flag
Looks all sane to me, series is Acked-by: Gerd Hoffmann kraxel@redhat.com
Thomas Zimmermann (5): drm/ast: Don't call ast_show_cursor() from ast_cursor_move() drm/ast: Move ast_{show,hide}_cursor() within source file drm/ast: Move cursor update code to ast_show_cursor() drm/ast: Move cursor offset swapping into ast_show_cursor() drm/ast: Allocate cursor BOs at high end of video memory
drivers/gpu/drm/ast/ast_drv.h | 43 +++--- drivers/gpu/drm/ast/ast_mode.c | 235 +++++++++++++++++++-------------- 2 files changed, 158 insertions(+), 120 deletions(-)
-- 2.23.0
Hi
Am 02.10.19 um 10:59 schrieb Gerd Hoffmann:
On Fri, Sep 27, 2019 at 11:03:04AM +0200, Thomas Zimmermann wrote:
(was: drm/ast/mgag200: Place cursor BOs at VRAM high-end)
This patchset cleans up the memory management of HW cursors in ast. It further moves the allocated cursor BOs to the of the video RAM to reduce memory fragmentation.
The ast driver manages cursor memory in a dedicated GEM VRAM buffer object. It uses a double-buffering scheme of alternating between offsets within the GEM BO. The code is convoluted and can lead to memory fragmentation if the BO is stored the middle of VRAM. This is especially a problem as ast devices only have a small amount of video memory (e.g., 8 MiB).
With this patchset, the cursor handling in ast is first split up into separate functions for copying cursor images, managing buffer objects, setting scanout addresses, and moving and hiding the cursor. Furthermore, the driver dedicates a few KiB at the high end of the device's video memory to storing the cursor's buffer objects. This prevents memory fragmentation.
The patchset has been tested on ast hardware.
v3:
- split-off ast patches into separate series
- move around ast_{show,hide}_cursor in a separate patch
- fix space-before-tab error near AST_HWC_SIGNATURE_CHECKSUM
v2:
- remove VRAM buffers in favor of GEM BOs
- manage BO placement with pin flag
Looks all sane to me, series is Acked-by: Gerd Hoffmann kraxel@redhat.com
Thanks for taking the time to review patches for this fairly obscure code.
Best regards Thomas
Thomas Zimmermann (5): drm/ast: Don't call ast_show_cursor() from ast_cursor_move() drm/ast: Move ast_{show,hide}_cursor() within source file drm/ast: Move cursor update code to ast_show_cursor() drm/ast: Move cursor offset swapping into ast_show_cursor() drm/ast: Allocate cursor BOs at high end of video memory
drivers/gpu/drm/ast/ast_drv.h | 43 +++--- drivers/gpu/drm/ast/ast_mode.c | 235 +++++++++++++++++++-------------- 2 files changed, 158 insertions(+), 120 deletions(-)
-- 2.23.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Gerd Hoffmann -
Thomas Zimmermann