Re: [Intel-gfx] [PATCH 05/24] drm/i915/vma: fix struct i915_vma_bindinfo kernel-doc

[Rodrigo Vivi <rodrigo.vivi@kernel.org>]

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
>