[PATCH v2 0/6] drm/ast: Managed MM

[PATCH v2 0/6] drm/ast: Managed MM

[Thomas Zimmermann]

This is the second patchset for converting ast to managed DRM interfaces.
This one addresses memory management. There will be another, final round
of patches for converting DRM device structures as well.

Patch #1 introduces managed initialization for VRAM MM. Other drivers
using the VRAM helpers should be converted to this at some point.

Patches #2 to #4 do some preparation that make ast look slightly nicer.

Patch #5 fixes a long-standing bug that I found as part of the rework.
Posting the GPU requires information about the installed DRAM. So the DRAM
detection has to run before the GPU-posting code. This got reversed by a
fix in v4.11. The patch restores the original correct order of these
operations. As the GPU is usually posted by the VGA BIOS, the problem might
not have shown up in practice.

With all the cleanups in place, patch #6 switches memory management to
mnaged interfaces.

Tested on AST2100 HW.

v2:
	* reworked managed VRAM MM; new interface name, returns errno
	  code, improved documentation (Sam)

Thomas Zimmermann (6):
  drm/vram-helper: Managed vram helpers
  drm/ast: Rename ast_ttm.c to ast_mm.c
  drm/ast: Use managed VRAM-helper initialization
  drm/ast: Move VRAM size detection to ast_mm.c
  drm/ast: Initialize DRAM type before posting GPU
  drm/ast: Use managed MM initialization

 drivers/gpu/drm/ast/Makefile                |  2 +-
 drivers/gpu/drm/ast/ast_drv.h               |  2 -
 drivers/gpu/drm/ast/ast_main.c              | 45 ++---------
 drivers/gpu/drm/ast/{ast_ttm.c => ast_mm.c} | 77 ++++++++++++++-----
 drivers/gpu/drm/drm_gem_vram_helper.c       | 84 ++++++++++++---------
 include/drm/drm_gem_vram_helper.h           |  3 +
 6 files changed, 115 insertions(+), 98 deletions(-)
 rename drivers/gpu/drm/ast/{ast_ttm.c => ast_mm.c} (63%)

--
2.27.0

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

[PATCH v2 1/6] drm/vram-helper: Managed vram helpers

[Thomas Zimmermann]

Calling drmm_vram_helper_init() sets up a managed instance of
VRAM MM. Releasing the DRM device also frees the memory manager.

The patch also updates the DRM documentation for VRAM helpers. The
tutorial now describes the new managed interface. The old interfaces
are deprecated and should not be used in new code.

v2:
	* rename init function to drmm_vram_helper_init()
	* return errno code from init function; caller does not
	  need vram_mm anyway
	* update documentation and remove docs for deprecated
	  un-managed functions

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

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index ad096600d89f..c96d54f7c180 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -10,6 +10,7 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_gem_ttm_helper.h>
 #include <drm/drm_gem_vram_helper.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_mode.h>
 #include <drm/drm_plane.h>
 #include <drm/drm_prime.h>
@@ -40,12 +41,11 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
  * the frame's scanout buffer or the cursor image. If there's no more space
  * left in VRAM, inactive GEM objects can be moved to system memory.
  *
- * The easiest way to use the VRAM helper library is to call
- * drm_vram_helper_alloc_mm(). The function allocates and initializes an
- * instance of &struct drm_vram_mm in &struct drm_device.vram_mm . Use
- * &DRM_GEM_VRAM_DRIVER to initialize &struct drm_driver and
- * &DRM_VRAM_MM_FILE_OPERATIONS to initialize &struct file_operations;
- * as illustrated below.
+ * To initialize the VRAM helper library call drmm_vram_helper_alloc_mm().
+ * The function allocates and initializes an instance of &struct drm_vram_mm
+ * in &struct drm_device.vram_mm . Use &DRM_GEM_VRAM_DRIVER to initialize
+ * &struct drm_driver and  &DRM_VRAM_MM_FILE_OPERATIONS to initialize
+ * &struct file_operations; as illustrated below.
  *
  * .. code-block:: c
  *
@@ -69,7 +69,7 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
  *		// setup device, vram base and size
  *		// ...
  *
- *		ret = drm_vram_helper_alloc_mm(dev, vram_base, vram_size);
+ *		ret = drmm_vram_helper_alloc_mm(dev, vram_base, vram_size);
  *		if (ret)
  *			return ret;
  *		return 0;
