All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/12] drm/ast/mgag200: Place cursor BOs at VRAM high-end
@ 2019-09-23 17:27 ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

(was: drm/vram: Add VRAM buffers for HW cursors)

This patchset cleans up the memory management of HW cursors in ast and
mgag200. It further moves the allocated cursor BOs to the of the video
RAM to reduce memory fragmentation.

The ast and mgag200 drivers support HW cursors of uncommon pixel formats.
Both drivers manage cursor memory in dedicated GEM VRAM buffer objects
with a double buffering scheme; either by alternating between two GEM BOs,
or by alternating between offsets within the same 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 on devices with only a small
amount of video memory.

With this patchset, the cursor handling in ast and mgag200 is first split
up into separate functions for copying cursor images, managing buffer
objects, setting scanout addresses, and moving and hiding the cursor.
Furthermore, each driver dedicates a few KiB at the high end of each
device's video memory to storing the cursor's buffer objects. This reduces
memory fragmentation, which may prevent displaying the framebuffer on
low-memory devices.

The patchset has been tested on ast and mgag200 hardware.

v2:
	* remove VRAM buffers in favor of GEM BOs
	* manage BO placement with pin flag

Thomas Zimmermann (12):
  drm/vram: Support top-down placement flag
  drm/ast: Don't call ast_show_cursor() from ast_cursor_move()
  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
  drm/mgag200: Rename cursor functions to use mgag200_ prefix
  drm/mgag200: Add init and fini functions for cursor handling
  drm/mgag200: Add separate move-cursor function
  drm/mgag200: Move cursor-image update to mgag200_show_cursor()
  drm/mgag200: Move cursor BO swapping into mgag200_show_cursor()
  drm/mgag200: Reserve video memory for cursor plane
  drm/mgag200: 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 +++++++++--------
 drivers/gpu/drm/drm_gem_vram_helper.c    |  14 +-
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 316 ++++++++++++++---------
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  22 +-
 drivers/gpu/drm/mgag200/mgag200_main.c   |  20 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c   |   6 +-
 drivers/gpu/drm/mgag200/mgag200_ttm.c    |   4 +
 include/drm/drm_gem_vram_helper.h        |   1 +
 9 files changed, 387 insertions(+), 274 deletions(-)

--
2.23.0


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

* [PATCH v2 00/12] drm/ast/mgag200: Place cursor BOs at VRAM high-end
@ 2019-09-23 17:27 ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

(was: drm/vram: Add VRAM buffers for HW cursors)

This patchset cleans up the memory management of HW cursors in ast and
mgag200. It further moves the allocated cursor BOs to the of the video
RAM to reduce memory fragmentation.

The ast and mgag200 drivers support HW cursors of uncommon pixel formats.
Both drivers manage cursor memory in dedicated GEM VRAM buffer objects
with a double buffering scheme; either by alternating between two GEM BOs,
or by alternating between offsets within the same 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 on devices with only a small
amount of video memory.

With this patchset, the cursor handling in ast and mgag200 is first split
up into separate functions for copying cursor images, managing buffer
objects, setting scanout addresses, and moving and hiding the cursor.
Furthermore, each driver dedicates a few KiB at the high end of each
device's video memory to storing the cursor's buffer objects. This reduces
memory fragmentation, which may prevent displaying the framebuffer on
low-memory devices.

The patchset has been tested on ast and mgag200 hardware.

v2:
	* remove VRAM buffers in favor of GEM BOs
	* manage BO placement with pin flag

Thomas Zimmermann (12):
  drm/vram: Support top-down placement flag
  drm/ast: Don't call ast_show_cursor() from ast_cursor_move()
  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
  drm/mgag200: Rename cursor functions to use mgag200_ prefix
  drm/mgag200: Add init and fini functions for cursor handling
  drm/mgag200: Add separate move-cursor function
  drm/mgag200: Move cursor-image update to mgag200_show_cursor()
  drm/mgag200: Move cursor BO swapping into mgag200_show_cursor()
  drm/mgag200: Reserve video memory for cursor plane
  drm/mgag200: 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 +++++++++--------
 drivers/gpu/drm/drm_gem_vram_helper.c    |  14 +-
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 316 ++++++++++++++---------
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  22 +-
 drivers/gpu/drm/mgag200/mgag200_main.c   |  20 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c   |   6 +-
 drivers/gpu/drm/mgag200/mgag200_ttm.c    |   4 +
 include/drm/drm_gem_vram_helper.h        |   1 +
 9 files changed, 387 insertions(+), 274 deletions(-)

--
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 01/12] drm/vram: Support top-down placement flag
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

Pinning lots of small buffer objects, such as cursors or sprites, to video
memory can lead to fragmentation, which is a problem for devices with only
a small amount of memory. As a result, framebuffer images might not get
pinned, even though there's enough space available overall.

The flag DRM_GEM_VRAM_PL_FLAG_TOPDOWN marks buffer objects to be pinned at
the high end of video memory. This leaves contiguous space available at
the memory's low end.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 14 ++++++++++----
 include/drm/drm_gem_vram_helper.h     |  1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 49588de88959..91baf3f279f1 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -58,6 +58,7 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
 {
 	unsigned int i;
 	unsigned int c = 0;
+	u32 invariant_flags = pl_flag & TTM_PL_FLAG_TOPDOWN;
 
 	gbo->placement.placement = gbo->placements;
 	gbo->placement.busy_placement = gbo->placements;
@@ -65,15 +66,18 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
 	if (pl_flag & TTM_PL_FLAG_VRAM)
 		gbo->placements[c++].flags = TTM_PL_FLAG_WC |
 					     TTM_PL_FLAG_UNCACHED |
-					     TTM_PL_FLAG_VRAM;
+					     TTM_PL_FLAG_VRAM |
+					     invariant_flags;
 
 	if (pl_flag & TTM_PL_FLAG_SYSTEM)
 		gbo->placements[c++].flags = TTM_PL_MASK_CACHING |
-					     TTM_PL_FLAG_SYSTEM;
+					     TTM_PL_FLAG_SYSTEM |
+					     invariant_flags;
 
 	if (!c)
 		gbo->placements[c++].flags = TTM_PL_MASK_CACHING |
-					     TTM_PL_FLAG_SYSTEM;
+					     TTM_PL_FLAG_SYSTEM |
+					     invariant_flags;
 
 	gbo->placement.num_placement = c;
 	gbo->placement.num_busy_placement = c;
@@ -236,7 +240,9 @@ static int drm_gem_vram_pin_locked(struct drm_gem_vram_object *gbo,
  * a memory region. A pinned buffer object has to be unpinned before
  * it can be pinned to another region. If the pl_flag argument is 0,
  * the buffer is pinned at its current location (video RAM or system
- * memory).
+ * memory). The modifier DRM_GEM_VRAM_PL_FLAG_TOPDOWN marks the buffer
+ * object to be pinned at the high end of the memory region. Set this
+ * flag to avoid memory fragmentation.
  *
  * Returns:
  * 0 on success, or
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 418eb1122861..354a9cd358a3 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -19,6 +19,7 @@ struct vm_area_struct;
 
 #define DRM_GEM_VRAM_PL_FLAG_VRAM	TTM_PL_FLAG_VRAM
 #define DRM_GEM_VRAM_PL_FLAG_SYSTEM	TTM_PL_FLAG_SYSTEM
+#define DRM_GEM_VRAM_PL_FLAG_TOPDOWN	TTM_PL_FLAG_TOPDOWN
 
 /*
  * Buffer-object helpers
-- 
2.23.0


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

* [PATCH v2 01/12] drm/vram: Support top-down placement flag
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

Pinning lots of small buffer objects, such as cursors or sprites, to video
memory can lead to fragmentation, which is a problem for devices with only
a small amount of memory. As a result, framebuffer images might not get
pinned, even though there's enough space available overall.

The flag DRM_GEM_VRAM_PL_FLAG_TOPDOWN marks buffer objects to be pinned at
the high end of video memory. This leaves contiguous space available at
the memory's low end.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 14 ++++++++++----
 include/drm/drm_gem_vram_helper.h     |  1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 49588de88959..91baf3f279f1 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -58,6 +58,7 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
 {
 	unsigned int i;
 	unsigned int c = 0;
+	u32 invariant_flags = pl_flag & TTM_PL_FLAG_TOPDOWN;
 
 	gbo->placement.placement = gbo->placements;
 	gbo->placement.busy_placement = gbo->placements;
@@ -65,15 +66,18 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
 	if (pl_flag & TTM_PL_FLAG_VRAM)
 		gbo->placements[c++].flags = TTM_PL_FLAG_WC |
 					     TTM_PL_FLAG_UNCACHED |
-					     TTM_PL_FLAG_VRAM;
+					     TTM_PL_FLAG_VRAM |
+					     invariant_flags;
 
 	if (pl_flag & TTM_PL_FLAG_SYSTEM)
 		gbo->placements[c++].flags = TTM_PL_MASK_CACHING |
-					     TTM_PL_FLAG_SYSTEM;
+					     TTM_PL_FLAG_SYSTEM |
+					     invariant_flags;
 
 	if (!c)
 		gbo->placements[c++].flags = TTM_PL_MASK_CACHING |
-					     TTM_PL_FLAG_SYSTEM;
+					     TTM_PL_FLAG_SYSTEM |
+					     invariant_flags;
 
 	gbo->placement.num_placement = c;
 	gbo->placement.num_busy_placement = c;
@@ -236,7 +240,9 @@ static int drm_gem_vram_pin_locked(struct drm_gem_vram_object *gbo,
  * a memory region. A pinned buffer object has to be unpinned before
  * it can be pinned to another region. If the pl_flag argument is 0,
  * the buffer is pinned at its current location (video RAM or system
- * memory).
+ * memory). The modifier DRM_GEM_VRAM_PL_FLAG_TOPDOWN marks the buffer
+ * object to be pinned at the high end of the memory region. Set this
+ * flag to avoid memory fragmentation.
  *
  * Returns:
  * 0 on success, or
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 418eb1122861..354a9cd358a3 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -19,6 +19,7 @@ struct vm_area_struct;
 
 #define DRM_GEM_VRAM_PL_FLAG_VRAM	TTM_PL_FLAG_VRAM
 #define DRM_GEM_VRAM_PL_FLAG_SYSTEM	TTM_PL_FLAG_SYSTEM
+#define DRM_GEM_VRAM_PL_FLAG_TOPDOWN	TTM_PL_FLAG_TOPDOWN
 
 /*
  * Buffer-object helpers
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 02/12] drm/ast: Don't call ast_show_cursor() from ast_cursor_move()
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

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;
 }
-- 
2.23.0


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

* [PATCH v2 02/12] drm/ast: Don't call ast_show_cursor() from ast_cursor_move()
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

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;
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 03/12] drm/ast: Move cursor update code to ast_show_cursor()
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 120 +++++++++++++++++++--------------
 1 file changed, 69 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index a4cbf2d5ee0a..1294f0612fd5 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,72 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height)
 	return csum;
 }
 
+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);
+}
+
 static int ast_cursor_set(struct drm_crtc *crtc,
 			  struct drm_file *file_priv,
 			  uint32_t handle,
@@ -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);
-- 
2.23.0


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

* [PATCH v2 03/12] drm/ast: Move cursor update code to ast_show_cursor()
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 120 +++++++++++++++++++--------------
 1 file changed, 69 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index a4cbf2d5ee0a..1294f0612fd5 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,72 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height)
 	return csum;
 }
 
+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);
+}
+
 static int ast_cursor_set(struct drm_crtc *crtc,
 			  struct drm_file *file_priv,
 			  uint32_t handle,
@@ -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);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 04/12] drm/ast: Move cursor offset swapping into ast_show_cursor()
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

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);
-- 
2.23.0


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

* [PATCH v2 04/12] drm/ast: Move cursor offset swapping into ast_show_cursor()
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

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);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 05/12] drm/ast: Allocate cursor BOs at high end of video memory
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

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.

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..fc35163c7541 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;
 }
-- 
2.23.0


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

* [PATCH v2 05/12] drm/ast: Allocate cursor BOs at high end of video memory
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

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.

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..fc35163c7541 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;
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 06/12] drm/mgag200: Rename cursor functions to use mgag200_ prefix
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

Although the driver source code is fairly inconsistent wrt naming, the
prefix should be mgag200. Rename cursor functions accordingly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 19 ++++++++-----------
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  6 +++---
 drivers/gpu/drm/mgag200/mgag200_mode.c   |  4 ++--
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 89f61573a497..3df70d86af21 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -13,10 +13,10 @@ static bool warn_transparent = true;
 static bool warn_palette = true;
 
 /*
-  Hide the cursor off screen. We can't disable the cursor hardware because it
-  takes too long to re-activate and causes momentary corruption
-*/
-static void mga_hide_cursor(struct mga_device *mdev)
+ * Hide the cursor off screen. We can't disable the cursor hardware because
+ * it takes too long to re-activate and causes momentary corruption.
+ */
+static void mgag200_hide_cursor(struct mga_device *mdev)
 {
 	WREG8(MGA_CURPOSXL, 0);
 	WREG8(MGA_CURPOSXH, 0);
@@ -25,11 +25,8 @@ static void mga_hide_cursor(struct mga_device *mdev)
 	mdev->cursor.pixels_current = NULL;
 }
 
-int mga_crtc_cursor_set(struct drm_crtc *crtc,
-			struct drm_file *file_priv,
-			uint32_t handle,
-			uint32_t width,
-			uint32_t height)
+int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
+			    uint32_t handle, uint32_t width, uint32_t height)
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = (struct mga_device *)dev->dev_private;
@@ -66,7 +63,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	}
 
 	if (!handle || !file_priv) {
-		mga_hide_cursor(mdev);
+		mgag200_hide_cursor(mdev);
 		return 0;
 	}
 
