[PATCH v2 00/14] drm/ast: Managed modesetting

[PATCH v2 00/14] drm/ast: Managed modesetting

[Thomas Zimmermann]

This is the first patchset to convert ast to use managed interfaces. These
patches address modesetting. I expect that there will be at least one more
set of patches for memory management and one for device structures.

Patches 1 to 11 tackle HW cursor handling. The overall point is to get
cursor support out of the plane functions and put the code into helpers.
There are quite a few improvements that make cursors easier and faster to
use. Patch 10 converts cursors to managed release.

With cursors out of the way, modesetting initialization is much more
simple. Patches 12 to 14 put all related code next to each other and
switch to managed initialization.

The patchset was tested on AST2100 hardware.

v2:
	* use to_ast_private() instead of
	  struct drm_device.dev_private (Sam)

Thomas Zimmermann (14):
  drm/ast: Move cursor functions to ast_cursor.c
  drm/ast: Pass struct ast_private instance to cursor init/fini
    functions
  drm/ast: Move cursor fb pinning and mapping into helper
  drm/ast: Update cursor image and checksum from same function
  drm/ast: Move cursor pageflip into helper
  drm/ast: Replace ast_cursor_move() with ast_cursor_show()
  drm/ast: Don't enable HW cursors twice during atomic update
  drm/ast: Add helper to hide cursor
  drm/ast: Keep cursor HW BOs mapped
  drm/ast: Managed cursor release
  drm/ast: Init cursors before creating modesetting structures
  drm/ast: Replace struct ast_crtc with struct drm_crtc
  drm/ast: Use managed mode-config init
  drm/ast: Initialize mode setting in ast_mode_config_init()

 drivers/gpu/drm/ast/Makefile     |   3 +-
 drivers/gpu/drm/ast/ast_cursor.c | 289 +++++++++++++++++++++++++++
 drivers/gpu/drm/ast/ast_drv.h    |  19 +-
 drivers/gpu/drm/ast/ast_main.c   |  35 +---
 drivers/gpu/drm/ast/ast_mode.c   | 322 ++++++-------------------------
 5 files changed, 359 insertions(+), 309 deletions(-)
 create mode 100644 drivers/gpu/drm/ast/ast_cursor.c

--
2.27.0

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

[PATCH v2 01/14] drm/ast: Move cursor functions to ast_cursor.c

[Thomas Zimmermann]

The cursor manipulation functions are unrelated to modesetting. Move
them into their own file.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/ast/Makefile     |   3 +-
 drivers/gpu/drm/ast/ast_cursor.c | 218 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/ast/ast_drv.h    |   9 ++
 drivers/gpu/drm/ast/ast_mode.c   | 195 ---------------------------
 4 files changed, 229 insertions(+), 196 deletions(-)
 create mode 100644 drivers/gpu/drm/ast/ast_cursor.c

diff --git a/drivers/gpu/drm/ast/Makefile b/drivers/gpu/drm/ast/Makefile
index 561f7c4199e4..6a5b3b247316 100644
--- a/drivers/gpu/drm/ast/Makefile
+++ b/drivers/gpu/drm/ast/Makefile
@@ -3,6 +3,7 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-ast-y := ast_drv.o ast_main.o ast_mode.o ast_ttm.o ast_post.o ast_dp501.o
+ast-y := ast_cursor.o ast_drv.o ast_main.o ast_mode.o ast_ttm.o ast_post.o \
+	 ast_dp501.o
 
 obj-$(CONFIG_DRM_AST) := ast.o
diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
new file mode 100644
index 000000000000..53bb6eebc7cd
--- /dev/null
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -0,0 +1,218 @@
+/*
+ * Copyright 2012 Red Hat Inc.
+ * Parts based on xf86-video-ast
+ * Copyright (c) 2005 ASPEED Technology Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ */
+/*
+ * Authors: Dave Airlie <airlied@redhat.com>
+ */
+
+#include <drm/drm_gem_vram_helper.h>
+
+#include "ast_drv.h"
+
+/*
+ * Allocate cursor BOs and pins them at the end of VRAM.
+ */
+int ast_cursor_init(struct drm_device *dev)
+{
+	struct ast_private *ast = to_ast_private(dev);
+	size_t size, i;
+	struct drm_gem_vram_object *gbo;
+	int ret;
+
+	size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
+
+	for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
+		gbo = drm_gem_vram_create(dev, size, 0);
+		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;
+		}
+
+		ast->cursor.gbo[i] = gbo;
+	}
+
+	return 0;
+
+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;
+}
+
+void ast_cursor_fini(struct drm_device *dev)
+{
+	struct ast_private *ast = to_ast_private(dev);
+	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);
+	}
+}
+
+static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height)
+{
+	union {
+		u32 ul;
+		u8 b[4];
+	} srcdata32[2], data32;
+	union {
+		u16 us;
+		u8 b[2];
+	} data16;
+	u32 csum = 0;
+	s32 alpha_dst_delta, last_alpha_dst_delta;
+	u8 *srcxor, *dstxor;
+	int i, j;
+	u32 per_pixel_copy, two_pixel_copy;
+
+	alpha_dst_delta = AST_MAX_HWC_WIDTH << 1;
+	last_alpha_dst_delta = alpha_dst_delta - (width << 1);
+
+	srcxor = src;
+	dstxor = (u8 *)dst + last_alpha_dst_delta + (AST_MAX_HWC_HEIGHT - height) * alpha_dst_delta;
+	per_pixel_copy = width & 1;
+	two_pixel_copy = width >> 1;
+
+	for (j = 0; j < height; j++) {
+		for (i = 0; i < two_pixel_copy; i++) {
+			srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0;
+			srcdata32[1].ul = *((u32 *)(srcxor + 4)) & 0xf0f0f0f0;
+			data32.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 4);
+			data32.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 4);
+			data32.b[2] = srcdata32[1].b[1] | (srcdata32[1].b[0] >> 4);
+			data32.b[3] = srcdata32[1].b[3] | (srcdata32[1].b[2] >> 4);
+
+			writel(data32.ul, dstxor);
+			csum += data32.ul;
+
+			dstxor += 4;
+			srcxor += 8;
+
+		}
+
+		for (i = 0; i < per_pixel_copy; i++) {
+			srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0;
+			data16.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 4);
+			data16.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 4);
+			writew(data16.us, dstxor);
+			csum += (u32)data16.us;
+
+			dstxor += 2;
+			srcxor += 4;
+		}
+		dstxor += last_alpha_dst_delta;
+	}
+	return csum;
+}
+
+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;
+}
+
+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);
+}
+
+int ast_cursor_move(struct drm_crtc *crtc, int x, int y)
+{
+	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+	struct ast_private *ast = to_ast_private(crtc->dev);
+	struct drm_gem_vram_object *gbo;
+	int x_offset, y_offset;
+	u8 *dst, *sig;
+	u8 jreg;
+
+	gbo = ast->cursor.gbo[ast->cursor.next_index];
+	dst = drm_gem_vram_vmap(gbo);
+	if (IS_ERR(dst))
+		return PTR_ERR(dst);
+
+	sig = dst + AST_HWC_SIZE;
+	writel(x, sig + AST_HWC_SIGNATURE_X);
+	writel(y, sig + AST_HWC_SIGNATURE_Y);
+
+	x_offset = ast_crtc->offset_x;
+	y_offset = ast_crtc->offset_y;
+	if (x < 0) {
+		x_offset = (-x) + ast_crtc->offset_x;
+		x = 0;
+	}
+
+	if (y < 0) {
+		y_offset = (-y) + ast_crtc->offset_y;
+		y = 0;
+	}
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc2, x_offset);
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc3, y_offset);
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc4, (x & 0xff));
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc5, ((x >> 8) & 0x0f));
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc6, (y & 0xff));
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc7, ((y >> 8) & 0x07));
+
+	/* dummy write to fire HWC */
+	jreg = 0x02 |
+	       0x01; /* enable ARGB4444 cursor */
+	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
+
+	drm_gem_vram_vunmap(gbo, dst);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index c44c1376c697..245ed2e2d775 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -314,4 +314,13 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata);
 u8 ast_get_dp501_max_clk(struct drm_device *dev);
 void ast_init_3rdtx(struct drm_device *dev);
 void ast_release_firmware(struct drm_device *dev);
