Re: [PATCH 5/7] drm/ttm: add ttm_sys_manager

2021-04-14 Thread Christian König 



Am 14.04.21 um 12:05 schrieb Matthew Auld:

On Tue, 13 Apr 2021 at 14:53, Christian König
 wrote:

Add a separate manager for the system domain and make function tables
mandatory.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/ttm/Makefile  |  2 +-
  drivers/gpu/drm/ttm/ttm_device.c  | 17 +-
  drivers/gpu/drm/ttm/ttm_module.h  |  3 ++
  drivers/gpu/drm/ttm/ttm_resource.c| 10 ++
  drivers/gpu/drm/ttm/ttm_sys_manager.c | 46 +++
  5 files changed, 53 insertions(+), 25 deletions(-)
  create mode 100644 drivers/gpu/drm/ttm/ttm_sys_manager.c

diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
index 40e5e9da7953..f906b22959cf 100644
--- a/drivers/gpu/drm/ttm/Makefile
+++ b/drivers/gpu/drm/ttm/Makefile
@@ -4,7 +4,7 @@

  ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
 ttm_execbuf_util.o ttm_range_manager.o ttm_resource.o ttm_pool.o \
-   ttm_device.o
+   ttm_device.o ttm_sys_manager.o
  ttm-$(CONFIG_AGP) += ttm_agp_backend.o

  obj-$(CONFIG_DRM_TTM) += ttm.o
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 1f2024164d72..39956e08b4c3 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -165,21 +165,6 @@ int ttm_device_swapout(struct ttm_device *bdev, struct 
ttm_operation_ctx *ctx,
  }
  EXPORT_SYMBOL(ttm_device_swapout);

-static void ttm_init_sysman(struct ttm_device *bdev)
-{
-   struct ttm_resource_manager *man = &bdev->sysman;
-
-   /*
-* Initialize the system memory buffer type.
-* Other types need to be driver / IOCTL initialized.
-*/
-   man->use_tt = true;
-
-   ttm_resource_manager_init(man, 0);
-   ttm_set_driver_manager(bdev, TTM_PL_SYSTEM, man);
-   ttm_resource_manager_set_used(man, true);
-}
-
  static void ttm_device_delayed_workqueue(struct work_struct *work)
  {
 struct ttm_device *bdev =
@@ -222,7 +207,7 @@ int ttm_device_init(struct ttm_device *bdev, struct 
ttm_device_funcs *funcs,

 bdev->funcs = funcs;

-   ttm_init_sysman(bdev);
+   ttm_sys_man_init(bdev);
 ttm_pool_init(&bdev->pool, dev, use_dma_alloc, use_dma32);

 bdev->vma_manager = vma_manager;
diff --git a/drivers/gpu/drm/ttm/ttm_module.h b/drivers/gpu/drm/ttm/ttm_module.h
index d7cac5d4b835..26564a98958f 100644
--- a/drivers/gpu/drm/ttm/ttm_module.h
+++ b/drivers/gpu/drm/ttm/ttm_module.h
@@ -34,7 +34,10 @@
  #define TTM_PFX "[TTM] "

  struct dentry;
+struct ttm_device;

  extern struct dentry *ttm_debugfs_root;

+int ttm_sys_man_init(struct ttm_device *bdev);
+
  #endif /* _TTM_MODULE_H_ */
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c 
b/drivers/gpu/drm/ttm/ttm_resource.c
index 04f2eef653ab..a6900b82f31a 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -33,9 +33,6 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
 ttm_manager_type(bo->bdev, res->mem_type);

 res->mm_node = NULL;
-   if (!man->func || !man->func->alloc)
-   return 0;
-
 return man->func->alloc(man, bo, place, res);
  }

@@ -44,9 +41,7 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct 
ttm_resource *res)
 struct ttm_resource_manager *man =
 ttm_manager_type(bo->bdev, res->mem_type);

-   if (man->func && man->func->free)
-   man->func->free(man, res);
-
+   man->func->free(man, res);
 res->mm_node = NULL;
 res->mem_type = TTM_PL_SYSTEM;
  }
@@ -139,7 +134,6 @@ void ttm_resource_manager_debug(struct ttm_resource_manager 
*man,
 drm_printf(p, "  use_type: %d\n", man->use_type);
 drm_printf(p, "  use_tt: %d\n", man->use_tt);
 drm_printf(p, "  size: %llu\n", man->size);
-   if (man->func && man->func->debug)
-   (*man->func->debug)(man, p);
+   man->func->debug(man, p);

Can we drop this one? It looks like nouveau has manager_funcs without
the .debug hook? Like nouveau_gart_manager?


Ah! I thought that was another check for !man, but you are right that 
will break nouveau.


Thanks,
Christian.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/7] drm/ttm: add ttm_sys_manager

2021-04-14 Thread Matthew Auld 
On Tue, 13 Apr 2021 at 14:53, Christian König
 wrote:
>
> Add a separate manager for the system domain and make function tables
> mandatory.
>
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/ttm/Makefile  |  2 +-
>  drivers/gpu/drm/ttm/ttm_device.c  | 17 +-
>  drivers/gpu/drm/ttm/ttm_module.h  |  3 ++
>  drivers/gpu/drm/ttm/ttm_resource.c| 10 ++
>  drivers/gpu/drm/ttm/ttm_sys_manager.c | 46 +++
>  5 files changed, 53 insertions(+), 25 deletions(-)
>  create mode 100644 drivers/gpu/drm/ttm/ttm_sys_manager.c
>
> diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
> index 40e5e9da7953..f906b22959cf 100644
> --- a/drivers/gpu/drm/ttm/Makefile
> +++ b/drivers/gpu/drm/ttm/Makefile
> @@ -4,7 +4,7 @@
>
>  ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
> ttm_execbuf_util.o ttm_range_manager.o ttm_resource.o ttm_pool.o \
> -   ttm_device.o
> +   ttm_device.o ttm_sys_manager.o
>  ttm-$(CONFIG_AGP) += ttm_agp_backend.o
>
>  obj-$(CONFIG_DRM_TTM) += ttm.o
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
> b/drivers/gpu/drm/ttm/ttm_device.c
> index 1f2024164d72..39956e08b4c3 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -165,21 +165,6 @@ int ttm_device_swapout(struct ttm_device *bdev, struct 
> ttm_operation_ctx *ctx,
>  }
>  EXPORT_SYMBOL(ttm_device_swapout);
>
> -static void ttm_init_sysman(struct ttm_device *bdev)
> -{
> -   struct ttm_resource_manager *man = &bdev->sysman;
> -
> -   /*
> -* Initialize the system memory buffer type.
> -* Other types need to be driver / IOCTL initialized.
> -*/
> -   man->use_tt = true;
> -
> -   ttm_resource_manager_init(man, 0);
> -   ttm_set_driver_manager(bdev, TTM_PL_SYSTEM, man);
> -   ttm_resource_manager_set_used(man, true);
> -}
> -
>  static void ttm_device_delayed_workqueue(struct work_struct *work)
>  {
> struct ttm_device *bdev =
> @@ -222,7 +207,7 @@ int ttm_device_init(struct ttm_device *bdev, struct 
> ttm_device_funcs *funcs,
>
> bdev->funcs = funcs;
>
> -   ttm_init_sysman(bdev);
> +   ttm_sys_man_init(bdev);
> ttm_pool_init(&bdev->pool, dev, use_dma_alloc, use_dma32);
>
> bdev->vma_manager = vma_manager;
> diff --git a/drivers/gpu/drm/ttm/ttm_module.h 
> b/drivers/gpu/drm/ttm/ttm_module.h
> index d7cac5d4b835..26564a98958f 100644
> --- a/drivers/gpu/drm/ttm/ttm_module.h
> +++ b/drivers/gpu/drm/ttm/ttm_module.h
> @@ -34,7 +34,10 @@
>  #define TTM_PFX "[TTM] "
>
>  struct dentry;
> +struct ttm_device;
>
>  extern struct dentry *ttm_debugfs_root;
>
> +int ttm_sys_man_init(struct ttm_device *bdev);
> +
>  #endif /* _TTM_MODULE_H_ */
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c 
> b/drivers/gpu/drm/ttm/ttm_resource.c
> index 04f2eef653ab..a6900b82f31a 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -33,9 +33,6 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
> ttm_manager_type(bo->bdev, res->mem_type);
>
> res->mm_node = NULL;
> -   if (!man->func || !man->func->alloc)
> -   return 0;
> -
> return man->func->alloc(man, bo, place, res);
>  }
>
> @@ -44,9 +41,7 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct 
> ttm_resource *res)
> struct ttm_resource_manager *man =
> ttm_manager_type(bo->bdev, res->mem_type);
>
> -   if (man->func && man->func->free)
> -   man->func->free(man, res);
> -
> +   man->func->free(man, res);
> res->mm_node = NULL;
> res->mem_type = TTM_PL_SYSTEM;
>  }
> @@ -139,7 +134,6 @@ void ttm_resource_manager_debug(struct 
> ttm_resource_manager *man,
> drm_printf(p, "  use_type: %d\n", man->use_type);
> drm_printf(p, "  use_tt: %d\n", man->use_tt);
> drm_printf(p, "  size: %llu\n", man->size);
> -   if (man->func && man->func->debug)
> -   (*man->func->debug)(man, p);
> +   man->func->debug(man, p);

Can we drop this one? It looks like nouveau has manager_funcs without
the .debug hook? Like nouveau_gart_manager?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/7] drm/ttm: add ttm_sys_manager

2021-04-13 Thread Christian König 
Add a separate manager for the system domain and make function tables
mandatory.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/Makefile  |  2 +-
 drivers/gpu/drm/ttm/ttm_device.c  | 17 +-
 drivers/gpu/drm/ttm/ttm_module.h  |  3 ++
 drivers/gpu/drm/ttm/ttm_resource.c| 10 ++
 drivers/gpu/drm/ttm/ttm_sys_manager.c | 46 +++
 5 files changed, 53 insertions(+), 25 deletions(-)
 create mode 100644 drivers/gpu/drm/ttm/ttm_sys_manager.c

diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
index 40e5e9da7953..f906b22959cf 100644
--- a/drivers/gpu/drm/ttm/Makefile
+++ b/drivers/gpu/drm/ttm/Makefile
@@ -4,7 +4,7 @@
 
 ttm-y := ttm_tt.o ttm_bo.o ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
ttm_execbuf_util.o ttm_range_manager.o ttm_resource.o ttm_pool.o \
-   ttm_device.o
+   ttm_device.o ttm_sys_manager.o
 ttm-$(CONFIG_AGP) += ttm_agp_backend.o
 
 obj-$(CONFIG_DRM_TTM) += ttm.o
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 1f2024164d72..39956e08b4c3 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -165,21 +165,6 @@ int ttm_device_swapout(struct ttm_device *bdev, struct 
ttm_operation_ctx *ctx,
 }
 EXPORT_SYMBOL(ttm_device_swapout);
 
-static void ttm_init_sysman(struct ttm_device *bdev)
-{
-   struct ttm_resource_manager *man = &bdev->sysman;
-
-   /*
-* Initialize the system memory buffer type.
-* Other types need to be driver / IOCTL initialized.
-*/
-   man->use_tt = true;
-
-   ttm_resource_manager_init(man, 0);
-   ttm_set_driver_manager(bdev, TTM_PL_SYSTEM, man);
-   ttm_resource_manager_set_used(man, true);
-}
-
 static void ttm_device_delayed_workqueue(struct work_struct *work)
 {
struct ttm_device *bdev =
@@ -222,7 +207,7 @@ int ttm_device_init(struct ttm_device *bdev, struct 
ttm_device_funcs *funcs,
 
bdev->funcs = funcs;
 
-   ttm_init_sysman(bdev);
+   ttm_sys_man_init(bdev);
ttm_pool_init(&bdev->pool, dev, use_dma_alloc, use_dma32);
 
bdev->vma_manager = vma_manager;
diff --git a/drivers/gpu/drm/ttm/ttm_module.h b/drivers/gpu/drm/ttm/ttm_module.h
index d7cac5d4b835..26564a98958f 100644
--- a/drivers/gpu/drm/ttm/ttm_module.h
+++ b/drivers/gpu/drm/ttm/ttm_module.h
@@ -34,7 +34,10 @@
 #define TTM_PFX "[TTM] "
 
 struct dentry;
+struct ttm_device;
 
 extern struct dentry *ttm_debugfs_root;
 
+int ttm_sys_man_init(struct ttm_device *bdev);
+
 #endif /* _TTM_MODULE_H_ */
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c 
b/drivers/gpu/drm/ttm/ttm_resource.c
index 04f2eef653ab..a6900b82f31a 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -33,9 +33,6 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
ttm_manager_type(bo->bdev, res->mem_type);
 
res->mm_node = NULL;
-   if (!man->func || !man->func->alloc)
-   return 0;
-
return man->func->alloc(man, bo, place, res);
 }
 
@@ -44,9 +41,7 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct 
ttm_resource *res)
struct ttm_resource_manager *man =
ttm_manager_type(bo->bdev, res->mem_type);
 
-   if (man->func && man->func->free)
-   man->func->free(man, res);
-
+   man->func->free(man, res);
res->mm_node = NULL;
res->mem_type = TTM_PL_SYSTEM;
 }
@@ -139,7 +134,6 @@ void ttm_resource_manager_debug(struct ttm_resource_manager 
*man,
drm_printf(p, "  use_type: %d\n", man->use_type);
drm_printf(p, "  use_tt: %d\n", man->use_tt);
drm_printf(p, "  size: %llu\n", man->size);
-   if (man->func && man->func->debug)
-   (*man->func->debug)(man, p);
+   man->func->debug(man, p);
 }
 EXPORT_SYMBOL(ttm_resource_manager_debug);
diff --git a/drivers/gpu/drm/ttm/ttm_sys_manager.c 
b/drivers/gpu/drm/ttm/ttm_sys_manager.c
new file mode 100644
index ..32bc16f022f9
--- /dev/null
+++ b/drivers/gpu/drm/ttm/ttm_sys_manager.c
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+
+#include 
+#include 
+#include 
+
+static int ttm_sys_man_alloc(struct ttm_resource_manager *man,
+struct ttm_buffer_object *bo,
+const struct ttm_place *place,
+struct ttm_resource *mem)
+{
+   return 0;
+}
+
+static void ttm_sys_man_free(struct ttm_resource_manager *man,
+struct ttm_resource *mem)
+{
+}
+
+static void ttm_sys_man_debug(struct ttm_resource_manager *man,
+ struct drm_printer *printer)
+{
+}
+
+static const struct ttm_resource_manager_func ttm_sys_manager_func = {
+   .alloc = ttm_sys_man_alloc,
+   .free = ttm_sys_man_free,
+   .debug = ttm_sys_man_debug
+};
+
+int ttm_sys_man_init(struct ttm_device *bdev)
+{
+   struct ttm_resource_manager *man = &b