@@ -224,7 +221,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	return ret;
 }
 
-int mga_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
+int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 {
 	struct mga_device *mdev = (struct mga_device *)crtc->dev->dev_private;
 	/* Our origin is at (64,64) */
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 37c003ed57c0..5244e3fa4203 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -203,8 +203,8 @@ int mgag200_mm_init(struct mga_device *mdev);
 void mgag200_mm_fini(struct mga_device *mdev);
 int mgag200_mmap(struct file *filp, struct vm_area_struct *vma);
 
-int mga_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
-						uint32_t handle, uint32_t width, uint32_t height);
-int mga_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
+int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
+			    uint32_t handle, uint32_t width, uint32_t height);
+int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
 
 #endif				/* __MGAG200_DRV_H__ */
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 68226556044b..0cf5608c3644 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1413,8 +1413,8 @@ static void mga_crtc_disable(struct drm_crtc *crtc)
 
 /* These provide the minimum set of functions required to handle a CRTC */
 static const struct drm_crtc_funcs mga_crtc_funcs = {
-	.cursor_set = mga_crtc_cursor_set,
-	.cursor_move = mga_crtc_cursor_move,
+	.cursor_set = mgag200_crtc_cursor_set,
+	.cursor_move = mgag200_crtc_cursor_move,
 	.gamma_set = mga_crtc_gamma_set,
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = mga_crtc_destroy,
-- 
2.23.0


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

* [PATCH v2 06/12] drm/mgag200: Rename cursor functions to use mgag200_ prefix
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

Although the driver source code is fairly inconsistent wrt naming, the
prefix should be mgag200. Rename cursor functions accordingly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 19 ++++++++-----------
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  6 +++---
 drivers/gpu/drm/mgag200/mgag200_mode.c   |  4 ++--
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 89f61573a497..3df70d86af21 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -13,10 +13,10 @@ static bool warn_transparent = true;
 static bool warn_palette = true;
 
 /*
-  Hide the cursor off screen. We can't disable the cursor hardware because it
-  takes too long to re-activate and causes momentary corruption
-*/
-static void mga_hide_cursor(struct mga_device *mdev)
+ * Hide the cursor off screen. We can't disable the cursor hardware because
+ * it takes too long to re-activate and causes momentary corruption.
+ */
+static void mgag200_hide_cursor(struct mga_device *mdev)
 {
 	WREG8(MGA_CURPOSXL, 0);
 	WREG8(MGA_CURPOSXH, 0);
@@ -25,11 +25,8 @@ static void mga_hide_cursor(struct mga_device *mdev)
 	mdev->cursor.pixels_current = NULL;
 }
 
-int mga_crtc_cursor_set(struct drm_crtc *crtc,
-			struct drm_file *file_priv,
-			uint32_t handle,
-			uint32_t width,
-			uint32_t height)
+int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
+			    uint32_t handle, uint32_t width, uint32_t height)
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = (struct mga_device *)dev->dev_private;
@@ -66,7 +63,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	}
 
 	if (!handle || !file_priv) {
-		mga_hide_cursor(mdev);
+		mgag200_hide_cursor(mdev);
 		return 0;
 	}
 
@@ -224,7 +221,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	return ret;
 }
 
-int mga_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
+int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 {
 	struct mga_device *mdev = (struct mga_device *)crtc->dev->dev_private;
 	/* Our origin is at (64,64) */
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 37c003ed57c0..5244e3fa4203 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -203,8 +203,8 @@ int mgag200_mm_init(struct mga_device *mdev);
 void mgag200_mm_fini(struct mga_device *mdev);
 int mgag200_mmap(struct file *filp, struct vm_area_struct *vma);
 
-int mga_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
-						uint32_t handle, uint32_t width, uint32_t height);
-int mga_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
+int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
+			    uint32_t handle, uint32_t width, uint32_t height);
+int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
 
 #endif				/* __MGAG200_DRV_H__ */
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 68226556044b..0cf5608c3644 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1413,8 +1413,8 @@ static void mga_crtc_disable(struct drm_crtc *crtc)
 
 /* These provide the minimum set of functions required to handle a CRTC */
 static const struct drm_crtc_funcs mga_crtc_funcs = {
-	.cursor_set = mga_crtc_cursor_set,
-	.cursor_move = mga_crtc_cursor_move,
+	.cursor_set = mgag200_crtc_cursor_set,
+	.cursor_move = mgag200_crtc_cursor_move,
 	.gamma_set = mga_crtc_gamma_set,
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = mga_crtc_destroy,
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 07/12] drm/mgag200: Add init and fini functions for cursor handling
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

Moving the cursor initialization and cleanup into separate functions
makes the overall code slightly more readable.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 28 ++++++++++++++++++++++++
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  2 ++
 drivers/gpu/drm/mgag200/mgag200_main.c   | 18 +++++----------
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 3df70d86af21..d39e2bc57a70 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -25,6 +25,34 @@ static void mgag200_hide_cursor(struct mga_device *mdev)
 	mdev->cursor.pixels_current = NULL;
 }
 
+int mgag200_cursor_init(struct mga_device *mdev)
+{
+	struct drm_device *dev = mdev->dev;
+
+	/*
+	 * Make small buffers to store a hardware cursor (double
+	 * buffered icon updates)
+	 */
+	mdev->cursor.pixels_1 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
+						    roundup(48*64, PAGE_SIZE),
+						    0, 0);
+	mdev->cursor.pixels_2 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
+						    roundup(48*64, PAGE_SIZE),
+						    0, 0);
+	if (IS_ERR(mdev->cursor.pixels_2) || IS_ERR(mdev->cursor.pixels_1)) {
+		mdev->cursor.pixels_1 = NULL;
+		mdev->cursor.pixels_2 = NULL;
+		dev_warn(&dev->pdev->dev,
+			"Could not allocate space for cursors. Not doing hardware cursors.\n");
+	}
+	mdev->cursor.pixels_current = NULL;
+
+	return 0;
+}
+
+void mgag200_cursor_fini(struct mga_device *mdev)
+{ }
+
 int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 			    uint32_t handle, uint32_t width, uint32_t height)
 {
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 5244e3fa4203..01243fa6397c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -203,6 +203,8 @@ int mgag200_mm_init(struct mga_device *mdev);
 void mgag200_mm_fini(struct mga_device *mdev);
 int mgag200_mmap(struct file *filp, struct vm_area_struct *vma);
 
+int mgag200_cursor_init(struct mga_device *mdev);
+void mgag200_cursor_fini(struct mga_device *mdev);
 int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 			    uint32_t handle, uint32_t width, uint32_t height);
 int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index a9773334dedf..2b59280777a5 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -171,20 +171,10 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 		goto err_modeset;
 	}
 
-	/* Make small buffers to store a hardware cursor (double buffered icon updates) */
-	mdev->cursor.pixels_1 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    roundup(48*64, PAGE_SIZE),
-						    0, 0);
-	mdev->cursor.pixels_2 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    roundup(48*64, PAGE_SIZE),
-						    0, 0);
-	if (IS_ERR(mdev->cursor.pixels_2) || IS_ERR(mdev->cursor.pixels_1)) {
-		mdev->cursor.pixels_1 = NULL;
-		mdev->cursor.pixels_2 = NULL;
+	r = mgag200_cursor_init(mdev);
+	if (r)
 		dev_warn(&dev->pdev->dev,
-			"Could not allocate space for cursors. Not doing hardware cursors.\n");
-	}
-	mdev->cursor.pixels_current = NULL;
+			"Could not initialize cursors. Not doing hardware cursors.\n");
 
 	r = drm_fbdev_generic_setup(mdev->dev, 0);
 	if (r)
@@ -194,6 +184,7 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 
 err_modeset:
 	drm_mode_config_cleanup(dev);
+	mgag200_cursor_fini(mdev);
 	mgag200_mm_fini(mdev);
 err_mm:
 	dev->dev_private = NULL;
@@ -209,6 +200,7 @@ void mgag200_driver_unload(struct drm_device *dev)
 		return;
 	mgag200_modeset_fini(mdev);
 	drm_mode_config_cleanup(dev);
+	mgag200_cursor_fini(mdev);
 	mgag200_mm_fini(mdev);
 	dev->dev_private = NULL;
 }
-- 
2.23.0


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

* [PATCH v2 07/12] drm/mgag200: Add init and fini functions for cursor handling
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

Moving the cursor initialization and cleanup into separate functions
makes the overall code slightly more readable.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 28 ++++++++++++++++++++++++
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  2 ++
 drivers/gpu/drm/mgag200/mgag200_main.c   | 18 +++++----------
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 3df70d86af21..d39e2bc57a70 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -25,6 +25,34 @@ static void mgag200_hide_cursor(struct mga_device *mdev)
 	mdev->cursor.pixels_current = NULL;
 }
 
