Re: [PATCH v2] Update function level documentation for GPUVM v2
Thanks guys, note taken. Andrey On 06/13/2018 10:01 AM, Christian König wrote: Am 13.06.2018 um 15:57 schrieb Michel Dänzer: On 2018-06-13 03:52 PM, Andrey Grodzovsky wrote: Yea, will take care of this, this particular warning is all over the place when you compile so I didn't notice my own among others. To avoid that problem, I recommend generating documentation first without one's patches applied, then (without cleaning the generated documentation first) again with them applied. The second run should only re-generate things affected by one's patches, so it should be easy to spot any warnings related to them. You can also just issue an "touch $file" command to force only regeneration the file you're interested in. Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] Update function level documentation for GPUVM v2
Am 13.06.2018 um 15:57 schrieb Michel Dänzer: On 2018-06-13 03:52 PM, Andrey Grodzovsky wrote: Yea, will take care of this, this particular warning is all over the place when you compile so I didn't notice my own among others. To avoid that problem, I recommend generating documentation first without one's patches applied, then (without cleaning the generated documentation first) again with them applied. The second run should only re-generate things affected by one's patches, so it should be easy to spot any warnings related to them. You can also just issue an "touch $file" command to force only regeneration the file you're interested in. Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] Update function level documentation for GPUVM v2
Hi Andrey, On 2018-06-12 04:05 PM, Andrey Grodzovsky wrote: > Add/update function level documentation and add reference to amdgpu_vm.c > in amdgpu.rst > > v2: > Fix reference in rst file. > Fix compilation warnings. > Add space between function names and params list where > it's missing. > > Signed-off-by: Andrey Grodzovsky The warnings below now appear while generating documentation, can you fix these up? ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:359: warning: Function parameter or member 'pte_support_ats' not described in 'amdgpu_vm_clear_bo' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:697: warning: Function parameter or member 'job' not described in 'amdgpu_vm_flush' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1781: warning: Function parameter or member '_cb' not described in 'amdgpu_vm_prt_cb' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2070: warning: Function parameter or member 'size' not described in 'amdgpu_vm_bo_map' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2134: warning: Function parameter or member 'size' not described in 'amdgpu_vm_bo_replace_map' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2347: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_bo_lookup_mapping' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2401: warning: Function parameter or member 'evicted' not described in 'amdgpu_vm_bo_invalidate' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2465: warning: Function parameter or member 'fragment_size_default' not described in 'amdgpu_vm_adjust_size' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2465: warning: Function parameter or member 'max_level' not described in 'amdgpu_vm_adjust_size' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2465: warning: Function parameter or member 'max_bits' not described in 'amdgpu_vm_adjust_size' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2536: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_init' -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] Update function level documentation for GPUVM v2
On 2018-06-13 03:52 PM, Andrey Grodzovsky wrote: > Yea, will take care of this, this particular warning is all over the > place when you compile so I didn't notice my own among others. To avoid that problem, I recommend generating documentation first without one's patches applied, then (without cleaning the generated documentation first) again with them applied. The second run should only re-generate things affected by one's patches, so it should be easy to spot any warnings related to them. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] Update function level documentation for GPUVM v2
Yea, will take care of this, this particular warning is all over the place when you compile so I didn't notice my own among others. Andrey On 06/13/2018 09:48 AM, Michel Dänzer wrote: Hi Andrey, On 2018-06-12 04:05 PM, Andrey Grodzovsky wrote: Add/update function level documentation and add reference to amdgpu_vm.c in amdgpu.rst v2: Fix reference in rst file. Fix compilation warnings. Add space between function names and params list where it's missing. Signed-off-by: Andrey Grodzovsky The warnings below now appear while generating documentation, can you fix these up? ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:359: warning: Function parameter or member 'pte_support_ats' not described in 'amdgpu_vm_clear_bo' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:697: warning: Function parameter or member 'job' not described in 'amdgpu_vm_flush' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1781: warning: Function parameter or member '_cb' not described in 'amdgpu_vm_prt_cb' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2070: warning: Function parameter or member 'size' not described in 'amdgpu_vm_bo_map' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2134: warning: Function parameter or member 'size' not described in 'amdgpu_vm_bo_replace_map' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2347: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_bo_lookup_mapping' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2401: warning: Function parameter or member 'evicted' not described in 'amdgpu_vm_bo_invalidate' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2465: warning: Function parameter or member 'fragment_size_default' not described in 'amdgpu_vm_adjust_size' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2465: warning: Function parameter or member 'max_level' not described in 'amdgpu_vm_adjust_size' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2465: warning: Function parameter or member 'max_bits' not described in 'amdgpu_vm_adjust_size' ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2536: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_init' ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] Update function level documentation for GPUVM v2
There are multiple occasions of "adev->gmc.visible_vram_size < adev->gmc.real_vram_size" in amdgpu_cs.c. It's basically the same check as in amdgpu_vm_is_large_bar(). I suggest to name it something like amdgpu_gmc_vram_full_visible() or similar since the BAR actually doesn't needs to be large for that. Thanks, Christian. Am 12.06.2018 um 17:06 schrieb Andrey Grodzovsky: I didn't find that check in amdgpu_cs.c, can you clarify please ? Andrey On 06/12/2018 10:41 AM, Christian König wrote: Unrelated to this patch, but we should probably move that function into amdgpu_gmc.h. There are a couple of more occasions of that check waiting for cleanup in amdgpu_cs.c. Can you take care of cleaning that up as well? Thanks in advance. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] Update function level documentation for GPUVM v2
I didn't find that check in amdgpu_cs.c, can you clarify please ? Andrey On 06/12/2018 10:41 AM, Christian König wrote: Unrelated to this patch, but we should probably move that function into amdgpu_gmc.h. There are a couple of more occasions of that check waiting for cleanup in amdgpu_cs.c. Can you take care of cleaning that up as well? Thanks in advance. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] Update function level documentation for GPUVM v2
Please update the subject line and add a "drm/amdgpu: " prefix.
Am 12.06.2018 um 16:05 schrieb Andrey Grodzovsky:
Add/update function level documentation and add reference to amdgpu_vm.c
in amdgpu.rst
v2:
Fix reference in rst file.
Fix compilation warnings.
Add space between function names and params list where
it's missing.
Signed-off-by: Andrey Grodzovsky
---
Documentation/gpu/amdgpu.rst | 9 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 199 -
2 files changed, 179 insertions(+), 29 deletions(-)
diff --git a/Documentation/gpu/amdgpu.rst b/Documentation/gpu/amdgpu.rst
index a4852f9..3ffb2a2 100644
--- a/Documentation/gpu/amdgpu.rst
+++ b/Documentation/gpu/amdgpu.rst
@@ -44,3 +44,12 @@ MMU Notifier
.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
:internal:
+
+AMDGPU Virtual Memory
+-
+
+.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+ :doc: GPUVM
+
+.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+ :internal:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d88687b..345fb3c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -34,8 +34,9 @@
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
-/*
- * GPUVM
+/**
+ * DOC: GPUVM
+ *
* GPUVM is similar to the legacy gart on older asics, however
* rather than there being a single global gart table
* for the entire GPU, there are multiple VM page tables active
@@ -63,7 +64,8 @@ INTERVAL_TREE_DEFINE(struct amdgpu_bo_va_mapping, rb,
uint64_t, __subtree_last,
#undef START
#undef LAST
-/* Local structure. Encapsulate some VM table update parameters to reduce
+/*
+ * Local structure. Encapsulate some VM table update parameters to reduce
* the number of function parameters
*/
While at it you could change the field comments into proper kernel
documentation as well.
struct amdgpu_pte_update_params {
@@ -94,6 +96,16 @@ struct amdgpu_prt_cb {
struct dma_fence_cb cb;
};
+/**
+ * amdgpu_vm_bo_base_init - Adds bo to the list of bos associated with the vm
Better use something like "Initialize a bo_va_base structure and add it
to the appropriate lists.".
+ *
+ * @base: base structure for tracking BO usage in a VM
+ * @vm: vm to which bo is to be added
+ * @bo: amdgpu buffer object
+ *
+ * Adds bo to the list of bos associated with the vm
+ *
+ */
static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
struct amdgpu_vm *vm,
struct amdgpu_bo *bo)
@@ -126,8 +138,10 @@ static void amdgpu_vm_bo_base_init(struct
amdgpu_vm_bo_base *base,
* amdgpu_vm_level_shift - return the addr shift for each level
*
* @adev: amdgpu_device pointer
+ * @level: VMPT level
*
- * Returns the number of bits the pfn needs to be right shifted for a level.
+ * Returns:
+ * The number of bits the pfn needs to be right shifted for a level.
*/
static unsigned amdgpu_vm_level_shift(struct amdgpu_device *adev,
unsigned level)
@@ -155,8 +169,10 @@ static unsigned amdgpu_vm_level_shift(struct amdgpu_device
*adev,
* amdgpu_vm_num_entries - return the number of entries in a PD/PT
*
* @adev: amdgpu_device pointer
+ * @level: VMPT level
*
- * Calculate the number of entries in a page directory or page table.
+ * Returns:
+ * The number of entries in a page directory or page table.
*/
static unsigned amdgpu_vm_num_entries(struct amdgpu_device *adev,
unsigned level)
@@ -179,8 +195,10 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device
*adev,
* amdgpu_vm_bo_size - returns the size of the BOs in bytes
*
* @adev: amdgpu_device pointer
+ * @level: VMPT level
*
- * Calculate the size of the BO for a page directory or page table in bytes.
+ * Returns:
+ * The size of the BO for a page directory or page table in bytes.
*/
static unsigned amdgpu_vm_bo_size(struct amdgpu_device *adev, unsigned level)
{
@@ -218,6 +236,9 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
* @param: parameter for the validation callback
*
* Validate the page table BOs on command submission if neccessary.
+ *
+ * Returns:
+ * Validation result.
*/
int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm
*vm,
int (*validate)(void *p, struct amdgpu_bo *bo),
@@ -273,6 +294,9 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
* @vm: VM to check
*
* Check if all VM PDs/PTs are ready for updates
+ *
+ * Returns:
+ * True if eviction list is empty.
*/
bool amdgpu_vm_ready(struct amdgpu_vm *vm)
{
@@ -283,10 +307,14 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
* amdgpu_vm_clear_bo - initially clear the PDs/PTs
*
* @adev: amdgpu_device pointer
+ * @vm: VM
[PATCH v2] Update function level documentation for GPUVM v2
Add/update function level documentation and add reference to amdgpu_vm.c
in amdgpu.rst
v2:
Fix reference in rst file.
Fix compilation warnings.
Add space between function names and params list where
it's missing.
Signed-off-by: Andrey Grodzovsky
---
Documentation/gpu/amdgpu.rst | 9 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 199 -
2 files changed, 179 insertions(+), 29 deletions(-)
diff --git a/Documentation/gpu/amdgpu.rst b/Documentation/gpu/amdgpu.rst
index a4852f9..3ffb2a2 100644
--- a/Documentation/gpu/amdgpu.rst
+++ b/Documentation/gpu/amdgpu.rst
@@ -44,3 +44,12 @@ MMU Notifier
.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
:internal:
+
+AMDGPU Virtual Memory
+-
+
+.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+ :doc: GPUVM
+
+.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+ :internal:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d88687b..345fb3c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -34,8 +34,9 @@
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
-/*
- * GPUVM
+/**
+ * DOC: GPUVM
+ *
* GPUVM is similar to the legacy gart on older asics, however
* rather than there being a single global gart table
* for the entire GPU, there are multiple VM page tables active
@@ -63,7 +64,8 @@ INTERVAL_TREE_DEFINE(struct amdgpu_bo_va_mapping, rb,
uint64_t, __subtree_last,
#undef START
#undef LAST
-/* Local structure. Encapsulate some VM table update parameters to reduce
+/*
+ * Local structure. Encapsulate some VM table update parameters to reduce
* the number of function parameters
*/
struct amdgpu_pte_update_params {
@@ -94,6 +96,16 @@ struct amdgpu_prt_cb {
struct dma_fence_cb cb;
};
+/**
+ * amdgpu_vm_bo_base_init - Adds bo to the list of bos associated with the vm
+ *
+ * @base: base structure for tracking BO usage in a VM
+ * @vm: vm to which bo is to be added
+ * @bo: amdgpu buffer object
+ *
+ * Adds bo to the list of bos associated with the vm
+ *
+ */
static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
struct amdgpu_vm *vm,
struct amdgpu_bo *bo)
@@ -126,8 +138,10 @@ static void amdgpu_vm_bo_base_init(struct
amdgpu_vm_bo_base *base,
* amdgpu_vm_level_shift - return the addr shift for each level
*
* @adev: amdgpu_device pointer
+ * @level: VMPT level
*
- * Returns the number of bits the pfn needs to be right shifted for a level.
+ * Returns:
+ * The number of bits the pfn needs to be right shifted for a level.
*/
static unsigned amdgpu_vm_level_shift(struct amdgpu_device *adev,
unsigned level)
@@ -155,8 +169,10 @@ static unsigned amdgpu_vm_level_shift(struct amdgpu_device
*adev,
* amdgpu_vm_num_entries - return the number of entries in a PD/PT
*
* @adev: amdgpu_device pointer
+ * @level: VMPT level
*
- * Calculate the number of entries in a page directory or page table.
+ * Returns:
+ * The number of entries in a page directory or page table.
*/
static unsigned amdgpu_vm_num_entries(struct amdgpu_device *adev,
unsigned level)
@@ -179,8 +195,10 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device
*adev,
* amdgpu_vm_bo_size - returns the size of the BOs in bytes
*
* @adev: amdgpu_device pointer
+ * @level: VMPT level
*
- * Calculate the size of the BO for a page directory or page table in bytes.
+ * Returns:
+ * The size of the BO for a page directory or page table in bytes.
*/
static unsigned amdgpu_vm_bo_size(struct amdgpu_device *adev, unsigned level)
{
@@ -218,6 +236,9 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
* @param: parameter for the validation callback
*
* Validate the page table BOs on command submission if neccessary.
+ *
+ * Returns:
+ * Validation result.
*/
int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
int (*validate)(void *p, struct amdgpu_bo *bo),
@@ -273,6 +294,9 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
* @vm: VM to check
*
* Check if all VM PDs/PTs are ready for updates
+ *
+ * Returns:
+ * True if eviction list is empty.
*/
bool amdgpu_vm_ready(struct amdgpu_vm *vm)
{
@@ -283,10 +307,14 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
* amdgpu_vm_clear_bo - initially clear the PDs/PTs
*
* @adev: amdgpu_device pointer
+ * @vm: VM to clear BO from
* @bo: BO to clear
* @level: level this BO is at
*
* Root PD needs to be reserved when calling this.
+ *
+ * Returns:
+ * 0 on success, errno otherwise.
*/
static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
struct amdgpu_vm *vm, struct amdgpu_bo *bo,
@@ -382,10 +410,16 @@ static int amdgpu_vm_clear_bo(struct
