All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Update function level documentation for GPUVM v2
@ 2018-06-12 14:05 Andrey Grodzovsky
       [not found] ` <1528812303-4720-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Andrey Grodzovsky @ 2018-06-12 14:05 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Christian.Koenig-5C7GfCeVMHo
  Cc: Andrey Grodzovsky, michel-otUistvHUpPR7s880joybQ

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 <andrey.grodzovsky@amd.com>
---
 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 amdgpu_device *adev,
  *
  * @adev: amdgpu_device pointer
  * @vm: requested vm
+ * @parent: parent PT
  * @saddr: start of the address range
  * @eaddr: end of the address range
+ * @level: VMPT level
+ * @ats: indicate ATS support from PTE
  *
  * Make sure the page directories and page tables are allocated
+ *
+ * Returns:
+ * 0 on success, errno otherwise.
  */
 static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
 				  struct amdgpu_vm *vm,
@@ -494,6 +528,9 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
  * @size: Size from start address we need.
  *
  * Make sure the page tables are allocated.
+ *
+ * Returns:
+ * 0 on success, errno otherwise.
  */
 int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 			struct amdgpu_vm *vm,
@@ -559,6 +596,15 @@ void amdgpu_vm_check_compute_bug(struct amdgpu_device *adev)
 	}
 }
 
+/**
+ * amdgpu_vm_need_pipeline_sync - Check if pipe sync is needed for job.
+ *
+ * @ring: ring on which the job will be submitted
+ * @job: job to submit
+ *
+ * Returns:
+ * True if sync is needed.
+ */
 bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
 				  struct amdgpu_job *job)
 {
@@ -586,6 +632,14 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
 	return vm_flush_needed || gds_switch_needed;
 }
 
+/**
+ * amdgpu_vm_is_large_bar - Check if BAR is large enough
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Returns:
+ * True if BAR is large enough.
+ */
 static bool amdgpu_vm_is_large_bar(struct amdgpu_device *adev)
 {
 	return (adev->gmc.real_vram_size == adev->gmc.visible_vram_size);
@@ -595,10 +649,12 @@ static bool amdgpu_vm_is_large_bar(struct amdgpu_device *adev)
  * amdgpu_vm_flush - hardware flush the vm
  *
  * @ring: ring to use for flush
- * @vmid: vmid number to use
- * @pd_addr: address of the page directory
+ * @need_pipe_sync: is pipe sync needed
  *
  * Emit a VM flush when it is necessary.
+ *
+ * Returns:
+ * 0 on success, errno otherwise.
  */
 int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job, bool need_pipe_sync)
 {
@@ -706,6 +762,9 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job, bool need_
  * Returns the found bo_va or NULL if none is found
  *
  * Object has to be reserved!
+ *
+ * Returns:
+ * Found bo_va or NULL.
  */
 struct amdgpu_bo_va *amdgpu_vm_bo_find(struct amdgpu_vm *vm,
 				       struct amdgpu_bo *bo)
@@ -787,7 +846,10 @@ static void amdgpu_vm_do_copy_ptes(struct amdgpu_pte_update_params *params,
  * @addr: the unmapped addr
  *
  * Look up the physical address of the page that the pte resolves
- * to and return the pointer for the page table entry.
+ * to.
+ *
+ * Returns:
+ * The pointer for the page table entry.
  */
 static uint64_t amdgpu_vm_map_gart(const dma_addr_t *pages_addr, uint64_t addr)
 {
@@ -840,6 +902,17 @@ static void amdgpu_vm_cpu_set_ptes(struct amdgpu_pte_update_params *params,
 	}
 }
 
+
+/**
+ * amdgpu_vm_wait_pd - Wait for PT BOs to be free.
+ *
+ * @adev: amdgpu_device pointer
+ * @vm: related vm
+ * @owner: fence owner
+ *
+ * Returns:
+ * 0 on success, errno otherwise.
+ */
 static int amdgpu_vm_wait_pd(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			     void *owner)
 {
@@ -893,7 +966,10 @@ static void amdgpu_vm_update_pde(struct amdgpu_pte_update_params *params,
 /*
  * amdgpu_vm_invalidate_level - mark all PD levels as invalid
  *
+ * @adev: amdgpu_device pointer
+ * @vm: related vm
  * @parent: parent PD
+ * @level: VMPT level
  *
  * Mark all PD level as invalid after an error.
  */
@@ -928,7 +1004,9 @@ static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
  * @vm: requested vm
  *
  * Makes sure all directories are up to date.
- * Returns 0 for success, error for failure.
+ *
+ * Returns:
+ * 0 for success, error for failure.
  */
 int amdgpu_vm_update_directories(struct amdgpu_device *adev,
 				 struct amdgpu_vm *vm)
@@ -1115,14 +1193,15 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
  * amdgpu_vm_update_ptes - make sure that page tables are valid
  *
  * @params: see amdgpu_pte_update_params definition
- * @vm: requested vm
  * @start: start of GPU address range
  * @end: end of GPU address range
  * @dst: destination address to map to, the next dst inside the function
  * @flags: mapping flags
  *
  * Update the page tables in the range @start - @end.
- * Returns 0 for success, -EINVAL for failure.
+ *
+ * Returns:
+ * 0 for success, -EINVAL for failure.
  */
 static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
 				  uint64_t start, uint64_t end,
@@ -1176,7 +1255,9 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
  * @end: last PTE to handle
  * @dst: addr those PTEs should point to
  * @flags: hw mapping flags
- * Returns 0 for success, -EINVAL for failure.
+ *
+ * Returns:
+ * 0 for success, -EINVAL for failure.
  */
 static int amdgpu_vm_frag_ptes(struct amdgpu_pte_update_params	*params,
 				uint64_t start, uint64_t end,
@@ -1248,7 +1329,9 @@ static int amdgpu_vm_frag_ptes(struct amdgpu_pte_update_params	*params,
  * @fence: optional resulting fence
  *
  * Fill in the page table entries between @start and @last.
- * Returns 0 for success, -EINVAL for failure.
+ *
+ * Returns:
+ * 0 for success, -EINVAL for failure.
  */
 static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 				       struct dma_fence *exclusive,
@@ -1403,7 +1486,9 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
  *
  * Split the mapping into smaller chunks so that each update fits
  * into a SDMA IB.
- * Returns 0 for success, -EINVAL for failure.
+ *
+ * Returns:
+ * 0 for success, -EINVAL for failure.
  */
 static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 				      struct dma_fence *exclusive,
@@ -1514,7 +1599,9 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
  * @clear: if true clear the entries
  *
  * Fill in the page table entries for @bo_va.
- * Returns 0 for success, -EINVAL for failure.
+ *
+ * Returns:
+ * 0 for success, -EINVAL for failure.
  */
 int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 			struct amdgpu_bo_va *bo_va,
@@ -1609,6 +1696,8 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 
 /**
  * amdgpu_vm_update_prt_state - update the global PRT state
+ *
+ * @adev: amdgpu_device pointer
  */
 static void amdgpu_vm_update_prt_state(struct amdgpu_device *adev)
 {
@@ -1623,6 +1712,8 @@ static void amdgpu_vm_update_prt_state(struct amdgpu_device *adev)
 
 /**
  * amdgpu_vm_prt_get - add a PRT user
+ *
+ * @adev: amdgpu_device pointer
  */
 static void amdgpu_vm_prt_get(struct amdgpu_device *adev)
 {
@@ -1635,6 +1726,8 @@ static void amdgpu_vm_prt_get(struct amdgpu_device *adev)
 
 /**
  * amdgpu_vm_prt_put - drop a PRT user
+ *
+ * @adev: amdgpu_device pointer
  */
 static void amdgpu_vm_prt_put(struct amdgpu_device *adev)
 {
@@ -1644,6 +1737,8 @@ static void amdgpu_vm_prt_put(struct amdgpu_device *adev)
 
 /**
  * amdgpu_vm_prt_cb - callback for updating the PRT status
+ *
+ * @fence: fence for the callback
  */
 static void amdgpu_vm_prt_cb(struct dma_fence *fence, struct dma_fence_cb *_cb)
 {
@@ -1655,6 +1750,9 @@ static void amdgpu_vm_prt_cb(struct dma_fence *fence, struct dma_fence_cb *_cb)
 
 /**
  * amdgpu_vm_add_prt_cb - add callback for updating the PRT status
+ *
+ * @adev: amdgpu_device pointer
+ * @fence: fence for the callback
  */
 static void amdgpu_vm_add_prt_cb(struct amdgpu_device *adev,
 				 struct dma_fence *fence)
@@ -1746,9 +1844,11 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
  * or if an error occurred)
  *
  * Make sure all freed BOs are cleared in the PT.
- * Returns 0 for success.
- *
  * PTs have to be reserved and mutex must be locked!
+ *
+ * Returns:
+ * 0 for success.
+ *
  */
 int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 			  struct amdgpu_vm *vm,
@@ -1793,10 +1893,11 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
  *
  * @adev: amdgpu_device pointer
  * @vm: requested vm
- * @sync: sync object to add fences to
  *
  * Make sure all BOs which are moved are updated in the PTs.
- * Returns 0 for success.
+ *
+ * Returns:
+ * 0 for success.
  *
  * PTs have to be reserved!
  */
@@ -1851,7 +1952,9 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
  *
  * Add @bo into the requested vm.
  * Add @bo to the list of bos associated with the vm
- * Returns newly added bo_va or NULL for failure
+ *
+ * Returns:
+ * Newly added bo_va or NULL for failure
  *
  * Object has to be reserved!
  */
@@ -1917,7 +2020,9 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device *adev,
  * @flags: attributes of pages (read/write/valid/etc.)
  *
  * Add a mapping of the BO at the specefied addr into the VM.
- * Returns 0 for success, error for failure.
+ *
+ * Returns:
+ * 0 for success, error for failure.
  *
  * Object has to be reserved and unreserved outside!
  */
@@ -1979,7 +2084,9 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
  *
  * Add a mapping of the BO at the specefied addr into the VM. Replace existing
  * mappings as we do so.
- * Returns 0 for success, error for failure.
+ *
+ * Returns:
+ * 0 for success, error for failure.
  *
  * Object has to be reserved and unreserved outside!
  */
@@ -2036,7 +2143,9 @@ int amdgpu_vm_bo_replace_map(struct amdgpu_device *adev,
  * @saddr: where to the BO is mapped
  *
  * Remove a mapping of the BO at the specefied addr from the VM.
- * Returns 0 for success, error for failure.
+ *
+ * Returns:
+ * 0 for success, error for failure.
  *
  * Object has to be reserved and unreserved outside!
  */
@@ -2090,7 +2199,9 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
  * @size: size of the range
  *
  * Remove all mappings in a range, split them as appropriate.
- * Returns 0 for success, error for failure.
+ *
+ * Returns:
+ * 0 for success, error for failure.
  */
 int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
 				struct amdgpu_vm *vm,
@@ -2189,6 +2300,10 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
  * @vm: the requested VM
  *
  * Find a mapping by it's address.
+ *
+ * Returns:
+ * The amdgpu_bo_va_mapping matching for addr or NULL
+ *
  */
 struct amdgpu_bo_va_mapping *amdgpu_vm_bo_lookup_mapping(struct amdgpu_vm *vm,
 							 uint64_t addr)
@@ -2240,7 +2355,6 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
  * amdgpu_vm_bo_invalidate - mark the bo as invalid
  *
  * @adev: amdgpu_device pointer
- * @vm: requested vm
  * @bo: amdgpu buffer object
  *
  * Mark @bo as invalid.
@@ -2281,6 +2395,14 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
 	}
 }
 
+/**
+ * amdgpu_vm_get_block_size - calculate VM page table size in bits
+ *
+ * @vm_size: VM size
+ *
+ * Returns:
+ * VM page table size in bits
+ */
 static uint32_t amdgpu_vm_get_block_size(uint64_t vm_size)
 {
 	/* Total bits covered by PD + PTs */
@@ -2368,6 +2490,9 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t vm_size,
  * @vm_context: Indicates if it GFX or Compute context
  *
  * Init @vm fields.
+ *
+ * Returns:
+ * 0 for success, error for failure.
  */
 int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		   int vm_context, unsigned int pasid)
@@ -2488,6 +2613,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 /**
  * amdgpu_vm_make_compute - Turn a GFX VM into a compute VM
  *
+ * @adev: amdgpu_device pointer
+ * @vm: requested vm
+ *
  * This only works on GFX VMs that don't have any BOs added and no
  * page tables allocated yet.
  *
@@ -2500,7 +2628,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
  * setting. May leave behind an unused shadow BO for the page
  * directory when switching from SDMA updates to CPU updates.
  *
- * Returns 0 for success, -errno for errors.
+ * Returns:
+ * 0 for success, -errno for errors.
  */
 int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 {
@@ -2655,8 +2784,10 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
  * @adev: amdgpu_device pointer
  * @pasid: PASID do identify the VM
  *
- * This function is expected to be called in interrupt context. Returns
- * true if there was fault credit, false otherwise
+ * This function is expected to be called in interrupt context.
+ *
+ * Returns:
+ * True if there was fault credit, false otherwise
  */
 bool amdgpu_vm_pasid_fault_credit(struct amdgpu_device *adev,
 				  unsigned int pasid)
@@ -2740,6 +2871,16 @@ void amdgpu_vm_manager_fini(struct amdgpu_device *adev)
 	amdgpu_vmid_mgr_fini(adev);
 }
 
+/**
+ * amdgpu_vm_ioctl - Manages VMID reservation for vm hubs.
+ *
+ * @dev: drm device pointer
+ * @data: drm_amdgpu_vm
+ * @filp: drm file pointer
+ *
+ * Returns:
+ * 0 for success, -errno for errors.
+ */
 int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 {
 	union drm_amdgpu_vm *args = data;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Update function level documentation for GPUVM v2
       [not found] ` <1528812303-4720-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