+int mgag200_cursor_init(struct mga_device *mdev)
+{
+	struct drm_device *dev = mdev->dev;
+
+	/*
+	 * Make small buffers to store a hardware cursor (double
+	 * buffered icon updates)
+	 */
+	mdev->cursor.pixels_1 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
+						    roundup(48*64, PAGE_SIZE),
+						    0, 0);
+	mdev->cursor.pixels_2 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
+						    roundup(48*64, PAGE_SIZE),
+						    0, 0);
+	if (IS_ERR(mdev->cursor.pixels_2) || IS_ERR(mdev->cursor.pixels_1)) {
+		mdev->cursor.pixels_1 = NULL;
+		mdev->cursor.pixels_2 = NULL;
+		dev_warn(&dev->pdev->dev,
+			"Could not allocate space for cursors. Not doing hardware cursors.\n");
+	}
+	mdev->cursor.pixels_current = NULL;
+
+	return 0;
+}
+
+void mgag200_cursor_fini(struct mga_device *mdev)
+{ }
+
 int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 			    uint32_t handle, uint32_t width, uint32_t height)
 {
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 5244e3fa4203..01243fa6397c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -203,6 +203,8 @@ int mgag200_mm_init(struct mga_device *mdev);
 void mgag200_mm_fini(struct mga_device *mdev);
 int mgag200_mmap(struct file *filp, struct vm_area_struct *vma);
 
+int mgag200_cursor_init(struct mga_device *mdev);
+void mgag200_cursor_fini(struct mga_device *mdev);
 int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 			    uint32_t handle, uint32_t width, uint32_t height);
 int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index a9773334dedf..2b59280777a5 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -171,20 +171,10 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 		goto err_modeset;
 	}
 
-	/* Make small buffers to store a hardware cursor (double buffered icon updates) */
-	mdev->cursor.pixels_1 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    roundup(48*64, PAGE_SIZE),
-						    0, 0);
-	mdev->cursor.pixels_2 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    roundup(48*64, PAGE_SIZE),
-						    0, 0);
-	if (IS_ERR(mdev->cursor.pixels_2) || IS_ERR(mdev->cursor.pixels_1)) {
-		mdev->cursor.pixels_1 = NULL;
-		mdev->cursor.pixels_2 = NULL;
+	r = mgag200_cursor_init(mdev);
+	if (r)
 		dev_warn(&dev->pdev->dev,
-			"Could not allocate space for cursors. Not doing hardware cursors.\n");
-	}
-	mdev->cursor.pixels_current = NULL;
+			"Could not initialize cursors. Not doing hardware cursors.\n");
 
 	r = drm_fbdev_generic_setup(mdev->dev, 0);
 	if (r)
@@ -194,6 +184,7 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 
 err_modeset:
 	drm_mode_config_cleanup(dev);
+	mgag200_cursor_fini(mdev);
 	mgag200_mm_fini(mdev);
 err_mm:
 	dev->dev_private = NULL;
@@ -209,6 +200,7 @@ void mgag200_driver_unload(struct drm_device *dev)
 		return;
 	mgag200_modeset_fini(mdev);
 	drm_mode_config_cleanup(dev);
+	mgag200_cursor_fini(mdev);
 	mgag200_mm_fini(mdev);
 	dev->dev_private = NULL;
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 08/12] drm/mgag200: Add separate move-cursor function
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

Adding mgag200_move_cursor() makes the cursor code more consistent and
will become handy when we move to universal cursor planes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 29 ++++++++++++++++--------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index d39e2bc57a70..621960723a3a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -25,6 +25,24 @@ static void mgag200_hide_cursor(struct mga_device *mdev)
 	mdev->cursor.pixels_current = NULL;
 }
 
+static void mgag200_move_cursor(struct mga_device *mdev, int x, int y)
+{
+	if (WARN_ON(x <= 0))
+		return;
+	if (WARN_ON(y <= 0))
+		return;
+	if (WARN_ON(x & ~0xffff))
+		return;
+	if (WARN_ON(y & ~0xffff))
+		return;
+
+	WREG8(MGA_CURPOSXL, x & 0xff);
+	WREG8(MGA_CURPOSXH, (x>>8) & 0xff);
+
+	WREG8(MGA_CURPOSYL, y & 0xff);
+	WREG8(MGA_CURPOSYH, (y>>8) & 0xff);
+}
+
 int mgag200_cursor_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
@@ -252,19 +270,12 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 {
 	struct mga_device *mdev = (struct mga_device *)crtc->dev->dev_private;
+
 	/* Our origin is at (64,64) */
 	x += 64;
 	y += 64;
 
-	BUG_ON(x <= 0);
-	BUG_ON(y <= 0);
-	BUG_ON(x & ~0xffff);
-	BUG_ON(y & ~0xffff);
+	mgag200_move_cursor(mdev, x, y);
 
-	WREG8(MGA_CURPOSXL, x & 0xff);
-	WREG8(MGA_CURPOSXH, (x>>8) & 0xff);
-
-	WREG8(MGA_CURPOSYL, y & 0xff);
-	WREG8(MGA_CURPOSYH, (y>>8) & 0xff);
 	return 0;
 }
-- 
2.23.0


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

* [PATCH v2 08/12] drm/mgag200: Add separate move-cursor function
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

Adding mgag200_move_cursor() makes the cursor code more consistent and
will become handy when we move to universal cursor planes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 29 ++++++++++++++++--------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index d39e2bc57a70..621960723a3a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -25,6 +25,24 @@ static void mgag200_hide_cursor(struct mga_device *mdev)
 	mdev->cursor.pixels_current = NULL;
 }
 
+static void mgag200_move_cursor(struct mga_device *mdev, int x, int y)
+{
+	if (WARN_ON(x <= 0))
+		return;
+	if (WARN_ON(y <= 0))
+		return;
+	if (WARN_ON(x & ~0xffff))
+		return;
+	if (WARN_ON(y & ~0xffff))
+		return;
+
+	WREG8(MGA_CURPOSXL, x & 0xff);
+	WREG8(MGA_CURPOSXH, (x>>8) & 0xff);
+
+	WREG8(MGA_CURPOSYL, y & 0xff);
+	WREG8(MGA_CURPOSYH, (y>>8) & 0xff);
+}
+
 int mgag200_cursor_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
@@ -252,19 +270,12 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 {
 	struct mga_device *mdev = (struct mga_device *)crtc->dev->dev_private;
+
 	/* Our origin is at (64,64) */
 	x += 64;
 	y += 64;
 
-	BUG_ON(x <= 0);
-	BUG_ON(y <= 0);
-	BUG_ON(x & ~0xffff);
-	BUG_ON(y & ~0xffff);
+	mgag200_move_cursor(mdev, x, y);
 
-	WREG8(MGA_CURPOSXL, x & 0xff);
-	WREG8(MGA_CURPOSXH, (x>>8) & 0xff);
-
-	WREG8(MGA_CURPOSYL, y & 0xff);
-	WREG8(MGA_CURPOSYH, (y>>8) & 0xff);
 	return 0;
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 09/12] drm/mgag200: Move cursor-image update to mgag200_show_cursor()
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

Separating the management of buffer objects from updating the hardware
cursor buffer gives the code more structure. While doing this, we can
further split the image-update code into code for writing the buffer,
setting the base scan-out address, and enabling the cursor. The first
two operations are in dedicated functions update() and set_base().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 221 +++++++++++++----------
 1 file changed, 126 insertions(+), 95 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 621960723a3a..13daa0ce1c9e 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -12,6 +12,128 @@
 static bool warn_transparent = true;
 static bool warn_palette = true;
 
+static int mgag200_cursor_update(struct mga_device *mdev, void *dst, void *src,
+				 unsigned int width, unsigned int height)
+{
+	struct drm_device *dev = mdev->dev;
+	unsigned int i, row, col;
+	uint32_t colour_set[16];
+	uint32_t *next_space = &colour_set[0];
+	uint32_t *palette_iter;
+	uint32_t this_colour;
+	bool found = false;
+	int colour_count = 0;
+	u8 reg_index;
+	u8 this_row[48];
+
+	memset(&colour_set[0], 0, sizeof(uint32_t)*16);
+	/* width*height*4 = 16384 */
+	for (i = 0; i < 16384; i += 4) {
+		this_colour = ioread32(src + i);
+		/* No transparency */
+		if (this_colour>>24 != 0xff &&
+			this_colour>>24 != 0x0) {
+			if (warn_transparent) {
+				dev_info(&dev->pdev->dev, "Video card doesn't support cursors with partial transparency.\n");
+				dev_info(&dev->pdev->dev, "Not enabling hardware cursor.\n");
+				warn_transparent = false; /* Only tell the user once. */
+			}
+			return -EINVAL;
+		}
+		/* Don't need to store transparent pixels as colours */
+		if (this_colour>>24 == 0x0)
+			continue;
+		found = false;
+		for (palette_iter = &colour_set[0]; palette_iter != next_space; palette_iter++) {
+			if (*palette_iter == this_colour) {
+				found = true;
+				break;
+			}
+		}
+		if (found)
+			continue;
+		/* We only support 4bit paletted cursors */
+		if (colour_count >= 16) {
+			if (warn_palette) {
+				dev_info(&dev->pdev->dev, "Video card only supports cursors with up to 16 colours.\n");
+				dev_info(&dev->pdev->dev, "Not enabling hardware cursor.\n");
+				warn_palette = false; /* Only tell the user once. */
+			}
+			return -EINVAL;
+		}
+		*next_space = this_colour;
+		next_space++;
+		colour_count++;
+	}
+
+	/* Program colours from cursor icon into palette */
+	for (i = 0; i < colour_count; i++) {
+		if (i <= 2)
+			reg_index = 0x8 + i*0x4;
+		else
+			reg_index = 0x60 + i*0x3;
+		WREG_DAC(reg_index, colour_set[i] & 0xff);
+		WREG_DAC(reg_index+1, colour_set[i]>>8 & 0xff);
+		WREG_DAC(reg_index+2, colour_set[i]>>16 & 0xff);
+		if (WARN_ON((colour_set[i]>>24 & 0xff) != 0xff))
+			return -EINVAL;
+	}
+
+	/* now write colour indices into hardware cursor buffer */
+	for (row = 0; row < 64; row++) {
+		memset(&this_row[0], 0, 48);
+		for (col = 0; col < 64; col++) {
+			this_colour = ioread32(src + 4*(col + 64*row));
+			/* write transparent pixels */
+			if (this_colour>>24 == 0x0) {
+				this_row[47 - col/8] |= 0x80>>(col%8);
+				continue;
+			}
+
+			/* write colour index here */
+			for (i = 0; i < colour_count; i++) {
+				if (colour_set[i] == this_colour) {
+					if (col % 2)
+						this_row[col/2] |= i<<4;
+					else
+						this_row[col/2] |= i;
+					break;
+				}
+			}
+		}
+		memcpy_toio(dst + row*48, &this_row[0], 48);
+	}
+
+	return 0;
+}
+
+static void mgag200_cursor_set_base(struct mga_device *mdev, u64 address)
+{
+	u8 addrl = (address >> 10) & 0xff;
+	u8 addrh = (address >> 18) & 0x3f;
+
+	/* Program gpu address of cursor buffer */
+	WREG_DAC(MGA1064_CURSOR_BASE_ADR_LOW, addrl);
+	WREG_DAC(MGA1064_CURSOR_BASE_ADR_HI, addrh);
+}
+
+static int mgag200_show_cursor(struct mga_device *mdev, void *dst, void *src,
+			       unsigned int width, unsigned int height,
+			       u64 dst_gpu)
+{
+	int ret;
+
+	ret = mgag200_cursor_update(mdev, dst, src, width, height);
+	if (ret)
+		return ret;
+	mgag200_cursor_set_base(mdev, dst_gpu);
+
+	/* Adjust cursor control register to turn on the cursor */
+	WREG_DAC(MGA1064_CURSOR_CTL, 4); /* 16-colour palletized cursor mode */
+
+	return 0;
+}
+
 /*
  * Hide the cursor off screen. We can't disable the cursor hardware because
  * it takes too long to re-activate and causes momentary corruption.
@@ -82,19 +204,10 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 	struct drm_gem_vram_object *pixels_next;
 	struct drm_gem_object *obj;
 	struct drm_gem_vram_object *gbo = NULL;
-	int ret = 0;
+	int ret;
 	u8 *src, *dst;
-	unsigned int i, row, col;
-	uint32_t colour_set[16];
-	uint32_t *next_space = &colour_set[0];
-	uint32_t *palette_iter;
-	uint32_t this_colour;
-	bool found = false;
-	int colour_count = 0;
 	s64 gpu_addr;
 	u64 dst_gpu;
-	u8 reg_index;
-	u8 this_row[48];
 
 	if (!pixels_1 || !pixels_2) {
 		WREG8(MGA_CURPOSXL, 0);
@@ -159,91 +272,9 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 	}
 	dst_gpu = (u64)gpu_addr;
 
-	memset(&colour_set[0], 0, sizeof(uint32_t)*16);
-	/* width*height*4 = 16384 */
-	for (i = 0; i < 16384; i += 4) {
-		this_colour = ioread32(src + i);
-		/* No transparency */
-		if (this_colour>>24 != 0xff &&
-			this_colour>>24 != 0x0) {
-			if (warn_transparent) {
-				dev_info(&dev->pdev->dev, "Video card doesn't support cursors with partial transparency.\n");
-				dev_info(&dev->pdev->dev, "Not enabling hardware cursor.\n");
-				warn_transparent = false; /* Only tell the user once. */
-			}
-			ret = -EINVAL;
-			goto err_drm_gem_vram_kunmap_dst;
-		}
-		/* Don't need to store transparent pixels as colours */
-		if (this_colour>>24 == 0x0)
-			continue;
-		found = false;
-		for (palette_iter = &colour_set[0]; palette_iter != next_space; palette_iter++) {
-			if (*palette_iter == this_colour) {
-				found = true;
-				break;
-			}
-		}
-		if (found)
-			continue;
-		/* We only support 4bit paletted cursors */
-		if (colour_count >= 16) {
-			if (warn_palette) {
-				dev_info(&dev->pdev->dev, "Video card only supports cursors with up to 16 colours.\n");
-				dev_info(&dev->pdev->dev, "Not enabling hardware cursor.\n");
-				warn_palette = false; /* Only tell the user once. */
-			}
-			ret = -EINVAL;
-			goto err_drm_gem_vram_kunmap_dst;
-		}
-		*next_space = this_colour;
-		next_space++;
-		colour_count++;
-	}
-
-	/* Program colours from cursor icon into palette */
-	for (i = 0; i < colour_count; i++) {
-		if (i <= 2)
-			reg_index = 0x8 + i*0x4;
-		else
-			reg_index = 0x60 + i*0x3;
-		WREG_DAC(reg_index, colour_set[i] & 0xff);
-		WREG_DAC(reg_index+1, colour_set[i]>>8 & 0xff);
-		WREG_DAC(reg_index+2, colour_set[i]>>16 & 0xff);
-		BUG_ON((colour_set[i]>>24 & 0xff) != 0xff);
-	}
-
-	/* now write colour indices into hardware cursor buffer */
-	for (row = 0; row < 64; row++) {
-		memset(&this_row[0], 0, 48);
-		for (col = 0; col < 64; col++) {
-			this_colour = ioread32(src + 4*(col + 64*row));
-			/* write transparent pixels */
-			if (this_colour>>24 == 0x0) {
-				this_row[47 - col/8] |= 0x80>>(col%8);
-				continue;
-			}
-
-			/* write colour index here */
-			for (i = 0; i < colour_count; i++) {
-				if (colour_set[i] == this_colour) {
-					if (col % 2)
-						this_row[col/2] |= i<<4;
-					else
-						this_row[col/2] |= i;
-					break;
-				}
-			}
-		}
-		memcpy_toio(dst + row*48, &this_row[0], 48);
-	}
-
-	/* Program gpu address of cursor buffer */
-	WREG_DAC(MGA1064_CURSOR_BASE_ADR_LOW, (u8)((dst_gpu>>10) & 0xff));
-	WREG_DAC(MGA1064_CURSOR_BASE_ADR_HI, (u8)((dst_gpu>>18) & 0x3f));
-
-	/* Adjust cursor control register to turn on the cursor */
-	WREG_DAC(MGA1064_CURSOR_CTL, 4); /* 16-colour palletized cursor mode */
+	ret = mgag200_show_cursor(mdev, dst, src, width, height, dst_gpu);
+	if (ret)
+		goto err_drm_gem_vram_kunmap_dst;
 
 	/* Now update internal buffer pointers */
 	if (pixels_current)