+
+/* ast_cursor.c */
+int ast_cursor_init(struct drm_device *dev);
+void ast_cursor_fini(struct drm_device *dev);
+int ast_cursor_update(void *dst, void *src, unsigned int width,
+		      unsigned int height);
+void ast_cursor_set_base(struct ast_private *ast, u64 address);
+int ast_cursor_move(struct drm_crtc *crtc, int x, int y);
+
 #endif
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 510ffb497344..c8399699d773 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -47,16 +47,6 @@
 
 static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
 static void ast_i2c_destroy(struct ast_i2c_chan *i2c);
-static int ast_cursor_move(struct drm_crtc *crtc,
-			   int x, int y);
-
-
-static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
-static int ast_cursor_update(void *dst, void *src, unsigned int width,
-			     unsigned int height);
-static void ast_cursor_set_base(struct ast_private *ast, u64 address);
-static int ast_cursor_move(struct drm_crtc *crtc,
-			   int x, int y);
 
 static inline void ast_load_palette_index(struct ast_private *ast,
 				     u8 index, u8 red, u8 green,
@@ -1129,58 +1119,6 @@ static int ast_connector_init(struct drm_device *dev)
 	return 0;
 }
 
-/* allocate cursor cache and pin at start of VRAM */
-static int ast_cursor_init(struct drm_device *dev)
-{
-	struct ast_private *ast = to_ast_private(dev);
-	size_t size, i;
-	struct drm_gem_vram_object *gbo;
-	int ret;
-
-	size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
-
-	for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
-		gbo = drm_gem_vram_create(dev, size, 0);
-		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;
-		}
-
-		ast->cursor.gbo[i] = gbo;
-	}
-
-	return 0;
-
-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 = to_ast_private(dev);
-	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)
 {
 	struct ast_private *ast = to_ast_private(dev);
@@ -1344,136 +1282,3 @@ static void ast_i2c_destroy(struct ast_i2c_chan *i2c)
 	i2c_del_adapter(&i2c->adapter);
 	kfree(i2c);
 }
-
-static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height)
-{
-	union {
-		u32 ul;
-		u8 b[4];
-	} srcdata32[2], data32;
-	union {
-		u16 us;
-		u8 b[2];
-	} data16;
-	u32 csum = 0;
-	s32 alpha_dst_delta, last_alpha_dst_delta;
-	u8 *srcxor, *dstxor;
-	int i, j;
-	u32 per_pixel_copy, two_pixel_copy;
-
-	alpha_dst_delta = AST_MAX_HWC_WIDTH << 1;
-	last_alpha_dst_delta = alpha_dst_delta - (width << 1);
-
-	srcxor = src;
-	dstxor = (u8 *)dst + last_alpha_dst_delta + (AST_MAX_HWC_HEIGHT - height) * alpha_dst_delta;
-	per_pixel_copy = width & 1;
-	two_pixel_copy = width >> 1;
-
-	for (j = 0; j < height; j++) {
-		for (i = 0; i < two_pixel_copy; i++) {
-			srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0;
-			srcdata32[1].ul = *((u32 *)(srcxor + 4)) & 0xf0f0f0f0;
-			data32.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 4);
-			data32.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 4);
-			data32.b[2] = srcdata32[1].b[1] | (srcdata32[1].b[0] >> 4);
-			data32.b[3] = srcdata32[1].b[3] | (srcdata32[1].b[2] >> 4);
-
-			writel(data32.ul, dstxor);
-			csum += data32.ul;
-
-			dstxor += 4;
-			srcxor += 8;
-
-		}
-
-		for (i = 0; i < per_pixel_copy; i++) {
-			srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0;
-			data16.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 4);
-			data16.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 4);
-			writew(data16.us, dstxor);
-			csum += (u32)data16.us;
-
-			dstxor += 2;
-			srcxor += 4;
-		}
-		dstxor += last_alpha_dst_delta;
-	}
-	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_cursor_move(struct drm_crtc *crtc,
-			   int x, int y)
-{
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	struct ast_private *ast = to_ast_private(crtc->dev);
-	struct drm_gem_vram_object *gbo;
-	int x_offset, y_offset;
-	u8 *dst, *sig;
-	u8 jreg;
-
-	gbo = ast->cursor.gbo[ast->cursor.next_index];
-	dst = drm_gem_vram_vmap(gbo);
-	if (IS_ERR(dst))
-		return PTR_ERR(dst);
-
-	sig = dst + AST_HWC_SIZE;
-	writel(x, sig + AST_HWC_SIGNATURE_X);
-	writel(y, sig + AST_HWC_SIGNATURE_Y);
-
-	x_offset = ast_crtc->offset_x;
-	y_offset = ast_crtc->offset_y;
-	if (x < 0) {
-		x_offset = (-x) + ast_crtc->offset_x;
-		x = 0;
-	}
-
-	if (y < 0) {
-		y_offset = (-y) + ast_crtc->offset_y;
-		y = 0;
-	}
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc2, x_offset);
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc3, y_offset);
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc4, (x & 0xff));
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc5, ((x >> 8) & 0x0f));
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc6, (y & 0xff));
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc7, ((y >> 8) & 0x07));
-
-	/* dummy write to fire HWC */
-	jreg = 0x02 |
-	       0x01; /* enable ARGB4444 cursor */
-	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
-
-	drm_gem_vram_vunmap(gbo, dst);
-
-	return 0;
-}
-- 
2.27.0

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

[PATCH v2 02/14] drm/ast: Pass struct ast_private instance to cursor init/fini functions

[Thomas Zimmermann]

Removes some typecasting.

v2:
	* use to_ast_private() instead of struct drm_device.dev_private

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/ast/ast_cursor.c | 7 +++----
 drivers/gpu/drm/ast/ast_drv.h    | 4 ++--
 drivers/gpu/drm/ast/ast_mode.c   | 6 ++++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 53bb6eebc7cd..1d4f51a7fe22 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -34,9 +34,9 @@
 /*
  * Allocate cursor BOs and pins them at the end of VRAM.
  */
-int ast_cursor_init(struct drm_device *dev)
+int ast_cursor_init(struct ast_private *ast)
 {
-	struct ast_private *ast = to_ast_private(dev);
+	struct drm_device *dev = ast->dev;
 	size_t size, i;
 	struct drm_gem_vram_object *gbo;
 	int ret;
@@ -72,9 +72,8 @@ int ast_cursor_init(struct drm_device *dev)
 	return ret;
 }
 