@@ -81,20 +81,12 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
  * manages an area of video RAM with VRAM MM and provides GEM VRAM objects
  * to userspace.
  *
- * To clean up the VRAM memory management, call drm_vram_helper_release_mm()
- * in the driver's clean-up code.
+ * You don't have to clean up the instance of VRAM MM.
+ * drmm_vram_helper_alloc_mm() is a managed interface that installs a
+ * clean-up handler to run during the DRM device's release.
  *
- * .. code-block:: c
- *
- *	void fini_drm_driver()
- *	{
- *		struct drm_device *dev = ...;
- *
- *		drm_vram_helper_release_mm(dev);
- *	}
- *
- * For drawing or scanout operations, buffer object have to be pinned in video
- * RAM. Call drm_gem_vram_pin() with &DRM_GEM_VRAM_PL_FLAG_VRAM or
+ * For drawing or scanout operations, rsp. buffer objects have to be pinned
+ * in video RAM. Call drm_gem_vram_pin() with &DRM_GEM_VRAM_PL_FLAG_VRAM or
  * &DRM_GEM_VRAM_PL_FLAG_SYSTEM to pin a buffer object in video RAM or system
  * memory. Call drm_gem_vram_unpin() to release the pinned object afterwards.
  *
@@ -1176,17 +1168,7 @@ static void drm_vram_mm_cleanup(struct drm_vram_mm *vmm)
  * Helpers for integration with struct drm_device
  */
 