@ 2018-06-12 14:41   ` Christian König
       [not found]     ` <fe0b61a8-000d-8679-d2f0-b7eaec4e092f-5C7GfCeVMHo@public.gmane.org>
  2018-06-13 13:48   ` Michel Dänzer
  1 sibling, 1 reply; 9+ messages in thread
From: Christian König @ 2018-06-12 14:41 UTC (permalink / raw)
  To: Andrey Grodzovsky, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: michel-otUistvHUpPR7s880joybQ

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 <andrey.grodzovsky@amd.com>
> ---
>   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 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 amdgpu_device *adev,
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requested vm
> + * @parent: parent PT
>    * @saddr: start of the address range
>    * @eaddr: end of the address range
> + * @level: VMPT level
> + * @ats: indicate ATS support from PTE
>    *
>    * Make sure the page directories and page tables are allocated
> + *
> + * Returns:
> + * 0 on success, errno otherwise.
>    */
>   static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   				  struct amdgpu_vm *vm,
> @@ -494,6 +528,9 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>    * @size: Size from start address we need.
>    *
>    * Make sure the page tables are allocated.
> + *
> + * Returns:
> + * 0 on success, errno otherwise.
>    */
>   int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   			struct amdgpu_vm *vm,
> @@ -559,6 +596,15 @@ void amdgpu_vm_check_compute_bug(struct amdgpu_device *adev)
>   	}
>   }
>   
> +/**
> + * amdgpu_vm_need_pipeline_sync - Check if pipe sync is needed for job.
> + *
> + * @ring: ring on which the job will be submitted
> + * @job: job to submit
> + *
> + * Returns:
> + * True if sync is needed.
> + */
>   bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   				  struct amdgpu_job *job)
>   {
> @@ -586,6 +632,14 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   	return vm_flush_needed || gds_switch_needed;
>   }
>   
> +/**
> + * amdgpu_vm_is_large_bar - Check if BAR is large enough
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * Returns:
> + * True if BAR is large enough.
> + */

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.

>   static bool amdgpu_vm_is_large_bar(struct amdgpu_device *adev)
>   {
>   	return (adev->gmc.real_vram_size == adev->gmc.visible_vram_size);
> @@ -595,10 +649,12 @@ static bool amdgpu_vm_is_large_bar(struct amdgpu_device *adev)
>    * amdgpu_vm_flush - hardware flush the vm
>    *
>    * @ring: ring to use for flush
> - * @vmid: vmid number to use
> - * @pd_addr: address of the page directory
> + * @need_pipe_sync: is pipe sync needed
>    *
>    * Emit a VM flush when it is necessary.
> + *
> + * Returns:
> + * 0 on success, errno otherwise.
>    */
>   int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job, bool need_pipe_sync)
>   {
> @@ -706,6 +762,9 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job, bool need_
>    * Returns the found bo_va or NULL if none is found
>    *
>    * Object has to be reserved!
> + *
> + * Returns:
> + * Found bo_va or NULL.
>    */
>   struct amdgpu_bo_va *amdgpu_vm_bo_find(struct amdgpu_vm *vm,
>   				       struct amdgpu_bo *bo)
> @@ -787,7 +846,10 @@ static void amdgpu_vm_do_copy_ptes(struct amdgpu_pte_update_params *params,
>    * @addr: the unmapped addr
>    *
>    * Look up the physical address of the page that the pte resolves
> - * to and return the pointer for the page table entry.
> + * to.
> + *
> + * Returns:
> + * The pointer for the page table entry.
>    */
>   static uint64_t amdgpu_vm_map_gart(const dma_addr_t *pages_addr, uint64_t addr)
>   {
> @@ -840,6 +902,17 @@ static void amdgpu_vm_cpu_set_ptes(struct amdgpu_pte_update_params *params,
>   	}
>   }
>   
> +
> +/**
> + * amdgpu_vm_wait_pd - Wait for PT BOs to be free.
> + *
> + * @adev: amdgpu_device pointer
> + * @vm: related vm
> + * @owner: fence owner
> + *
> + * Returns:
> + * 0 on success, errno otherwise.
> + */
>   static int amdgpu_vm_wait_pd(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			     void *owner)
>   {
> @@ -893,7 +966,10 @@ static void amdgpu_vm_update_pde(struct amdgpu_pte_update_params *params,
>   /*
>    * amdgpu_vm_invalidate_level - mark all PD levels as invalid
>    *
> + * @adev: amdgpu_device pointer
> + * @vm: related vm
>    * @parent: parent PD
> + * @level: VMPT level
>    *
>    * Mark all PD level as invalid after an error.
>    */
> @@ -928,7 +1004,9 @@ static void amdgpu_vm_invalidate_level(struct amdgpu_device *adev,
>    * @vm: requested vm
>    *
>    * Makes sure all directories are up to date.
> - * Returns 0 for success, error for failure.
> + *
> + * Returns:
> + * 0 for success, error for failure.
>    */
>   int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>   				 struct amdgpu_vm *vm)
> @@ -1115,14 +1193,15 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
>    * amdgpu_vm_update_ptes - make sure that page tables are valid
>    *
>    * @params: see amdgpu_pte_update_params definition
> - * @vm: requested vm
>    * @start: start of GPU address range
>    * @end: end of GPU address range
>    * @dst: destination address to map to, the next dst inside the function
>    * @flags: mapping flags
>    *
>    * Update the page tables in the range @start - @end.
> - * Returns 0 for success, -EINVAL for failure.
> + *
> + * Returns:
> + * 0 for success, -EINVAL for failure.
>    */
>   static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>   				  uint64_t start, uint64_t end,
> @@ -1176,7 +1255,9 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>    * @end: last PTE to handle
>    * @dst: addr those PTEs should point to
>    * @flags: hw mapping flags
> - * Returns 0 for success, -EINVAL for failure.
> + *
> + * Returns:
> + * 0 for success, -EINVAL for failure.
>    */
>   static int amdgpu_vm_frag_ptes(struct amdgpu_pte_update_params	*params,
>   				uint64_t start, uint64_t end,
> @@ -1248,7 +1329,9 @@ static int amdgpu_vm_frag_ptes(struct amdgpu_pte_update_params	*params,
>    * @fence: optional resulting fence
>    *
>    * Fill in the page table entries between @start and @last.
> - * Returns 0 for success, -EINVAL for failure.
> + *
> + * Returns:
> + * 0 for success, -EINVAL for failure.
>    */
>   static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>   				       struct dma_fence *exclusive,
> @@ -1403,7 +1486,9 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>    *
>    * Split the mapping into smaller chunks so that each update fits
>    * into a SDMA IB.
> - * Returns 0 for success, -EINVAL for failure.
> + *
> + * Returns:
> + * 0 for success, -EINVAL for failure.
>    */
>   static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   				      struct dma_fence *exclusive,
> @@ -1514,7 +1599,9 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>    * @clear: if true clear the entries
>    *
>    * Fill in the page table entries for @bo_va.
> - * Returns 0 for success, -EINVAL for failure.
> + *
> + * Returns:
> + * 0 for success, -EINVAL for failure.
>    */
>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va *bo_va,
> @@ -1609,6 +1696,8 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   
>   /**
>    * amdgpu_vm_update_prt_state - update the global PRT state
> + *
> + * @adev: amdgpu_device pointer
>    */
>   static void amdgpu_vm_update_prt_state(struct amdgpu_device *adev)
>   {
> @@ -1623,6 +1712,8 @@ static void amdgpu_vm_update_prt_state(struct amdgpu_device *adev)
>   
>   /**
>    * amdgpu_vm_prt_get - add a PRT user
> + *
> + * @adev: amdgpu_device pointer
>    */
>   static void amdgpu_vm_prt_get(struct amdgpu_device *adev)
>   {
> @@ -1635,6 +1726,8 @@ static void amdgpu_vm_prt_get(struct amdgpu_device *adev)
>   
>   /**
>    * amdgpu_vm_prt_put - drop a PRT user
> + *
> + * @adev: amdgpu_device pointer
>    */
>   static void amdgpu_vm_prt_put(struct amdgpu_device *adev)
>   {
> @@ -1644,6 +1737,8 @@ static void amdgpu_vm_prt_put(struct amdgpu_device *adev)
>   
>   /**
>    * amdgpu_vm_prt_cb - callback for updating the PRT status
> + *
> + * @fence: fence for the callback
>    */
>   static void amdgpu_vm_prt_cb(struct dma_fence *fence, struct dma_fence_cb *_cb)
>   {
> @@ -1655,6 +1750,9 @@ static void amdgpu_vm_prt_cb(struct dma_fence *fence, struct dma_fence_cb *_cb)
>   
>   /**
>    * amdgpu_vm_add_prt_cb - add callback for updating the PRT status
> + *
> + * @adev: amdgpu_device pointer
> + * @fence: fence for the callback
>    */
>   static void amdgpu_vm_add_prt_cb(struct amdgpu_device *adev,
>   				 struct dma_fence *fence)
> @@ -1746,9 +1844,11 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>    * or if an error occurred)
>    *
>    * Make sure all freed BOs are cleared in the PT.
> - * Returns 0 for success.
> - *
>    * PTs have to be reserved and mutex must be locked!
> + *
> + * Returns:
> + * 0 for success.
> + *
>    */
>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>   			  struct amdgpu_vm *vm,
> @@ -1793,10 +1893,11 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requested vm
> - * @sync: sync object to add fences to
>    *
>    * Make sure all BOs which are moved are updated in the PTs.
> - * Returns 0 for success.
> + *
> + * Returns:
> + * 0 for success.
>    *
>    * PTs have to be reserved!
>    */
> @@ -1851,7 +1952,9 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>    *
>    * Add @bo into the requested vm.
>    * Add @bo to the list of bos associated with the vm
> - * Returns newly added bo_va or NULL for failure
> + *
> + * Returns:
> + * Newly added bo_va or NULL for failure
>    *
>    * Object has to be reserved!
>    */
> @@ -1917,7 +2020,9 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device *adev,
>    * @flags: attributes of pages (read/write/valid/etc.)
>    *
>    * Add a mapping of the BO at the specefied addr into the VM.
> - * Returns 0 for success, error for failure.
> + *
> + * Returns:
> + * 0 for success, error for failure.
>    *
>    * Object has to be reserved and unreserved outside!
>    */
> @@ -1979,7 +2084,9 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
>    *
>    * Add a mapping of the BO at the specefied addr into the VM. Replace existing
>    * mappings as we do so.
> - * Returns 0 for success, error for failure.
> + *
> + * Returns:
> + * 0 for success, error for failure.
>    *
>    * Object has to be reserved and unreserved outside!
>    */
> @@ -2036,7 +2143,9 @@ int amdgpu_vm_bo_replace_map(struct amdgpu_device *adev,
>    * @saddr: where to the BO is mapped
>    *
>    * Remove a mapping of the BO at the specefied addr from the VM.
> - * Returns 0 for success, error for failure.
> + *
> + * Returns:
> + * 0 for success, error for failure.
>    *
>    * Object has to be reserved and unreserved outside!
>    */
> @@ -2090,7 +2199,9 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>    * @size: size of the range
>    *
>    * Remove all mappings in a range, split them as appropriate.
> - * Returns 0 for success, error for failure.
> + *
> + * Returns:
> + * 0 for success, error for failure.
>    */
>   int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
>   				struct amdgpu_vm *vm,
> @@ -2189,6 +2300,10 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
>    * @vm: the requested VM
>    *
>    * Find a mapping by it's address.
> + *
> + * Returns:
> + * The amdgpu_bo_va_mapping matching for addr or NULL
> + *
>    */
>   struct amdgpu_bo_va_mapping *amdgpu_vm_bo_lookup_mapping(struct amdgpu_vm *vm,
>   							 uint64_t addr)
> @@ -2240,7 +2355,6 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>    * amdgpu_vm_bo_invalidate - mark the bo as invalid
>    *
>    * @adev: amdgpu_device pointer
> - * @vm: requested vm
>    * @bo: amdgpu buffer object
>    *
>    * Mark @bo as invalid.
> @@ -2281,6 +2395,14 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   	}
>   }
>   
> +/**
> + * amdgpu_vm_get_block_size - calculate VM page table size in bits

Better "calculate VM page table size as power of two".

Nice cleanup, with the minor issues fixed Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Thanks,
Christian.

> + *
> + * @vm_size: VM size
> + *
> + * Returns:
> + * VM page table size in bits
> + */
>   static uint32_t amdgpu_vm_get_block_size(uint64_t vm_size)
>   {
>   	/* Total bits covered by PD + PTs */
> @@ -2368,6 +2490,9 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t vm_size,
>    * @vm_context: Indicates if it GFX or Compute context
>    *
>    * Init @vm fields.
> + *
> + * Returns:
> + * 0 for success, error for failure.
>    */
>   int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		   int vm_context, unsigned int pasid)
> @@ -2488,6 +2613,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   /**
>    * amdgpu_vm_make_compute - Turn a GFX VM into a compute VM
>    *
> + * @adev: amdgpu_device pointer
> + * @vm: requested vm
> + *
>    * This only works on GFX VMs that don't have any BOs added and no
>    * page tables allocated yet.
>    *
> @@ -2500,7 +2628,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>    * setting. May leave behind an unused shadow BO for the page
>    * directory when switching from SDMA updates to CPU updates.
>    *
> - * Returns 0 for success, -errno for errors.
> + * Returns:
> + * 0 for success, -errno for errors.
>    */
>   int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>   {
> @@ -2655,8 +2784,10 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>    * @adev: amdgpu_device pointer
>    * @pasid: PASID do identify the VM
>    *
> - * This function is expected to be called in interrupt context. Returns
> - * true if there was fault credit, false otherwise
> + * This function is expected to be called in interrupt context.
> + *
> + * Returns:
> + * True if there was fault credit, false otherwise
>    */
>   bool amdgpu_vm_pasid_fault_credit(struct amdgpu_device *adev,
>   				  unsigned int pasid)
> @@ -2740,6 +2871,16 @@ void amdgpu_vm_manager_fini(struct amdgpu_device *adev)
>   	amdgpu_vmid_mgr_fini(adev);
>   }
>   
> +/**
> + * amdgpu_vm_ioctl - Manages VMID reservation for vm hubs.
> + *
> + * @dev: drm device pointer
> + * @data: drm_amdgpu_vm
> + * @filp: drm file pointer
> + *
> + * Returns:
> + * 0 for success, -errno for errors.
> + */
>   int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>   {
>   	union drm_amdgpu_vm *args = data;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Update function level documentation for GPUVM v2
       [not found]     ` <fe0b61a8-000d-8679-d2f0-b7eaec4e092f-5C7GfCeVMHo@public.gmane.org>