-void ast_cursor_fini(struct drm_device *dev)
+void ast_cursor_fini(struct ast_private *ast)
 {
-	struct ast_private *ast = to_ast_private(dev);
 	size_t i;
 	struct drm_gem_vram_object *gbo;
 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 245ed2e2d775..f7b120f862a8 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -316,8 +316,8 @@ void ast_init_3rdtx(struct drm_device *dev);
 void ast_release_firmware(struct drm_device *dev);
 
 /* ast_cursor.c */
-int ast_cursor_init(struct drm_device *dev);
-void ast_cursor_fini(struct drm_device *dev);
+int ast_cursor_init(struct ast_private *ast);
+void ast_cursor_fini(struct ast_private *ast);
 int ast_cursor_update(void *dst, void *src, unsigned int width,
 		      unsigned int height);
 void ast_cursor_set_base(struct ast_private *ast, u64 address);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index c8399699d773..e69965f5636d 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1149,7 +1149,7 @@ int ast_mode_init(struct drm_device *dev)
 	drm_plane_helper_add(&ast->cursor_plane,
 			     &ast_cursor_plane_helper_funcs);
 
-	ast_cursor_init(dev);
+	ast_cursor_init(ast);
 	ast_crtc_init(dev);
 	ast_encoder_init(dev);
 	ast_connector_init(dev);
@@ -1159,7 +1159,9 @@ int ast_mode_init(struct drm_device *dev)
 
 void ast_mode_fini(struct drm_device *dev)
 {
-	ast_cursor_fini(dev);
+	struct ast_private *ast = to_ast_private(dev);
+
+	ast_cursor_fini(ast);
 }
 
 static int get_clock(void *i2c_priv)
-- 
2.27.0

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

[PATCH v2 03/14] drm/ast: Move cursor fb pinning and mapping into helper

[Thomas Zimmermann]

The new helper ast_cursor_blit() updates a cursor's backbuffer HW
BO from a framebuffer structure. The cursor plane's prepare_fb()
function now uses the new interface.

Pinning and mapping of BOs is done automatically by the helper. This
includes the source BO, which was not pinned by the original code in
prepare_fb().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_cursor.c | 57 ++++++++++++++++++++++++++++++--
 drivers/gpu/drm/ast/ast_drv.h    |  3 +-
 drivers/gpu/drm/ast/ast_mode.c   | 41 ++---------------------
 3 files changed, 59 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 1d4f51a7fe22..8f94d4712f66 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -140,8 +140,8 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height)
 	return csum;
 }
 
-int ast_cursor_update(void *dst, void *src, unsigned int width,
-		      unsigned int height)
+static int ast_cursor_update(void *dst, void *src, unsigned int width,
+			     unsigned int height)
 {
 	u32 csum;
 
@@ -159,6 +159,59 @@ int ast_cursor_update(void *dst, void *src, unsigned int width,
 	return 0;
 }
 
+int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb)
+{
+	struct drm_device *dev = ast->dev;
+	struct drm_gem_vram_object *gbo;
+	int ret;
+	void *src;
+	void *dst;
+
+	if (drm_WARN_ON_ONCE(dev, fb->width > AST_MAX_HWC_WIDTH) ||
+	    drm_WARN_ON_ONCE(dev, fb->height > AST_MAX_HWC_HEIGHT))
+		return -EINVAL;
+
+	gbo = drm_gem_vram_of_gem(fb->obj[0]);
+
+	ret = drm_gem_vram_pin(gbo, 0);
+	if (ret)
+		return ret;
+	src = drm_gem_vram_vmap(gbo);
+	if (IS_ERR(src)) {
+		ret = PTR_ERR(src);
+		goto err_drm_gem_vram_unpin;
+	}
+
+	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
+	if (IS_ERR(dst)) {
+		ret = PTR_ERR(dst);
+		goto err_drm_gem_vram_vunmap_src;
+	}
+
+	ret = ast_cursor_update(dst, src, fb->width, fb->height);
+	if (ret)
+		goto err_drm_gem_vram_vunmap_dst;
+
+	/*
+	 * Always unmap buffers here. Destination buffers are
+	 * perma-pinned while the driver is active. We're only
+	 * changing ref-counters here.
+	 */
+	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
+	drm_gem_vram_vunmap(gbo, src);
+	drm_gem_vram_unpin(gbo);
+
+	return 0;
+
+err_drm_gem_vram_vunmap_dst:
+	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
+err_drm_gem_vram_vunmap_src:
+	drm_gem_vram_vunmap(gbo, src);
+err_drm_gem_vram_unpin:
+	drm_gem_vram_unpin(gbo);
+	return ret;
+}
+
 void ast_cursor_set_base(struct ast_private *ast, u64 address)
 {
 	u8 addr0 = (address >> 3) & 0xff;
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index f7b120f862a8..9bc1bb76ec91 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -318,8 +318,7 @@ void ast_release_firmware(struct drm_device *dev);
 /* ast_cursor.c */
 int ast_cursor_init(struct ast_private *ast);
 void ast_cursor_fini(struct ast_private *ast);
-int ast_cursor_update(void *dst, void *src, unsigned int width,
-		      unsigned int height);
+int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb);
 void ast_cursor_set_base(struct ast_private *ast, u64 address);
 int ast_cursor_move(struct drm_crtc *crtc, int x, int y);
 
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index e69965f5636d..701211050832 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -611,56 +611,21 @@ static int
 ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
 				   struct drm_plane_state *new_state)
 {
-	struct drm_device *dev = plane->dev;
 	struct drm_framebuffer *fb = new_state->fb;
 	struct drm_crtc *crtc = new_state->crtc;
-	struct drm_gem_vram_object *gbo;
 	struct ast_private *ast;
 	int ret;
-	void *src, *dst;
 
 	if (!crtc || !fb)
 		return 0;
 
-	if (drm_WARN_ON_ONCE(dev, fb->width > AST_MAX_HWC_WIDTH) ||
-	    drm_WARN_ON_ONCE(dev, fb->height > AST_MAX_HWC_HEIGHT))
-		return -EINVAL; /* BUG: didn't test in atomic_check() */
-
-	ast = to_ast_private(dev);
-
-	gbo = drm_gem_vram_of_gem(fb->obj[0]);
-	src = drm_gem_vram_vmap(gbo);
-	if (IS_ERR(src)) {
-		ret = PTR_ERR(src);
-		goto err_drm_gem_vram_unpin;
-	}
-
-	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
-	if (IS_ERR(dst)) {
-		ret = PTR_ERR(dst);
-		goto err_drm_gem_vram_vunmap_src;
-	}
+	ast = to_ast_private(plane->dev);
 
-	ret = ast_cursor_update(dst, src, fb->width, fb->height);
+	ret = ast_cursor_blit(ast, fb);
 	if (ret)
-		goto err_drm_gem_vram_vunmap_dst;
-
-	/* Always unmap buffers here. Destination buffers are
-	 * perma-pinned while the driver is active. We're only
-	 * changing ref-counters here.
-	 */
-	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
-	drm_gem_vram_vunmap(gbo, src);
+		return ret;
 
 	return 0;
-
-err_drm_gem_vram_vunmap_dst:
-	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
-err_drm_gem_vram_vunmap_src:
-	drm_gem_vram_vunmap(gbo, src);
-err_drm_gem_vram_unpin:
-	drm_gem_vram_unpin(gbo);
-	return ret;
 }
 
 static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
-- 
2.27.0

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

[PATCH v2 04/14] drm/ast: Update cursor image and checksum from same function

