[PATCH 1/8] drm/ttm: remove the outdated kerneldoc section

[PATCH 1/8] drm/ttm: remove the outdated kerneldoc section

[Christian König]

Clean up to start over with new and more accurate documentation.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 Documentation/gpu/drm-mm.rst | 49 ------------------------------------
 1 file changed, 49 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 0198fa43d254..8ca981065e1a 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -30,55 +30,6 @@ The Translation Table Manager (TTM)
 
 TTM design background and information belongs here.
 
-TTM initialization
-------------------
-
-    **Warning**
-    This section is outdated.
-
-Drivers wishing to support TTM must pass a filled :c:type:`ttm_bo_driver
-<ttm_bo_driver>` structure to ttm_bo_device_init, together with an
-initialized global reference to the memory manager.  The ttm_bo_driver
-structure contains several fields with function pointers for
-initializing the TTM, allocating and freeing memory, waiting for command
-completion and fence synchronization, and memory migration.
-
-The :c:type:`struct drm_global_reference <drm_global_reference>` is made
-up of several fields:
-
-.. code-block:: c
-
-              struct drm_global_reference {
-                      enum ttm_global_types global_type;
-                      size_t size;
-                      void *object;
-                      int (*init) (struct drm_global_reference *);
-                      void (*release) (struct drm_global_reference *);
-              };
-
-
-There should be one global reference structure for your memory manager
-as a whole, and there will be others for each object created by the
-memory manager at runtime. Your global TTM should have a type of
-TTM_GLOBAL_TTM_MEM. The size field for the global object should be
-sizeof(struct ttm_mem_global), and the init and release hooks should
-point at your driver-specific init and release routines, which probably
-eventually call ttm_mem_global_init and ttm_mem_global_release,
-respectively.
-
-Once your global TTM accounting structure is set up and initialized by
-calling ttm_global_item_ref() on it, you need to create a buffer
-object TTM to provide a pool for buffer object allocation by clients and
-the kernel itself. The type of this object should be
-TTM_GLOBAL_TTM_BO, and its size should be sizeof(struct
-ttm_bo_global). Again, driver-specific init and release functions may
-be provided, likely eventually calling ttm_bo_global_ref_init() and
-ttm_bo_global_ref_release(), respectively. Also, like the previous
-object, ttm_global_item_ref() is used to create an initial reference
-count for the TTM, which will call your initialization function.
-
-See the radeon_ttm.c file for an example of usage.
-
 The Graphics Execution Manager (GEM)
 ====================================
 
-- 
2.25.1

[PATCH 2/8] drm/ttm: add some general module kerneldoc

[Christian König]

For now just a brief description of what TTM is all about.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 Documentation/gpu/drm-mm.rst     |  3 ++-
 drivers/gpu/drm/ttm/ttm_module.c | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 8ca981065e1a..6b7717af4f88 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -28,7 +28,8 @@ UMA devices.
 The Translation Table Manager (TTM)
 ===================================
 
-TTM design background and information belongs here.
+.. kernel-doc:: drivers/gpu/drm/ttm/ttm_module.c
+   :doc: TTM
 
 The Graphics Execution Manager (GEM)
 ====================================
diff --git a/drivers/gpu/drm/ttm/ttm_module.c b/drivers/gpu/drm/ttm/ttm_module.c
index 997c458f68a9..11b59cf03ec3 100644
--- a/drivers/gpu/drm/ttm/ttm_module.c
+++ b/drivers/gpu/drm/ttm/ttm_module.c
@@ -39,6 +39,18 @@
 
 #include "ttm_module.h"
 
+/**
+ * DOC: TTM
+ *
+ * TTM is a memory manager for graphics devices with dedicated video memory.
+ *
+ * The basic idea is that resources are grouped together in buffer objects of
+ * certain size and TTM handles lifetime, movement and CPU mappings of those
+ * objects.
+ *
+ * TODO: Add more design background and information here.
+ */
+
 /**
  * ttm_prot_from_caching - Modify the page protection according to the
  * ttm cacing mode
-- 
2.25.1

[PATCH 3/8] drm/ttm: add kerneldoc for enum ttm_caching

[Christian König]

Briefly describe what this is all about.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 Documentation/gpu/drm-mm.rst  |  3 +++
 include/drm/ttm/ttm_caching.h | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 6b7717af4f88..f22c9f9a2c0e 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -31,6 +31,9 @@ The Translation Table Manager (TTM)
 .. kernel-doc:: drivers/gpu/drm/ttm/ttm_module.c
    :doc: TTM
 
+.. kernel-doc:: include/drm/ttm/ttm_caching.h
+   :internal:
+
 The Graphics Execution Manager (GEM)
 ====================================
 
diff --git a/include/drm/ttm/ttm_caching.h b/include/drm/ttm/ttm_caching.h
index 3c9dd65f5aaf..235a743d90e1 100644
--- a/include/drm/ttm/ttm_caching.h
+++ b/include/drm/ttm/ttm_caching.h
@@ -27,9 +27,26 @@
 
 #define TTM_NUM_CACHING_TYPES	3
 
+/**
+ * enum ttm_caching - CPU caching and BUS snooping behavior.
+ */
 enum ttm_caching {
+	/**
+	 * @ttm_uncached: Most defensive option for device mappings,
+	 * don't even allow write combining.
+	 */
 	ttm_uncached,
+
+	/**
+	 * @ttm_write_combined: Don't cache read accesses, but allow at least
+	 * writes to be combined.
+	 */
 	ttm_write_combined,
+
+	/**
+	 * @ttm_cached: Fully cached like normal system memory, requires that
+	 * devices snoop the CPU cache on accesses.
+	 */
 	ttm_cached
 };
 
