On 2025-01-13 03:18, Simon Ser wrote: > This patch should probably come after all patches introducing the > properties referenced in the docs, e.g. NEXT and COLOR_PIPELINE. > Probably after "[13/45] drm/colorop: Introduce > DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE"? > >> +/** >> + * DOC: overview >> + * >> + * A colorop represents a single color operation. Colorops are chained >> + * via the NEXT property and make up color pipelines. Color pipelines >> + * are advertised and selected via the COLOR_PIPELINE &drm_plane >> + * property. >> + * >> + * A colorop will be of a certain type, advertised by the read-only TYPE >> + * property. Each type of colorop will advertise a different set of >> + * properties and is programmed in a different manner. Types can be >> + * enumerated 1D curves, 1D LUTs, 3D LUTs, matrices, etc. See the >> + * &drm_colorop_type documentation for information on each type. > > It's not super nice to refer to internal kernel docs here, because AFAIU > this section is mostly written towards user-space developers. User-space > developers have no idea how internal kernel structs work. > > It would be nicer to have a list of colorop types here, without referring > to kernel internals. For instance, we have a list of > I'm not sure I follow. This is linking to the drm_colorop_type (from drm_mode.h) enum documentation in drm-uapi.html. Duplicating it here would mean that sooner or later the two docs will get out of sync. I agree with the rest and we'll reflect that in v8. Harry >> + * If a colorop advertises the BYPASS property it can be bypassed. >> + * >> + * Since colorops cannot stand-alone and are used to describe colorop >> + * operations on a plane they don't have their own locking mechanism but >> + * are locked and programmed along with their associated &drm_plane. > > This sounds a bit too internal for overview docs - maybe should be in > the struct drm_colorop docs instead? > >> + * Colorops are only advertised and valid for atomic drivers and atomic >> + * userspace that signals the DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE client > > Nit: this cap can be linkified with a "&". > >> + * cap. When a driver advertises the COLOR_PIPELINE property on a >> + * &drm_plane and userspace signals the >> + * DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE the driver shall ignore all other >> + * plane color properties, such as COLOR_ENCODING and COLOR_RANGE. > > Perhaps this should appear first in the doc? Start with instructions to > enable, then describe how it works? > >> + * More information about colorops and color pipelines can be found at >> + * rfc/color_pipeline.rst. > > Perhaps we should note that this document contains information about > design/architectural decisions? The "reference" should be this section, the > RFC doc is just about the motivation I believe. That is, if the API evolves, > the RFC is unliekly to get updated, but this document will.