[Thomas Zimmermann]

Cursor image and checksum go hand in hand. Update both in the same
place. The helper cannot fail, so remove the return type.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_cursor.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 8f94d4712f66..7474baddf048 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -84,7 +84,7 @@ void ast_cursor_fini(struct ast_private *ast)
 	}
 }
 
-static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height)
+static void update_cursor_image(u8 __iomem *dst, const u8 *src, int width, int height)
 {
 	union {
 		u32 ul;
@@ -96,7 +96,8 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height)
 	} data16;
 	u32 csum = 0;
 	s32 alpha_dst_delta, last_alpha_dst_delta;
-	u8 *srcxor, *dstxor;
+	u8 __iomem *dstxor;
+	const u8 *srcxor;
 	int i, j;
 	u32 per_pixel_copy, two_pixel_copy;
 
@@ -137,16 +138,6 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height)
 		}
 		dstxor += last_alpha_dst_delta;
 	}
-	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;
@@ -155,8 +146,6 @@ static int ast_cursor_update(void *dst, void *src, unsigned int width,
 	writel(height, dst + AST_HWC_SIGNATURE_SizeY);
 	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
 	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
-
-	return 0;
 }
 
 int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb)
@@ -188,9 +177,8 @@ int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb)
 		goto err_drm_gem_vram_vunmap_src;
 	}
 
-	ret = ast_cursor_update(dst, src, fb->width, fb->height);
-	if (ret)
-		goto err_drm_gem_vram_vunmap_dst;
+	/* do data transfer to cursor BO */
+	update_cursor_image(dst, src, fb->width, fb->height);
 
 	/*
 	 * Always unmap buffers here. Destination buffers are
@@ -203,8 +191,6 @@ int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb)
 
 	return 0;
 
-err_drm_gem_vram_vunmap_dst:
-	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
 err_drm_gem_vram_vunmap_src:
 	drm_gem_vram_vunmap(gbo, src);
 err_drm_gem_vram_unpin:
-- 
2.27.0

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

[PATCH v2 05/14] drm/ast: Move cursor pageflip into helper

[Thomas Zimmermann]

The new helper ast_cursor_page_flip() switches the cursor's front and
back BOs. This simplifies the cursor plane's update helper.

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

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 7474baddf048..815d95b2f392 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -198,7 +198,7 @@ int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb)
 	return ret;
 }
 
-void ast_cursor_set_base(struct ast_private *ast, u64 address)
+static void ast_cursor_set_base(struct ast_private *ast, u64 address)
 {
 	u8 addr0 = (address >> 3) & 0xff;
 	u8 addr1 = (address >> 11) & 0xff;
@@ -209,6 +209,24 @@ void ast_cursor_set_base(struct ast_private *ast, u64 address)
 	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xca, addr2);
 }
 
+void ast_cursor_page_flip(struct ast_private *ast)
+{
+	struct drm_device *dev = ast->dev;
+	struct drm_gem_vram_object *gbo;
+	s64 off;
+
+	gbo = ast->cursor.gbo[ast->cursor.next_index];
+
+	off = drm_gem_vram_offset(gbo);
+	if (drm_WARN_ON_ONCE(dev, off < 0))
+		return; /* Bug: we didn't pin the cursor HW BO to VRAM. */
+
+	ast_cursor_set_base(ast, off);
+
+	++ast->cursor.next_index;
+	ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
+}
+
 int ast_cursor_move(struct drm_crtc *crtc, int x, int y)
 {
 	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 9bc1bb76ec91..e973c1ab96cb 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -319,7 +319,7 @@ void ast_release_firmware(struct drm_device *dev);
 int ast_cursor_init(struct ast_private *ast);
 void ast_cursor_fini(struct ast_private *ast);
 int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb);
-void ast_cursor_set_base(struct ast_private *ast, u64 address);
+void ast_cursor_page_flip(struct ast_private *ast);
 int ast_cursor_move(struct drm_crtc *crtc, int x, int y);
 
 #endif
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 701211050832..32365f16c78c 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -660,14 +660,11 @@ static void
 ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
 				      struct drm_plane_state *old_state)
 {
-	struct drm_device *dev = plane->dev;
 	struct drm_plane_state *state = plane->state;
 	struct drm_crtc *crtc = state->crtc;
 	struct drm_framebuffer *fb = state->fb;
 	struct ast_private *ast = to_ast_private(plane->dev);
 	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	struct drm_gem_vram_object *gbo;
-	s64 off;
 	u8 jreg;
 
 	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
@@ -675,14 +672,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
 
 	if (state->fb != old_state->fb) {
 		/* A new cursor image was installed. */
-		gbo = ast->cursor.gbo[ast->cursor.next_index];
-		off = drm_gem_vram_offset(gbo);
-		if (drm_WARN_ON_ONCE(dev, off < 0))
-			return; /* Bug: we didn't pin cursor HW BO to VRAM. */
-		ast_cursor_set_base(ast, off);
-
-		++ast->cursor.next_index;
-		ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
+		ast_cursor_page_flip(ast);
 	}
 
 	ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
-- 
2.27.0

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

[PATCH v2 06/14] drm/ast: Replace ast_cursor_move() with ast_cursor_show()

[Thomas Zimmermann]

Having a cursor move function is misleading, as it actually enables the
cursor's image for displaying. So rename it to ast_cursor_show(). It's
semantics is to show a cursor at the specified location on the screen.
The displayed cursor is always the image in the cursor front BO.

This change also simplifies struct ast_crtc to being a mere wrapper around
around struct drm_crtc. It will be removed by a later patch.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_cursor.c | 42 ++++++++++++++++++++------------
 drivers/gpu/drm/ast/ast_drv.h    |  4 +--
 drivers/gpu/drm/ast/ast_mode.c   | 12 ++++-----
 3 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 815d95b2f392..8f8fdc831830 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -227,12 +227,27 @@ void ast_cursor_page_flip(struct ast_private *ast)
 	ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
 }
 
-int ast_cursor_move(struct drm_crtc *crtc, int x, int y)
+static void ast_cursor_set_location(struct ast_private *ast, u16 x, u16 y,
+				    u8 x_offset, u8 y_offset)
+{
+	u8 x0 = (x & 0x00ff);
+	u8 x1 = (x & 0x0f00) >> 8;
+	u8 y0 = (y & 0x00ff);
+	u8 y1 = (y & 0x0700) >> 8;
+
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc2, x_offset);
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc3, y_offset);
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc4, x0);
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc5, x1);
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc6, y0);
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc7, y1);
+}
+
+int ast_cursor_show(struct ast_private *ast, int x, int y,
+		    unsigned int offset_x, unsigned int offset_y)
 {
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	struct ast_private *ast = to_ast_private(crtc->dev);
 	struct drm_gem_vram_object *gbo;
-	int x_offset, y_offset;
+	u8 x_offset, y_offset;
 	u8 *dst, *sig;
 	u8 jreg;
 
@@ -245,23 +260,20 @@ int ast_cursor_move(struct drm_crtc *crtc, int x, int y)
 	writel(x, sig + AST_HWC_SIGNATURE_X);
 	writel(y, sig + AST_HWC_SIGNATURE_Y);
 
-	x_offset = ast_crtc->offset_x;
-	y_offset = ast_crtc->offset_y;
 	if (x < 0) {
-		x_offset = (-x) + ast_crtc->offset_x;
+		x_offset = (-x) + offset_x;
 		x = 0;
+	} else {
+		x_offset = offset_x;
 	}
-
 	if (y < 0) {
-		y_offset = (-y) + ast_crtc->offset_y;
+		y_offset = (-y) + offset_y;
 		y = 0;
+	} else {
+		y_offset = offset_y;
 	}
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc2, x_offset);
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc3, y_offset);
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc4, (x & 0xff));
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc5, ((x >> 8) & 0x0f));
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc6, (y & 0xff));
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc7, ((y >> 8) & 0x07));
+
+	ast_cursor_set_location(ast, x, y, x_offset, y_offset);
 
 	/* dummy write to fire HWC */
 	jreg = 0x02 |
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index e973c1ab96cb..b00091798ef5 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -239,7 +239,6 @@ struct ast_connector {
 
 struct ast_crtc {
 	struct drm_crtc base;
-	u8 offset_x, offset_y;
 };
 
 #define to_ast_crtc(x) container_of(x, struct ast_crtc, base)
@@ -320,6 +319,7 @@ int ast_cursor_init(struct ast_private *ast);
 void ast_cursor_fini(struct ast_private *ast);
 int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb);
 void ast_cursor_page_flip(struct ast_private *ast);