-/**
- * drm_vram_helper_alloc_mm - Allocates a device's instance of \
-	&struct drm_vram_mm
- * @dev:	the DRM device
- * @vram_base:	the base address of the video memory
- * @vram_size:	the size of the video memory in bytes
- *
- * Returns:
- * The new instance of &struct drm_vram_mm on success, or
- * an ERR_PTR()-encoded errno code otherwise.
- */
+/* deprecated; use drmm_vram_mm_init() */
 struct drm_vram_mm *drm_vram_helper_alloc_mm(
 	struct drm_device *dev, uint64_t vram_base, size_t vram_size)
 {
@@ -1212,11 +1194,6 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm(
 }
 EXPORT_SYMBOL(drm_vram_helper_alloc_mm);
 
-/**
- * drm_vram_helper_release_mm - Releases a device's instance of \
-	&struct drm_vram_mm
- * @dev:	the DRM device
- */
 void drm_vram_helper_release_mm(struct drm_device *dev)
 {
 	if (!dev->vram_mm)
@@ -1228,6 +1205,41 @@ void drm_vram_helper_release_mm(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_vram_helper_release_mm);
 
+static void drm_vram_mm_release(struct drm_device *dev, void *ptr)
+{
+	drm_vram_helper_release_mm(dev);
+}
+
+/**
+ * drmm_vram_helper_init - Initializes a device's instance of
+ *                         &struct drm_vram_mm
+ * @dev:	the DRM device
+ * @vram_base:	the base address of the video memory
+ * @vram_size:	the size of the video memory in bytes
+ *
+ * Creates a new instance of &struct drm_vram_mm and stores it in
+ * struct &drm_device.vram_mm. The instance is auto-managed and cleaned
+ * up as part of device cleanup. Calling this function multiple times
+ * will generate an error message.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drmm_vram_helper_init(struct drm_device *dev, uint64_t vram_base,
+			  size_t vram_size)
+{
+	struct drm_vram_mm *vram_mm;
+
+	if (drm_WARN_ON_ONCE(dev, dev->vram_mm))
+		return 0;
+
+	vram_mm = drm_vram_helper_alloc_mm(dev, vram_base, vram_size);
+	if (IS_ERR(vram_mm))
+		return PTR_ERR(vram_mm);
+	return drmm_add_action_or_reset(dev, drm_vram_mm_release, NULL);
+}
+EXPORT_SYMBOL(drmm_vram_helper_init);
+
 /*
  * Mode-config helpers
  */
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index b63bcd1b996d..035332f3723f 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -206,6 +206,9 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm(
 	struct drm_device *dev, uint64_t vram_base, size_t vram_size);
 void drm_vram_helper_release_mm(struct drm_device *dev);
 
+int drmm_vram_helper_init(struct drm_device *dev, uint64_t vram_base,
+			  size_t vram_size);
+
 /*
  * Mode-config helpers
  */
-- 
2.27.0

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

[PATCH v2 2/6] drm/ast: Rename ast_ttm.c to ast_mm.c

[Thomas Zimmermann]

Although built upon TTM, the ast driver's VRAM MM helper does not
expose TTM to its users. Rename the related ast file to ast_mm.c.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/Makefile                | 2 +-
 drivers/gpu/drm/ast/{ast_ttm.c => ast_mm.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/gpu/drm/ast/{ast_ttm.c => ast_mm.c} (100%)

diff --git a/drivers/gpu/drm/ast/Makefile b/drivers/gpu/drm/ast/Makefile
index 6a5b3b247316..2265a8a624dd 100644
--- a/drivers/gpu/drm/ast/Makefile
+++ b/drivers/gpu/drm/ast/Makefile
@@ -3,7 +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_cursor.o ast_drv.o ast_main.o ast_mode.o ast_ttm.o ast_post.o \
+ast-y := ast_cursor.o ast_drv.o ast_main.o ast_mm.o ast_mode.o ast_post.o \
 	 ast_dp501.o
 
 obj-$(CONFIG_DRM_AST) := ast.o
diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_mm.c
similarity index 100%
rename from drivers/gpu/drm/ast/ast_ttm.c
rename to drivers/gpu/drm/ast/ast_mm.c
-- 
2.27.0

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

[PATCH v2 3/6] drm/ast: Use managed VRAM-helper initialization

[Thomas Zimmermann]

As a first step to managed MM code in ast, switch over VRAM MM helpers.

v2:
	* updated to use drmm_vram_helper_init()

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mm.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mm.c b/drivers/gpu/drm/ast/ast_mm.c
index 9c3788a4c1c5..f0a96cae68c9 100644
--- a/drivers/gpu/drm/ast/ast_mm.c
+++ b/drivers/gpu/drm/ast/ast_mm.c
@@ -35,15 +35,12 @@
 
 int ast_mm_init(struct ast_private *ast)
 {
-	struct drm_vram_mm *vmm;
 	int ret;
 	struct drm_device *dev = ast->dev;
 
-	vmm = drm_vram_helper_alloc_mm(
-		dev, pci_resource_start(dev->pdev, 0),
-		ast->vram_size);
-	if (IS_ERR(vmm)) {
-		ret = PTR_ERR(vmm);
+	ret = drmm_vram_helper_init(dev, pci_resource_start(dev->pdev, 0),
+				    ast->vram_size);
+	if (ret) {
 		drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
 		return ret;
 	}
@@ -60,8 +57,6 @@ void ast_mm_fini(struct ast_private *ast)
 {
 	struct drm_device *dev = ast->dev;
 
-	drm_vram_helper_release_mm(dev);
-
 	arch_phys_wc_del(ast->fb_mtrr);
 	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
 				pci_resource_len(dev->pdev, 0));
-- 
2.27.0

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

[PATCH v2 4/6] drm/ast: Move VRAM size detection to ast_mm.c

[Thomas Zimmermann]

VRAM size detection is only relevant to the memory management. Move
the code into ast_mm.c.

While at it, rename the function to ast_get_vram_size(). The function
argument's type is now struct ast_private. The result is stored in a
local variable and not in struct ast_private any longer.

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

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index c8c442e9efdc..9a770e5b36d1 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -110,7 +110,6 @@ struct ast_private {
 	uint32_t dram_bus_width;
 	uint32_t dram_type;
 	uint32_t mclk;
-	uint32_t vram_size;
 
 	int fb_mtrr;
 
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 860a43a64b31..b162cc82204d 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -378,38 +378,6 @@ static int ast_get_dram_info(struct drm_device *dev)
 	return 0;
 }
 
-static u32 ast_get_vram_info(struct drm_device *dev)
-{
-	struct ast_private *ast = to_ast_private(dev);
-	u8 jreg;
-	u32 vram_size;
-	ast_open_key(ast);
-
-	vram_size = AST_VIDMEM_DEFAULT_SIZE;
-	jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xaa, 0xff);
-	switch (jreg & 3) {
-	case 0: vram_size = AST_VIDMEM_SIZE_8M; break;
-	case 1: vram_size = AST_VIDMEM_SIZE_16M; break;
-	case 2: vram_size = AST_VIDMEM_SIZE_32M; break;
-	case 3: vram_size = AST_VIDMEM_SIZE_64M; break;
-	}
-
-	jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x99, 0xff);
-	switch (jreg & 0x03) {
-	case 1:
-		vram_size -= 0x100000;
-		break;
-	case 2:
-		vram_size -= 0x200000;
-		break;
-	case 3:
-		vram_size -= 0x400000;
-		break;
-	}
-
-	return vram_size;
-}
-
 int ast_driver_load(struct drm_device *dev, unsigned long flags)
 {
 	struct ast_private *ast;
@@ -456,10 +424,8 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
 	ret = ast_get_dram_info(dev);
 	if (ret)
 		goto out_free;
-	ast->vram_size = ast_get_vram_info(dev);
-	drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
-		 ast->mclk, ast->dram_type,
-		 ast->dram_bus_width, ast->vram_size);
+	drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d\n",
+		 ast->mclk, ast->dram_type, ast->dram_bus_width);
 
 	ret = ast_mm_init(ast);
 	if (ret)
diff --git a/drivers/gpu/drm/ast/ast_mm.c b/drivers/gpu/drm/ast/ast_mm.c
index f0a96cae68c9..aaeb19d01052 100644
--- a/drivers/gpu/drm/ast/ast_mm.c
+++ b/drivers/gpu/drm/ast/ast_mm.c
@@ -33,13 +33,56 @@
 
 #include "ast_drv.h"
 
+static u32 ast_get_vram_size(struct ast_private *ast)
+{
+	u8 jreg;
+	u32 vram_size;
+
+	ast_open_key(ast);
+
+	vram_size = AST_VIDMEM_DEFAULT_SIZE;
+	jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xaa, 0xff);
+	switch (jreg & 3) {
+	case 0:
+		vram_size = AST_VIDMEM_SIZE_8M;
+		break;
+	case 1:
+		vram_size = AST_VIDMEM_SIZE_16M;
+		break;
+	case 2:
+		vram_size = AST_VIDMEM_SIZE_32M;
+		break;
+	case 3:
+		vram_size = AST_VIDMEM_SIZE_64M;
+		break;
+	}
+
+	jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x99, 0xff);
+	switch (jreg & 0x03) {
+	case 1:
+		vram_size -= 0x100000;
+		break;
+	case 2:
+		vram_size -= 0x200000;
+		break;
+	case 3:
+		vram_size -= 0x400000;
+		break;
+	}
+
+	return vram_size;
+}
+
 int ast_mm_init(struct ast_private *ast)
 {
+	u32 vram_size;
 	int ret;
 	struct drm_device *dev = ast->dev;
 
+	vram_size = ast_get_vram_size(ast);
+
 	ret = drmm_vram_helper_init(dev, pci_resource_start(dev->pdev, 0),
-				    ast->vram_size);
+				    vram_size);
 	if (ret) {
 		drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
 		return ret;
-- 
2.27.0

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

[PATCH v2 5/6] drm/ast: Initialize DRAM type before posting GPU

[Thomas Zimmermann]

Posting the GPU requires the correct DRAM type to be stored in
struct ast_private. Therefore first initialize the DRAM info and
then post the GPU. This restores the original order of instructions
in this function.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Fixes: bad09da6deab ("drm/ast: Fixed vram size incorrect issue on POWER")
Cc: Joel Stanley <joel@jms.id.au>
Cc: Y.C. Chen <yc_chen@aspeedtech.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Cc: "Y.C. Chen" <yc_chen@aspeedtech.com>
Cc: <stable@vger.kernel.org> # v4.11+
---
 drivers/gpu/drm/ast/ast_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index b162cc82204d..87e5baded2a7 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -418,15 +418,15 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
 
 	ast_detect_chip(dev, &need_post);
 
-	if (need_post)
-		ast_post_gpu(dev);
-
 	ret = ast_get_dram_info(dev);
 	if (ret)
 		goto out_free;
 	drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d\n",
 		 ast->mclk, ast->dram_type, ast->dram_bus_width);
 
