[PATCH v5 05/44] drm/colorop: Introduce new drm_colorop mode object

[Louis Chauvet]

Le 19/08/24 - 16:56, Harry Wentland a écrit :

[...]

> +#ifndef __DRM_COLOROP_H__
> +#define __DRM_COLOROP_H__
> +
> +#include <drm/drm_mode_object.h>
> +#include <drm/drm_mode.h>
> +#include <drm/drm_property.h>
> +
> +/**
> + * struct drm_colorop_state - mutable colorop state
> + */
> +struct drm_colorop_state {
> +	/** @colorop: backpointer to the colorop */
> +	struct drm_colorop *colorop;
> +
> +	/* colorop properties */

Can you add a comment like this, so it is clear that not all the fields 
are valid and only a few of them will be used.

	/*
	 * Color properties
	 *
	 * The following fields are not always valid, their usage depends
	 * on the colorop type. See their associated comment for more 
	 * information.
	 */

> +
> +	/** @state: backpointer to global drm_atomic_state */
> +	struct drm_atomic_state *state;
> +};
> +
> +/**
> + * struct drm_colorop - DRM color operation control structure
> + *
> + * A colorop represents one color operation. They can be chained via
> + * the 'next' pointer to build a color pipeline.
> + */
> +struct drm_colorop {
> +	/** @dev: parent DRM device */
> +	struct drm_device *dev;
> +
> +	/**
> +	 * @head:
> +	 *
> +	 * List of all colorops on @dev, linked from &drm_mode_config.colorop_list.
> +	 * Invariant over the lifetime of @dev and therefore does not need
> +	 * locking.
> +	 */
> +	struct list_head head;
> +
> +	/**
> +	 * @index: Position inside the mode_config.list, can be used as an array
> +	 * index. It is invariant over the lifetime of the colorop.
> +	 */
> +	unsigned index;
> +
> +	/** @base: base mode object */
> +	struct drm_mode_object base;
> +
> +	/**
> +	 * @plane:
> +	 *
> +	 * The plane on which the colorop sits. A drm_colorop is always unique
> +	 * to a plane.
> +	 */
> +	struct drm_plane *plane;
> +
> +	/**
> +	 * @state:
> +	 *
> +	 * Current atomic state for this colorop.
> +	 *
> +	 * This is protected by @mutex. Note that nonblocking atomic commits
> +	 * access the current colorop state without taking locks.
> +	 */
> +	struct drm_colorop_state *state;
> +
> +	/* colorop properties */

Maybe the same kind of comment here?

> +	/** @properties: property tracking for this colorop */
> +	struct drm_object_properties properties;
> +
> +};
> +

[...]

Louis Chauvet