-int ast_cursor_move(struct drm_crtc *crtc, int x, int y);
+int ast_cursor_show(struct ast_private *ast, int x, int y,
+		    unsigned int offset_x, unsigned int offset_y);
 
 #endif
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 32365f16c78c..b452f9e28d7c 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -661,21 +661,21 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
 				      struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = plane->state;
-	struct drm_crtc *crtc = state->crtc;
 	struct drm_framebuffer *fb = state->fb;
-	struct ast_private *ast = to_ast_private(plane->dev);
-	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+	struct ast_private *ast = plane->dev->dev_private;
+	unsigned int offset_x, offset_y;
 	u8 jreg;
 
-	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
-	ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
+	offset_x = AST_MAX_HWC_WIDTH - fb->width;
+	offset_y = AST_MAX_HWC_WIDTH - fb->height;
 
 	if (state->fb != old_state->fb) {
 		/* A new cursor image was installed. */
 		ast_cursor_page_flip(ast);
 	}
 
-	ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
+	ast_cursor_show(ast, state->crtc_x, state->crtc_y,
+			offset_x, offset_y);
 
 	jreg = 0x2;
 	/* enable ARGB cursor */
-- 
2.27.0

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

[PATCH v2 07/14] drm/ast: Don't enable HW cursors twice during atomic update

[Thomas Zimmermann]

The ast_cursor_show() helper enables the cursor to be displayed. No need
to repeat that operation in the plane's atomic-update function.

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

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index b452f9e28d7c..5c41a91f5630 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -664,7 +664,6 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
 	struct drm_framebuffer *fb = state->fb;
 	struct ast_private *ast = plane->dev->dev_private;
 	unsigned int offset_x, offset_y;
-	u8 jreg;
 
 	offset_x = AST_MAX_HWC_WIDTH - fb->width;
 	offset_y = AST_MAX_HWC_WIDTH - fb->height;
@@ -676,11 +675,6 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
 
 	ast_cursor_show(ast, state->crtc_x, state->crtc_y,
 			offset_x, offset_y);
-
-	jreg = 0x2;
-	/* enable ARGB cursor */
-	jreg |= 1;
-	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
 }
 
 static void
-- 
2.27.0

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

[PATCH v2 08/14] drm/ast: Add helper to hide cursor

[Thomas Zimmermann]

As the inverse to ast_cursor_show(), ast_cursor_hide() disables the
HW cursor.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_cursor.c | 5 +++++
 drivers/gpu/drm/ast/ast_drv.h    | 1 +
 drivers/gpu/drm/ast/ast_mode.c   | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 8f8fdc831830..5421241015d6 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -284,3 +284,8 @@ int ast_cursor_show(struct ast_private *ast, int x, int y,
 
 	return 0;
 }
+
+void ast_cursor_hide(struct ast_private *ast)
+{
+	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
+}
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index b00091798ef5..92af0637ac48 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -321,5 +321,6 @@ int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb);
 void ast_cursor_page_flip(struct ast_private *ast);
 int ast_cursor_show(struct ast_private *ast, int x, int y,
 		    unsigned int offset_x, unsigned int offset_y);
+void ast_cursor_hide(struct ast_private *ast);
 
 #endif
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 5c41a91f5630..8fdc46401814 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -683,7 +683,7 @@ ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
 {
 	struct ast_private *ast = to_ast_private(plane->dev);
 
-	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
+	ast_cursor_hide(ast);
 }
 
 static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
-- 
2.27.0

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

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

[Thomas Zimmermann]

Updating the image in a cursor's HW BO requires a mapping of the BO's
buffer in the kernel's address space. Cursor image updates can happen
frequently and create CPU overhead.

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

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

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

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

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

[PATCH v2 10/14] drm/ast: Managed cursor release

[Thomas Zimmermann]

Register a release function to finalize cursors. The _fini() function
gets un-exported from the source file.

The function ast_mode_fini() is now empty and will be removed by a
later patch.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_cursor.c | 38 ++++++++++++++++++--------------
 drivers/gpu/drm/ast/ast_drv.h    |  1 -
 drivers/gpu/drm/ast/ast_mode.c   |  3 ---
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 35680402e410..acf0d23514e8 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -28,9 +28,30 @@
  */
 
 #include <drm/drm_gem_vram_helper.h>
+#include <drm/drm_managed.h>
 
 #include "ast_drv.h"
 
+static void ast_cursor_fini(struct ast_private *ast)
+{
+	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_vunmap(gbo, ast->cursor.vaddr[i]);
+		drm_gem_vram_unpin(gbo);
+		drm_gem_vram_put(gbo);
+	}
+}
+
+static void ast_cursor_release(struct drm_device *dev, void *ptr)
+{
+	struct ast_private *ast = dev->dev_private;
+
+	ast_cursor_fini(ast);
+}
+
 /*
  * Allocate cursor BOs and pins them at the end of VRAM.
  */
@@ -68,7 +89,7 @@ int ast_cursor_init(struct ast_private *ast)
 		ast->cursor.vaddr[i] = vaddr;
 	}
 
-	return 0;
+	return drmm_add_action_or_reset(dev, ast_cursor_release, NULL);
 
 err_drm_gem_vram_put:
 	while (i) {
@@ -77,25 +98,10 @@ int ast_cursor_init(struct ast_private *ast)
 		drm_gem_vram_vunmap(gbo, ast->cursor.vaddr[i]);
 		drm_gem_vram_unpin(gbo);
 		drm_gem_vram_put(gbo);
-		ast->cursor.gbo[i] = NULL;
-		ast->cursor.vaddr[i] = NULL;
 	}
 	return ret;
 }
 