-- 
2.23.0


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

* [PATCH v2 09/12] drm/mgag200: Move cursor-image update to mgag200_show_cursor()
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

Separating the management of buffer objects from updating the hardware
cursor buffer gives the code more structure. While doing this, we can
further split the image-update code into code for writing the buffer,
setting the base scan-out address, and enabling the cursor. The first
two operations are in dedicated functions update() and set_base().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 221 +++++++++++++----------
 1 file changed, 126 insertions(+), 95 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 621960723a3a..13daa0ce1c9e 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -12,6 +12,128 @@
 static bool warn_transparent = true;
 static bool warn_palette = true;
 
+static int mgag200_cursor_update(struct mga_device *mdev, void *dst, void *src,
+				 unsigned int width, unsigned int height)
+{
+	struct drm_device *dev = mdev->dev;
+	unsigned int i, row, col;
+	uint32_t colour_set[16];
+	uint32_t *next_space = &colour_set[0];
+	uint32_t *palette_iter;
+	uint32_t this_colour;
+	bool found = false;
+	int colour_count = 0;
+	u8 reg_index;
+	u8 this_row[48];
+
+	memset(&colour_set[0], 0, sizeof(uint32_t)*16);
+	/* width*height*4 = 16384 */
+	for (i = 0; i < 16384; i += 4) {
+		this_colour = ioread32(src + i);
+		/* No transparency */
+		if (this_colour>>24 != 0xff &&
+			this_colour>>24 != 0x0) {
+			if (warn_transparent) {
+				dev_info(&dev->pdev->dev, "Video card doesn't support cursors with partial transparency.\n");
+				dev_info(&dev->pdev->dev, "Not enabling hardware cursor.\n");
+				warn_transparent = false; /* Only tell the user once. */
+			}
+			return -EINVAL;
+		}
+		/* Don't need to store transparent pixels as colours */
+		if (this_colour>>24 == 0x0)
+			continue;
+		found = false;
+		for (palette_iter = &colour_set[0]; palette_iter != next_space; palette_iter++) {
+			if (*palette_iter == this_colour) {
+				found = true;
+				break;
+			}
+		}
+		if (found)
+			continue;
+		/* We only support 4bit paletted cursors */
+		if (colour_count >= 16) {
+			if (warn_palette) {
+				dev_info(&dev->pdev->dev, "Video card only supports cursors with up to 16 colours.\n");
+				dev_info(&dev->pdev->dev, "Not enabling hardware cursor.\n");
+				warn_palette = false; /* Only tell the user once. */
+			}
+			return -EINVAL;
+		}
+		*next_space = this_colour;
+		next_space++;
+		colour_count++;
+	}
+
+	/* Program colours from cursor icon into palette */
+	for (i = 0; i < colour_count; i++) {
+		if (i <= 2)
+			reg_index = 0x8 + i*0x4;
+		else
+			reg_index = 0x60 + i*0x3;
+		WREG_DAC(reg_index, colour_set[i] & 0xff);
+		WREG_DAC(reg_index+1, colour_set[i]>>8 & 0xff);
+		WREG_DAC(reg_index+2, colour_set[i]>>16 & 0xff);
+		if (WARN_ON((colour_set[i]>>24 & 0xff) != 0xff))
+			return -EINVAL;
+	}
+
+	/* now write colour indices into hardware cursor buffer */
+	for (row = 0; row < 64; row++) {
+		memset(&this_row[0], 0, 48);
+		for (col = 0; col < 64; col++) {
+			this_colour = ioread32(src + 4*(col + 64*row));
+			/* write transparent pixels */
+			if (this_colour>>24 == 0x0) {
+				this_row[47 - col/8] |= 0x80>>(col%8);
+				continue;
+			}
+
+			/* write colour index here */
+			for (i = 0; i < colour_count; i++) {
+				if (colour_set[i] == this_colour) {
+					if (col % 2)
+						this_row[col/2] |= i<<4;
+					else
+						this_row[col/2] |= i;
+					break;
+				}
+			}
+		}
+		memcpy_toio(dst + row*48, &this_row[0], 48);
+	}
+
+	return 0;
+}
+
+static void mgag200_cursor_set_base(struct mga_device *mdev, u64 address)
+{
+	u8 addrl = (address >> 10) & 0xff;
+	u8 addrh = (address >> 18) & 0x3f;
+
+	/* Program gpu address of cursor buffer */
+	WREG_DAC(MGA1064_CURSOR_BASE_ADR_LOW, addrl);
+	WREG_DAC(MGA1064_CURSOR_BASE_ADR_HI, addrh);
+}
+
+static int mgag200_show_cursor(struct mga_device *mdev, void *dst, void *src,
+			       unsigned int width, unsigned int height,
+			       u64 dst_gpu)
+{
+	int ret;
+
+	ret = mgag200_cursor_update(mdev, dst, src, width, height);
+	if (ret)
+		return ret;
+	mgag200_cursor_set_base(mdev, dst_gpu);
+
+	/* Adjust cursor control register to turn on the cursor */
+	WREG_DAC(MGA1064_CURSOR_CTL, 4); /* 16-colour palletized cursor mode */
+
+	return 0;
+}
+
 /*
  * Hide the cursor off screen. We can't disable the cursor hardware because
  * it takes too long to re-activate and causes momentary corruption.
@@ -82,19 +204,10 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 	struct drm_gem_vram_object *pixels_next;
 	struct drm_gem_object *obj;
 	struct drm_gem_vram_object *gbo = NULL;
-	int ret = 0;
+	int ret;
 	u8 *src, *dst;
-	unsigned int i, row, col;
-	uint32_t colour_set[16];
-	uint32_t *next_space = &colour_set[0];
-	uint32_t *palette_iter;
-	uint32_t this_colour;
-	bool found = false;
-	int colour_count = 0;
 	s64 gpu_addr;
 	u64 dst_gpu;
-	u8 reg_index;
-	u8 this_row[48];
 
 	if (!pixels_1 || !pixels_2) {
 		WREG8(MGA_CURPOSXL, 0);
@@ -159,91 +272,9 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 	}
 	dst_gpu = (u64)gpu_addr;
 
-	memset(&colour_set[0], 0, sizeof(uint32_t)*16);
-	/* width*height*4 = 16384 */
-	for (i = 0; i < 16384; i += 4) {
-		this_colour = ioread32(src + i);
-		/* No transparency */
-		if (this_colour>>24 != 0xff &&
-			this_colour>>24 != 0x0) {
-			if (warn_transparent) {
-				dev_info(&dev->pdev->dev, "Video card doesn't support cursors with partial transparency.\n");
-				dev_info(&dev->pdev->dev, "Not enabling hardware cursor.\n");
-				warn_transparent = false; /* Only tell the user once. */
-			}
-			ret = -EINVAL;
-			goto err_drm_gem_vram_kunmap_dst;
-		}
-		/* Don't need to store transparent pixels as colours */
-		if (this_colour>>24 == 0x0)
-			continue;
-		found = false;
-		for (palette_iter = &colour_set[0]; palette_iter != next_space; palette_iter++) {
-			if (*palette_iter == this_colour) {
-				found = true;
-				break;
-			}
-		}
-		if (found)
-			continue;
-		/* We only support 4bit paletted cursors */
-		if (colour_count >= 16) {
-			if (warn_palette) {
-				dev_info(&dev->pdev->dev, "Video card only supports cursors with up to 16 colours.\n");
-				dev_info(&dev->pdev->dev, "Not enabling hardware cursor.\n");
-				warn_palette = false; /* Only tell the user once. */
-			}
-			ret = -EINVAL;
-			goto err_drm_gem_vram_kunmap_dst;
-		}
-		*next_space = this_colour;
-		next_space++;
-		colour_count++;
-	}
-
-	/* Program colours from cursor icon into palette */
-	for (i = 0; i < colour_count; i++) {
-		if (i <= 2)
-			reg_index = 0x8 + i*0x4;
-		else
-			reg_index = 0x60 + i*0x3;
-		WREG_DAC(reg_index, colour_set[i] & 0xff);
-		WREG_DAC(reg_index+1, colour_set[i]>>8 & 0xff);
-		WREG_DAC(reg_index+2, colour_set[i]>>16 & 0xff);
-		BUG_ON((colour_set[i]>>24 & 0xff) != 0xff);
-	}
-
-	/* now write colour indices into hardware cursor buffer */
-	for (row = 0; row < 64; row++) {
-		memset(&this_row[0], 0, 48);
-		for (col = 0; col < 64; col++) {
-			this_colour = ioread32(src + 4*(col + 64*row));
-			/* write transparent pixels */
-			if (this_colour>>24 == 0x0) {
-				this_row[47 - col/8] |= 0x80>>(col%8);
-				continue;
-			}
-
-			/* write colour index here */
-			for (i = 0; i < colour_count; i++) {
-				if (colour_set[i] == this_colour) {
-					if (col % 2)
-						this_row[col/2] |= i<<4;
-					else
-						this_row[col/2] |= i;
-					break;
-				}
-			}
-		}
-		memcpy_toio(dst + row*48, &this_row[0], 48);
-	}
-
-	/* Program gpu address of cursor buffer */
-	WREG_DAC(MGA1064_CURSOR_BASE_ADR_LOW, (u8)((dst_gpu>>10) & 0xff));
-	WREG_DAC(MGA1064_CURSOR_BASE_ADR_HI, (u8)((dst_gpu>>18) & 0x3f));
-
-	/* Adjust cursor control register to turn on the cursor */
-	WREG_DAC(MGA1064_CURSOR_CTL, 4); /* 16-colour palletized cursor mode */
+	ret = mgag200_show_cursor(mdev, dst, src, width, height, dst_gpu);
+	if (ret)
+		goto err_drm_gem_vram_kunmap_dst;
 
 	/* Now update internal buffer pointers */
 	if (pixels_current)
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 10/12] drm/mgag200: Move cursor BO swapping into mgag200_show_cursor()
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