+	if (need_post)
+		ast_post_gpu(dev);
+
 	ret = ast_mm_init(ast);
 	if (ret)
 		goto out_free;
-- 
2.27.0

[PATCH v2 5/6] drm/ast: Initialize DRAM type before posting GPU

[Thomas Zimmermann]

Posting the GPU requires the correct DRAM type to be stored in
struct ast_private. Therefore first initialize the DRAM info and
then post the GPU. This restores the original order of instructions
in this function.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Fixes: bad09da6deab ("drm/ast: Fixed vram size incorrect issue on POWER")
Cc: Joel Stanley <joel@jms.id.au>
Cc: Y.C. Chen <yc_chen@aspeedtech.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Cc: "Y.C. Chen" <yc_chen@aspeedtech.com>
Cc: <stable@vger.kernel.org> # v4.11+
---
 drivers/gpu/drm/ast/ast_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index b162cc82204d..87e5baded2a7 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -418,15 +418,15 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
 
 	ast_detect_chip(dev, &need_post);
 
-	if (need_post)
-		ast_post_gpu(dev);
-
 	ret = ast_get_dram_info(dev);
 	if (ret)
 		goto out_free;
 	drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d\n",
 		 ast->mclk, ast->dram_type, ast->dram_bus_width);
 
+	if (need_post)
+		ast_post_gpu(dev);
+
 	ret = ast_mm_init(ast);
 	if (ret)
 		goto out_free;