-void ast_cursor_fini(struct ast_private *ast)
-{
-	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_vunmap(gbo, ast->cursor.vaddr[i]);
-		drm_gem_vram_unpin(gbo);
-		drm_gem_vram_put(gbo);
-	}
-}
-
 static void update_cursor_image(u8 __iomem *dst, const u8 *src, int width, int height)
 {
 	union {
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index f465e0c0984b..ea4de3dce2c4 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -317,7 +317,6 @@ void ast_release_firmware(struct drm_device *dev);
 
 /* ast_cursor.c */
 int ast_cursor_init(struct ast_private *ast);
-void ast_cursor_fini(struct ast_private *ast);
 int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb);
 void ast_cursor_page_flip(struct ast_private *ast);
 void ast_cursor_show(struct ast_private *ast, int x, int y,
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 8fdc46401814..4724a38c001a 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1108,9 +1108,6 @@ int ast_mode_init(struct drm_device *dev)
 
 void ast_mode_fini(struct drm_device *dev)
 {
-	struct ast_private *ast = to_ast_private(dev);
-
-	ast_cursor_fini(ast);
 }
 
 static int get_clock(void *i2c_priv)
-- 
2.27.0

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

[PATCH v2 11/14] drm/ast: Init cursors before creating modesetting structures

[Thomas Zimmermann]

The cursor helpers reserve buffer objects in VRAM and update their
content. So although tied to modesetting, cursor helpers are more
of a memory manager. The modesetting's cursor plane requires this
functionality, so initialize cursors before modesetting.

While at it, also add an error check for ast_cursor_init().

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 4724a38c001a..89d9ee0a9e81 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1073,6 +1073,10 @@ int ast_mode_init(struct drm_device *dev)
 	struct ast_private *ast = to_ast_private(dev);
 	int ret;
 
+	ret = ast_cursor_init(ast);
+	if (ret)
+		return ret;
+
 	memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
 	ret = drm_universal_plane_init(dev, &ast->primary_plane, 0x01,
 				       &ast_primary_plane_funcs,
@@ -1098,7 +1102,6 @@ int ast_mode_init(struct drm_device *dev)
 	drm_plane_helper_add(&ast->cursor_plane,
 			     &ast_cursor_plane_helper_funcs);
 
-	ast_cursor_init(ast);
 	ast_crtc_init(dev);
 	ast_encoder_init(dev);
 	ast_connector_init(dev);
-- 
2.27.0

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

[PATCH v2 12/14] drm/ast: Replace struct ast_crtc with struct drm_crtc

[Thomas Zimmermann]

Struct ast_crtc has been cleaned up and it's now a wrapper around the
DRM CRTC structure struct drm_crtc. This patch converts the driver to
struct drm_crtc and removes struct ast_crtc.

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

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index ea4de3dce2c4..77226e2fd7c0 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -238,11 +238,6 @@ struct ast_connector {
 	struct ast_i2c_chan *i2c;
 };
 
-struct ast_crtc {
-	struct drm_crtc base;
-};
-
-#define to_ast_crtc(x) container_of(x, struct ast_crtc, base)
 #define to_ast_connector(x) container_of(x, struct ast_connector, base)
 
 struct ast_vbios_stdtable {
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 89d9ee0a9e81..43c9686ba0f7 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -881,21 +881,22 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
 static int ast_crtc_init(struct drm_device *dev)
 {
 	struct ast_private *ast = to_ast_private(dev);
-	struct ast_crtc *crtc;
+	struct drm_crtc *crtc;
 	int ret;
 
-	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
+	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
 	if (!crtc)
 		return -ENOMEM;
 
-	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
+	ret = drm_crtc_init_with_planes(dev, crtc, &ast->primary_plane,
 					&ast->cursor_plane, &ast_crtc_funcs,
 					NULL);
 	if (ret)
 		goto err_kfree;
 
-	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
-	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
+	drm_mode_crtc_set_gamma_size(crtc, 256);
+	drm_crtc_helper_add(crtc, &ast_crtc_helper_funcs);
+
 	return 0;
 
 err_kfree:
-- 
2.27.0

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

[PATCH v2 13/14] drm/ast: Use managed mode-config init

[Thomas Zimmermann]

Using drmm_mode_config_init() sets up managed release of modesetting
resources. The existing modesetting's finalizer is empty, so remove it.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  | 1 -
 drivers/gpu/drm/ast/ast_main.c | 6 +++---
 drivers/gpu/drm/ast/ast_mode.c | 4 ----
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 77226e2fd7c0..ad1d0b14ef12 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -287,7 +287,6 @@ struct ast_crtc_state {
 #define to_ast_crtc_state(state) container_of(state, struct ast_crtc_state, base)
 
 extern int ast_mode_init(struct drm_device *dev);
-extern void ast_mode_fini(struct drm_device *dev);
 
 #define AST_MM_ALIGN_SHIFT 4
 #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 2eab19a9056f..2642e6d0152b 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -473,7 +473,9 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret)
 		goto out_free;
 
-	drm_mode_config_init(dev);
+	ret = drmm_mode_config_init(dev);
+	if (ret)
+		goto out_free;
 
 	dev->mode_config.funcs = (void *)&ast_mode_funcs;
 	dev->mode_config.min_width = 0;
@@ -516,8 +518,6 @@ void ast_driver_unload(struct drm_device *dev)
 
 	ast_release_firmware(dev);
 	kfree(ast->dp501_fw_addr);
-	ast_mode_fini(dev);
-	drm_mode_config_cleanup(dev);
 
 	ast_mm_fini(ast);
 	kfree(ast);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 43c9686ba0f7..33b4b6f3e394 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1110,10 +1110,6 @@ int ast_mode_init(struct drm_device *dev)
 	return 0;
 }
 
-void ast_mode_fini(struct drm_device *dev)
-{
-}
-
 static int get_clock(void *i2c_priv)
 {
 	struct ast_i2c_chan *i2c = i2c_priv;
-- 
2.27.0

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

[PATCH v2 14/14] drm/ast: Initialize mode setting in ast_mode_config_init()

[Thomas Zimmermann]

There's modesetting init code in ast_main.c. Move it to ast_mode.c and
merge it with the modesetting init code in ast_mode_init(). The result
is ast_mode_config_init(), which initalizes the whole modesetting.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  |  2 +-
 drivers/gpu/drm/ast/ast_main.c | 35 +----------------------------
 drivers/gpu/drm/ast/ast_mode.c | 41 ++++++++++++++++++++++++++++++++--
 3 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index ad1d0b14ef12..c8c442e9efdc 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -286,7 +286,7 @@ struct ast_crtc_state {
 
 #define to_ast_crtc_state(state) container_of(state, struct ast_crtc_state, base)
 
-extern int ast_mode_init(struct drm_device *dev);
+int ast_mode_config_init(struct ast_private *ast);
 
 #define AST_MM_ALIGN_SHIFT 4
 #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 2642e6d0152b..860a43a64b31 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -31,7 +31,6 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem.h>
-#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_gem_vram_helper.h>
 
 #include "ast_drv.h"
@@ -379,13 +378,6 @@ static int ast_get_dram_info(struct drm_device *dev)
 	return 0;
 }
 
-static const struct drm_mode_config_funcs ast_mode_funcs = {
-	.fb_create = drm_gem_fb_create,
-	.mode_valid = drm_vram_helper_mode_valid,
-	.atomic_check = drm_atomic_helper_check,
-	.atomic_commit = drm_atomic_helper_commit,
-};
-
 static u32 ast_get_vram_info(struct drm_device *dev)
 {
 	struct ast_private *ast = to_ast_private(dev);
@@ -473,35 +465,10 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret)
 		goto out_free;
 
-	ret = drmm_mode_config_init(dev);
-	if (ret)
-		goto out_free;
-
-	dev->mode_config.funcs = (void *)&ast_mode_funcs;
-	dev->mode_config.min_width = 0;
-	dev->mode_config.min_height = 0;
-	dev->mode_config.preferred_depth = 24;
-	dev->mode_config.prefer_shadow = 1;
-	dev->mode_config.fb_base = pci_resource_start(ast->dev->pdev, 0);
-
-	if (ast->chip == AST2100 ||
-	    ast->chip == AST2200 ||
-	    ast->chip == AST2300 ||
-	    ast->chip == AST2400 ||
-	    ast->chip == AST2500) {
-		dev->mode_config.max_width = 1920;
-		dev->mode_config.max_height = 2048;
-	} else {
-		dev->mode_config.max_width = 1600;
-		dev->mode_config.max_height = 1200;
-	}
-
-	ret = ast_mode_init(dev);
+	ret = ast_mode_config_init(ast);
 	if (ret)
 		goto out_free;
 
-	drm_mode_config_reset(dev);
-
 	return 0;
 out_free:
 	kfree(ast);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 33b4b6f3e394..154cd877d9d1 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -37,6 +37,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_gem_vram_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
@@ -1069,15 +1070,49 @@ static int ast_connector_init(struct drm_device *dev)
 	return 0;
 }
 
-int ast_mode_init(struct drm_device *dev)
+/*
+ * Mode config
+ */
+
+static const struct drm_mode_config_funcs ast_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create,
+	.mode_valid = drm_vram_helper_mode_valid,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+int ast_mode_config_init(struct ast_private *ast)
 {
-	struct ast_private *ast = to_ast_private(dev);
+	struct drm_device *dev = ast->dev;
 	int ret;
 
 	ret = ast_cursor_init(ast);
 	if (ret)
 		return ret;
 
+	ret = drmm_mode_config_init(dev);
+	if (ret)
+		return ret;
+
+	dev->mode_config.funcs = &ast_mode_config_funcs;
+	dev->mode_config.min_width = 0;
+	dev->mode_config.min_height = 0;
+	dev->mode_config.preferred_depth = 24;
+	dev->mode_config.prefer_shadow = 1;
+	dev->mode_config.fb_base = pci_resource_start(ast->dev->pdev, 0);
+
+	if (ast->chip == AST2100 ||
+	    ast->chip == AST2200 ||
+	    ast->chip == AST2300 ||
+	    ast->chip == AST2400 ||
+	    ast->chip == AST2500) {
+		dev->mode_config.max_width = 1920;
+		dev->mode_config.max_height = 2048;
+	} else {
+		dev->mode_config.max_width = 1600;
+		dev->mode_config.max_height = 1200;
+	}
+
 	memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
 	ret = drm_universal_plane_init(dev, &ast->primary_plane, 0x01,
 				       &ast_primary_plane_funcs,
@@ -1107,6 +1142,8 @@ int ast_mode_init(struct drm_device *dev)
 	ast_encoder_init(dev);
 	ast_connector_init(dev);
 
+	drm_mode_config_reset(dev);
+
 	return 0;
 }
 
-- 
2.27.0

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

Re: [PATCH v2 12/14] drm/ast: Replace struct ast_crtc with struct drm_crtc

[Sam Ravnborg]

Hi Thomas.

Just browsing code..

On Thu, Jul 02, 2020 at 01:50:27PM +0200, Thomas Zimmermann wrote:
> Struct ast_crtc has been cleaned up and it's now a wrapper around the
> DRM CRTC structure struct drm_crtc. This patch converts the driver to
> struct drm_crtc and removes struct ast_crtc.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Why is it we cannot embed struct drm_crtc?
And I also failed to see where is is de-allocated - but surely I miss
something obvious here.

	Sam

> ---
>  drivers/gpu/drm/ast/ast_drv.h  |  5 -----
>  drivers/gpu/drm/ast/ast_mode.c | 11 ++++++-----
>  2 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index ea4de3dce2c4..77226e2fd7c0 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -238,11 +238,6 @@ struct ast_connector {
>  	struct ast_i2c_chan *i2c;
>  };
>  
> -struct ast_crtc {
> -	struct drm_crtc base;
> -};
> -
> -#define to_ast_crtc(x) container_of(x, struct ast_crtc, base)
>  #define to_ast_connector(x) container_of(x, struct ast_connector, base)
>  
>  struct ast_vbios_stdtable {
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 89d9ee0a9e81..43c9686ba0f7 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -881,21 +881,22 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>  static int ast_crtc_init(struct drm_device *dev)
>  {
>  	struct ast_private *ast = to_ast_private(dev);
> -	struct ast_crtc *crtc;
> +	struct drm_crtc *crtc;
>  	int ret;
>  
> -	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
> +	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
>  	if (!crtc)
>  		return -ENOMEM;
>  
> -	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
> +	ret = drm_crtc_init_with_planes(dev, crtc, &ast->primary_plane,
>  					&ast->cursor_plane, &ast_crtc_funcs,
>  					NULL);
>  	if (ret)
>  		goto err_kfree;
>  
> -	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
> -	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
> +	drm_mode_crtc_set_gamma_size(crtc, 256);
> +	drm_crtc_helper_add(crtc, &ast_crtc_helper_funcs);
> +
>  	return 0;
>  
>  err_kfree:
> -- 
> 2.27.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/14] drm/ast: Managed modesetting

[Sam Ravnborg]

Hi Thomas.

On Thu, Jul 02, 2020 at 01:50:15PM +0200, Thomas Zimmermann wrote:
> This is the first patchset to convert ast to use managed interfaces. These
> patches address modesetting. I expect that there will be at least one more
> set of patches for memory management and one for device structures.
> 
> Patches 1 to 11 tackle HW cursor handling. The overall point is to get
> cursor support out of the plane functions and put the code into helpers.
> There are quite a few improvements that make cursors easier and faster to
> use. Patch 10 converts cursors to managed release.
> 
> With cursors out of the way, modesetting initialization is much more
> simple. Patches 12 to 14 put all related code next to each other and
> switch to managed initialization.
> 
> The patchset was tested on AST2100 hardware.
> 
> v2:
> 	* use to_ast_private() instead of
> 	  struct drm_device.dev_private (Sam)
> 
> Thomas Zimmermann (14):
>   drm/ast: Move cursor functions to ast_cursor.c
>   drm/ast: Pass struct ast_private instance to cursor init/fini
>     functions
>   drm/ast: Move cursor fb pinning and mapping into helper
>   drm/ast: Update cursor image and checksum from same function
>   drm/ast: Move cursor pageflip into helper
>   drm/ast: Replace ast_cursor_move() with ast_cursor_show()
>   drm/ast: Don't enable HW cursors twice during atomic update
>   drm/ast: Add helper to hide cursor
>   drm/ast: Keep cursor HW BOs mapped
>   drm/ast: Managed cursor release
>   drm/ast: Init cursors before creating modesetting structures
>   drm/ast: Replace struct ast_crtc with struct drm_crtc
>   drm/ast: Use managed mode-config init
>   drm/ast: Initialize mode setting in ast_mode_config_init()
I have browsed the patch-set. Nothing jumped at me but then
most of the bo handling I do not understand.

Feel free to add:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
to all patches.

But I hope someone else that knows this better could jump in and
take a look too.

	Sam

> 
>  drivers/gpu/drm/ast/Makefile     |   3 +-
>  drivers/gpu/drm/ast/ast_cursor.c | 289 +++++++++++++++++++++++++++
>  drivers/gpu/drm/ast/ast_drv.h    |  19 +-
>  drivers/gpu/drm/ast/ast_main.c   |  35 +---
>  drivers/gpu/drm/ast/ast_mode.c   | 322 ++++++-------------------------
>  5 files changed, 359 insertions(+), 309 deletions(-)
>  create mode 100644 drivers/gpu/drm/ast/ast_cursor.c
> 
> --
> 2.27.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 12/14] drm/ast: Replace struct ast_crtc with struct drm_crtc

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3604 bytes --]

Hi Sam

Am 03.07.20 um 08:38 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> Just browsing code..
> 
> On Thu, Jul 02, 2020 at 01:50:27PM +0200, Thomas Zimmermann wrote:
>> Struct ast_crtc has been cleaned up and it's now a wrapper around the
>> DRM CRTC structure struct drm_crtc. This patch converts the driver to
>> struct drm_crtc and removes struct ast_crtc.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Why is it we cannot embed struct drm_crtc?

I want to do that in a later patchset. The conversion to managed code is
fairly large, so thought it might be better to do it in several rounds.

This patchset is only for modesetting. I have another patchset that
converts the memory management to managed interfaces. After that the
final patchset will address device structures. Embedding everything CRTC
and other structures in struct ast_private would be part of this.

If you prefer a longer patchset that does everything, let me know.

> And I also failed to see where is is de-allocated - but surely I miss
> something obvious here.

It's freed in ast_crtc_destroy().

Best regards
Thomas

> 
> 	Sam
> 
>> ---
>>  drivers/gpu/drm/ast/ast_drv.h  |  5 -----
>>  drivers/gpu/drm/ast/ast_mode.c | 11 ++++++-----
>>  2 files changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>> index ea4de3dce2c4..77226e2fd7c0 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -238,11 +238,6 @@ struct ast_connector {
>>  	struct ast_i2c_chan *i2c;
>>  };
>>  
>> -struct ast_crtc {
>> -	struct drm_crtc base;
>> -};
>> -
>> -#define to_ast_crtc(x) container_of(x, struct ast_crtc, base)
>>  #define to_ast_connector(x) container_of(x, struct ast_connector, base)
>>  
>>  struct ast_vbios_stdtable {
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 89d9ee0a9e81..43c9686ba0f7 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -881,21 +881,22 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>>  static int ast_crtc_init(struct drm_device *dev)
>>  {
>>  	struct ast_private *ast = to_ast_private(dev);
>> -	struct ast_crtc *crtc;
>> +	struct drm_crtc *crtc;
>>  	int ret;
>>  
>> -	crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
>> +	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
>>  	if (!crtc)
>>  		return -ENOMEM;
>>  
>> -	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
>> +	ret = drm_crtc_init_with_planes(dev, crtc, &ast->primary_plane,
>>  					&ast->cursor_plane, &ast_crtc_funcs,
>>  					NULL);
>>  	if (ret)
>>  		goto err_kfree;
>>  
>> -	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>> -	drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
>> +	drm_mode_crtc_set_gamma_size(crtc, 256);
>> +	drm_crtc_helper_add(crtc, &ast_crtc_helper_funcs);
>> +
>>  	return 0;
>>  
>>  err_kfree:
>> -- 
>> 2.27.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v2 00/14] drm/ast: Managed modesetting

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3255 bytes --]

Hi

Am 03.07.20 um 08:44 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Thu, Jul 02, 2020 at 01:50:15PM +0200, Thomas Zimmermann wrote:
>> This is the first patchset to convert ast to use managed interfaces. These
>> patches address modesetting. I expect that there will be at least one more
>> set of patches for memory management and one for device structures.
>>
>> Patches 1 to 11 tackle HW cursor handling. The overall point is to get
>> cursor support out of the plane functions and put the code into helpers.
>> There are quite a few improvements that make cursors easier and faster to
>> use. Patch 10 converts cursors to managed release.
>>
>> With cursors out of the way, modesetting initialization is much more
>> simple. Patches 12 to 14 put all related code next to each other and
>> switch to managed initialization.
>>
>> The patchset was tested on AST2100 hardware.
>>
>> v2:
>> 	* use to_ast_private() instead of
>> 	  struct drm_device.dev_private (Sam)
>>
>> Thomas Zimmermann (14):
>>   drm/ast: Move cursor functions to ast_cursor.c
>>   drm/ast: Pass struct ast_private instance to cursor init/fini
>>     functions
>>   drm/ast: Move cursor fb pinning and mapping into helper
>>   drm/ast: Update cursor image and checksum from same function
>>   drm/ast: Move cursor pageflip into helper
>>   drm/ast: Replace ast_cursor_move() with ast_cursor_show()
>>   drm/ast: Don't enable HW cursors twice during atomic update
>>   drm/ast: Add helper to hide cursor
>>   drm/ast: Keep cursor HW BOs mapped
>>   drm/ast: Managed cursor release
>>   drm/ast: Init cursors before creating modesetting structures
>>   drm/ast: Replace struct ast_crtc with struct drm_crtc
>>   drm/ast: Use managed mode-config init
>>   drm/ast: Initialize mode setting in ast_mode_config_init()
> I have browsed the patch-set. Nothing jumped at me but then
> most of the bo handling I do not understand.
> 
> Feel free to add:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> to all patches.
> 
> But I hope someone else that knows this better could jump in and
> take a look too.

Thank you so much. Finding reviewers for such obscure drivers is not
easy. I appreciate your feedback.

Best regards
Thomas

> 
> 	Sam
> 
>>
>>  drivers/gpu/drm/ast/Makefile     |   3 +-
>>  drivers/gpu/drm/ast/ast_cursor.c | 289 +++++++++++++++++++++++++++
>>  drivers/gpu/drm/ast/ast_drv.h    |  19 +-
>>  drivers/gpu/drm/ast/ast_main.c   |  35 +---
>>  drivers/gpu/drm/ast/ast_mode.c   | 322 ++++++-------------------------
>>  5 files changed, 359 insertions(+), 309 deletions(-)
>>  create mode 100644 drivers/gpu/drm/ast/ast_cursor.c
>>
>> --
>> 2.27.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v2 12/14] drm/ast: Replace struct ast_crtc with struct drm_crtc

[Sam Ravnborg]

Hi Thomas.

On Fri, Jul 03, 2020 at 08:51:31AM +0200, Thomas Zimmermann wrote:
> Hi Sam
> 
> Am 03.07.20 um 08:38 schrieb Sam Ravnborg:
> > Hi Thomas.
> > 
> > Just browsing code..
> > 
> > On Thu, Jul 02, 2020 at 01:50:27PM +0200, Thomas Zimmermann wrote:
> >> Struct ast_crtc has been cleaned up and it's now a wrapper around the
> >> DRM CRTC structure struct drm_crtc. This patch converts the driver to
> >> struct drm_crtc and removes struct ast_crtc.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > 
> > Why is it we cannot embed struct drm_crtc?
> 
> I want to do that in a later patchset. The conversion to managed code is
> fairly large, so thought it might be better to do it in several rounds.

Understood, and several rounds are good.

> 
> This patchset is only for modesetting. I have another patchset that
> converts the memory management to managed interfaces. After that the
> final patchset will address device structures. Embedding everything CRTC
> and other structures in struct ast_private would be part of this.
> 
> If you prefer a longer patchset that does everything, let me know.
> 
> > And I also failed to see where is is de-allocated - but surely I miss
> > something obvious here.
> 
> It's freed in ast_crtc_destroy().

Ohh, one of the places that worked/works because struct crtc was/is the
first member.

I get it now, thanks for the explanation.

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