Selecting the correct BO for the new cursor image is not relevant
outside of mgag200_show_cursor(). Let the function do the work.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 120 +++++++++++------------
 1 file changed, 56 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 13daa0ce1c9e..4a5b1aa921e0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -117,21 +117,69 @@ static void mgag200_cursor_set_base(struct mga_device *mdev, u64 address)
 	WREG_DAC(MGA1064_CURSOR_BASE_ADR_HI, addrh);
 }
 
-static int mgag200_show_cursor(struct mga_device *mdev, void *dst, void *src,
-			       unsigned int width, unsigned int height,
-			       u64 dst_gpu)
+static int mgag200_show_cursor(struct mga_device *mdev, void *src,
+			       unsigned int width, unsigned int height)
 {
+	struct drm_device *dev = mdev->dev;
+	struct drm_gem_vram_object *pixels_1 = mdev->cursor.pixels_1;
+	struct drm_gem_vram_object *pixels_2 = mdev->cursor.pixels_2;
+	struct drm_gem_vram_object *pixels_current = mdev->cursor.pixels_current;
+	struct drm_gem_vram_object *pixels_next;
+	void *dst;
+	s64 off;
 	int ret;
 
+	if (!pixels_1 || !pixels_2) {
+		WREG8(MGA_CURPOSXL, 0);
+		WREG8(MGA_CURPOSXH, 0);
+		return -ENOTSUPP; /* Didn't allocate space for cursors */
+	}
+
+	if (WARN_ON(pixels_current &&
+		    pixels_1 != pixels_current &&
+		    pixels_2 != pixels_current)) {
+		return -ENOTSUPP; /* inconsistent state */
+	}
+
+	if (pixels_current == pixels_1)
+		pixels_next = pixels_2;
+	else
+		pixels_next = pixels_1;
+
+	dst = drm_gem_vram_vmap(pixels_next);
+	if (IS_ERR(dst)) {
+		ret = PTR_ERR(dst);
+		dev_err(&dev->pdev->dev,
+			"failed to map cursor updates: %d\n", ret);
+		return ret;
+	}
+	off = drm_gem_vram_offset(pixels_next);
+	if (off < 0) {
+		ret = (int)off;
+		dev_err(&dev->pdev->dev,
+			"failed to get cursor scanout address: %d\n", ret);
+		goto err_drm_gem_vram_vunmap;
+	}
+
 	ret = mgag200_cursor_update(mdev, dst, src, width, height);
 	if (ret)
-		return ret;
-	mgag200_cursor_set_base(mdev, dst_gpu);
+		goto err_drm_gem_vram_vunmap;
+	mgag200_cursor_set_base(mdev, off);
 
 	/* Adjust cursor control register to turn on the cursor */
 	WREG_DAC(MGA1064_CURSOR_CTL, 4); /* 16-colour palletized cursor mode */
 
+	if (pixels_current)
+		drm_gem_vram_unpin(pixels_current);
+	mdev->cursor.pixels_current = pixels_next;
+
+	drm_gem_vram_vunmap(pixels_next, dst);
+
 	return 0;
+
+err_drm_gem_vram_vunmap:
+	drm_gem_vram_vunmap(pixels_next, dst);
+	return ret;
 }
 
 /*
@@ -198,28 +246,10 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = (struct mga_device *)dev->dev_private;
-	struct drm_gem_vram_object *pixels_1 = mdev->cursor.pixels_1;
-	struct drm_gem_vram_object *pixels_2 = mdev->cursor.pixels_2;
-	struct drm_gem_vram_object *pixels_current = mdev->cursor.pixels_current;
-	struct drm_gem_vram_object *pixels_next;
 	struct drm_gem_object *obj;
 	struct drm_gem_vram_object *gbo = NULL;
 	int ret;
-	u8 *src, *dst;
-	s64 gpu_addr;
-	u64 dst_gpu;
-
-	if (!pixels_1 || !pixels_2) {
-		WREG8(MGA_CURPOSXL, 0);
-		WREG8(MGA_CURPOSXH, 0);
-		return -ENOTSUPP; /* Didn't allocate space for cursors */
-	}
-
-	if (WARN_ON(pixels_current &&
-		    pixels_1 != pixels_current &&
-		    pixels_2 != pixels_current)) {
-		return -ENOTSUPP; /* inconsistent state */
-	}
+	u8 *src;
 
 	if (!handle || !file_priv) {
 		mgag200_hide_cursor(mdev);
@@ -232,11 +262,6 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 		return -EINVAL;
 	}
 
-	if (pixels_current == pixels_1)
-		pixels_next = pixels_2;
-	else
-		pixels_next = pixels_1;
-
 	obj = drm_gem_object_lookup(file_priv, handle);
 	if (!obj)
 		return -ENOENT;
@@ -249,48 +274,15 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 		goto err_drm_gem_object_put_unlocked;
 	}
 
-	/* Pin and map up-coming buffer to write colour indices */
-	ret = drm_gem_vram_pin(pixels_next, DRM_GEM_VRAM_PL_FLAG_VRAM);
-	if (ret) {
-		dev_err(&dev->pdev->dev,
-			"failed to pin cursor buffer: %d\n", ret);
-		goto err_drm_gem_vram_vunmap;
-	}
-	dst = drm_gem_vram_kmap(pixels_next, true, NULL);
-	if (IS_ERR(dst)) {
-		ret = PTR_ERR(dst);
-		dev_err(&dev->pdev->dev,
-			"failed to kmap cursor updates: %d\n", ret);
-		goto err_drm_gem_vram_unpin_dst;
-	}
-	gpu_addr = drm_gem_vram_offset(pixels_next);
-	if (gpu_addr < 0) {
-		ret = (int)gpu_addr;
-		dev_err(&dev->pdev->dev,
-			"failed to get cursor scanout address: %d\n", ret);
-		goto err_drm_gem_vram_kunmap_dst;
-	}
-	dst_gpu = (u64)gpu_addr;
-
-	ret = mgag200_show_cursor(mdev, dst, src, width, height, dst_gpu);
+	ret = mgag200_show_cursor(mdev, src, width, height);
 	if (ret)
-		goto err_drm_gem_vram_kunmap_dst;
+		goto err_drm_gem_vram_vunmap;
 
 	/* Now update internal buffer pointers */
-	if (pixels_current)
-		drm_gem_vram_unpin(pixels_current);
-	mdev->cursor.pixels_current = pixels_next;
-
-	drm_gem_vram_kunmap(pixels_next);
 	drm_gem_vram_vunmap(gbo, src);
 	drm_gem_object_put_unlocked(obj);
 
 	return 0;
-
-err_drm_gem_vram_kunmap_dst:
-	drm_gem_vram_kunmap(pixels_next);
-err_drm_gem_vram_unpin_dst:
-	drm_gem_vram_unpin(pixels_next);
 err_drm_gem_vram_vunmap:
 	drm_gem_vram_vunmap(gbo, src);
 err_drm_gem_object_put_unlocked:
-- 
2.23.0


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

* [PATCH v2 10/12] drm/mgag200: Move cursor BO swapping into mgag200_show_cursor()
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

Selecting the correct BO for the new cursor image is not relevant
outside of mgag200_show_cursor(). Let the function do the work.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 120 +++++++++++------------
 1 file changed, 56 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 13daa0ce1c9e..4a5b1aa921e0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -117,21 +117,69 @@ static void mgag200_cursor_set_base(struct mga_device *mdev, u64 address)
 	WREG_DAC(MGA1064_CURSOR_BASE_ADR_HI, addrh);
 }
 
