| Message ID | 89ab5cf179566c429383cc57db746038f75cba0d.1683041799.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: fix kernel-doc warnings, enable kernel-doc -Werror | expand |
On Tue, May 02, 2023 at 06:37:22PM +0300, Jani Nikula wrote: > You can't document both a sub-struct type and a struct member at the > same time. Separate them. another way would be to kill the 'i915_vma_bindinfo' name entirely and document only as '@bi:' and then move the individual documentations near their definitions. > > drivers/gpu/drm/i915/i915_vma_resource.h:91: warning: Incorrect use of kernel-doc format: * struct i915_vma_bindinfo - Information needed for async bind > drivers/gpu/drm/i915/i915_vma_resource.h:129: warning: Function parameter or member 'bi' not described in 'i915_vma_resource' > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_vma_resource.h | 45 ++++++++++++++---------- > 1 file changed, 27 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_vma_resource.h b/drivers/gpu/drm/i915/i915_vma_resource.h > index 2053c037dbfb..ca2b0f7f59bc 100644 > --- a/drivers/gpu/drm/i915/i915_vma_resource.h > +++ b/drivers/gpu/drm/i915/i915_vma_resource.h > @@ -33,6 +33,27 @@ struct i915_page_sizes { > unsigned int sg; > }; > > +/** > + * struct i915_vma_bindinfo - Information needed for async bind > + * only but that can be dropped after the bind has taken place. > + * Consider making this a separate argument to the bind_vma > + * op, coalescing with other arguments like vm, stash, cache_level > + * and flags > + * @pages: The pages sg-table. > + * @page_sizes: Page sizes of the pages. > + * @pages_rsgt: Refcounted sg-table when delayed object destruction > + * is supported. May be NULL. > + * @readonly: Whether the vma should be bound read-only. > + * @lmem: Whether the vma points to lmem. > + */ > +struct i915_vma_bindinfo { > + struct sg_table *pages; > + struct i915_page_sizes page_sizes; > + struct i915_refct_sgt *pages_rsgt; > + bool readonly:1; > + bool lmem:1; btw, I believe we should move all the individual docs near to its declarations. easier to forget updating the documentation when updating fields here. > +}; > + > /** > * struct i915_vma_resource - Snapshotted unbind information. > * @unbind_fence: Fence to mark unbinding complete. Note that this fence > @@ -89,25 +110,13 @@ struct i915_vma_resource { > intel_wakeref_t wakeref; > > /** > - * struct i915_vma_bindinfo - Information needed for async bind > - * only but that can be dropped after the bind has taken place. > - * Consider making this a separate argument to the bind_vma > - * op, coalescing with other arguments like vm, stash, cache_level > - * and flags > - * @pages: The pages sg-table. > - * @page_sizes: Page sizes of the pages. > - * @pages_rsgt: Refcounted sg-table when delayed object destruction > - * is supported. May be NULL. > - * @readonly: Whether the vma should be bound read-only. > - * @lmem: Whether the vma points to lmem. > + * @bi: Information needed for async bind only but that can be dropped > + * after the bind has taken place. > + * > + * Consider making this a separate argument to the bind_vma op, > + * coalescing with other arguments like vm, stash, cache_level and flags > */ > - struct i915_vma_bindinfo { > - struct sg_table *pages; > - struct i915_page_sizes page_sizes; > - struct i915_refct_sgt *pages_rsgt; > - bool readonly:1; > - bool lmem:1; > - } bi; > + struct i915_vma_bindinfo bi; > > #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) > struct intel_memory_region *mr; > -- > 2.39.2 >
On Wed, 03 May 2023, Rodrigo Vivi <rodrigo.vivi@kernel.org> wrote: > On Tue, May 02, 2023 at 06:37:22PM +0300, Jani Nikula wrote: >> You can't document both a sub-struct type and a struct member at the >> same time. Separate them. > > another way would be to kill the 'i915_vma_bindinfo' name entirely and > document only as '@bi:' and then move the individual documentations near > their definitions. I don't think that will work, because AFAIK kernel-doc does not descend into struct members recursively. You can either declare and document the sub-structs as separate types (the patch at hand), or you can document sub-struct members directly from the embedding struct as shown below. I don't think the latter is very nice. BR, Jani. index 2053c037dbfb..ee767cc4de43 100644 --- a/drivers/gpu/drm/i915/i915_vma_resource.h +++ b/drivers/gpu/drm/i915/i915_vma_resource.h @@ -48,6 +48,12 @@ struct i915_page_sizes { * @rb: Rb node for the vm's pending unbind interval tree. * @__subtree_last: Interval tree private member. * @wakeref: wakeref. + * @bi.pages: The pages sg-table. + * @bi.page_sizes: Page sizes of the pages. + * @bi.pages_rsgt: Refcounted sg-table when delayed object destruction + * is supported. May be NULL. + * @bi.readonly: Whether the vma should be bound read-only. + * @bi.lmem: Whether the vma points to lmem. * @vm: non-refcounted pointer to the vm. This is for internal use only and * this member is cleared after vm_resource unbind. * @mr: The memory region of the object pointed to by the vma. @@ -89,17 +95,11 @@ struct i915_vma_resource { intel_wakeref_t wakeref; /** - * struct i915_vma_bindinfo - Information needed for async bind - * only but that can be dropped after the bind has taken place. - * Consider making this a separate argument to the bind_vma - * op, coalescing with other arguments like vm, stash, cache_level - * and flags - * @pages: The pages sg-table. - * @page_sizes: Page sizes of the pages. - * @pages_rsgt: Refcounted sg-table when delayed object destruction - * is supported. May be NULL. - * @readonly: Whether the vma should be bound read-only. - * @lmem: Whether the vma points to lmem. + * @bi: Information needed for async bind only but that can be dropped + * after the bind has taken place. + * + * Consider making this a separate argument to the bind_vma op, + * coalescing with other arguments like vm, stash, cache_level and flags */ struct i915_vma_bindinfo { struct sg_table *pages; > >> >> drivers/gpu/drm/i915/i915_vma_resource.h:91: warning: Incorrect use of kernel-doc format: * struct i915_vma_bindinfo - Information needed for async bind >> drivers/gpu/drm/i915/i915_vma_resource.h:129: warning: Function parameter or member 'bi' not described in 'i915_vma_resource' >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/i915_vma_resource.h | 45 ++++++++++++++---------- >> 1 file changed, 27 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_vma_resource.h b/drivers/gpu/drm/i915/i915_vma_resource.h >> index 2053c037dbfb..ca2b0f7f59bc 100644 >> --- a/drivers/gpu/drm/i915/i915_vma_resource.h >> +++ b/drivers/gpu/drm/i915/i915_vma_resource.h >> @@ -33,6 +33,27 @@ struct i915_page_sizes { >> unsigned int sg; >> }; >> >> +/** >> + * struct i915_vma_bindinfo - Information needed for async bind >> + * only but that can be dropped after the bind has taken place. >> + * Consider making this a separate argument to the bind_vma >> + * op, coalescing with other arguments like vm, stash, cache_level >> + * and flags >> + * @pages: The pages sg-table. >> + * @page_sizes: Page sizes of the pages. >> + * @pages_rsgt: Refcounted sg-table when delayed object destruction >> + * is supported. May be NULL. >> + * @readonly: Whether the vma should be bound read-only. >> + * @lmem: Whether the vma points to lmem. >> + */ >> +struct i915_vma_bindinfo { >> + struct sg_table *pages; >> + struct i915_page_sizes page_sizes; >> + struct i915_refct_sgt *pages_rsgt; >> + bool readonly:1; >> + bool lmem:1; > > btw, I believe we should move all the individual docs near to its > declarations. easier to forget updating the documentation when updating > fields here. > >> +}; >> + >> /** >> * struct i915_vma_resource - Snapshotted unbind information. >> * @unbind_fence: Fence to mark unbinding complete. Note that this fence >> @@ -89,25 +110,13 @@ struct i915_vma_resource { >> intel_wakeref_t wakeref; >> >> /** >> - * struct i915_vma_bindinfo - Information needed for async bind >> - * only but that can be dropped after the bind has taken place. >> - * Consider making this a separate argument to the bind_vma >> - * op, coalescing with other arguments like vm, stash, cache_level >> - * and flags >> - * @pages: The pages sg-table. >> - * @page_sizes: Page sizes of the pages. >> - * @pages_rsgt: Refcounted sg-table when delayed object destruction >> - * is supported. May be NULL. >> - * @readonly: Whether the vma should be bound read-only. >> - * @lmem: Whether the vma points to lmem. >> + * @bi: Information needed for async bind only but that can be dropped >> + * after the bind has taken place. >> + * >> + * Consider making this a separate argument to the bind_vma op, >> + * coalescing with other arguments like vm, stash, cache_level and flags >> */ >> - struct i915_vma_bindinfo { >> - struct sg_table *pages; >> - struct i915_page_sizes page_sizes; >> - struct i915_refct_sgt *pages_rsgt; >> - bool readonly:1; >> - bool lmem:1; >> - } bi; >> + struct i915_vma_bindinfo bi; >> >> #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) >> struct intel_memory_region *mr; >> -- >> 2.39.2 >>
On Thu, May 04, 2023 at 12:43:05PM +0300, Jani Nikula wrote: > On Wed, 03 May 2023, Rodrigo Vivi <rodrigo.vivi@kernel.org> wrote: > > On Tue, May 02, 2023 at 06:37:22PM +0300, Jani Nikula wrote: > >> You can't document both a sub-struct type and a struct member at the > >> same time. Separate them. > > > > another way would be to kill the 'i915_vma_bindinfo' name entirely and > > document only as '@bi:' and then move the individual documentations near > > their definitions. > > I don't think that will work, because AFAIK kernel-doc does not descend > into struct members recursively. It does: 'In-line member documentation comments' definition of: Documentation/doc-guide/kernel-doc.rst > > You can either declare and document the sub-structs as separate types > (the patch at hand), or you can document sub-struct members directly > from the embedding struct as shown below. I don't think the latter is > very nice. what I had in mind was something more like: /** - * struct i915_vma_bindinfo - Information needed for async bind - * only but that can be dropped after the bind has taken place. - * Consider making this a separate argument to the bind_vma - * op, coalescing with other arguments like vm, stash, cache_level - * and flags - * @pages: The pages sg-table. - * @page_sizes: Page sizes of the pages. - * @pages_rsgt: Refcounted sg-table when delayed object destruction - * is supported. May be NULL. - * @readonly: Whether the vma should be bound read-only. - * @lmem: Whether the vma points to lmem. + * @bi: Information needed for async bind only but that can be dropped + * after the bind has taken place. + * Consider making this a separate argument to the bind_vma + * op, coalescing with other arguments like vm, stash, cache_level + * and flags */ - struct i915_vma_bindinfo { + struct { + /** @pages: The pages sg-table. */ struct sg_table *pages; + /** @page_sizes: Page sizes of the pages. */ struct i915_page_sizes page_sizes; + /** + * @pages_rsgt: Refcounted sg-table when delayed object + * destruction is supported. May be NULL. + */ struct i915_refct_sgt *pages_rsgt; + /** @readonly: Whether the vma should be bound read-only. */ bool readonly:1; + /** @lmem: Whether the vma points to lmem. */ bool lmem:1; } bi; But let's not block the progress of the much needed fixes. It's your call: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > BR, > Jani. > > > index 2053c037dbfb..ee767cc4de43 100644 > --- a/drivers/gpu/drm/i915/i915_vma_resource.h > +++ b/drivers/gpu/drm/i915/i915_vma_resource.h > @@ -48,6 +48,12 @@ struct i915_page_sizes { > * @rb: Rb node for the vm's pending unbind interval tree. > * @__subtree_last: Interval tree private member. > * @wakeref: wakeref. > + * @bi.pages: The pages sg-table. > + * @bi.page_sizes: Page sizes of the pages. > + * @bi.pages_rsgt: Refcounted sg-table when delayed object destruction > + * is supported. May be NULL. > + * @bi.readonly: Whether the vma should be bound read-only. > + * @bi.lmem: Whether the vma points to lmem. > * @vm: non-refcounted pointer to the vm. This is for internal use only and > * this member is cleared after vm_resource unbind. > * @mr: The memory region of the object pointed to by the vma. > @@ -89,17 +95,11 @@ struct i915_vma_resource { > intel_wakeref_t wakeref; > > /** > - * struct i915_vma_bindinfo - Information needed for async bind > - * only but that can be dropped after the bind has taken place. > - * Consider making this a separate argument to the bind_vma > - * op, coalescing with other arguments like vm, stash, cache_level > - * and flags > - * @pages: The pages sg-table. > - * @page_sizes: Page sizes of the pages. > - * @pages_rsgt: Refcounted sg-table when delayed object destruction > - * is supported. May be NULL. > - * @readonly: Whether the vma should be bound read-only. > - * @lmem: Whether the vma points to lmem. > + * @bi: Information needed for async bind only but that can be dropped > + * after the bind has taken place. > + * > + * Consider making this a separate argument to the bind_vma op, > + * coalescing with other arguments like vm, stash, cache_level and flags > */ > struct i915_vma_bindinfo { > struct sg_table *pages; > > > > > >> > >> drivers/gpu/drm/i915/i915_vma_resource.h:91: warning: Incorrect use of kernel-doc format: * struct i915_vma_bindinfo - Information needed for async bind > >> drivers/gpu/drm/i915/i915_vma_resource.h:129: warning: Function parameter or member 'bi' not described in 'i915_vma_resource' > >> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_vma_resource.h | 45 ++++++++++++++---------- > >> 1 file changed, 27 insertions(+), 18 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_vma_resource.h b/drivers/gpu/drm/i915/i915_vma_resource.h > >> index 2053c037dbfb..ca2b0f7f59bc 100644 > >> --- a/drivers/gpu/drm/i915/i915_vma_resource.h > >> +++ b/drivers/gpu/drm/i915/i915_vma_resource.h > >> @@ -33,6 +33,27 @@ struct i915_page_sizes { > >> unsigned int sg; > >> }; > >> > >> +/** > >> + * struct i915_vma_bindinfo - Information needed for async bind > >> + * only but that can be dropped after the bind has taken place. > >> + * Consider making this a separate argument to the bind_vma > >> + * op, coalescing with other arguments like vm, stash, cache_level > >> + * and flags > >> + * @pages: The pages sg-table. > >> + * @page_sizes: Page sizes of the pages. > >> + * @pages_rsgt: Refcounted sg-table when delayed object destruction > >> + * is supported. May be NULL. > >> + * @readonly: Whether the vma should be bound read-only. > >> + * @lmem: Whether the vma points to lmem. > >> + */ > >> +struct i915_vma_bindinfo { > >> + struct sg_table *pages; > >> + struct i915_page_sizes page_sizes; > >> + struct i915_refct_sgt *pages_rsgt; > >> + bool readonly:1; > >> + bool lmem:1; > > > > btw, I believe we should move all the individual docs near to its > > declarations. easier to forget updating the documentation when updating > > fields here. > > > >> +}; > >> + > >> /** > >> * struct i915_vma_resource - Snapshotted unbind information. > >> * @unbind_fence: Fence to mark unbinding complete. Note that this fence > >> @@ -89,25 +110,13 @@ struct i915_vma_resource { > >> intel_wakeref_t wakeref; > >> > >> /** > >> - * struct i915_vma_bindinfo - Information needed for async bind > >> - * only but that can be dropped after the bind has taken place. > >> - * Consider making this a separate argument to the bind_vma > >> - * op, coalescing with other arguments like vm, stash, cache_level > >> - * and flags > >> - * @pages: The pages sg-table. > >> - * @page_sizes: Page sizes of the pages. > >> - * @pages_rsgt: Refcounted sg-table when delayed object destruction > >> - * is supported. May be NULL. > >> - * @readonly: Whether the vma should be bound read-only. > >> - * @lmem: Whether the vma points to lmem. > >> + * @bi: Information needed for async bind only but that can be dropped > >> + * after the bind has taken place. > >> + * > >> + * Consider making this a separate argument to the bind_vma op, > >> + * coalescing with other arguments like vm, stash, cache_level and flags > >> */ > >> - struct i915_vma_bindinfo { > >> - struct sg_table *pages; > >> - struct i915_page_sizes page_sizes; > >> - struct i915_refct_sgt *pages_rsgt; > >> - bool readonly:1; > >> - bool lmem:1; > >> - } bi; > >> + struct i915_vma_bindinfo bi; > >> > >> #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) > >> struct intel_memory_region *mr; > >> -- > >> 2.39.2 > >> > > -- > Jani Nikula, Intel Open Source Graphics Center
On Thu, 04 May 2023, Rodrigo Vivi <rodrigo.vivi@kernel.org> wrote: > On Thu, May 04, 2023 at 12:43:05PM +0300, Jani Nikula wrote: >> On Wed, 03 May 2023, Rodrigo Vivi <rodrigo.vivi@kernel.org> wrote: >> > On Tue, May 02, 2023 at 06:37:22PM +0300, Jani Nikula wrote: >> >> You can't document both a sub-struct type and a struct member at the >> >> same time. Separate them. >> > >> > another way would be to kill the 'i915_vma_bindinfo' name entirely and >> > document only as '@bi:' and then move the individual documentations near >> > their definitions. >> >> I don't think that will work, because AFAIK kernel-doc does not descend >> into struct members recursively. > > It does: > > 'In-line member documentation comments' definition of: > Documentation/doc-guide/kernel-doc.rst Regardless of what that says, I can't make it work. :( $ scripts/kernel-doc -function i915_vma_resource drivers/gpu/drm/i915/i915_vma_resource.h only documents ``bi`` but not the sub-struct. Regardless of whether I refer to the members as @bi.pages: or @pages: etc. Let me know if and how you can make it work. BR, Jani. > >> >> You can either declare and document the sub-structs as separate types >> (the patch at hand), or you can document sub-struct members directly >> from the embedding struct as shown below. I don't think the latter is >> very nice. > > what I had in mind was something more like: > > /** > - * struct i915_vma_bindinfo - Information needed for async bind > - * only but that can be dropped after the bind has taken place. > - * Consider making this a separate argument to the bind_vma > - * op, coalescing with other arguments like vm, stash, cache_level > - * and flags > - * @pages: The pages sg-table. > - * @page_sizes: Page sizes of the pages. > - * @pages_rsgt: Refcounted sg-table when delayed object destruction > - * is supported. May be NULL. > - * @readonly: Whether the vma should be bound read-only. > - * @lmem: Whether the vma points to lmem. > + * @bi: Information needed for async bind only but that can be dropped > + * after the bind has taken place. > + * Consider making this a separate argument to the bind_vma > + * op, coalescing with other arguments like vm, stash, cache_level > + * and flags > */ > - struct i915_vma_bindinfo { > + struct { > + /** @pages: The pages sg-table. */ > struct sg_table *pages; > + /** @page_sizes: Page sizes of the pages. */ > struct i915_page_sizes page_sizes; > + /** > + * @pages_rsgt: Refcounted sg-table when delayed object > + * destruction is supported. May be NULL. > + */ > struct i915_refct_sgt *pages_rsgt; > + /** @readonly: Whether the vma should be bound read-only. */ > bool readonly:1; > + /** @lmem: Whether the vma points to lmem. */ > bool lmem:1; > } bi; > > > But let's not block the progress of the much needed fixes. It's your call: > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > >> >> BR, >> Jani. >> >> >> index 2053c037dbfb..ee767cc4de43 100644 >> --- a/drivers/gpu/drm/i915/i915_vma_resource.h >> +++ b/drivers/gpu/drm/i915/i915_vma_resource.h >> @@ -48,6 +48,12 @@ struct i915_page_sizes { >> * @rb: Rb node for the vm's pending unbind interval tree. >> * @__subtree_last: Interval tree private member. >> * @wakeref: wakeref. >> + * @bi.pages: The pages sg-table. >> + * @bi.page_sizes: Page sizes of the pages. >> + * @bi.pages_rsgt: Refcounted sg-table when delayed object destruction >> + * is supported. May be NULL. >> + * @bi.readonly: Whether the vma should be bound read-only. >> + * @bi.lmem: Whether the vma points to lmem. >> * @vm: non-refcounted pointer to the vm. This is for internal use only and >> * this member is cleared after vm_resource unbind. >> * @mr: The memory region of the object pointed to by the vma. >> @@ -89,17 +95,11 @@ struct i915_vma_resource { >> intel_wakeref_t wakeref; >> >> /** >> - * struct i915_vma_bindinfo - Information needed for async bind >> - * only but that can be dropped after the bind has taken place. >> - * Consider making this a separate argument to the bind_vma >> - * op, coalescing with other arguments like vm, stash, cache_level >> - * and flags >> - * @pages: The pages sg-table. >> - * @page_sizes: Page sizes of the pages. >> - * @pages_rsgt: Refcounted sg-table when delayed object destruction >> - * is supported. May be NULL. >> - * @readonly: Whether the vma should be bound read-only. >> - * @lmem: Whether the vma points to lmem. >> + * @bi: Information needed for async bind only but that can be dropped >> + * after the bind has taken place. >> + * >> + * Consider making this a separate argument to the bind_vma op, >> + * coalescing with other arguments like vm, stash, cache_level and flags >> */ >> struct i915_vma_bindinfo { >> struct sg_table *pages; >> >> >> > >> >> >> >> drivers/gpu/drm/i915/i915_vma_resource.h:91: warning: Incorrect use of kernel-doc format: * struct i915_vma_bindinfo - Information needed for async bind >> >> drivers/gpu/drm/i915/i915_vma_resource.h:129: warning: Function parameter or member 'bi' not described in 'i915_vma_resource' >> >> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/i915_vma_resource.h | 45 ++++++++++++++---------- >> >> 1 file changed, 27 insertions(+), 18 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/i915_vma_resource.h b/drivers/gpu/drm/i915/i915_vma_resource.h >> >> index 2053c037dbfb..ca2b0f7f59bc 100644 >> >> --- a/drivers/gpu/drm/i915/i915_vma_resource.h >> >> +++ b/drivers/gpu/drm/i915/i915_vma_resource.h >> >> @@ -33,6 +33,27 @@ struct i915_page_sizes { >> >> unsigned int sg; >> >> }; >> >> >> >> +/** >> >> + * struct i915_vma_bindinfo - Information needed for async bind >> >> + * only but that can be dropped after the bind has taken place. >> >> + * Consider making this a separate argument to the bind_vma >> >> + * op, coalescing with other arguments like vm, stash, cache_level >> >> + * and flags >> >> + * @pages: The pages sg-table. >> >> + * @page_sizes: Page sizes of the pages. >> >> + * @pages_rsgt: Refcounted sg-table when delayed object destruction >> >> + * is supported. May be NULL. >> >> + * @readonly: Whether the vma should be bound read-only. >> >> + * @lmem: Whether the vma points to lmem. >> >> + */ >> >> +struct i915_vma_bindinfo { >> >> + struct sg_table *pages; >> >> + struct i915_page_sizes page_sizes; >> >> + struct i915_refct_sgt *pages_rsgt; >> >> + bool readonly:1; >> >> + bool lmem:1; >> > >> > btw, I believe we should move all the individual docs near to its >> > declarations. easier to forget updating the documentation when updating >> > fields here. >> > >> >> +}; >> >> + >> >> /** >> >> * struct i915_vma_resource - Snapshotted unbind information. >> >> * @unbind_fence: Fence to mark unbinding complete. Note that this fence >> >> @@ -89,25 +110,13 @@ struct i915_vma_resource { >> >> intel_wakeref_t wakeref; >> >> >> >> /** >> >> - * struct i915_vma_bindinfo - Information needed for async bind >> >> - * only but that can be dropped after the bind has taken place. >> >> - * Consider making this a separate argument to the bind_vma >> >> - * op, coalescing with other arguments like vm, stash, cache_level >> >> - * and flags >> >> - * @pages: The pages sg-table. >> >> - * @page_sizes: Page sizes of the pages. >> >> - * @pages_rsgt: Refcounted sg-table when delayed object destruction >> >> - * is supported. May be NULL. >> >> - * @readonly: Whether the vma should be bound read-only. >> >> - * @lmem: Whether the vma points to lmem. >> >> + * @bi: Information needed for async bind only but that can be dropped >> >> + * after the bind has taken place. >> >> + * >> >> + * Consider making this a separate argument to the bind_vma op, >> >> + * coalescing with other arguments like vm, stash, cache_level and flags >> >> */ >> >> - struct i915_vma_bindinfo { >> >> - struct sg_table *pages; >> >> - struct i915_page_sizes page_sizes; >> >> - struct i915_refct_sgt *pages_rsgt; >> >> - bool readonly:1; >> >> - bool lmem:1; >> >> - } bi; >> >> + struct i915_vma_bindinfo bi; >> >> >> >> #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) >> >> struct intel_memory_region *mr; >> >> -- >> >> 2.39.2 >> >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
On Thu, 04 May 2023, Rodrigo Vivi <rodrigo.vivi@kernel.org> wrote: > But let's not block the progress of the much needed fixes. It's your call: > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Thanks, pushed to drm-intel-gt-next as-is. BR, Jani.
diff --git a/drivers/gpu/drm/i915/i915_vma_resource.h b/drivers/gpu/drm/i915/i915_vma_resource.h index 2053c037dbfb..ca2b0f7f59bc 100644 --- a/drivers/gpu/drm/i915/i915_vma_resource.h +++ b/drivers/gpu/drm/i915/i915_vma_resource.h @@ -33,6 +33,27 @@ struct i915_page_sizes { unsigned int sg; }; +/** + * struct i915_vma_bindinfo - Information needed for async bind + * only but that can be dropped after the bind has taken place. + * Consider making this a separate argument to the bind_vma + * op, coalescing with other arguments like vm, stash, cache_level + * and flags + * @pages: The pages sg-table. + * @page_sizes: Page sizes of the pages. + * @pages_rsgt: Refcounted sg-table when delayed object destruction + * is supported. May be NULL. + * @readonly: Whether the vma should be bound read-only. + * @lmem: Whether the vma points to lmem. + */ +struct i915_vma_bindinfo { + struct sg_table *pages; + struct i915_page_sizes page_sizes; + struct i915_refct_sgt *pages_rsgt; + bool readonly:1; + bool lmem:1; +}; + /** * struct i915_vma_resource - Snapshotted unbind information. * @unbind_fence: Fence to mark unbinding complete. Note that this fence @@ -89,25 +110,13 @@ struct i915_vma_resource { intel_wakeref_t wakeref; /** - * struct i915_vma_bindinfo - Information needed for async bind - * only but that can be dropped after the bind has taken place. - * Consider making this a separate argument to the bind_vma - * op, coalescing with other arguments like vm, stash, cache_level - * and flags - * @pages: The pages sg-table. - * @page_sizes: Page sizes of the pages. - * @pages_rsgt: Refcounted sg-table when delayed object destruction - * is supported. May be NULL. - * @readonly: Whether the vma should be bound read-only. - * @lmem: Whether the vma points to lmem. + * @bi: Information needed for async bind only but that can be dropped + * after the bind has taken place. + * + * Consider making this a separate argument to the bind_vma op, + * coalescing with other arguments like vm, stash, cache_level and flags */ - struct i915_vma_bindinfo { - struct sg_table *pages; - struct i915_page_sizes page_sizes; - struct i915_refct_sgt *pages_rsgt; - bool readonly:1; - bool lmem:1; - } bi; + struct i915_vma_bindinfo bi; #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) struct intel_memory_region *mr;
You can't document both a sub-struct type and a struct member at the same time. Separate them. drivers/gpu/drm/i915/i915_vma_resource.h:91: warning: Incorrect use of kernel-doc format: * struct i915_vma_bindinfo - Information needed for async bind drivers/gpu/drm/i915/i915_vma_resource.h:129: warning: Function parameter or member 'bi' not described in 'i915_vma_resource' Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_vma_resource.h | 45 ++++++++++++++---------- 1 file changed, 27 insertions(+), 18 deletions(-)