-- 
2.25.1

[PATCH 4/8] drm/ttm: enable TTM device object kerneldoc

[Christian König]

Fix the remaining warnings, switch to inline structure documentation
and finally enable this.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 Documentation/gpu/drm-mm.rst |  9 +++++
 include/drm/ttm/ttm_device.h | 73 +++++++++++++++++++++---------------
 2 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index f22c9f9a2c0e..3da81b7b4e71 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -34,6 +34,15 @@ The Translation Table Manager (TTM)
 .. kernel-doc:: include/drm/ttm/ttm_caching.h
    :internal:
 
+TTM device object reference
+---------------------------
+
+.. kernel-doc:: include/drm/ttm/ttm_device.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/ttm/ttm_device.c
+   :export:
+
 The Graphics Execution Manager (GEM)
 ====================================
 
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 07d722950d5b..0b31ec731e66 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -39,31 +39,23 @@ struct ttm_operation_ctx;
 
 /**
  * struct ttm_global - Buffer object driver global data.
- *
- * @dummy_read_page: Pointer to a dummy page used for mapping requests
- * of unpopulated pages.
- * @shrink: A shrink callback object used for buffer object swap.
- * @device_list_mutex: Mutex protecting the device list.
- * This mutex is held while traversing the device list for pm options.
- * @lru_lock: Spinlock protecting the bo subsystem lru lists.
- * @device_list: List of buffer object devices.
- * @swap_lru: Lru list of buffer objects used for swapping.
  */
 extern struct ttm_global {
 
 	/**
-	 * Constant after init.
+	 * @dummy_read_page: Pointer to a dummy page used for mapping requests
+	 * of unpopulated pages. Constant after init.
 	 */
-
 	struct page *dummy_read_page;
 
 	/**
-	 * Protected by ttm_global_mutex.
+	 * @device_list: List of buffer object devices. Protected by
+	 * ttm_global_mutex.
 	 */
 	struct list_head device_list;
 
 	/**
-	 * Internal protection.
+	 * @bo_count: Number of buffer objects allocated by devices.
 	 */
 	atomic_t bo_count;
 } ttm_glob;