-static int mgag200_show_cursor(struct mga_device *mdev, void *dst, void *src,
-			       unsigned int width, unsigned int height,
-			       u64 dst_gpu)
+static int mgag200_show_cursor(struct mga_device *mdev, void *src,
+			       unsigned int width, unsigned int height)
 {
+	struct drm_device *dev = mdev->dev;
+	struct drm_gem_vram_object *pixels_1 = mdev->cursor.pixels_1;
+	struct drm_gem_vram_object *pixels_2 = mdev->cursor.pixels_2;
+	struct drm_gem_vram_object *pixels_current = mdev->cursor.pixels_current;
+	struct drm_gem_vram_object *pixels_next;
+	void *dst;
+	s64 off;
 	int ret;
 
+	if (!pixels_1 || !pixels_2) {
+		WREG8(MGA_CURPOSXL, 0);
+		WREG8(MGA_CURPOSXH, 0);
+		return -ENOTSUPP; /* Didn't allocate space for cursors */
+	}
+
+	if (WARN_ON(pixels_current &&
+		    pixels_1 != pixels_current &&
+		    pixels_2 != pixels_current)) {
+		return -ENOTSUPP; /* inconsistent state */
+	}
+
+	if (pixels_current == pixels_1)
+		pixels_next = pixels_2;
+	else
+		pixels_next = pixels_1;
+
+	dst = drm_gem_vram_vmap(pixels_next);
+	if (IS_ERR(dst)) {
+		ret = PTR_ERR(dst);
+		dev_err(&dev->pdev->dev,
+			"failed to map cursor updates: %d\n", ret);
+		return ret;
+	}
+	off = drm_gem_vram_offset(pixels_next);
+	if (off < 0) {
+		ret = (int)off;
+		dev_err(&dev->pdev->dev,
+			"failed to get cursor scanout address: %d\n", ret);
+		goto err_drm_gem_vram_vunmap;
+	}
+
 	ret = mgag200_cursor_update(mdev, dst, src, width, height);
 	if (ret)
-		return ret;
-	mgag200_cursor_set_base(mdev, dst_gpu);
+		goto err_drm_gem_vram_vunmap;
+	mgag200_cursor_set_base(mdev, off);
 
 	/* Adjust cursor control register to turn on the cursor */
 	WREG_DAC(MGA1064_CURSOR_CTL, 4); /* 16-colour palletized cursor mode */
 
+	if (pixels_current)
+		drm_gem_vram_unpin(pixels_current);
+	mdev->cursor.pixels_current = pixels_next;
+
+	drm_gem_vram_vunmap(pixels_next, dst);
+
 	return 0;
+
+err_drm_gem_vram_vunmap:
+	drm_gem_vram_vunmap(pixels_next, dst);
+	return ret;
 }
 
 /*
@@ -198,28 +246,10 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = (struct mga_device *)dev->dev_private;
-	struct drm_gem_vram_object *pixels_1 = mdev->cursor.pixels_1;
-	struct drm_gem_vram_object *pixels_2 = mdev->cursor.pixels_2;
-	struct drm_gem_vram_object *pixels_current = mdev->cursor.pixels_current;
-	struct drm_gem_vram_object *pixels_next;
 	struct drm_gem_object *obj;
 	struct drm_gem_vram_object *gbo = NULL;
 	int ret;
-	u8 *src, *dst;
-	s64 gpu_addr;
-	u64 dst_gpu;
-
-	if (!pixels_1 || !pixels_2) {
-		WREG8(MGA_CURPOSXL, 0);
-		WREG8(MGA_CURPOSXH, 0);
-		return -ENOTSUPP; /* Didn't allocate space for cursors */
-	}
-
-	if (WARN_ON(pixels_current &&
-		    pixels_1 != pixels_current &&
-		    pixels_2 != pixels_current)) {
-		return -ENOTSUPP; /* inconsistent state */
-	}
+	u8 *src;
 
 	if (!handle || !file_priv) {
 		mgag200_hide_cursor(mdev);
@@ -232,11 +262,6 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 		return -EINVAL;
 	}
 
-	if (pixels_current == pixels_1)
-		pixels_next = pixels_2;
-	else
-		pixels_next = pixels_1;
-
 	obj = drm_gem_object_lookup(file_priv, handle);
 	if (!obj)
 		return -ENOENT;
@@ -249,48 +274,15 @@ int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 		goto err_drm_gem_object_put_unlocked;
 	}
 
-	/* Pin and map up-coming buffer to write colour indices */
-	ret = drm_gem_vram_pin(pixels_next, DRM_GEM_VRAM_PL_FLAG_VRAM);
-	if (ret) {
-		dev_err(&dev->pdev->dev,
-			"failed to pin cursor buffer: %d\n", ret);
-		goto err_drm_gem_vram_vunmap;
-	}
-	dst = drm_gem_vram_kmap(pixels_next, true, NULL);
-	if (IS_ERR(dst)) {
-		ret = PTR_ERR(dst);
-		dev_err(&dev->pdev->dev,
-			"failed to kmap cursor updates: %d\n", ret);
-		goto err_drm_gem_vram_unpin_dst;
-	}
-	gpu_addr = drm_gem_vram_offset(pixels_next);
-	if (gpu_addr < 0) {
-		ret = (int)gpu_addr;
-		dev_err(&dev->pdev->dev,
-			"failed to get cursor scanout address: %d\n", ret);
-		goto err_drm_gem_vram_kunmap_dst;
-	}
-	dst_gpu = (u64)gpu_addr;
-
-	ret = mgag200_show_cursor(mdev, dst, src, width, height, dst_gpu);
+	ret = mgag200_show_cursor(mdev, src, width, height);
 	if (ret)
-		goto err_drm_gem_vram_kunmap_dst;
+		goto err_drm_gem_vram_vunmap;
 
 	/* Now update internal buffer pointers */
-	if (pixels_current)
-		drm_gem_vram_unpin(pixels_current);
-	mdev->cursor.pixels_current = pixels_next;
-
-	drm_gem_vram_kunmap(pixels_next);
 	drm_gem_vram_vunmap(gbo, src);
 	drm_gem_object_put_unlocked(obj);
 
 	return 0;
-
-err_drm_gem_vram_kunmap_dst:
-	drm_gem_vram_kunmap(pixels_next);
-err_drm_gem_vram_unpin_dst:
-	drm_gem_vram_unpin(pixels_next);
 err_drm_gem_vram_vunmap:
 	drm_gem_vram_vunmap(gbo, src);
 err_drm_gem_object_put_unlocked:
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 11/12] drm/mgag200: Reserve video memory for cursor plane
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

The double-buffered cursor image is currently stored in video memory
by creating two BOs and pinning them to VRAM. The exact location is
chosen by VRAM helpers. The pinned cursor BOs can conflict with
framebuffer BOs and prevent the primary plane from displaying its
framebuffer.

As a first step to solving this problem, we reserve dedicated space at
the high end of the video memory for the cursor images. As the amount
of video memory now differs from the amount of available framebuffer
memory, size tests are adapted accordingly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 19 +++++++++++++++----
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  2 ++
 drivers/gpu/drm/mgag200/mgag200_main.c   |  2 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c   |  2 +-
 drivers/gpu/drm/mgag200/mgag200_ttm.c    |  4 ++++
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 4a5b1aa921e0..7f48abf80a6a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -216,17 +216,20 @@ static void mgag200_move_cursor(struct mga_device *mdev, int x, int y)
 int mgag200_cursor_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
+	size_t size;
+
+	size = roundup(64 * 48, PAGE_SIZE);
+	if (size * 2 > mdev->vram_fb_available)
+		return -ENOMEM;
 
 	/*
 	 * Make small buffers to store a hardware cursor (double
 	 * buffered icon updates)
 	 */
 	mdev->cursor.pixels_1 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    roundup(48*64, PAGE_SIZE),
-						    0, 0);
+						    size, 0, 0);
 	mdev->cursor.pixels_2 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    roundup(48*64, PAGE_SIZE),
-						    0, 0);
+						    size, 0, 0);
 	if (IS_ERR(mdev->cursor.pixels_2) || IS_ERR(mdev->cursor.pixels_1)) {
 		mdev->cursor.pixels_1 = NULL;
 		mdev->cursor.pixels_2 = NULL;
@@ -235,6 +238,14 @@ int mgag200_cursor_init(struct mga_device *mdev)
 	}
 	mdev->cursor.pixels_current = NULL;
 
+	/*
+	 * At the high end of video memory, we reserve space for
+	 * buffer objects. The cursor plane uses this memory to store
+	 * a double-buffered image of the current cursor. Hence, it's
+	 * not available for framebuffers.
+	 */
+	mdev->vram_fb_available -= 2 * size;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 01243fa6397c..5d6cfc88697a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -173,6 +173,8 @@ struct mga_device {
 
 	struct mga_cursor cursor;
 
+	size_t vram_fb_available;
+
 	bool				suspended;
 	int				num_crtc;
 	enum mga_type			type;
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 2b59280777a5..5f74aabcd3df 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -159,7 +159,7 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 
 	drm_mode_config_init(dev);
 	dev->mode_config.funcs = (void *)&mga_mode_funcs;
-	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024))
+	if (IS_G200_SE(mdev) && mdev->vram_fb_available < (2048*1024))
 		dev->mode_config.preferred_depth = 16;
 	else
 		dev->mode_config.preferred_depth = 32;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 0cf5608c3644..5ec697148fc1 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1629,7 +1629,7 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
 			bpp = connector->cmdline_mode.bpp;
 	}
 
-	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->mc.vram_size) {
+	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->vram_fb_available) {
 		if (connector->cmdline_mode.specified)
 			connector->cmdline_mode.specified = false;
 		return MODE_BAD;
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 69c81ebf3745..99997d737362 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -50,6 +50,8 @@ int mgag200_mm_init(struct mga_device *mdev)
 	mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
 					 pci_resource_len(dev->pdev, 0));
 
+	mdev->vram_fb_available = mdev->mc.vram_size;
+
 	return 0;
 }
 
@@ -57,6 +59,8 @@ void mgag200_mm_fini(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
 
+	mdev->vram_fb_available = 0;
+
 	drm_vram_helper_release_mm(dev);
 
 	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
-- 
2.23.0


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

* [PATCH v2 11/12] drm/mgag200: Reserve video memory for cursor plane
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

The double-buffered cursor image is currently stored in video memory
by creating two BOs and pinning them to VRAM. The exact location is
chosen by VRAM helpers. The pinned cursor BOs can conflict with
framebuffer BOs and prevent the primary plane from displaying its
framebuffer.

As a first step to solving this problem, we reserve dedicated space at
the high end of the video memory for the cursor images. As the amount
of video memory now differs from the amount of available framebuffer
memory, size tests are adapted accordingly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 19 +++++++++++++++----
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  2 ++
 drivers/gpu/drm/mgag200/mgag200_main.c   |  2 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c   |  2 +-
 drivers/gpu/drm/mgag200/mgag200_ttm.c    |  4 ++++
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 4a5b1aa921e0..7f48abf80a6a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -216,17 +216,20 @@ static void mgag200_move_cursor(struct mga_device *mdev, int x, int y)
 int mgag200_cursor_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
+	size_t size;
+
+	size = roundup(64 * 48, PAGE_SIZE);
+	if (size * 2 > mdev->vram_fb_available)
+		return -ENOMEM;
 
 	/*
 	 * Make small buffers to store a hardware cursor (double
 	 * buffered icon updates)
 	 */
 	mdev->cursor.pixels_1 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    roundup(48*64, PAGE_SIZE),
-						    0, 0);
+						    size, 0, 0);
 	mdev->cursor.pixels_2 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    roundup(48*64, PAGE_SIZE),
-						    0, 0);
+						    size, 0, 0);
 	if (IS_ERR(mdev->cursor.pixels_2) || IS_ERR(mdev->cursor.pixels_1)) {
 		mdev->cursor.pixels_1 = NULL;
 		mdev->cursor.pixels_2 = NULL;
@@ -235,6 +238,14 @@ int mgag200_cursor_init(struct mga_device *mdev)
 	}
 	mdev->cursor.pixels_current = NULL;
 
+	/*
+	 * At the high end of video memory, we reserve space for
+	 * buffer objects. The cursor plane uses this memory to store
+	 * a double-buffered image of the current cursor. Hence, it's
+	 * not available for framebuffers.
+	 */
+	mdev->vram_fb_available -= 2 * size;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 01243fa6397c..5d6cfc88697a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -173,6 +173,8 @@ struct mga_device {
 
 	struct mga_cursor cursor;
 
+	size_t vram_fb_available;
+
 	bool				suspended;
 	int				num_crtc;
 	enum mga_type			type;
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 2b59280777a5..5f74aabcd3df 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -159,7 +159,7 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 
 	drm_mode_config_init(dev);
 	dev->mode_config.funcs = (void *)&mga_mode_funcs;
-	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024))
+	if (IS_G200_SE(mdev) && mdev->vram_fb_available < (2048*1024))
 		dev->mode_config.preferred_depth = 16;
 	else
 		dev->mode_config.preferred_depth = 32;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 0cf5608c3644..5ec697148fc1 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1629,7 +1629,7 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
 			bpp = connector->cmdline_mode.bpp;
 	}
 