-- 
2.27.0

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

[PATCH v2 6/6] drm/ast: Use managed MM initialization

[Thomas Zimmermann]

Cleaning up ast's MM code with ast_mm_fini() resets the write-combine
flags on the VRAM I/O memory. Drop ast_mm_fini() in favor of an auto-
release callback. Releasing the device also executes the callback.

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

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 9a770e5b36d1..e3a264ac7ee2 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -291,7 +291,6 @@ int ast_mode_config_init(struct ast_private *ast);
 #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
 
 int ast_mm_init(struct ast_private *ast);
-void ast_mm_fini(struct ast_private *ast);
 
 /* ast post */
 void ast_enable_vga(struct drm_device *dev);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 87e5baded2a7..dd12b55d57a2 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -452,6 +452,5 @@ void ast_driver_unload(struct drm_device *dev)
 	ast_release_firmware(dev);
 	kfree(ast->dp501_fw_addr);
 
-	ast_mm_fini(ast);
 	kfree(ast);
 }
diff --git a/drivers/gpu/drm/ast/ast_mm.c b/drivers/gpu/drm/ast/ast_mm.c
index aaeb19d01052..9186ec3ebbe0 100644
--- a/drivers/gpu/drm/ast/ast_mm.c
+++ b/drivers/gpu/drm/ast/ast_mm.c
@@ -28,8 +28,9 @@
 
 #include <linux/pci.h>
 
-#include <drm/drm_print.h>
 #include <drm/drm_gem_vram_helper.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_print.h>
 
 #include "ast_drv.h"
 
@@ -73,6 +74,15 @@ static u32 ast_get_vram_size(struct ast_private *ast)
 	return vram_size;
 }
 
+static void ast_mm_release(struct drm_device *dev, void *ptr)
+{
+	struct ast_private *ast = to_ast_private(dev);
+
+	arch_phys_wc_del(ast->fb_mtrr);
+	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
+				pci_resource_len(dev->pdev, 0));
+}
+
 int ast_mm_init(struct ast_private *ast)
 {
 	u32 vram_size;
@@ -93,14 +103,5 @@ int ast_mm_init(struct ast_private *ast)
 	ast->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
 					pci_resource_len(dev->pdev, 0));
 
-	return 0;
-}
-
-void ast_mm_fini(struct ast_private *ast)
-{
-	struct drm_device *dev = ast->dev;
-
-	arch_phys_wc_del(ast->fb_mtrr);
-	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
-				pci_resource_len(dev->pdev, 0));
+	return drmm_add_action_or_reset(dev, ast_mm_release, NULL);
 }
-- 
2.27.0

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

Re: [PATCH v2 0/6] drm/ast: Managed MM

[Sam Ravnborg]

Hi Thomas.

