RE: [PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
I can see them now. Sam > -Original Message- > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf > Of Samuel Li > Sent: Friday, June 01, 2018 5:10 PM > To: Michel Dänzer ; Liu, Leo > Cc: amd-gfx@lists.freedesktop.org > Subject: Re: [PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c > file > > > > On 2018-05-31 12:30 PM, Michel Dänzer wrote: > > On 2018-05-30 08:42 PM, Leo Liu wrote: > >> There are four ioctls in this files, and DOC gives details of data > >> structures for each of ioctls, and their functionalities. > >> > >> Signed-off-by: Leo Liu > > > > This isn't enough to actually make this part of the generated > > documentation. It needs to be hooked up to a *.rst file for that. > > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 2018-05-31 12:30 PM, Michel Dänzer wrote: > On 2018-05-30 08:42 PM, Leo Liu wrote: >> There are four ioctls in this files, and DOC gives details of >> data structures for each of ioctls, and their functionalities. >> >> Signed-off-by: Leo Liu > > This isn't enough to actually make this part of the generated > documentation. It needs to be hooked up to a *.rst file for that. > > I'm adding an amdgpu.rst file in > https://patchwork.freedesktop.org/series/44035/ , where you could hook > it up accordingly. Michel, have you submitted this one? So we can hook up the files gradually. I do not see the patches in this list. Sam > > Please check that generating the documentation (e.g. with make htmldocs) > doesn't produce any warnings about amdgpu_cs.c, and that the result > looks good in Documentation/output/gpu/amdgpu.html . The documentation > format itself is documented in Documentation/output/doc-guide/index.html . > > [...] ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 2018-05-30 08:42 PM, Leo Liu wrote: > There are four ioctls in this files, and DOC gives details of > data structures for each of ioctls, and their functionalities. > > Signed-off-by: Leo Liu > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 142 + > 1 file changed, 142 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 12f0d18c6ee8..343ff115cff1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, > return 0; > } > > +/** > + * DOC: amdgpu_cs_ioctl > + * > + * This ioctl processes user space command submission chunks, > + * return a fence sequence number associated to a amdgpu fence. > + * > + * In data structure: > + * > + * __u32 ctx_id: > + * Integer ID, created via DRM_AMDGPU_CTX ioctl call when user space > + * command submission context created. It will be used as ID for later > + * command submission context. > + * > + * __u32 bo_list_handle: > + * Handle of resources list associated with this CS, created via > + * DRM_AMDGPU_BO_LIST ioctl call before command submission, and > + * the BOs in the list will be validated. > + * > + * __u32 num_chunks: > + * Number of chunks, their types include: > + * AMDGPU_CHUNK_ID_IB > + * The data will be filled into IB buffer, and mappped to a HW ring. > + * > + * AMDGPU_CHUNK_ID_FENCE > + * The data will be used to find user fence BO and its offset. > + * > + * AMDGPU_CHUNK_ID_DEPENDENCIES > + * AMDGPU_CHUNK_ID_SYNCOBJ_IN > + * AMDGPU_CHUNK_ID_SYNCOBJ_OUT > + * These will be parsed as fence dependencies in given requirement, > + * and will be remembered and to be synced later. > + * > + * __u32 _pad: > + * > + * __u64 chunks: > + * Point to the CS chunks. BTW, this kind of formatting isn't preserved in the generated documentation, so it probably won't look as intended. -- 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] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 2018-05-31 08:02 PM, Leo Liu wrote: > On 05/31/2018 01:04 PM, Michel Dänzer wrote: >> On 2018-05-31 06:49 PM, Leo Liu wrote: >>> On 05/31/2018 12:47 PM, Michel Dänzer wrote: On 2018-05-31 06:39 PM, Leo Liu wrote: > On 05/31/2018 12:30 PM, Michel Dänzer wrote: >> On 2018-05-30 08:42 PM, Leo Liu wrote: >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> index 12f0d18c6ee8..343ff115cff1 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct >>> amdgpu_cs_parser *p, >>> return 0; >>> } >>> +/** >>> + * DOC: amdgpu_cs_ioctl >> DOC comments shouldn't be used for functions, see >> Documentation/output/doc-guide/kernel-doc.html#function-documentation >> > This doc is not for the functions, it's like something in commit > message > about the details of in/out data, and what is the functionality for > this > ioctl. Data structures should be documented in comments directly above their definitions, per Documentation/output/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation . What an ioctl does should be described in the function documentation comment above its implementation, as described above. >>> Then what do you think the kernel doc for ioctl should do? >> The data structure should be documented in a comment above its >> definition, and what the ioctl does should be documented in a comment >> above the function implementing it. The source files need to be hooked >> up to the amdgpu.rst file. >> >> Does that answer your question? > Sort of. It tells me what should not be in the kernel DOC. I will put > those details about data and functionalities where they should be. > And focus on what the ioctl is used for for the kernel DOC section. I'm afraid I don't see DOC comments being useful for documenting ioctls. They're for general/overview kind of documentation. For ioctls, the data structure and function documentation comments should be sufficient. You can reference a data structure in a function documentation comment like this: /** * amdgpu_cs_ioctl - ... * @...: ... * * ... * * The userspace parameters for this ioctl are passed in via * _amdgpu_cs. * * Returns: * ... */ See e.g. the documentation about i915_gem_get/set_tiling_ioctl in drivers/gpu/drm/i915/i915_gem_tiling.c. -- 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] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 05/31/2018 01:04 PM, Michel Dänzer wrote: On 2018-05-31 06:49 PM, Leo Liu wrote: On 05/31/2018 12:47 PM, Michel Dänzer wrote: On 2018-05-31 06:39 PM, Leo Liu wrote: On 05/31/2018 12:30 PM, Michel Dänzer wrote: On 2018-05-30 08:42 PM, Leo Liu wrote: diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 12f0d18c6ee8..343ff115cff1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, return 0; } +/** + * DOC: amdgpu_cs_ioctl DOC comments shouldn't be used for functions, see Documentation/output/doc-guide/kernel-doc.html#function-documentation This doc is not for the functions, it's like something in commit message about the details of in/out data, and what is the functionality for this ioctl. Data structures should be documented in comments directly above their definitions, per Documentation/output/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation . What an ioctl does should be described in the function documentation comment above its implementation, as described above. Then what do you think the kernel doc for ioctl should do? The data structure should be documented in a comment above its definition, and what the ioctl does should be documented in a comment above the function implementing it. The source files need to be hooked up to the amdgpu.rst file. Does that answer your question? Sort of. It tells me what should not be in the kernel DOC. I will put those details about data and functionalities where they should be. And focus on what the ioctl is used for for the kernel DOC section. Thanks Michel, the information is helpful. Leo ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 2018-05-31 06:49 PM, Leo Liu wrote: > On 05/31/2018 12:47 PM, Michel Dänzer wrote: >> On 2018-05-31 06:39 PM, Leo Liu wrote: >>> On 05/31/2018 12:30 PM, Michel Dänzer wrote: On 2018-05-30 08:42 PM, Leo Liu wrote: > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 12f0d18c6ee8..343ff115cff1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct > amdgpu_cs_parser *p, > return 0; > } > +/** > + * DOC: amdgpu_cs_ioctl DOC comments shouldn't be used for functions, see Documentation/output/doc-guide/kernel-doc.html#function-documentation >>> This doc is not for the functions, it's like something in commit message >>> about the details of in/out data, and what is the functionality for this >>> ioctl. >> Data structures should be documented in comments directly above their >> definitions, per >> Documentation/output/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation >> >> . What an ioctl does should be described in the function documentation >> comment above its implementation, as described above. > Then what do you think the kernel doc for ioctl should do? The data structure should be documented in a comment above its definition, and what the ioctl does should be documented in a comment above the function implementing it. The source files need to be hooked up to the amdgpu.rst file. Does that answer your question? -- 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] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 05/31/2018 12:47 PM, Michel Dänzer wrote: On 2018-05-31 06:39 PM, Leo Liu wrote: On 05/31/2018 12:30 PM, Michel Dänzer wrote: On 2018-05-30 08:42 PM, Leo Liu wrote: diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 12f0d18c6ee8..343ff115cff1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, return 0; } +/** + * DOC: amdgpu_cs_ioctl DOC comments shouldn't be used for functions, see Documentation/output/doc-guide/kernel-doc.html#function-documentation This doc is not for the functions, it's like something in commit message about the details of in/out data, and what is the functionality for this ioctl. Data structures should be documented in comments directly above their definitions, per Documentation/output/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation . What an ioctl does should be described in the function documentation comment above its implementation, as described above. Then what do you think the kernel doc for ioctl should do? Leo ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 05/31/2018 12:39 PM, Leo Liu wrote: On 05/31/2018 12:30 PM, Michel Dänzer wrote: On 2018-05-30 08:42 PM, Leo Liu wrote: There are four ioctls in this files, and DOC gives details of data structures for each of ioctls, and their functionalities. Signed-off-by: Leo Liu This isn't enough to actually make this part of the generated documentation. It needs to be hooked up to a *.rst file for that. I'm adding an amdgpu.rst file in https://patchwork.freedesktop.org/series/44035/ , where you could hook it up accordingly. Please check that generating the documentation (e.g. with make htmldocs) doesn't produce any warnings about amdgpu_cs.c, and that the result looks good in Documentation/output/gpu/amdgpu.html . The documentation format itself is documented in Documentation/output/doc-guide/index.html . I will take a look this. diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 12f0d18c6ee8..343ff115cff1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, return 0; } +/** + * DOC: amdgpu_cs_ioctl DOC comments shouldn't be used for functions, see Documentation/output/doc-guide/kernel-doc.html#function-documentation This doc is not for the functions, it's like something in commit message about the details of in/out data, and what is the functionality for this ioctl. @@ -1524,6 +1630,42 @@ static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev, return r; } +/** + * DOC: amdgpu_cs_wait_fences_ioctl + * + * This ioctl checks either all fences or any fence from multiple fences + * to be signaled or waits to be signaled till timeout. So it's used to + * check and wait multiple CS to be completed. + * + * In data structure: + * + * __u64 fences + * Point to the multiple fences + * + * __u32 fence_count + * number of fences + * + * __u32 wait_all + * ways to wait either wait_all or wait_any + * + * __u64 timeout_ns + * Absolute timeout to wait + * + * The function will extract user space fences based on pointer and counts, + * then mapping them amdgpu fences and check if they are signaled or wait + * to timeout. + * + * Out data: + * + *__u32 status + * 0 CS completed + * 1 CS busy + * + *__u32 first_signaled; + * First signaled fence index + * + */ + /** * amdgpu_cs_wait_fences_ioctl - wait for multiple command submissions to finish * Any reason for not adding the above to the existing function documentation comment here? The existing is the function document, I am adding kernel doc for this ioctl. Leo Yes. It's used by close source Vulkan, where it might be lack of BO fence dependencies implementations like we do in Mesa. Leo ___ 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] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 2018-05-31 06:39 PM, Leo Liu wrote: > On 05/31/2018 12:30 PM, Michel Dänzer wrote: >> On 2018-05-30 08:42 PM, Leo Liu wrote: > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> index 12f0d18c6ee8..343ff115cff1 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct >>> amdgpu_cs_parser *p, >>> return 0; >>> } >>> +/** >>> + * DOC: amdgpu_cs_ioctl >> DOC comments shouldn't be used for functions, see >> Documentation/output/doc-guide/kernel-doc.html#function-documentation >> > This doc is not for the functions, it's like something in commit message > about the details of in/out data, and what is the functionality for this > ioctl. Data structures should be documented in comments directly above their definitions, per Documentation/output/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation . What an ioctl does should be described in the function documentation comment above its implementation, as described above. -- 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] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 05/31/2018 12:30 PM, Michel Dänzer wrote: On 2018-05-30 08:42 PM, Leo Liu wrote: There are four ioctls in this files, and DOC gives details of data structures for each of ioctls, and their functionalities. Signed-off-by: Leo Liu This isn't enough to actually make this part of the generated documentation. It needs to be hooked up to a *.rst file for that. I'm adding an amdgpu.rst file in https://patchwork.freedesktop.org/series/44035/ , where you could hook it up accordingly. Please check that generating the documentation (e.g. with make htmldocs) doesn't produce any warnings about amdgpu_cs.c, and that the result looks good in Documentation/output/gpu/amdgpu.html . The documentation format itself is documented in Documentation/output/doc-guide/index.html . I will take a look this. diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 12f0d18c6ee8..343ff115cff1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, return 0; } +/** + * DOC: amdgpu_cs_ioctl DOC comments shouldn't be used for functions, see Documentation/output/doc-guide/kernel-doc.html#function-documentation This doc is not for the functions, it's like something in commit message about the details of in/out data, and what is the functionality for this ioctl. @@ -1524,6 +1630,42 @@ static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev, return r; } +/** + * DOC: amdgpu_cs_wait_fences_ioctl + * + * This ioctl checks either all fences or any fence from multiple fences + * to be signaled or waits to be signaled till timeout. So it's used to + * check and wait multiple CS to be completed. + * + * In data structure: + * + * __u64 fences + * Point to the multiple fences + * + * __u32 fence_count + * number of fences + * + * __u32 wait_all + * ways to wait either wait_all or wait_any + * + * __u64 timeout_ns + * Absolute timeout to wait + * + * The function will extract user space fences based on pointer and counts, + * then mapping them amdgpu fences and check if they are signaled or wait + * to timeout. + * + * Out data: + * + *__u32 status + * 0 CS completed + * 1 CS busy + * + *__u32 first_signaled; + * First signaled fence index + * + */ + /** * amdgpu_cs_wait_fences_ioctl - wait for multiple command submissions to finish * Any reason for not adding the above to the existing function documentation comment here? Yes. It's used by close source Vulkan, where it might be lack of BO fence dependencies implementations like we do in Mesa. Leo ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 2018-05-30 08:42 PM, Leo Liu wrote: > There are four ioctls in this files, and DOC gives details of > data structures for each of ioctls, and their functionalities. > > Signed-off-by: Leo Liu This isn't enough to actually make this part of the generated documentation. It needs to be hooked up to a *.rst file for that. I'm adding an amdgpu.rst file in https://patchwork.freedesktop.org/series/44035/ , where you could hook it up accordingly. Please check that generating the documentation (e.g. with make htmldocs) doesn't produce any warnings about amdgpu_cs.c, and that the result looks good in Documentation/output/gpu/amdgpu.html . The documentation format itself is documented in Documentation/output/doc-guide/index.html . > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 12f0d18c6ee8..343ff115cff1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, > return 0; > } > > +/** > + * DOC: amdgpu_cs_ioctl DOC comments shouldn't be used for functions, see Documentation/output/doc-guide/kernel-doc.html#function-documentation > @@ -1524,6 +1630,42 @@ static int amdgpu_cs_wait_any_fence(struct > amdgpu_device *adev, > return r; > } > > +/** > + * DOC: amdgpu_cs_wait_fences_ioctl > + * > + * This ioctl checks either all fences or any fence from multiple fences > + * to be signaled or waits to be signaled till timeout. So it's used to > + * check and wait multiple CS to be completed. > + * > + * In data structure: > + * > + * __u64 fences > + * Point to the multiple fences > + * > + * __u32 fence_count > + * number of fences > + * > + * __u32 wait_all > + * ways to wait either wait_all or wait_any > + * > + * __u64 timeout_ns > + * Absolute timeout to wait > + * > + * The function will extract user space fences based on pointer and counts, > + * then mapping them amdgpu fences and check if they are signaled or wait > + * to timeout. > + * > + * Out data: > + * > + *__u32 status > + * 0 CS completed > + * 1 CS busy > + * > + *__u32 first_signaled; > + * First signaled fence index > + * > + */ > + > /** > * amdgpu_cs_wait_fences_ioctl - wait for multiple command submissions to > finish > * Any reason for not adding the above to the existing function documentation comment here? -- 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] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On 05/31/2018 11:43 AM, Alex Deucher wrote:
On Wed, May 30, 2018 at 2:42 PM, Leo Liu wrote:
There are four ioctls in this files, and DOC gives details of
data structures for each of ioctls, and their functionalities.
Signed-off-by: Leo Liu
A few comments below about readability. With those fixed:
Reviewed-by: Alex Deucher
Thanks for the reviews, I will update it accordingly.
Leo
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 142 +
1 file changed, 142 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 12f0d18c6ee8..343ff115cff1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
return 0;
}
+/**
+ * DOC: amdgpu_cs_ioctl
+ *
+ * This ioctl processes user space command submission chunks,
Maybe give an overview of why we use the ioctl. Something like:
The CS (Command Submission) ioctl is used to submit command buffers to
the kernel for scheduling on hw queues. The chunk interface gives us
the flexibility to add new features to the ioctl by simply adding a
new chunk type.
+ * return a fence sequence number associated to a amdgpu fence.
+ *
+ * In data structure:
+ *
+ * __u32 ctx_id:
+ * Integer ID, created via DRM_AMDGPU_CTX ioctl call when user space
+ * command submission context created. It will be used as ID for later
+ * command submission context.
Required for all command submissions. The kernel uses this to track
dependencies and guilt for command submissions that lead to an engine
lock up.
+ *
+ * __u32 bo_list_handle:
+ * Handle of resources list associated with this CS, created via
+ * DRM_AMDGPU_BO_LIST ioctl call before command submission, and
+ * the BOs in the list will be validated.
to make sure they are resident when the command buffer executes.
+ *
+ * __u32 num_chunks:
+ * Number of chunks, their types include:
+ * AMDGPU_CHUNK_ID_IB
+ * The data will be filled into IB buffer, and mappped to a HW ring.
+ *
Expand this a bit. E.g.,
An IB (Indirect Buffer) is a stand alone command buffer that can be
scheduled fetching for execution on a hw queue.
+ * AMDGPU_CHUNK_ID_FENCE
+ * The data will be used to find user fence BO and its offset.
+ *
Expand this a bit. E.g., add something like:
Allows user space to specify their own fences within the command stream.
+ * AMDGPU_CHUNK_ID_DEPENDENCIES
+ * AMDGPU_CHUNK_ID_SYNCOBJ_IN
+ * AMDGPU_CHUNK_ID_SYNCOBJ_OUT
+ * These will be parsed as fence dependencies in given requirement,
+ * and will be remembered and to be synced later.
+ *
+ * __u32 _pad:
+ *
+ * __u64 chunks:
+ * Point to the CS chunks.
Pointer
+ *
+ * amdgpu_cs_submit() function will be called to initialize a scheduler
+ * job, and associate it to a HW ring, add a new fence to the context,
associate it with a
+ * and then push the job to the queue for scheduler to process,
+ * it will return fence sequence number to user space.
+ *
+ */
int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
{
struct amdgpu_device *adev = dev->dev_private;
@@ -1272,6 +1315,38 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
return r;
}
+/**
+ * DOC: amdgpu_cs_wait_ioctl
+ *
+ * This ioctl checks a user space fence associated to a amdgpu fence whether
+ * it's signaled or waits to be signaled till timeout with kernel dma fence.
This reads sort of oddly. how about:
The CS (Command Submission) wait ioctl waits for the specified fence
on the specified hw queue to signal or for the timeout, whichever
comes first.
+ * Once signaled, the associated CS is completed.
+ *
+ * In data structure:
+ *
+ * __u64 handle:
+ * The fence sequence number from amdgpu_cs_ioctl returned.
+ *
+ * __u64 timeout:
+ * Absolute timeout to wait.
+ *
+ * __u32 ip_type:
+ * __u32 ip_instance:
+ * __u32 ring:
+ * Map user space ring to a kernel HW ring, then use the seq(handle) to
+ * find the amdgpu fence, that will be checked and waited.
+ *
+ * __u32 ctx_id:
+ * ID for command submission context
+ *
+ * Out data:
+ *
+ * __u64 status:
+ * 0 CS completed
+ * 1 CS busy
+ *
+ */
+
/**
* amdgpu_cs_wait_ioctl - wait for a command submission to finish
*
@@ -1358,6 +1433,37 @@ static struct dma_fence *amdgpu_cs_get_fence(struct
amdgpu_device *adev,
return fence;
}
+/**
+ * DOC: amdgpu_cs_fence_to_handle_ioctl
+ *
+ * This ioctl converts a user space fence into a fence object handle or fd,
+ * or file fd based on the purpose in “what”, since using handles or fd will
+ * be more efficient than ioctl call from user space to check signaled.
This reads sort of oddly. how about:
The CS (Command Submission) fence to handle ioctl converts a CS fence
into a drm sync object as specified in the what parameter.
+ *
+ * In data structure:
+ *
+ * struct drm_amdgpu_fence fence:
+
Re: [PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
On Wed, May 30, 2018 at 2:42 PM, Leo Liu wrote:
> There are four ioctls in this files, and DOC gives details of
> data structures for each of ioctls, and their functionalities.
>
> Signed-off-by: Leo Liu
A few comments below about readability. With those fixed:
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 142 +
> 1 file changed, 142 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 12f0d18c6ee8..343ff115cff1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
> return 0;
> }
>
> +/**
> + * DOC: amdgpu_cs_ioctl
> + *
> + * This ioctl processes user space command submission chunks,
Maybe give an overview of why we use the ioctl. Something like:
The CS (Command Submission) ioctl is used to submit command buffers to
the kernel for scheduling on hw queues. The chunk interface gives us
the flexibility to add new features to the ioctl by simply adding a
new chunk type.
> + * return a fence sequence number associated to a amdgpu fence.
> + *
> + * In data structure:
> + *
> + * __u32 ctx_id:
> + * Integer ID, created via DRM_AMDGPU_CTX ioctl call when user space
> + * command submission context created. It will be used as ID for later
> + * command submission context.
Required for all command submissions. The kernel uses this to track
dependencies and guilt for command submissions that lead to an engine
lock up.
> + *
> + * __u32 bo_list_handle:
> + * Handle of resources list associated with this CS, created via
> + * DRM_AMDGPU_BO_LIST ioctl call before command submission, and
> + * the BOs in the list will be validated.
to make sure they are resident when the command buffer executes.
> + *
> + * __u32 num_chunks:
> + * Number of chunks, their types include:
> + * AMDGPU_CHUNK_ID_IB
> + * The data will be filled into IB buffer, and mappped to a HW ring.
> + *
Expand this a bit. E.g.,
An IB (Indirect Buffer) is a stand alone command buffer that can be
scheduled fetching for execution on a hw queue.
> + * AMDGPU_CHUNK_ID_FENCE
> + * The data will be used to find user fence BO and its offset.
> + *
Expand this a bit. E.g., add something like:
Allows user space to specify their own fences within the command stream.
> + * AMDGPU_CHUNK_ID_DEPENDENCIES
> + * AMDGPU_CHUNK_ID_SYNCOBJ_IN
> + * AMDGPU_CHUNK_ID_SYNCOBJ_OUT
> + * These will be parsed as fence dependencies in given requirement,
> + * and will be remembered and to be synced later.
> + *
> + * __u32 _pad:
> + *
> + * __u64 chunks:
> + * Point to the CS chunks.
Pointer
> + *
> + * amdgpu_cs_submit() function will be called to initialize a scheduler
> + * job, and associate it to a HW ring, add a new fence to the context,
associate it with a
> + * and then push the job to the queue for scheduler to process,
> + * it will return fence sequence number to user space.
> + *
> + */
> int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file
> *filp)
> {
> struct amdgpu_device *adev = dev->dev_private;
> @@ -1272,6 +1315,38 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void
> *data, struct drm_file *filp)
> return r;
> }
>
> +/**
> + * DOC: amdgpu_cs_wait_ioctl
> + *
> + * This ioctl checks a user space fence associated to a amdgpu fence whether
> + * it's signaled or waits to be signaled till timeout with kernel dma fence.
This reads sort of oddly. how about:
The CS (Command Submission) wait ioctl waits for the specified fence
on the specified hw queue to signal or for the timeout, whichever
comes first.
> + * Once signaled, the associated CS is completed.
> + *
> + * In data structure:
> + *
> + * __u64 handle:
> + * The fence sequence number from amdgpu_cs_ioctl returned.
> + *
> + * __u64 timeout:
> + * Absolute timeout to wait.
> + *
> + * __u32 ip_type:
> + * __u32 ip_instance:
> + * __u32 ring:
> + * Map user space ring to a kernel HW ring, then use the seq(handle) to
> + * find the amdgpu fence, that will be checked and waited.
> + *
> + * __u32 ctx_id:
> + * ID for command submission context
> + *
> + * Out data:
> + *
> + * __u64 status:
> + * 0 CS completed
> + * 1 CS busy
> + *
> + */
> +
> /**
> * amdgpu_cs_wait_ioctl - wait for a command submission to finish
> *
> @@ -1358,6 +1433,37 @@ static struct dma_fence *amdgpu_cs_get_fence(struct
> amdgpu_device *adev,
> return fence;
> }
>
> +/**
> + * DOC: amdgpu_cs_fence_to_handle_ioctl
> + *
> + * This ioctl converts a user space fence into a fence object handle or fd,
> + * or file fd based on the purpose in “what”, since using handles or fd will
> + * be more efficient than ioctl call from user space to check signaled.
This reads sort of oddly. how about:
The CS (Command Submission) fence to handle ioctl converts a CS fence
into a drm sync object as specified in
[PATCH] drm/amdgpu: add kernel DOC for ioctls in amdgpu_cs.c file
There are four ioctls in this files, and DOC gives details of
data structures for each of ioctls, and their functionalities.
Signed-off-by: Leo Liu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 142 +
1 file changed, 142 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 12f0d18c6ee8..343ff115cff1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1217,6 +1217,49 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
return 0;
}
+/**
+ * DOC: amdgpu_cs_ioctl
+ *
+ * This ioctl processes user space command submission chunks,
+ * return a fence sequence number associated to a amdgpu fence.
+ *
+ * In data structure:
+ *
+ * __u32 ctx_id:
+ * Integer ID, created via DRM_AMDGPU_CTX ioctl call when user space
+ * command submission context created. It will be used as ID for later
+ * command submission context.
+ *
+ * __u32 bo_list_handle:
+ * Handle of resources list associated with this CS, created via
+ * DRM_AMDGPU_BO_LIST ioctl call before command submission, and
+ * the BOs in the list will be validated.
+ *
+ * __u32 num_chunks:
+ * Number of chunks, their types include:
+ * AMDGPU_CHUNK_ID_IB
+ * The data will be filled into IB buffer, and mappped to a HW ring.
+ *
+ * AMDGPU_CHUNK_ID_FENCE
+ * The data will be used to find user fence BO and its offset.
+ *
+ * AMDGPU_CHUNK_ID_DEPENDENCIES
+ * AMDGPU_CHUNK_ID_SYNCOBJ_IN
+ * AMDGPU_CHUNK_ID_SYNCOBJ_OUT
+ * These will be parsed as fence dependencies in given requirement,
+ * and will be remembered and to be synced later.
+ *
+ * __u32 _pad:
+ *
+ * __u64 chunks:
+ * Point to the CS chunks.
+ *
+ * amdgpu_cs_submit() function will be called to initialize a scheduler
+ * job, and associate it to a HW ring, add a new fence to the context,
+ * and then push the job to the queue for scheduler to process,
+ * it will return fence sequence number to user space.
+ *
+ */
int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
{
struct amdgpu_device *adev = dev->dev_private;
@@ -1272,6 +1315,38 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
return r;
}
+/**
+ * DOC: amdgpu_cs_wait_ioctl
+ *
+ * This ioctl checks a user space fence associated to a amdgpu fence whether
+ * it's signaled or waits to be signaled till timeout with kernel dma fence.
+ * Once signaled, the associated CS is completed.
+ *
+ * In data structure:
+ *
+ * __u64 handle:
+ * The fence sequence number from amdgpu_cs_ioctl returned.
+ *
+ * __u64 timeout:
+ * Absolute timeout to wait.
+ *
+ * __u32 ip_type:
+ * __u32 ip_instance:
+ * __u32 ring:
+ * Map user space ring to a kernel HW ring, then use the seq(handle) to
+ * find the amdgpu fence, that will be checked and waited.
+ *
+ * __u32 ctx_id:
+ * ID for command submission context
+ *
+ * Out data:
+ *
+ * __u64 status:
+ * 0 CS completed
+ * 1 CS busy
+ *
+ */
+
/**
* amdgpu_cs_wait_ioctl - wait for a command submission to finish
*
@@ -1358,6 +1433,37 @@ static struct dma_fence *amdgpu_cs_get_fence(struct
amdgpu_device *adev,
return fence;
}
+/**
+ * DOC: amdgpu_cs_fence_to_handle_ioctl
+ *
+ * This ioctl converts a user space fence into a fence object handle or fd,
+ * or file fd based on the purpose in “what”, since using handles or fd will
+ * be more efficient than ioctl call from user space to check signaled.
+ *
+ * In data structure:
+ *
+ * struct drm_amdgpu_fence fence:
+ * User space fence structured with ctx_id, ip_type, ip_instance, ring, seq_no
+ *
+ * __u32 what:
+ * Types of handle to convert include:
+ *
+ * AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ
+ * It is required to return a process-dependent handle for a sync object
+ *
+ * AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ_FD
+ * It is required to return a process-independent fd for a sync object
+ *
+ * AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD
+ * It is required to return a process-independent fd to a sync file
+ *
+ * __u32 pad
+ *
+ * Out data:
+ * __u32 handle:
+ * Handle to be returned to user space
+ *
+ */
int amdgpu_cs_fence_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
@@ -1524,6 +1630,42 @@ static int amdgpu_cs_wait_any_fence(struct amdgpu_device
*adev,
return r;
}
+/**
+ * DOC: amdgpu_cs_wait_fences_ioctl
+ *
+ * This ioctl checks either all fences or any fence from multiple fences
+ * to be signaled or waits to be signaled till timeout. So it's used to
+ * check and wait multiple CS to be completed.
+ *
+ * In data structure:
+ *
+ * __u64 fences
+ * Point to the multiple fences
+ *
+ * __u32 fence_count
+ * number of fences
+ *
+ * __u32 wait_all
+ * ways to wait either wait_all or wait_any
+ *
+ * __u64 timeout_ns
+ * Absolute timeout to wait
+ *
+ * The function will extract user space fences based on pointer and counts,
+ * then mapping them