-	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->mc.vram_size) {
+	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->vram_fb_available) {
 		if (connector->cmdline_mode.specified)
 			connector->cmdline_mode.specified = false;
 		return MODE_BAD;
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 69c81ebf3745..99997d737362 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -50,6 +50,8 @@ int mgag200_mm_init(struct mga_device *mdev)
 	mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
 					 pci_resource_len(dev->pdev, 0));
 
+	mdev->vram_fb_available = mdev->mc.vram_size;
+
 	return 0;
 }
 
@@ -57,6 +59,8 @@ void mgag200_mm_fini(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
 
+	mdev->vram_fb_available = 0;
+
 	drm_vram_helper_release_mm(dev);
 
 	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 12/12] drm/mgag200: Allocate cursor BOs at high end of video memory
  2019-09-23 17:27 ` Thomas Zimmermann
@ 2019-09-23 17:27   ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: dri-devel, linux-doc, Thomas Zimmermann

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 and aligns it with the
ast driver. If there are more drivers with similar requirements, the
code could be moved into a shared place.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 94 +++++++++++++-----------
 drivers/gpu/drm/mgag200/mgag200_drv.h    | 12 +--
 2 files changed, 52 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 7f48abf80a6a..d65ee94d540c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -121,39 +121,25 @@ static int mgag200_show_cursor(struct mga_device *mdev, void *src,
 			       unsigned int width, unsigned int height)
 {
 	struct drm_device *dev = mdev->dev;
-	struct drm_gem_vram_object *pixels_1 = mdev->cursor.pixels_1;
-	struct drm_gem_vram_object *pixels_2 = mdev->cursor.pixels_2;
-	struct drm_gem_vram_object *pixels_current = mdev->cursor.pixels_current;
-	struct drm_gem_vram_object *pixels_next;
+	struct drm_gem_vram_object *gbo;
 	void *dst;
 	s64 off;
 	int ret;
 
-	if (!pixels_1 || !pixels_2) {
+	gbo = mdev->cursor.gbo[mdev->cursor.next_index];
+	if (!gbo) {
 		WREG8(MGA_CURPOSXL, 0);
 		WREG8(MGA_CURPOSXH, 0);
 		return -ENOTSUPP; /* Didn't allocate space for cursors */
 	}
-
-	if (WARN_ON(pixels_current &&
-		    pixels_1 != pixels_current &&
-		    pixels_2 != pixels_current)) {
-		return -ENOTSUPP; /* inconsistent state */
-	}
-
-	if (pixels_current == pixels_1)
-		pixels_next = pixels_2;
-	else
-		pixels_next = pixels_1;
-
-	dst = drm_gem_vram_vmap(pixels_next);
+	dst = drm_gem_vram_vmap(gbo);
 	if (IS_ERR(dst)) {
 		ret = PTR_ERR(dst);
 		dev_err(&dev->pdev->dev,
 			"failed to map cursor updates: %d\n", ret);
 		return ret;
 	}
-	off = drm_gem_vram_offset(pixels_next);
+	off = drm_gem_vram_offset(gbo);
 	if (off < 0) {
 		ret = (int)off;
 		dev_err(&dev->pdev->dev,
@@ -169,16 +155,15 @@ static int mgag200_show_cursor(struct mga_device *mdev, void *src,
 	/* Adjust cursor control register to turn on the cursor */
 	WREG_DAC(MGA1064_CURSOR_CTL, 4); /* 16-colour palletized cursor mode */
 
-	if (pixels_current)
-		drm_gem_vram_unpin(pixels_current);
-	mdev->cursor.pixels_current = pixels_next;
+	drm_gem_vram_vunmap(gbo, dst);
 
-	drm_gem_vram_vunmap(pixels_next, dst);
+	++mdev->cursor.next_index;
+	mdev->cursor.next_index %= ARRAY_SIZE(mdev->cursor.gbo);
 
 	return 0;
 
 err_drm_gem_vram_vunmap:
-	drm_gem_vram_vunmap(pixels_next, dst);
+	drm_gem_vram_vunmap(gbo, dst);
 	return ret;
 }
 
@@ -190,9 +175,6 @@ static void mgag200_hide_cursor(struct mga_device *mdev)
 {
 	WREG8(MGA_CURPOSXL, 0);
 	WREG8(MGA_CURPOSXH, 0);
-	if (mdev->cursor.pixels_current)
-		drm_gem_vram_unpin(mdev->cursor.pixels_current);
-	mdev->cursor.pixels_current = NULL;
 }
 
 static void mgag200_move_cursor(struct mga_device *mdev, int x, int y)
@@ -216,27 +198,32 @@ static void mgag200_move_cursor(struct mga_device *mdev, int x, int y)
 int mgag200_cursor_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
+	size_t ncursors = ARRAY_SIZE(mdev->cursor.gbo);
 	size_t size;
+	int ret;
+	size_t i;
+	struct drm_gem_vram_object *gbo;
 
 	size = roundup(64 * 48, PAGE_SIZE);
-	if (size * 2 > mdev->vram_fb_available)
+	if (size * ncursors > mdev->vram_fb_available)
 		return -ENOMEM;
 
-	/*
-	 * Make small buffers to store a hardware cursor (double
-	 * buffered icon updates)
-	 */
-	mdev->cursor.pixels_1 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    size, 0, 0);
-	mdev->cursor.pixels_2 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    size, 0, 0);
-	if (IS_ERR(mdev->cursor.pixels_2) || IS_ERR(mdev->cursor.pixels_1)) {
-		mdev->cursor.pixels_1 = NULL;
-		mdev->cursor.pixels_2 = NULL;
-		dev_warn(&dev->pdev->dev,
-			"Could not allocate space for cursors. Not doing hardware cursors.\n");
+	for (i = 0; i < ncursors; ++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;
+		}
+
+		mdev->cursor.gbo[i] = gbo;
 	}
-	mdev->cursor.pixels_current = NULL;
 
 	/*
 	 * At the high end of video memory, we reserve space for
@@ -244,13 +231,32 @@ int mgag200_cursor_init(struct mga_device *mdev)
 	 * a double-buffered image of the current cursor. Hence, it's
 	 * not available for framebuffers.
 	 */
-	mdev->vram_fb_available -= 2 * size;
+	mdev->vram_fb_available -= ncursors * size;
 
 	return 0;
+
+err_drm_gem_vram_put:
+	while (i) {
+		--i;
+		gbo = mdev->cursor.gbo[i];
+		drm_gem_vram_unpin(gbo);
+		drm_gem_vram_put(gbo);
+		mdev->cursor.gbo[i] = NULL;
+	}
+	return ret;
 }
 
 void mgag200_cursor_fini(struct mga_device *mdev)
-{ }
+{
+	size_t i;
+	struct drm_gem_vram_object *gbo;
+
+	for (i = 0; i < ARRAY_SIZE(mdev->cursor.gbo); ++i) {
+		gbo = mdev->cursor.gbo[i];
+		drm_gem_vram_unpin(gbo);
+		drm_gem_vram_put(gbo);
+	}
+}
 
 int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 			    uint32_t handle, uint32_t width, uint32_t height)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 5d6cfc88697a..0ea9a525e57d 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -129,16 +129,8 @@ struct mga_connector {
 };
 
 struct mga_cursor {
-	/*
-	   We have to have 2 buffers for the cursor to avoid occasional
-	   corruption while switching cursor icons.
-	   If either of these is NULL, then don't do hardware cursors, and
-	   fall back to software.
-	*/
-	struct drm_gem_vram_object *pixels_1;
-	struct drm_gem_vram_object *pixels_2;
-	/* The currently displayed icon, this points to one of pixels_1, or pixels_2 */
-	struct drm_gem_vram_object *pixels_current;
+	struct drm_gem_vram_object *gbo[2];
+	unsigned int next_index;
 };
 
 struct mga_mc {
-- 
2.23.0


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

* [PATCH v2 12/12] drm/mgag200: Allocate cursor BOs at high end of video memory
@ 2019-09-23 17:27   ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-23 17:27 UTC (permalink / raw)
  To: airlied, daniel, kraxel, sam, yc_chen, corbet
  Cc: Thomas Zimmermann, dri-devel, linux-doc

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 and aligns it with the
ast driver. If there are more drivers with similar requirements, the
code could be moved into a shared place.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 94 +++++++++++++-----------
 drivers/gpu/drm/mgag200/mgag200_drv.h    | 12 +--
 2 files changed, 52 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 7f48abf80a6a..d65ee94d540c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -121,39 +121,25 @@ static int mgag200_show_cursor(struct mga_device *mdev, void *src,
 			       unsigned int width, unsigned int height)
 {
 	struct drm_device *dev = mdev->dev;
-	struct drm_gem_vram_object *pixels_1 = mdev->cursor.pixels_1;
-	struct drm_gem_vram_object *pixels_2 = mdev->cursor.pixels_2;
-	struct drm_gem_vram_object *pixels_current = mdev->cursor.pixels_current;
-	struct drm_gem_vram_object *pixels_next;
+	struct drm_gem_vram_object *gbo;
 	void *dst;
 	s64 off;
 	int ret;
 
-	if (!pixels_1 || !pixels_2) {
+	gbo = mdev->cursor.gbo[mdev->cursor.next_index];
+	if (!gbo) {
 		WREG8(MGA_CURPOSXL, 0);
 		WREG8(MGA_CURPOSXH, 0);
 		return -ENOTSUPP; /* Didn't allocate space for cursors */
 	}
-
-	if (WARN_ON(pixels_current &&
-		    pixels_1 != pixels_current &&
-		    pixels_2 != pixels_current)) {
-		return -ENOTSUPP; /* inconsistent state */
-	}
-
-	if (pixels_current == pixels_1)
-		pixels_next = pixels_2;
-	else
-		pixels_next = pixels_1;
-
-	dst = drm_gem_vram_vmap(pixels_next);
+	dst = drm_gem_vram_vmap(gbo);
 	if (IS_ERR(dst)) {
 		ret = PTR_ERR(dst);
 		dev_err(&dev->pdev->dev,
 			"failed to map cursor updates: %d\n", ret);
 		return ret;
 	}
-	off = drm_gem_vram_offset(pixels_next);
+	off = drm_gem_vram_offset(gbo);
 	if (off < 0) {
 		ret = (int)off;
 		dev_err(&dev->pdev->dev,
@@ -169,16 +155,15 @@ static int mgag200_show_cursor(struct mga_device *mdev, void *src,
 	/* Adjust cursor control register to turn on the cursor */
 	WREG_DAC(MGA1064_CURSOR_CTL, 4); /* 16-colour palletized cursor mode */
 
-	if (pixels_current)
-		drm_gem_vram_unpin(pixels_current);
-	mdev->cursor.pixels_current = pixels_next;
+	drm_gem_vram_vunmap(gbo, dst);
 
-	drm_gem_vram_vunmap(pixels_next, dst);
+	++mdev->cursor.next_index;
+	mdev->cursor.next_index %= ARRAY_SIZE(mdev->cursor.gbo);
 
 	return 0;
 
 err_drm_gem_vram_vunmap:
-	drm_gem_vram_vunmap(pixels_next, dst);
+	drm_gem_vram_vunmap(gbo, dst);
 	return ret;
 }
 
@@ -190,9 +175,6 @@ static void mgag200_hide_cursor(struct mga_device *mdev)
 {
 	WREG8(MGA_CURPOSXL, 0);
 	WREG8(MGA_CURPOSXH, 0);
-	if (mdev->cursor.pixels_current)
-		drm_gem_vram_unpin(mdev->cursor.pixels_current);
-	mdev->cursor.pixels_current = NULL;
 }
 
 static void mgag200_move_cursor(struct mga_device *mdev, int x, int y)
@@ -216,27 +198,32 @@ static void mgag200_move_cursor(struct mga_device *mdev, int x, int y)
 int mgag200_cursor_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
+	size_t ncursors = ARRAY_SIZE(mdev->cursor.gbo);
 	size_t size;
+	int ret;
+	size_t i;
+	struct drm_gem_vram_object *gbo;
 
 	size = roundup(64 * 48, PAGE_SIZE);
-	if (size * 2 > mdev->vram_fb_available)
+	if (size * ncursors > mdev->vram_fb_available)
 		return -ENOMEM;
 
-	/*
-	 * Make small buffers to store a hardware cursor (double
-	 * buffered icon updates)
-	 */
-	mdev->cursor.pixels_1 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    size, 0, 0);
-	mdev->cursor.pixels_2 = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
-						    size, 0, 0);
-	if (IS_ERR(mdev->cursor.pixels_2) || IS_ERR(mdev->cursor.pixels_1)) {
-		mdev->cursor.pixels_1 = NULL;
-		mdev->cursor.pixels_2 = NULL;
-		dev_warn(&dev->pdev->dev,
-			"Could not allocate space for cursors. Not doing hardware cursors.\n");
+	for (i = 0; i < ncursors; ++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;
+		}
+
+		mdev->cursor.gbo[i] = gbo;
 	}
-	mdev->cursor.pixels_current = NULL;
 
 	/*
 	 * At the high end of video memory, we reserve space for
@@ -244,13 +231,32 @@ int mgag200_cursor_init(struct mga_device *mdev)
 	 * a double-buffered image of the current cursor. Hence, it's
 	 * not available for framebuffers.
 	 */
-	mdev->vram_fb_available -= 2 * size;
+	mdev->vram_fb_available -= ncursors * size;
 
 	return 0;
+
+err_drm_gem_vram_put:
+	while (i) {
+		--i;
+		gbo = mdev->cursor.gbo[i];
+		drm_gem_vram_unpin(gbo);
+		drm_gem_vram_put(gbo);
+		mdev->cursor.gbo[i] = NULL;
+	}
+	return ret;
 }
 
 void mgag200_cursor_fini(struct mga_device *mdev)
-{ }
+{
+	size_t i;
+	struct drm_gem_vram_object *gbo;
+
+	for (i = 0; i < ARRAY_SIZE(mdev->cursor.gbo); ++i) {
+		gbo = mdev->cursor.gbo[i];
+		drm_gem_vram_unpin(gbo);
+		drm_gem_vram_put(gbo);
+	}
+}
 
 int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 			    uint32_t handle, uint32_t width, uint32_t height)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 5d6cfc88697a..0ea9a525e57d 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -129,16 +129,8 @@ struct mga_connector {
 };
 
 struct mga_cursor {
-	/*
-	   We have to have 2 buffers for the cursor to avoid occasional
-	   corruption while switching cursor icons.
-	   If either of these is NULL, then don't do hardware cursors, and
-	   fall back to software.
-	*/
-	struct drm_gem_vram_object *pixels_1;
-	struct drm_gem_vram_object *pixels_2;
-	/* The currently displayed icon, this points to one of pixels_1, or pixels_2 */
-	struct drm_gem_vram_object *pixels_current;
+	struct drm_gem_vram_object *gbo[2];
+	unsigned int next_index;
 };
 
 struct mga_mc {
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2 01/12] drm/vram: Support top-down placement flag
  2019-09-23 17:27   ` Thomas Zimmermann
