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

[PATCH 1/5] 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>
---
 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/5] 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>
---
 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 8ca981065e1a..56b7b581567d 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -30,6 +30,15 @@ The Translation Table Manager (TTM)
 
 TTM design background and information belongs here.
 
+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

Re: [PATCH 2/5] drm/ttm: enable TTM device object kerneldoc

[Matthew Auld]

On Fri, 3 Sept 2021 at 13:31, 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>
> ---
>  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 8ca981065e1a..56b7b581567d 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -30,6 +30,15 @@ The Translation Table Manager (TTM)
>
>  TTM design background and information belongs here.
>
> +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.

Would it be reasonable to move the ttm_global_mutex into ttm_global
here? That way everything is nicely grouped together, and we can
easily reference it here with @mutex or so?

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

Re: [PATCH 2/5] drm/ttm: enable TTM device object kerneldoc

[Christian König]

Am 03.09.21 um 15:16 schrieb Matthew Auld:
> On Fri, 3 Sept 2021 at 13:31, 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>
>> ---
>>   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 8ca981065e1a..56b7b581567d 100644
>> --- a/Documentation/gpu/drm-mm.rst
>> +++ b/Documentation/gpu/drm-mm.rst
>> @@ -30,6 +30,15 @@ The Translation Table Manager (TTM)
>>
>>   TTM design background and information belongs here.
>>
>> +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.
> Would it be reasonable to move the ttm_global_mutex into ttm_global
> here? That way everything is nicely grouped together, and we can
> easily reference it here with @mutex or so?

To be honest I'm in the process of decomposing the global structure. 
Those are essentially static information which can be kept inside the 
ttm_device.c file.

The only reason we had it in the first place was because we leaked the 
BO count and device list into other parts of TTM and even the driver.

Regards,
Christian.

>
>>           */
>>          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 3/5] drm/ttm: enable TTM resource object kerneldoc

[Christian König]

Fix the last two 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_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 56b7b581567d..094e367130db 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -39,6 +39,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..255fc8169d9a 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:		What CPU caching should be used
  *
  * Structure indicating the bus placement of an object.
  */
-- 
2.25.1

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

[Matthew Auld]

On Fri, 3 Sept 2021 at 13:31, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Fix the last two 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_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 56b7b581567d..094e367130db 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -39,6 +39,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..255fc8169d9a 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:           What CPU caching should be used

Maybe add "See enum ttm_caching" or something, so it generates a link,
once we also add kernel-doc for that?

>   *
>   * Structure indicating the bus placement of an object.
>   */
> --
> 2.25.1
>

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

[Christian König]

Am 03.09.21 um 15:48 schrieb Matthew Auld:
> On Fri, 3 Sept 2021 at 13:31, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Fix the last two 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_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 56b7b581567d..094e367130db 100644
>> --- a/Documentation/gpu/drm-mm.rst
>> +++ b/Documentation/gpu/drm-mm.rst
>> @@ -39,6 +39,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..255fc8169d9a 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:           What CPU caching should be used
> Maybe add "See enum ttm_caching" or something, so it generates a link,
> once we also add kernel-doc for that?

Good point, going to do that as well.

Thanks,
Christian.

>
>>    *
>>    * Structure indicating the bus placement of an object.
>>    */
>> --
>> 2.25.1
>>

[PATCH 4/5] 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>
---
 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 094e367130db..195a857fa1d8 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -39,6 +39,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 5/5] drm/ttm: enable TTM TT object 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_tt.h     | 9 +++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 195a857fa1d8..aa78e6ffc22e 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -54,6 +54,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..fbc13d69ee73 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -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 1/5] drm/ttm: remove the outdated kerneldoc section

[Matthew Auld]

On Fri, 3 Sept 2021 at 13:31, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Clean up to start over with new and more accurate documentation.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

For the series,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

We could maybe also bring in ttm_pool.[ch]? It looks like it already
has near complete kernel-doc?

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

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

[Christian König]

Am 03.09.21 um 16:22 schrieb Matthew Auld:
> On Fri, 3 Sept 2021 at 13:31, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Clean up to start over with new and more accurate documentation.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
> For the series,
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Thanks.

>
> We could maybe also bring in ttm_pool.[ch]? It looks like it already
> has near complete kernel-doc?

Yes, just didn't had time to cleanup the remaining fallout yet.

The last and most important remaining beast is the BO documentation, but 
that will still take a while.

Regards,
Christian.

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