@@ -230,50 +222,69 @@ struct ttm_device_funcs {
 
 /**
  * struct ttm_device - Buffer object driver device-specific data.
- *
- * @device_list: Our entry in the global device list.
- * @funcs: Function table for the device.
- * @sysman: Resource manager for the system domain.
- * @man_drv: An array of resource_managers.
- * @vma_manager: Address space manager.
- * @pool: page pool for the device.
- * @dev_mapping: A pointer to the struct address_space representing the
- * device address space.
- * @wq: Work queue structure for the delayed delete workqueue.
  */
 struct ttm_device {
-	/*
+	/**
+	 * @device_list: Our entry in the global device list.
 	 * Constant after bo device init
 	 */
 	struct list_head device_list;
+
+	/**
+	 * @funcs: Function table for the device.
+	 * Constant after bo device init
+	 */
 	struct ttm_device_funcs *funcs;
 
-	/*
+	/**
+	 * @sysman: Resource manager for the system domain.
 	 * Access via ttm_manager_type.
 	 */
 	struct ttm_resource_manager sysman;
+
+	/**
+	 * @man_drv: An array of resource_managers.
+	 */
 	struct ttm_resource_manager *man_drv[TTM_NUM_MEM_TYPES];
 
 	/*
 	 * Protected by internal locks.
 	 */
+
+	/**
+	 * @vma_manager: Address space manager for finding BOs to mmap.
+	 */
 	struct drm_vma_offset_manager *vma_manager;
+
+	/**
+	 * @pool: page pool for the device.
+	 */
 	struct ttm_pool pool;
 
-	/*
-	 * Protection for the per manager LRU and ddestroy lists.
+	/**
+	 * @lru_lock: Protection for the per manager LRU and ddestroy lists.
 	 */
 	spinlock_t lru_lock;
+
+	/**
+	 * @ddestroy: Destroyed but not yet cleaned up buffer objects.
+	 */
 	struct list_head ddestroy;
+
+	/**
+	 * @pinned: Buffer object which are pinned and so not on any LRU list.
+	 */
 	struct list_head pinned;
 
-	/*
-	 * Protected by load / firstopen / lastclose /unload sync.
+	/**
+	 * @dev_mapping: A pointer to the struct address_space for invalidating
+	 * CPU mappings on buffer move. Protected by load/unload sync.
 	 */
 	struct address_space *dev_mapping;
 
-	/*
-	 * Internal protection.
+	/**
+	 * @wq: Work queue structure for the delayed delete workqueue. Has
+	 * internal protection.
 	 */
 	struct delayed_work wq;
 };
-- 
2.25.1

[PATCH 5/8] drm/ttm: enable TTM resource object kerneldoc v2

[Christian König]

Fix the last two remaining warnings and finally enable this.

v2: add caching enum link

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 Documentation/gpu/drm-mm.rst   | 9 +++++++++
 include/drm/ttm/ttm_resource.h | 6 ++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 3da81b7b4e71..66d24b745c62 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -43,6 +43,15 @@ TTM device object reference
 .. kernel-doc:: drivers/gpu/drm/ttm/ttm_device.c
    :export:
 
+TTM resource object reference
+-----------------------------
+
+.. kernel-doc:: include/drm/ttm/ttm_resource.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/ttm/ttm_resource.c
+   :export:
+
 The Graphics Execution Manager (GEM)
 ====================================
 
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 32c5edd9e8b5..5952051091cd 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -103,10 +103,7 @@ struct ttm_resource_manager_func {
  * struct ttm_resource_manager
  *
  * @use_type: The memory type is enabled.
- * @flags: TTM_MEMTYPE_XX flags identifying the traits of the memory
- * managed by this memory type.
- * @gpu_offset: If used, the GPU offset of the first managed page of
- * fixed memory or the first managed location in an aperture.
+ * @use_tt: If a TT object should be used for the backing store.
  * @size: Size of the managed region.
  * @func: structure pointer implementing the range manager. See above
  * @move_lock: lock for move fence
@@ -144,6 +141,7 @@ struct ttm_resource_manager {
  * @addr:		mapped virtual address
  * @offset:		physical addr
  * @is_iomem:		is this io memory ?
+ * @caching:		See enum ttm_caching
  *
  * Structure indicating the bus placement of an object.
  */
-- 
2.25.1

[PATCH 6/8] drm/ttm: enable TTM placement kerneldoc

[Christian König]

Fix the last remaining warning and finally enable this.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 Documentation/gpu/drm-mm.rst    | 6 ++++++
 include/drm/ttm/ttm_placement.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 66d24b745c62..1c9930fb5e7d 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -43,6 +43,12 @@ TTM device object reference
 .. kernel-doc:: drivers/gpu/drm/ttm/ttm_device.c
    :export:
 
+TTM resource placement reference
+--------------------------------
+
+.. kernel-doc:: include/drm/ttm/ttm_placement.h
+   :internal:
+
 TTM resource object reference
 -----------------------------
 
diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
index 8995c9e4ec1b..76d1b9119a2b 100644
--- a/include/drm/ttm/ttm_placement.h
+++ b/include/drm/ttm/ttm_placement.h
@@ -58,6 +58,7 @@
  *
  * @fpfn:	first valid page frame number to put the object
  * @lpfn:	last valid page frame number to put the object
+ * @mem_type:	One of TTM_PL_* where the resource should be allocated from.
  * @flags:	memory domain and caching flags for the object
  *
  * Structure indicating a possible place to put an object.
-- 
2.25.1

[PATCH 7/8] drm/ttm: enable TTM TT object kerneldoc v2

[Christian König]

Fix the remaining warnings and finally enable this.

v2: add caching enum link

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 Documentation/gpu/drm-mm.rst |  9 +++++++++
 include/drm/ttm/ttm_tt.h     | 11 ++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 1c9930fb5e7d..69c4a20b95d0 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -58,6 +58,15 @@ TTM resource object reference
 .. kernel-doc:: drivers/gpu/drm/ttm/ttm_resource.c
    :export:
 
+TTM TT object reference
+-----------------------
+
+.. kernel-doc:: include/drm/ttm/ttm_tt.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/ttm/ttm_tt.c
+   :export:
+
 The Graphics Execution Manager (GEM)
 ====================================
 
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
index e402dab1d0f6..b3963ab12e1f 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -54,7 +54,7 @@ struct ttm_operation_ctx;
  * @dma_address: The DMA (bus) addresses of the pages
  * @swap_storage: Pointer to shmem struct file for swap storage.
  * @pages_list: used by some page allocation backend
- * @caching: The current caching state of the pages.
+ * @caching: The current caching state of the pages, see enum ttm_caching.
  *
  * This is a structure holding the pages, caching- and aperture binding
  * status for a buffer object that isn't backed by fixed (VRAM / AGP)
@@ -126,8 +126,9 @@ int ttm_sg_tt_init(struct ttm_tt *ttm_dma, struct ttm_buffer_object *bo,
 void ttm_tt_fini(struct ttm_tt *ttm);
 
 /**
- * ttm_ttm_destroy:
+ * ttm_tt_destroy:
  *
+ * @bdev: the ttm_device this object belongs to
  * @ttm: The struct ttm_tt.
  *
  * Unbind, unpopulate and destroy common struct ttm_tt.
@@ -148,15 +149,19 @@ int ttm_tt_swapout(struct ttm_device *bdev, struct ttm_tt *ttm,
 /**
  * ttm_tt_populate - allocate pages for a ttm
  *
+ * @bdev: the ttm_device this object belongs to
  * @ttm: Pointer to the ttm_tt structure
+ * @ctx: operation context for populating the tt object.
  *
  * Calls the driver method to allocate pages for a ttm
  */
-int ttm_tt_populate(struct ttm_device *bdev, struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
+int ttm_tt_populate(struct ttm_device *bdev, struct ttm_tt *ttm,
+		    struct ttm_operation_ctx *ctx);
 
 /**
  * ttm_tt_unpopulate - free pages from a ttm
  *
+ * @bdev: the ttm_device this object belongs to
  * @ttm: Pointer to the ttm_tt structure
  *
  * Calls the driver method to free all pages from a ttm
-- 
2.25.1

[PATCH 8/8] drm/ttm: enable TTM page pool kerneldoc

[Christian König]

Fix the remaining warnings and finally enable this.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 Documentation/gpu/drm-mm.rst | 9 +++++++++
 include/drm/ttm/ttm_pool.h   | 5 +++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 69c4a20b95d0..e0538083a2c0 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -67,6 +67,15 @@ TTM TT object reference
 .. kernel-doc:: drivers/gpu/drm/ttm/ttm_tt.c
    :export:
 
+TTM page pool reference
+-----------------------
+
+.. kernel-doc:: include/drm/ttm/ttm_pool.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/ttm/ttm_pool.c
+   :export:
+
 The Graphics Execution Manager (GEM)
 ====================================
 
diff --git a/include/drm/ttm/ttm_pool.h b/include/drm/ttm/ttm_pool.h
index 4321728bdd11..ef09b23d29e3 100644
--- a/include/drm/ttm/ttm_pool.h
+++ b/include/drm/ttm/ttm_pool.h
@@ -37,7 +37,7 @@ struct ttm_pool;
 struct ttm_operation_ctx;
 
 /**
- * ttm_pool_type - Pool for a certain memory type
+ * struct ttm_pool_type - Pool for a certain memory type
  *
  * @pool: the pool we belong to, might be NULL for the global ones
  * @order: the allocation order our pages have
@@ -58,8 +58,9 @@ struct ttm_pool_type {
 };
 
 /**
- * ttm_pool - Pool for all caching and orders
+ * struct ttm_pool - Pool for all caching and orders
  *
+ * @dev: the device we allocate pages for
  * @use_dma_alloc: if coherent DMA allocations should be used
  * @use_dma32: if GFP_DMA32 should be used
  * @caching: pools for each caching/order
-- 
2.25.1

Re: [PATCH 1/8] drm/ttm: remove the outdated kerneldoc section

[Daniel Vetter]

On Tue, Sep 07, 2021 at 10:01:28AM +0200, Christian König wrote:
> Clean up to start over with new and more accurate documentation.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Burried in mails, but thanks a lot for kicking this can so we can start
the journey!
-Daniel

> ---
>  Documentation/gpu/drm-mm.rst | 49 ------------------------------------
>  1 file changed, 49 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index 0198fa43d254..8ca981065e1a 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -30,55 +30,6 @@ The Translation Table Manager (TTM)
>  
>  TTM design background and information belongs here.
>  
> -TTM initialization
> -------------------
> -
> -    **Warning**
> -    This section is outdated.
> -
> -Drivers wishing to support TTM must pass a filled :c:type:`ttm_bo_driver
> -<ttm_bo_driver>` structure to ttm_bo_device_init, together with an
> -initialized global reference to the memory manager.  The ttm_bo_driver
> -structure contains several fields with function pointers for
> -initializing the TTM, allocating and freeing memory, waiting for command
> -completion and fence synchronization, and memory migration.
> -
> -The :c:type:`struct drm_global_reference <drm_global_reference>` is made
> -up of several fields:
> -
> -.. code-block:: c
> -
> -              struct drm_global_reference {
> -                      enum ttm_global_types global_type;
> -                      size_t size;
> -                      void *object;
> -                      int (*init) (struct drm_global_reference *);
> -                      void (*release) (struct drm_global_reference *);
> -              };
> -
> -
> -There should be one global reference structure for your memory manager
> -as a whole, and there will be others for each object created by the
> -memory manager at runtime. Your global TTM should have a type of
> -TTM_GLOBAL_TTM_MEM. The size field for the global object should be
> -sizeof(struct ttm_mem_global), and the init and release hooks should
> -point at your driver-specific init and release routines, which probably
> -eventually call ttm_mem_global_init and ttm_mem_global_release,
> -respectively.
> -
> -Once your global TTM accounting structure is set up and initialized by
> -calling ttm_global_item_ref() on it, you need to create a buffer
> -object TTM to provide a pool for buffer object allocation by clients and
> -the kernel itself. The type of this object should be
> -TTM_GLOBAL_TTM_BO, and its size should be sizeof(struct
> -ttm_bo_global). Again, driver-specific init and release functions may
> -be provided, likely eventually calling ttm_bo_global_ref_init() and
> -ttm_bo_global_ref_release(), respectively. Also, like the previous
> -object, ttm_global_item_ref() is used to create an initial reference
> -count for the TTM, which will call your initialization function.
> -
> -See the radeon_ttm.c file for an example of usage.
> -
>  The Graphics Execution Manager (GEM)
>  ====================================
>  
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/8] drm/ttm: add some general module kerneldoc

[Alex Deucher]

On Tue, Sep 7, 2021 at 4:01 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> For now just a brief description of what TTM is all about.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  Documentation/gpu/drm-mm.rst     |  3 ++-
>  drivers/gpu/drm/ttm/ttm_module.c | 12 ++++++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index 8ca981065e1a..6b7717af4f88 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -28,7 +28,8 @@ UMA devices.
>  The Translation Table Manager (TTM)
>  ===================================
>
> -TTM design background and information belongs here.
> +.. kernel-doc:: drivers/gpu/drm/ttm/ttm_module.c
> +   :doc: TTM
>
>  The Graphics Execution Manager (GEM)
>  ====================================
> diff --git a/drivers/gpu/drm/ttm/ttm_module.c b/drivers/gpu/drm/ttm/ttm_module.c
> index 997c458f68a9..11b59cf03ec3 100644
> --- a/drivers/gpu/drm/ttm/ttm_module.c
> +++ b/drivers/gpu/drm/ttm/ttm_module.c
> @@ -39,6 +39,18 @@
>
>  #include "ttm_module.h"
>
> +/**
> + * DOC: TTM
> + *
> + * TTM is a memory manager for graphics devices with dedicated video memory.

Maybe say "accelerator devices with dedicated memory" since this could
just as easily be applicable to compute only devices.

Alex


> + *
> + * The basic idea is that resources are grouped together in buffer objects of
> + * certain size and TTM handles lifetime, movement and CPU mappings of those
> + * objects.
> + *
> + * TODO: Add more design background and information here.
> + */
> +
>  /**
>   * ttm_prot_from_caching - Modify the page protection according to the
>   * ttm cacing mode
> --
> 2.25.1
>

Re: [PATCH 3/8] drm/ttm: add kerneldoc for enum ttm_caching

[Alex Deucher]

On Tue, Sep 7, 2021 at 4:01 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Briefly describe what this is all about.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  Documentation/gpu/drm-mm.rst  |  3 +++
>  include/drm/ttm/ttm_caching.h | 17 +++++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index 6b7717af4f88..f22c9f9a2c0e 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -31,6 +31,9 @@ The Translation Table Manager (TTM)
>  .. kernel-doc:: drivers/gpu/drm/ttm/ttm_module.c
>     :doc: TTM
>
> +.. kernel-doc:: include/drm/ttm/ttm_caching.h
> +   :internal:
> +
>  The Graphics Execution Manager (GEM)
>  ====================================
>
> diff --git a/include/drm/ttm/ttm_caching.h b/include/drm/ttm/ttm_caching.h
> index 3c9dd65f5aaf..235a743d90e1 100644
> --- a/include/drm/ttm/ttm_caching.h
> +++ b/include/drm/ttm/ttm_caching.h
> @@ -27,9 +27,26 @@
>
>  #define TTM_NUM_CACHING_TYPES  3
>
> +/**
> + * enum ttm_caching - CPU caching and BUS snooping behavior.
> + */
>  enum ttm_caching {
> +       /**
> +        * @ttm_uncached: Most defensive option for device mappings,
> +        * don't even allow write combining.
> +        */
>         ttm_uncached,
> +
> +       /**
> +        * @ttm_write_combined: Don't cache read accesses, but allow at least
> +        * writes to be combined.
> +        */
>         ttm_write_combined,
> +
> +       /**
> +        * @ttm_cached: Fully cached like normal system memory, requires that
> +        * devices snoop the CPU cache on accesses.
> +        */
>         ttm_cached
>  };
>
> --
> 2.25.1
>

Re: [PATCH 4/8] drm/ttm: enable TTM device object kerneldoc

[Alex Deucher]

On Tue, Sep 7, 2021 at 4:01 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Fix the remaining warnings, switch to inline structure documentation
> and finally enable this.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> ---
>  Documentation/gpu/drm-mm.rst |  9 +++++
>  include/drm/ttm/ttm_device.h | 73 +++++++++++++++++++++---------------
>  2 files changed, 51 insertions(+), 31 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index f22c9f9a2c0e..3da81b7b4e71 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -34,6 +34,15 @@ The Translation Table Manager (TTM)
>  .. kernel-doc:: include/drm/ttm/ttm_caching.h
>     :internal:
>
> +TTM device object reference
> +---------------------------
> +
> +.. kernel-doc:: include/drm/ttm/ttm_device.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/ttm/ttm_device.c
> +   :export:
> +
>  The Graphics Execution Manager (GEM)
>  ====================================
>
> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
> index 07d722950d5b..0b31ec731e66 100644
> --- a/include/drm/ttm/ttm_device.h
> +++ b/include/drm/ttm/ttm_device.h
> @@ -39,31 +39,23 @@ struct ttm_operation_ctx;
>
>  /**
>   * struct ttm_global - Buffer object driver global data.
> - *
> - * @dummy_read_page: Pointer to a dummy page used for mapping requests
> - * of unpopulated pages.
> - * @shrink: A shrink callback object used for buffer object swap.
> - * @device_list_mutex: Mutex protecting the device list.
> - * This mutex is held while traversing the device list for pm options.
> - * @lru_lock: Spinlock protecting the bo subsystem lru lists.
> - * @device_list: List of buffer object devices.
> - * @swap_lru: Lru list of buffer objects used for swapping.
>   */
>  extern struct ttm_global {
>
>         /**
> -        * Constant after init.
> +        * @dummy_read_page: Pointer to a dummy page used for mapping requests
> +        * of unpopulated pages. Constant after init.
>          */
> -
>         struct page *dummy_read_page;
>
>         /**
> -        * Protected by ttm_global_mutex.
> +        * @device_list: List of buffer object devices. Protected by
> +        * ttm_global_mutex.
>          */
>         struct list_head device_list;
>
>         /**
> -        * Internal protection.
> +        * @bo_count: Number of buffer objects allocated by devices.
>          */
>         atomic_t bo_count;
>  } ttm_glob;
> @@ -230,50 +222,69 @@ struct ttm_device_funcs {
>
>  /**
>   * struct ttm_device - Buffer object driver device-specific data.
> - *
> - * @device_list: Our entry in the global device list.
> - * @funcs: Function table for the device.
> - * @sysman: Resource manager for the system domain.
> - * @man_drv: An array of resource_managers.
> - * @vma_manager: Address space manager.
> - * @pool: page pool for the device.
> - * @dev_mapping: A pointer to the struct address_space representing the
> - * device address space.
> - * @wq: Work queue structure for the delayed delete workqueue.
>   */
>  struct ttm_device {
> -       /*
> +       /**
> +        * @device_list: Our entry in the global device list.
>          * Constant after bo device init
>          */
>         struct list_head device_list;
> +
> +       /**
> +        * @funcs: Function table for the device.
> +        * Constant after bo device init
> +        */
>         struct ttm_device_funcs *funcs;
>
> -       /*
> +       /**
> +        * @sysman: Resource manager for the system domain.
>          * Access via ttm_manager_type.
>          */
>         struct ttm_resource_manager sysman;
> +
> +       /**
> +        * @man_drv: An array of resource_managers.

Might want to provide some detail as to why there are multiple
resources managers.  E.g., one per pool.

> +        */
>         struct ttm_resource_manager *man_drv[TTM_NUM_MEM_TYPES];
>
>         /*
>          * Protected by internal locks.
>          */
> +
> +       /**
> +        * @vma_manager: Address space manager for finding BOs to mmap.
> +        */
>         struct drm_vma_offset_manager *vma_manager;
> +
> +       /**
> +        * @pool: page pool for the device.
> +        */
>         struct ttm_pool pool;
>
> -       /*
> -        * Protection for the per manager LRU and ddestroy lists.
> +       /**
> +        * @lru_lock: Protection for the per manager LRU and ddestroy lists.
>          */
>         spinlock_t lru_lock;
> +
> +       /**
> +        * @ddestroy: Destroyed but not yet cleaned up buffer objects.
> +        */
>         struct list_head ddestroy;
> +
> +       /**
> +        * @pinned: Buffer object which are pinned and so not on any LRU list.

objects

> +        */
>         struct list_head pinned;
>
> -       /*
> -        * Protected by load / firstopen / lastclose /unload sync.
> +       /**
> +        * @dev_mapping: A pointer to the struct address_space for invalidating
> +        * CPU mappings on buffer move. Protected by load/unload sync.
>          */
>         struct address_space *dev_mapping;
>
> -       /*
> -        * Internal protection.
> +       /**
> +        * @wq: Work queue structure for the delayed delete workqueue. Has
> +        * internal protection.

What does internal protection imply here?

>          */
>         struct delayed_work wq;
>  };
> --
> 2.25.1
>

Re: [PATCH 6/8] drm/ttm: enable TTM placement kerneldoc

[Alex Deucher]

On Tue, Sep 7, 2021 at 4:01 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Fix the last remaining warning and finally enable this.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  Documentation/gpu/drm-mm.rst    | 6 ++++++
>  include/drm/ttm/ttm_placement.h | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index 66d24b745c62..1c9930fb5e7d 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -43,6 +43,12 @@ TTM device object reference
>  .. kernel-doc:: drivers/gpu/drm/ttm/ttm_device.c
>     :export:
>
> +TTM resource placement reference
> +--------------------------------
> +
> +.. kernel-doc:: include/drm/ttm/ttm_placement.h
> +   :internal:
> +
>  TTM resource object reference
>  -----------------------------
>
> diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
> index 8995c9e4ec1b..76d1b9119a2b 100644
> --- a/include/drm/ttm/ttm_placement.h
> +++ b/include/drm/ttm/ttm_placement.h
> @@ -58,6 +58,7 @@
>   *
>   * @fpfn:      first valid page frame number to put the object
>   * @lpfn:      last valid page frame number to put the object
> + * @mem_type:  One of TTM_PL_* where the resource should be allocated from.
>   * @flags:     memory domain and caching flags for the object
>   *
>   * Structure indicating a possible place to put an object.
> --
> 2.25.1
>

Re: [PATCH 5/8] drm/ttm: enable TTM resource object kerneldoc v2

[Alex Deucher]

On Tue, Sep 7, 2021 at 4:01 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Fix the last two remaining warnings and finally enable this.
>
> v2: add caching enum link
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  Documentation/gpu/drm-mm.rst   | 9 +++++++++
>  include/drm/ttm/ttm_resource.h | 6 ++----
>  2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index 3da81b7b4e71..66d24b745c62 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -43,6 +43,15 @@ TTM device object reference
>  .. kernel-doc:: drivers/gpu/drm/ttm/ttm_device.c
>     :export:
>
> +TTM resource object reference
> +-----------------------------
> +
> +.. kernel-doc:: include/drm/ttm/ttm_resource.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/ttm/ttm_resource.c
> +   :export:
> +
>  The Graphics Execution Manager (GEM)
>  ====================================
>
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 32c5edd9e8b5..5952051091cd 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -103,10 +103,7 @@ struct ttm_resource_manager_func {
>   * struct ttm_resource_manager
>   *
>   * @use_type: The memory type is enabled.
> - * @flags: TTM_MEMTYPE_XX flags identifying the traits of the memory
> - * managed by this memory type.
> - * @gpu_offset: If used, the GPU offset of the first managed page of
> - * fixed memory or the first managed location in an aperture.
> + * @use_tt: If a TT object should be used for the backing store.
>   * @size: Size of the managed region.
>   * @func: structure pointer implementing the range manager. See above
>   * @move_lock: lock for move fence
> @@ -144,6 +141,7 @@ struct ttm_resource_manager {
>   * @addr:              mapped virtual address
>   * @offset:            physical addr
>   * @is_iomem:          is this io memory ?
> + * @caching:           See enum ttm_caching
>   *
>   * Structure indicating the bus placement of an object.
>   */
> --
> 2.25.1
>

Re: [PATCH 7/8] drm/ttm: enable TTM TT object kerneldoc v2

[Alex Deucher]

On Tue, Sep 7, 2021 at 4:01 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Fix the remaining warnings and finally enable this.
>
> v2: add caching enum link
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  Documentation/gpu/drm-mm.rst |  9 +++++++++
>  include/drm/ttm/ttm_tt.h     | 11 ++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index 1c9930fb5e7d..69c4a20b95d0 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -58,6 +58,15 @@ TTM resource object reference
>  .. kernel-doc:: drivers/gpu/drm/ttm/ttm_resource.c
>     :export:
>
> +TTM TT object reference
> +-----------------------
> +
> +.. kernel-doc:: include/drm/ttm/ttm_tt.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/ttm/ttm_tt.c
> +   :export:
> +
>  The Graphics Execution Manager (GEM)
>  ====================================
>
> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
> index e402dab1d0f6..b3963ab12e1f 100644
> --- a/include/drm/ttm/ttm_tt.h
> +++ b/include/drm/ttm/ttm_tt.h
> @@ -54,7 +54,7 @@ struct ttm_operation_ctx;
>   * @dma_address: The DMA (bus) addresses of the pages
>   * @swap_storage: Pointer to shmem struct file for swap storage.
>   * @pages_list: used by some page allocation backend
> - * @caching: The current caching state of the pages.
> + * @caching: The current caching state of the pages, see enum ttm_caching.
>   *
>   * This is a structure holding the pages, caching- and aperture binding
>   * status for a buffer object that isn't backed by fixed (VRAM / AGP)
> @@ -126,8 +126,9 @@ int ttm_sg_tt_init(struct ttm_tt *ttm_dma, struct ttm_buffer_object *bo,
>  void ttm_tt_fini(struct ttm_tt *ttm);
>
>  /**
> - * ttm_ttm_destroy:
> + * ttm_tt_destroy:
>   *
> + * @bdev: the ttm_device this object belongs to
>   * @ttm: The struct ttm_tt.
>   *
>   * Unbind, unpopulate and destroy common struct ttm_tt.
> @@ -148,15 +149,19 @@ int ttm_tt_swapout(struct ttm_device *bdev, struct ttm_tt *ttm,
>  /**
>   * ttm_tt_populate - allocate pages for a ttm
>   *
> + * @bdev: the ttm_device this object belongs to
>   * @ttm: Pointer to the ttm_tt structure
> + * @ctx: operation context for populating the tt object.
>   *
>   * Calls the driver method to allocate pages for a ttm
>   */
> -int ttm_tt_populate(struct ttm_device *bdev, struct ttm_tt *ttm, struct ttm_operation_ctx *ctx);
> +int ttm_tt_populate(struct ttm_device *bdev, struct ttm_tt *ttm,
> +                   struct ttm_operation_ctx *ctx);
>
>  /**
>   * ttm_tt_unpopulate - free pages from a ttm
>   *
> + * @bdev: the ttm_device this object belongs to
>   * @ttm: Pointer to the ttm_tt structure
>   *
>   * Calls the driver method to free all pages from a ttm
> --
> 2.25.1
>

Re: [PATCH 8/8] drm/ttm: enable TTM page pool kerneldoc

[Alex Deucher]

On Tue, Sep 7, 2021 at 4:01 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Fix the remaining warnings and finally enable this.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  Documentation/gpu/drm-mm.rst | 9 +++++++++
>  include/drm/ttm/ttm_pool.h   | 5 +++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index 69c4a20b95d0..e0538083a2c0 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -67,6 +67,15 @@ TTM TT object reference
>  .. kernel-doc:: drivers/gpu/drm/ttm/ttm_tt.c
>     :export:
>
> +TTM page pool reference
> +-----------------------
> +
> +.. kernel-doc:: include/drm/ttm/ttm_pool.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/ttm/ttm_pool.c
> +   :export:
> +
>  The Graphics Execution Manager (GEM)
>  ====================================
>
> diff --git a/include/drm/ttm/ttm_pool.h b/include/drm/ttm/ttm_pool.h
> index 4321728bdd11..ef09b23d29e3 100644
> --- a/include/drm/ttm/ttm_pool.h
> +++ b/include/drm/ttm/ttm_pool.h
> @@ -37,7 +37,7 @@ struct ttm_pool;
>  struct ttm_operation_ctx;
>
>  /**
> - * ttm_pool_type - Pool for a certain memory type
> + * struct ttm_pool_type - Pool for a certain memory type
>   *
>   * @pool: the pool we belong to, might be NULL for the global ones
>   * @order: the allocation order our pages have
> @@ -58,8 +58,9 @@ struct ttm_pool_type {
>  };
>
>  /**
> - * ttm_pool - Pool for all caching and orders
> + * struct ttm_pool - Pool for all caching and orders
>   *
> + * @dev: the device we allocate pages for
>   * @use_dma_alloc: if coherent DMA allocations should be used
>   * @use_dma32: if GFP_DMA32 should be used
>   * @caching: pools for each caching/order
> --
> 2.25.1
>

[PATCH 5/8] drm/ttm: enable TTM resource object kerneldoc v2

[Christian König]

Fix the last two remaining warnings and finally enable this.

v2: add caching enum link

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 Documentation/gpu/drm-mm.rst   | 9 +++++++++
 include/drm/ttm/ttm_resource.h | 6 ++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 3da81b7b4e71..66d24b745c62 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -43,6 +43,15 @@ TTM device object reference
 .. kernel-doc:: drivers/gpu/drm/ttm/ttm_device.c
    :export:
 
+TTM resource object reference
+-----------------------------
+
+.. kernel-doc:: include/drm/ttm/ttm_resource.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/ttm/ttm_resource.c
+   :export:
+
 The Graphics Execution Manager (GEM)
 ====================================
 
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 32c5edd9e8b5..5952051091cd 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -103,10 +103,7 @@ struct ttm_resource_manager_func {
  * struct ttm_resource_manager
  *
  * @use_type: The memory type is enabled.
- * @flags: TTM_MEMTYPE_XX flags identifying the traits of the memory
- * managed by this memory type.
- * @gpu_offset: If used, the GPU offset of the first managed page of
- * fixed memory or the first managed location in an aperture.
+ * @use_tt: If a TT object should be used for the backing store.
  * @size: Size of the managed region.
  * @func: structure pointer implementing the range manager. See above
  * @move_lock: lock for move fence
@@ -144,6 +141,7 @@ struct ttm_resource_manager {
  * @addr:		mapped virtual address
  * @offset:		physical addr
  * @is_iomem:		is this io memory ?
+ * @caching:		See enum ttm_caching
  *
  * Structure indicating the bus placement of an object.
  */
-- 
2.25.1