Re: [1/5] drm/atomic: Move the drm_atomic_state field doc inline

[Sui Jingfeng <sui.jingfeng@linux.dev>]

Hi,


On 2023/12/4 20:17, Maxime Ripard wrote:
> Some fields of drm_atomic_state have been documented in-line, but some
> were documented in the main kerneldoc block before the structure.
>
> Since the former is the preferred option in DRM, let's move all the
> fields to an inline documentation.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev>

Very nice cleanup, but I see a very very tiny concern, see below.

> ---
>   include/drm/drm_atomic.h | 50 ++++++++++++++++++++++++++++++++--------
>   1 file changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index cf8e1220a4ac..2a08030fcd75 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -347,24 +347,22 @@ struct __drm_private_objs_state {
>   
>   /**
>    * struct drm_atomic_state - the global state object for atomic updates
> - * @ref: count of all references to this state (will not be freed until zero)
> - * @dev: parent DRM device
> - * @async_update: hint for asynchronous plane update
> - * @planes: pointer to array of structures with per-plane data
> - * @crtcs: pointer to array of CRTC pointers
> - * @num_connector: size of the @connectors and @connector_states arrays
> - * @connectors: pointer to array of structures with per-connector data
> - * @num_private_objs: size of the @private_objs array
> - * @private_objs: pointer to array of private object pointers
> - * @acquire_ctx: acquire context for this atomic modeset state update
>    *
>    * States are added to an atomic update by calling drm_atomic_get_crtc_state(),
>    * drm_atomic_get_plane_state(), drm_atomic_get_connector_state(), or for
>    * private state structures, drm_atomic_get_private_obj_state().
>    */
>   struct drm_atomic_state {
> +	/**
> +	 * @ref:
> +	 *
> +	 * Count of all references to this update (will not be freed until zero).
> +	 */
>   	struct kref ref;
>   
> +	/**
> +	 * @dev: Parent DRM Device.
> +	 */

Well, I noticed that the first letter of 'Parent' get capitalized.
Do we also need capitalize the reset of documentation to keep consistent?


>   	struct drm_device *dev;
>   
>   	/**
> @@ -388,7 +386,12 @@ struct drm_atomic_state {
>   	 * flag are not allowed.
>   	 */
>   	bool legacy_cursor_update : 1;
> +
> +	/**
> +	 * @async_update: hint for asynchronous plane update
> +	 */
>   	bool async_update : 1;
> +
>   	/**
>   	 * @duplicated:
>   	 *
> @@ -398,13 +401,40 @@ struct drm_atomic_state {
>   	 * states.
>   	 */
>   	bool duplicated : 1;
> +
> +	/**
> +	 * @planes: pointer to array of structures with per-plane data
> +	 */
>   	struct __drm_planes_state *planes;
> +
> +	/**
> +	 * @crtcs: pointer to array of CRTC pointers
> +	 */
>   	struct __drm_crtcs_state *crtcs;
> +
> +	/**
> +	 * @num_connector: size of the @connectors and @connector_states arrays
> +	 */
>   	int num_connector;
> +
> +	/**
> +	 * @connectors: pointer to array of structures with per-connector data
> +	 */
>   	struct __drm_connnectors_state *connectors;
> +
> +	/**
> +	 * @num_private_objs: size of the @private_objs array
> +	 */
>   	int num_private_objs;
> +
> +	/**
> +	 * @private_objs: pointer to array of private object pointers
> +	 */
>   	struct __drm_private_objs_state *private_objs;
>   
> +	/**
> +	 * @acquire_ctx: acquire context for this atomic modeset state update
> +	 */
>   	struct drm_modeset_acquire_ctx *acquire_ctx;
>   
>   	/**