@ 2018-06-12 15:06       ` Andrey Grodzovsky
       [not found]         ` <5bbb4abe-9d35-65c0-9afe-47331160e7f1-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Andrey Grodzovsky @ 2018-06-12 15:06 UTC (permalink / raw)
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: michel-otUistvHUpPR7s880joybQ

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Update function level documentation for GPUVM v2
       [not found]         ` <5bbb4abe-9d35-65c0-9afe-47331160e7f1-5C7GfCeVMHo@public.gmane.org>
@ 2018-06-12 15:35           ` Christian König
  0 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2018-06-12 15:35 UTC (permalink / raw)
  To: Andrey Grodzovsky, Christian König,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: michel-otUistvHUpPR7s880joybQ

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Update function level documentation for GPUVM v2
       [not found] ` <1528812303-4720-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
  2018-06-12 14:41   ` Christian König
@ 2018-06-13 13:48   ` Michel Dänzer
       [not found]     ` <4f64a37b-433a-1c08-10e3-cd2b6c9ed560-otUistvHUpPR7s880joybQ@public.gmane.org>
  1 sibling, 1 reply; 9+ messages in thread
From: Michel Dänzer @ 2018-06-13 13:48 UTC (permalink / raw)
  To: Andrey Grodzovsky; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


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 <andrey.grodzovsky@amd.com>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Update function level documentation for GPUVM v2
       [not found]     ` <4f64a37b-433a-1c08-10e3-cd2b6c9ed560-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-06-13 13:52       ` Andrey Grodzovsky
       [not found]         ` <66dc2027-8ea2-4118-e493-7ed2e6f98997-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Andrey Grodzovsky @ 2018-06-13 13:52 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

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 <andrey.grodzovsky@amd.com>
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Update function level documentation for GPUVM v2
       [not found]         ` <66dc2027-8ea2-4118-e493-7ed2e6f98997-5C7GfCeVMHo@public.gmane.org>