@ 2019-09-24  7:18     ` Gerd Hoffmann
  -1 siblings, 0 replies; 34+ messages in thread
From: Gerd Hoffmann @ 2019-09-24  7:18 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: airlied, daniel, sam, yc_chen, corbet, dri-devel, linux-doc

  Hi,

> + * object to be pinned at the high end of the memory region. Set this
> + * flag to avoid memory fragmentation.

That is confusing, sounds like the flag should be set on all objects
which is not correct.  The description from the cover letter is better.

Otherwise the patch is fine, so with that fixed:
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd


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

* Re: [PATCH v2 01/12] drm/vram: Support top-down placement flag
@ 2019-09-24  7:18     ` Gerd Hoffmann
  0 siblings, 0 replies; 34+ messages in thread
From: Gerd Hoffmann @ 2019-09-24  7:18 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: corbet, airlied, linux-doc, dri-devel, sam

  Hi,

> + * object to be pinned at the high end of the memory region. Set this
> + * flag to avoid memory fragmentation.

That is confusing, sounds like the flag should be set on all objects
which is not correct.  The description from the cover letter is better.

Otherwise the patch is fine, so with that fixed:
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2 03/12] drm/ast: Move cursor update code to ast_show_cursor()
  2019-09-23 17:27   ` Thomas Zimmermann
@ 2019-09-24  7:47     ` Gerd Hoffmann
  -1 siblings, 0 replies; 34+ messages in thread
From: Gerd Hoffmann @ 2019-09-24  7:47 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: airlied, daniel, sam, yc_chen, corbet, dri-devel, linux-doc

  Hi,

> -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 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);
> +}

Please split this into two patches, one which only moves the functions
and one which does the code changes.  Then it is easier to see the
actual code changes for ast_{show,hide}_cursor in the diff output of the
second patch.

cheers,
  Gerd


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

* Re: [PATCH v2 03/12] drm/ast: Move cursor update code to ast_show_cursor()
@ 2019-09-24  7:47     ` Gerd Hoffmann
  0 siblings, 0 replies; 34+ messages in thread
From: Gerd Hoffmann @ 2019-09-24  7:47 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: corbet, airlied, linux-doc, dri-devel, sam

  Hi,

> -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 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);
> +}

Please split this into two patches, one which only moves the functions
and one which does the code changes.  Then it is easier to see the
actual code changes for ast_{show,hide}_cursor in the diff output of the
second patch.

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2 11/12] drm/mgag200: Reserve video memory for cursor plane
  2019-09-23 17:27   ` Thomas Zimmermann
@ 2019-09-24  7:55     ` Gerd Hoffmann
  -1 siblings, 0 replies; 34+ messages in thread
From: Gerd Hoffmann @ 2019-09-24  7:55 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: airlied, daniel, sam, yc_chen, corbet, dri-devel, linux-doc

  Hi,

> +	/*
> +	 * At the high end of video memory, we reserve space for
> +	 * buffer objects. The cursor plane uses this memory to store
> +	 * a double-buffered image of the current cursor. Hence, it's
> +	 * not available for framebuffers.
> +	 */
> +	mdev->vram_fb_available -= 2 * size;

Hmm, that looks like a leftover from the previous version of the patch
series ...

cheers,
  Gerd


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

* Re: [PATCH v2 11/12] drm/mgag200: Reserve video memory for cursor plane
@ 2019-09-24  7:55     ` Gerd Hoffmann
  0 siblings, 0 replies; 34+ messages in thread
From: Gerd Hoffmann @ 2019-09-24  7:55 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: corbet, airlied, linux-doc, dri-devel, sam

  Hi,

> +	/*
> +	 * At the high end of video memory, we reserve space for
> +	 * buffer objects. The cursor plane uses this memory to store
> +	 * a double-buffered image of the current cursor. Hence, it's
> +	 * not available for framebuffers.
> +	 */
> +	mdev->vram_fb_available -= 2 * size;

Hmm, that looks like a leftover from the previous version of the patch
series ...

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2 11/12] drm/mgag200: Reserve video memory for cursor plane
  2019-09-24  7:55     ` Gerd Hoffmann
@ 2019-09-24  8:14       ` Thomas Zimmermann
  -1 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-24  8:14 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: airlied, daniel, sam, yc_chen, corbet, dri-devel, linux-doc

Hi

Am 24.09.19 um 09:55 schrieb Gerd Hoffmann:
>    Hi,
> 
>> +	/*
>> +	 * At the high end of video memory, we reserve space for
>> +	 * buffer objects. The cursor plane uses this memory to store
>> +	 * a double-buffered image of the current cursor. Hence, it's
>> +	 * not available for framebuffers.
>> +	 */
>> +	mdev->vram_fb_available -= 2 * size;
> 
> Hmm, that looks like a leftover from the previous version of the patch
> series ...

This belongs here. For several tests the size of the available 
framebuffer memory needs to be known. It's stored in 
mdev->vram_fb_available. mgag200_mm_init() sets the value of 
mdev->vram_fb_available to the full size of the video RAM. The line 
above subtracts the size of 2 cursor buffers.

Best regards
Thomas

> 
> cheers,
>    Gerd
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

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

* Re: [PATCH v2 11/12] drm/mgag200: Reserve video memory for cursor plane
@ 2019-09-24  8:14       ` Thomas Zimmermann
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Zimmermann @ 2019-09-24  8:14 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: corbet, airlied, linux-doc, dri-devel, sam

Hi

Am 24.09.19 um 09:55 schrieb Gerd Hoffmann:
>    Hi,
> 
>> +	/*
>> +	 * At the high end of video memory, we reserve space for
>> +	 * buffer objects. The cursor plane uses this memory to store
>> +	 * a double-buffered image of the current cursor. Hence, it's
>> +	 * not available for framebuffers.
>> +	 */
>> +	mdev->vram_fb_available -= 2 * size;
> 
> Hmm, that looks like a leftover from the previous version of the patch
> series ...

This belongs here. For several tests the size of the available 
framebuffer memory needs to be known. It's stored in 
mdev->vram_fb_available. mgag200_mm_init() sets the value of 
mdev->vram_fb_available to the full size of the video RAM. The line 
above subtracts the size of 2 cursor buffers.

Best regards
Thomas

> 
> cheers,
>    Gerd
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2019-09-24  8:14 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-23 17:27 [PATCH v2 00/12] drm/ast/mgag200: Place cursor BOs at VRAM high-end Thomas Zimmermann
2019-09-23 17:27 ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 01/12] drm/vram: Support top-down placement flag Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-24  7:18   ` Gerd Hoffmann
2019-09-24  7:18     ` Gerd Hoffmann
2019-09-23 17:27 ` [PATCH v2 02/12] drm/ast: Don't call ast_show_cursor() from ast_cursor_move() Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 03/12] drm/ast: Move cursor update code to ast_show_cursor() Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-24  7:47   ` Gerd Hoffmann
2019-09-24  7:47     ` Gerd Hoffmann
2019-09-23 17:27 ` [PATCH v2 04/12] drm/ast: Move cursor offset swapping into ast_show_cursor() Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 05/12] drm/ast: Allocate cursor BOs at high end of video memory Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 06/12] drm/mgag200: Rename cursor functions to use mgag200_ prefix Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 07/12] drm/mgag200: Add init and fini functions for cursor handling Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 08/12] drm/mgag200: Add separate move-cursor function Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 09/12] drm/mgag200: Move cursor-image update to mgag200_show_cursor() Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 10/12] drm/mgag200: Move cursor BO swapping into mgag200_show_cursor() Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 11/12] drm/mgag200: Reserve video memory for cursor plane Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann
2019-09-24  7:55   ` Gerd Hoffmann
2019-09-24  7:55     ` Gerd Hoffmann
2019-09-24  8:14     ` Thomas Zimmermann
2019-09-24  8:14       ` Thomas Zimmermann
2019-09-23 17:27 ` [PATCH v2 12/12] drm/mgag200: Allocate cursor BOs at high end of video memory Thomas Zimmermann
2019-09-23 17:27   ` Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.