On Thu, Jul 16, 2020 at 02:53:47PM +0200, Thomas Zimmermann wrote:
> This is the second patchset for converting ast to managed DRM interfaces.
> This one addresses memory management. There will be another, final round
> of patches for converting DRM device structures as well.
> 
> Patch #1 introduces managed initialization for VRAM MM. Other drivers
> using the VRAM helpers should be converted to this at some point.
> 
> Patches #2 to #4 do some preparation that make ast look slightly nicer.
> 
> Patch #5 fixes a long-standing bug that I found as part of the rework.
> Posting the GPU requires information about the installed DRAM. So the DRAM
> detection has to run before the GPU-posting code. This got reversed by a
> fix in v4.11. The patch restores the original correct order of these
> operations. As the GPU is usually posted by the VGA BIOS, the problem might
> not have shown up in practice.
> 
> With all the cleanups in place, patch #6 switches memory management to
> mnaged interfaces.
> 
> Tested on AST2100 HW.
> 
> v2:
> 	* reworked managed VRAM MM; new interface name, returns errno
> 	  code, improved documentation (Sam)
> 
> Thomas Zimmermann (6):
>   drm/vram-helper: Managed vram helpers
>   drm/ast: Rename ast_ttm.c to ast_mm.c
>   drm/ast: Use managed VRAM-helper initialization
>   drm/ast: Move VRAM size detection to ast_mm.c
>   drm/ast: Initialize DRAM type before posting GPU
>   drm/ast: Use managed MM initialization

Series looks good now. All patches are:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>


	Sam
> 
>  drivers/gpu/drm/ast/Makefile                |  2 +-
>  drivers/gpu/drm/ast/ast_drv.h               |  2 -
>  drivers/gpu/drm/ast/ast_main.c              | 45 ++---------
>  drivers/gpu/drm/ast/{ast_ttm.c => ast_mm.c} | 77 ++++++++++++++-----
>  drivers/gpu/drm/drm_gem_vram_helper.c       | 84 ++++++++++++---------
>  include/drm/drm_gem_vram_helper.h           |  3 +
>  6 files changed, 115 insertions(+), 98 deletions(-)
>  rename drivers/gpu/drm/ast/{ast_ttm.c => ast_mm.c} (63%)
> 
> --
> 2.27.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/6] drm/ast: Managed MM

[Thomas Zimmermann]

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



Am 16.07.20 um 15:24 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Thu, Jul 16, 2020 at 02:53:47PM +0200, Thomas Zimmermann wrote:
>> This is the second patchset for converting ast to managed DRM interfaces.
>> This one addresses memory management. There will be another, final round
>> of patches for converting DRM device structures as well.
>>
>> Patch #1 introduces managed initialization for VRAM MM. Other drivers
>> using the VRAM helpers should be converted to this at some point.
>>
>> Patches #2 to #4 do some preparation that make ast look slightly nicer.
>>
>> Patch #5 fixes a long-standing bug that I found as part of the rework.
>> Posting the GPU requires information about the installed DRAM. So the DRAM
>> detection has to run before the GPU-posting code. This got reversed by a
>> fix in v4.11. The patch restores the original correct order of these
>> operations. As the GPU is usually posted by the VGA BIOS, the problem might
>> not have shown up in practice.
>>
>> With all the cleanups in place, patch #6 switches memory management to
>> mnaged interfaces.
>>
>> Tested on AST2100 HW.
>>
>> v2:
>> 	* reworked managed VRAM MM; new interface name, returns errno
>> 	  code, improved documentation (Sam)
>>
>> Thomas Zimmermann (6):
>>   drm/vram-helper: Managed vram helpers
>>   drm/ast: Rename ast_ttm.c to ast_mm.c
>>   drm/ast: Use managed VRAM-helper initialization
>>   drm/ast: Move VRAM size detection to ast_mm.c
>>   drm/ast: Initialize DRAM type before posting GPU
>>   drm/ast: Use managed MM initialization
> 
> Series looks good now. All patches are:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Thanks, Sam. I added the patches to drm-misc-next

> 
> 
> 	Sam
>>
>>  drivers/gpu/drm/ast/Makefile                |  2 +-
>>  drivers/gpu/drm/ast/ast_drv.h               |  2 -
>>  drivers/gpu/drm/ast/ast_main.c              | 45 ++---------
>>  drivers/gpu/drm/ast/{ast_ttm.c => ast_mm.c} | 77 ++++++++++++++-----
>>  drivers/gpu/drm/drm_gem_vram_helper.c       | 84 ++++++++++++---------
>>  include/drm/drm_gem_vram_helper.h           |  3 +
>>  6 files changed, 115 insertions(+), 98 deletions(-)
>>  rename drivers/gpu/drm/ast/{ast_ttm.c => ast_mm.c} (63%)
>>
>> --
>> 2.27.0

-- 
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