@ 2018-06-13 13:57           ` Michel Dänzer
       [not found]             ` <1d8844bf-472a-663b-2247-7c22dd31c114-otUistvHUpPR7s880joybQ@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Michel Dänzer @ 2018-06-13 13:57 UTC (permalink / raw)
  To: Andrey Grodzovsky; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Update function level documentation for GPUVM v2
       [not found]             ` <1d8844bf-472a-663b-2247-7c22dd31c114-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-06-13 14:01               ` Christian König
       [not found]                 ` <03738b7c-9b0d-bcfb-5781-354f4ff3e983-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2018-06-13 14:01 UTC (permalink / raw)
  To: Michel Dänzer, Andrey Grodzovsky
  Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Update function level documentation for GPUVM v2
       [not found]                 ` <03738b7c-9b0d-bcfb-5781-354f4ff3e983-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-06-13 14:12                   ` Andrey Grodzovsky
  0 siblings, 0 replies; 9+ messages in thread
From: Andrey Grodzovsky @ 2018-06-13 14:12 UTC (permalink / raw)
  To: christian.koenig-5C7GfCeVMHo, Michel Dänzer
  Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-06-13 14:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-12 14:05 [PATCH v2] Update function level documentation for GPUVM v2 Andrey Grodzovsky
     [not found] ` <1528812303-4720-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
2018-06-12 14:41   ` Christian König
     [not found]     ` <fe0b61a8-000d-8679-d2f0-b7eaec4e092f-5C7GfCeVMHo@public.gmane.org>
2018-06-12 15:06       ` Andrey Grodzovsky
     [not found]         ` <5bbb4abe-9d35-65c0-9afe-47331160e7f1-5C7GfCeVMHo@public.gmane.org>
2018-06-12 15:35           ` Christian König
2018-06-13 13:48   ` Michel Dänzer
     [not found]     ` <4f64a37b-433a-1c08-10e3-cd2b6c9ed560-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-06-13 13:52       ` Andrey Grodzovsky
     [not found]         ` <66dc2027-8ea2-4118-e493-7ed2e6f98997-5C7GfCeVMHo@public.gmane.org>
2018-06-13 13:57           ` Michel Dänzer
     [not found]             ` <1d8844bf-472a-663b-2247-7c22dd31c114-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-06-13 14:01               ` Christian König
     [not found]                 ` <03738b7c-9b0d-bcfb-5781-354f4ff3e983-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-06-13 14:12                   ` Andrey Grodzovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.