diff mbox series

[RFC,01/33] drm/doc/rfc: Add RFC document for proposed Plane Color Pipeline

Message ID 20230829160422.1251087-2-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Add Support for Plane Color Pipeline | expand

Commit Message

Uma Shankar Aug. 29, 2023, 4:03 p.m. UTC
Add the documentation for the new proposed Plane Color Pipeline.

Co-developed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
 1 file changed, 394 insertions(+)
 create mode 100644 Documentation/gpu/rfc/plane_color_pipeline.rst

Comments

Harry Wentland Aug. 29, 2023, 7:40 p.m. UTC | #1
On 2023-08-29 12:03, Uma Shankar wrote:
> Add the documentation for the new proposed Plane Color Pipeline.
> 
> Co-developed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
>   1 file changed, 394 insertions(+)
>   create mode 100644 Documentation/gpu/rfc/plane_color_pipeline.rst
> 
> diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst b/Documentation/gpu/rfc/plane_color_pipeline.rst
> new file mode 100644
> index 000000000000..60ce515b6ea7
> --- /dev/null
> +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst
> @@ -0,0 +1,394 @@
> +=======================================
> + Plane Color Pipeline: A UAPI proposal
> +=======================================
> +
> +To build the proposal on, lets take the premise of a color pipeline as shown
> +below.
> +
> + +-------------------------------------------+
> + |                RAM                        |
> + |  +------+    +---------+    +---------+   |
> + |  | FB 1 |    |  FB 2   |    | FB N    |   |
> + |  +------+    +---------+    +---------+   |
> + +-------------------------------------------+
> +       |  Plane Color Hardware Block |
> + +--------------------------------------------+
> + | +---v-----+   +---v-------+   +---v------+ |
> + | | Plane A |   | Plane B   |   | Plane N  | |
> + | | Pre-CSC |   | Pre-CSC   |   | Pre-CSC  | |
> + | +---+-----+   +---+-------+   +---+------+ |
> + |     |             |               |        |
> + | +---v-----+   +---v-------+   +---v------+ |
> + | |Plane A  |   | Plane B   |   | Plane N  | |
> + | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
> + | +---+-----+   +----+------+   +----+-----+ |
> + |     |              |               |       |
> + | +---v-----+   +----v------+   +----v-----+ |
> + | | Plane A |   | Plane B   |   | Plane N  | |
> + | |Post-CSC |   | Post-CSC  |   | Post-CSC | |
> + | +---+-----+   +----+------+   +----+-----+ |
> + |     |              |               |       |
> + +--------------------------------------------+
> ++------v--------------v---------------v-------|
> +||                                           ||
> +||           Pipe Blender                    ||
> ++--------------------+------------------------+
> +|                    |                        |
> +|        +-----------v----------+             |
> +|        |  Pipe Pre-CSC        |             |
> +|        |                      |             |
> +|        +-----------+----------+             |
> +|                    |            Pipe Color  |
> +|        +-----------v----------+ Hardware    |
> +|        |  Pipe CSC/CTM        |             |
> +|        |                      |             |
> +|        +-----------+----------+             |
> +|                    |                        |
> +|        +-----------v----------+             |
> +|        |  Pipe Post-CSC       |             |
> +|        |                      |             |
> +|        +-----------+----------+             |
> +|                    |                        |
> ++---------------------------------------------+
> +                     |
> +                     v
> +                Pipe Output
> +
> +Each plane consists of the following color blocks
> + * Pre-CSC : This block can used to linearize the input frame buffer data.
> +             The linear data then can be further acted on by the following
> +             color hardware blocks in the display hardware pipeline
> +
> + * CSC/CTM: Used to program color transformation matrix, this block is used
> +            to perform color space conversions like BT2020 to BT709 or BT601
> +            etc. This block acts on the linearized data coming from the
> +            Pre-CSC HW block.
> +
> + * Post-CSC: This HW block can be used to non-linearize frame buffer data to
> +             match the sink. Another use case of it could be to perform Tone
> +             mapping for HDR use-cases.
> +
> +Data from multiple planes will then be fed to pipe/crtc where it will get blended.
> +There is a similar set of HW blocks available at pipe/crtc level which acts on
> +this blended data.
> +
> +Below is a sample usecase fo video playback with sub-titles and playback
> +controls
> +
> +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐
> +│FB1         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │
> +│            ├───►│Linearize    ├────►│ BT709 to    ├───►│ SDR to HDR  │
> +│BT709 SDR   │    │             │     │ BT2020      │    │ Tone Mapping├─────┐
> +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘     │
> +(subtitles)                                                                  │
> +                                                                             │
> +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐     │
> +│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │     │
> +│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├───┐ │
> +│BT601 SDR   │    │             │     │ BT2020      │    │ Tone Mapping│   │ │
> +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘   │ │
> +(Playback controls UI)                                                     │ │
> +                                                                           │ │
> +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐   │ │
> +│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │   │ │
> +│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├─┐ │ │
> +│BT2020 HDR  │    │             │     │ pass-through│    │ pass-through│ │ │ │
> +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘ │ │ │
> +(video frame)                                                            │ │ │
> +                                                                         │ │ │
> +┌────────────────────────────────────────────────────────────────────────┴─┴─┘
> +│
> +│ ┌─────────────┐      ┌─────────────┐      ┌───────────────┐
> +│ │ CRTC        │      │ CRTC        │      │ CRTC          │
> +└─┤ PRE-CSC     ├─────►│ CSC/CTM     ├─────►│ POST-CSC      ├─────► TO Port
> +  │             │      │             │      │               │
> +  └─────────────┘      └─────────────┘      └───────────────┘
> +
> +This RFC is intended to propose an uAPI for the pre-blending color pipeline
> +(however, can be also extended to post blending pipeline).
> +
> +Below are the design considerations while designing the uAPI.
> +
> +1. Individual HW blocks can be muxed. (e.g. out of two HW blocks only one can be used)
> +2. Position of the HW block can be programmable
> +3. LUTs can be one dimentional or three dimentional
> +4. Number of LUT entries can vary across platforms
> +5. Precision of LUT entries can vary across platforms
> +6. Distribution of LUT entries may vary. e.g Mutli-segmented, Logarithmic,
> +   Piece-Wise Linear(PWL) etc
> +7. There can be parameterized/non-parameterized fixed function HW blocks.
> +   e.g. Just a hardware bit, to convert from one color space to another.
> +
> +Plane Color Pipeline: Design Proposal
> +=====================================
> +Each Color Hardware block will be represented by the structure drm_color_op.
> +
> +struct drm_color_op {
> +	enum color_op_block name;
> +	enum color_op_type type;
> +	u32 blob_id;
> +	u32 private_flags;
> +};
> +
> +The members of which will constitute:
> +1. name: A standardised enum for the color hardware block
> +2. type: The type of mathematical operation done by the hardware block.
> +         e.g. 1D Curve, 3D Curve, Matrix etc.
> +3. blob id: Id  pointing to a blob containing information about the hardware
> +         block advertising the respective capabilities to the userspace.
> +         It can be an optional field depending on the members "name" and "type".
> +4. private_flags: This can be used to provide vendor specific hints
> +         to user space
> +
> +
> +   For example to represent LUTs, we introduce the drm_color_lut_range
> +   structure. It can represent LUTs with varied number of entries and
> +   distributions (Multi segmented, Logarithmic etc).
> +
> +   struct drm_color_lut_range {
> +		/* DRM_MODE_LUT_* */
> +		__u32 flags;
> +		/* number of points on the curve */
> +		__u16 count;
> +		/* input/output bits per component */
> +		__u8 input_bpc, output_bpc;
> +		/* input start/end values */
> +		__s32 start, end;
> +		/* output min/max values */
> +		__s32 min, max;
> +   };
> +
> +Note: More details on exact usage and implementation of this structure can be
> +      found in the RFC. This structure is taken as is from the series.
> +      https://patchwork.freedesktop.org/series/90825/
> +      However, we can add more members to it to encompass all use-cases.
> +      For example. we can add a precision field to advertise the
> +      bitdepth of the LUTs. Similarly, we can reserve some bits in the flag
> +      field for vendor specific use cases.
> +
> +      At the same time, we don't need to pass any additional information for the
> +      CSC block as userspace and driver already agrees struct drm_color_ctm as
> +      a uAPI to pass on data.
> +
> +These color operations will form the building blocks of a color pipeline which
> +best represents the underlying Hardware. Color operations can be re-arranged,
> +substracted or added to create distinct color pipelines to accurately describe
> +the Hardware blocks present in the display engine.
> +

What do you mean when you're talking of re-arranging, or subtracting 
color operations?

> +In this proposal, a color pipeline is represented as an array of
> +struct drm_color_op.
> +
> +struct drm_color_op color_pipeline_1[]
> +
> +For example to represent the pre-blending color pipeline as described above
> +
> +We can create a color pipeline like this.
> +
> +struct drm_color_op color_pipeline_1[] = {
> +	{
> +		.name = DRM_CB_PRE_CSC,
> +		.type = CURVE_1D,
> +		.blob_id = 0; /* actual value to be populated during plane
> +						 initialization*/
> +	},
> +	{
> +		.name = DRM_CB_CSC,
> +		.type = MATRIX,
> +		.blob_id = 0;
> +	},
> +	{
> +		.name = DRM_CB_POST_CSC,
> +		.type = CURVE_1D,
> +		.blob_id = 0;
> +	},
> +};
> +
> +Then, for individual color operation, we add blobs to advertise the capability
> +of the particular Hardware block. In case of the example pipeline, we add
> +blobs of type drm_color_lut_range for the "pre-csc" and "post-csc".
> +For the "csc" block we pass no blob id to user space as it is known to both
> +user space and driver that drm_color_ctm structure is to be used for such
> +operation.
> +
> +To represent, this in a diagram.
> +
> +   struct drm_color_op color_pipeline_1[]
> +      +---------------------------+
> +      |                           |           drm_color_op
> +      |  +---------------------+--+-----------+---------------------+
> +      |  |                     |  |           |                     |
> +      |  |                     |  |           | +-----------------+ |
> +      |  |                     |  |           | |     name        | |
> +      |  |                     |  |           | +-----------------+ |
> +      |  |                     |  |           | |     type        | |
> +      |  |    color_op_1       |  |           | +-----------------+ |
> +      |  |                     |  |           | |     blob id     | +--------+
> +      |  |                     |  |           | +-----------------+ |        |
> +      |  |                     |  |           | |     private     | |        |
> +      |  |                     |  |           | |      flags      | |        |
> +      |  |                     |  |           | +-----------------+ |        |
> +      |  |                     |  |           |                     |        |
> +      |  +---------------------+--+-----------+---------------------+        |
> +      |                           |                                          |
> +      |                           |                                          |
> +      |  +---------------------+  |                                          |
> +      |  |                     |  |           drm_color_lut_range            |
> +      |  |    color_op_2       |  |           +-------------------------+    |
> +      |  |                     |  |           |                         |    |
> +      |  |                     |  |           | +---------------------+ |    |
> +      |  +---------------------+  |           | | segment 1 {         | |<---+
> +      |                           |           | |  ...                | |
> +      |  +---------------------+  |           | |  .input_bpc = 16,   | |
> +      |  |                     |  |           | |  .output_bpc = 16,  | |
> +      |  |    color_op_3       |  |           | |  ...                | |
> +      |  |                     |  |           | | }                   | |
> +      |  |                     |  |           | +---------------------+ |
> +      |  +---------------------+  |           |                         |
> +      |             .             |           | +---------------------+ |
> +      |             .             |           | | segment 2 {         | |
> +      |             .             |           | | ...                 | |
> +      +---------------------------+           | | }                   | |
> +                                              | |                     | |
> +                                              | |                     | |
> +                                              | |                     | |
> +                                              | +---------------------+ |
> +                                              |            .            |
> +                                              |            .            |
> +                                              |            .            |
> +                                              +-------------------------+
> +
> +
> +
> +This color pipeline is then packaged within a blob for the user space to
> +retrieve it. Details can be found in the next section
> +

Not sure I like blobs that contain other blob ids.

> +Exposing a color pipeline to user space
> +=======================================
> +
> +To advertise the available color pipelines, an immutable ENUM property
> +"GET_COLOR_PIPELINE" is introduced.
> +This enum property has blob id's as values. With each blob id representing
> +a distinct color pipeline based on underlying HW capabilities and their
> +respective combinations.
> +
> +The following output of drm_info [1], shows how color pipelines are visible
> +to userspace.
> +
> +├───Plane 0
> +    │   ├───Object ID: 31
> +    │   ├───CRTCs: {0}
> +    │   ├───Legacy info
> +        ...
> +    │       ├───"GET_COLOR_PIPELINE" (immutable): enum {no color pipeline,
> +						color pipeline 1, color pipeline 2}= no color pipeline
> +
> +To understand the capabilities of individual pipelines, first the userspace
> +need to retrieve the pipeline blob with the blob ids retrieved by reading the
> +enum property.
> +
> +Once the color pipeline is retrieved, user can then parse through
> +individual drm_color_op blocks to understand the capabilities of each
> +hardware block.
> +
> +Check IGT series to see how user space can parse through color pipelines.
> +Refer the IGT series here: https://patchwork.freedesktop.org/series/123018/
> +
> +Setting up a color pipeline
> +===========================
> +
> +Once the user space decides on a color pipeline, it can set the pipeline and
> +the corresponding data for the hardware blocks within the pipeline with
> +the BLOB property "SET_COLOR_PIPELINE".
> +
> +To achieve this two structures are introduced
> +
> +1.	struct drm_color_op_data: It represents data to be passed onto individual
> +							  color hardware blocks. It contains three members
> +                      a) name: to identify the color operation block
> +                      b) blob_id: pointing to the blob with data for the
> +                                  corresponding hardware block
> +
> +	struct drm_color_op_data {
> +		enum color_op_block name;
> +		u32 blob_id;
> +	};
> +
> +2.	struct drm_color_pipeline: This structure represents the aggregate
> +                                   pipeline to be set. it contains the following  members
> +					  a) num: pipeline number to be selected
> +					  b) size: size of the data to be passed onto the driver
> +					  c) data: array of struct drm_color_op_data with data for
> +                                                   the hardware block/s that userspace wants to
> +                                                   set values for.
> +
> +	struct drm_color_pipeline {
> +		int num;
> +		int size;
> +		struct drm_color_op_data *data;
> +	};
> +
> +	User can either:
> +	1. send data for all the color operations in a pipeline as shown in [2].
> +	   The color operation data need not be in order that the pipeline advertises
> +	   however, it should not contain data for any
> +	   color operation that is not present in the pipeline.
> +
> +	   Note: This check for pipeline is not yet implemented but if the
> +	   wider proposal is accepted we have few solutions in mind.
> +
> +	2. send data for a subset of color operations as shown in [3].For the
> +	   color operation that userspace does not send data will retain their
> +	   older state.
> +
> +	3. reset/disable the pipeline by setting the "SET_COLOR_PIPELINE" blob
> +	   property to NULL as shown in both [2] and [3]
> +
> +	4. change the color pipeline as demonstrated in [3].
> +	   On the new pipeline, the user is expected to setup all the color hardware block
> +	   Once the user requests a pipeline change, the driver will provide it a clean slate
> +           which means that all the data previously set by the user will be discarded even if
> +           there are common hardware blocks between the two(previous and current) pipelines.
> +
> +IGT implementation can be found here [4]
> +
> +Representing Fixed function hardware
> +====================================
> +
> +To provide support for fixed function hardware, the driver could expose vendor
> +specific struct drm_color_op with parameters that both the userspace and
> +driver agrees on. To demonstrate, let's consider a hyphothetical fixed
> +function hardware block that converts BT601 to BT2020.
> +The driver can choose to advertise the block as such.
> +
> +struct drm_color_op color_pipeline_X[] = {
> +	{
> +		.name = DRM_CB_PRIVATE,
> +		.type = FIXED_FUNCTION,
> +		.blob_id = 45;
> +	},
> +}
> +
> +Where the blob represents some vendor specific enums, strings or any other
> +appropriate data types which both the user-space and drivers are aligned on.
> +
> +blob:45 {
> +	VENDORXXX_BT602_TO_BT2020,
> +};
> +
> +For enabling or passing parameters to such blocks, the user can send data
> +to the driver wrapped within a blob like any other color operation block.
> +
> +	struct drm_color_op_data {
> +		.name = DRM_CB_PRIVATE;
> +		.blob_id = 46;
> +	} ;
> +
> +where blob with id 46 contains data to enable the fixed function hardware(s).
> +

One major thing missing from the RFC is an explanation on why we want to 
go with a prescriptive model, rather than a descriptive model. This 
question came up in Dave's response to Simon's RFC:
https://lore.kernel.org/dri-devel/CAPM=9tz54Jc1HSjdh5A7iG4X8Gvgg46qu7Ezvgnmj4N6gbY+Kw@mail.gmail.com/

This is a rough attempt at such an explanation:
https://gitlab.freedesktop.org/hwentland/linux/-/merge_requests/5/diffs?commit_id=bc99737623796b39925767d6f8cbc097ad0b4d8d

Harry

> +References
> +==========
> +
> +[1] https://gitlab.freedesktop.org/emersion/drm_info
> +[2] https://patchwork.freedesktop.org/patch/554827/?series=123018&rev=1
> +[3] https://patchwork.freedesktop.org/patch/554826/?series=123018&rev=1
> +[4] https://patchwork.freedesktop.org/series/123018/
Uma Shankar Aug. 30, 2023, 8:59 a.m. UTC | #2
> -----Original Message-----
> From: Harry Wentland <harry.wentland@amd.com>
> Sent: Wednesday, August 30, 2023 1:10 AM
> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; wayland-
> devel@lists.freedesktop.org
> Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane Color
> Pipeline
> 
> 
> 
> On 2023-08-29 12:03, Uma Shankar wrote:
> > Add the documentation for the new proposed Plane Color Pipeline.
> >
> > Co-developed-by: Chaitanya Kumar Borah
> > <chaitanya.kumar.borah@intel.com>
> > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
> >   1 file changed, 394 insertions(+)
> >   create mode 100644 Documentation/gpu/rfc/plane_color_pipeline.rst
> >
> > diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst
> > b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > new file mode 100644
> > index 000000000000..60ce515b6ea7
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > @@ -0,0 +1,394 @@
> > +=======================================
> > + Plane Color Pipeline: A UAPI proposal
> > +=======================================
> > +
> > +To build the proposal on, lets take the premise of a color pipeline
> > +as shown below.
> > +
> > + +-------------------------------------------+
> > + |                RAM                        |
> > + |  +------+    +---------+    +---------+   |
> > + |  | FB 1 |    |  FB 2   |    | FB N    |   |
> > + |  +------+    +---------+    +---------+   |
> > + +-------------------------------------------+
> > +       |  Plane Color Hardware Block |
> > + +--------------------------------------------+
> > + | +---v-----+   +---v-------+   +---v------+ |
> > + | | Plane A |   | Plane B   |   | Plane N  | |
> > + | | Pre-CSC |   | Pre-CSC   |   | Pre-CSC  | |
> > + | +---+-----+   +---+-------+   +---+------+ |
> > + |     |             |               |        |
> > + | +---v-----+   +---v-------+   +---v------+ |
> > + | |Plane A  |   | Plane B   |   | Plane N  | |
> > + | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
> > + | +---+-----+   +----+------+   +----+-----+ |
> > + |     |              |               |       |
> > + | +---v-----+   +----v------+   +----v-----+ |
> > + | | Plane A |   | Plane B   |   | Plane N  | |
> > + | |Post-CSC |   | Post-CSC  |   | Post-CSC | |
> > + | +---+-----+   +----+------+   +----+-----+ |
> > + |     |              |               |       |
> > + +--------------------------------------------+
> > ++------v--------------v---------------v-------|
> > +||                                           ||
> > +||           Pipe Blender                    ||
> > ++--------------------+------------------------+
> > +|                    |                        |
> > +|        +-----------v----------+             |
> > +|        |  Pipe Pre-CSC        |             |
> > +|        |                      |             |
> > +|        +-----------+----------+             |
> > +|                    |            Pipe Color  |
> > +|        +-----------v----------+ Hardware    |
> > +|        |  Pipe CSC/CTM        |             |
> > +|        |                      |             |
> > +|        +-----------+----------+             |
> > +|                    |                        |
> > +|        +-----------v----------+             |
> > +|        |  Pipe Post-CSC       |             |
> > +|        |                      |             |
> > +|        +-----------+----------+             |
> > +|                    |                        |
> > ++---------------------------------------------+
> > +                     |
> > +                     v
> > +                Pipe Output
> > +
> > +Each plane consists of the following color blocks
> > + * Pre-CSC : This block can used to linearize the input frame buffer data.
> > +             The linear data then can be further acted on by the following
> > +             color hardware blocks in the display hardware pipeline
> > +
> > + * CSC/CTM: Used to program color transformation matrix, this block is used
> > +            to perform color space conversions like BT2020 to BT709 or BT601
> > +            etc. This block acts on the linearized data coming from the
> > +            Pre-CSC HW block.
> > +
> > + * Post-CSC: This HW block can be used to non-linearize frame buffer data to
> > +             match the sink. Another use case of it could be to perform Tone
> > +             mapping for HDR use-cases.
> > +
> > +Data from multiple planes will then be fed to pipe/crtc where it will get blended.
> > +There is a similar set of HW blocks available at pipe/crtc level
> > +which acts on this blended data.
> > +
> > +Below is a sample usecase fo video playback with sub-titles and
> > +playback controls
> > +
> > +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐
> > +│FB1         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │
> > +│            ├───►│Linearize    ├────►│ BT709 to    ├───►│ SDR to HDR  │
> > +│BT709 SDR   │    │             │     │ BT2020      │    │ Tone Mapping├─────┐
> > +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘
> │
> > +(subtitles)                                                                  │
> > +                                                                             │
> > +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐
> │
> > +│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │     │
> > +│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├───┐ │
> > +│BT601 SDR   │    │             │     │ BT2020      │    │ Tone Mapping│   │ │
> > +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘
> │ │
> > +(Playback controls UI)                                                     │ │
> > +                                                                           │ │
> > +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐
> │ │
> > +│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │   │ │
> > +│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├─┐ │ │
> > +│BT2020 HDR  │    │             │     │ pass-through│    │ pass-through│ │ │ │
> > +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘ │
> │ │
> > +(video frame)                                                            │ │ │
> > +
> > +│ │ │
> >
> +┌──────────────────────────────────────────────────────────────────
> ──
> > +────┴─┴─┘
> > +│
> > +│ ┌─────────────┐      ┌─────────────┐      ┌───────────────┐
> > +│ │ CRTC        │      │ CRTC        │      │ CRTC          │
> > +└─┤ PRE-CSC     ├─────►│ CSC/CTM     ├─────►│ POST-CSC      ├─────► TO
> Port
> > +  │             │      │             │      │               │
> > +  └─────────────┘      └─────────────┘      └───────────────┘
> > +
> > +This RFC is intended to propose an uAPI for the pre-blending color
> > +pipeline (however, can be also extended to post blending pipeline).
> > +
> > +Below are the design considerations while designing the uAPI.
> > +
> > +1. Individual HW blocks can be muxed. (e.g. out of two HW blocks only
> > +one can be used) 2. Position of the HW block can be programmable 3.
> > +LUTs can be one dimentional or three dimentional 4. Number of LUT
> > +entries can vary across platforms 5. Precision of LUT entries can
> > +vary across platforms 6. Distribution of LUT entries may vary. e.g
> > +Mutli-segmented, Logarithmic,
> > +   Piece-Wise Linear(PWL) etc
> > +7. There can be parameterized/non-parameterized fixed function HW blocks.
> > +   e.g. Just a hardware bit, to convert from one color space to another.
> > +
> > +Plane Color Pipeline: Design Proposal
> > +=====================================
> > +Each Color Hardware block will be represented by the structure drm_color_op.
> > +
> > +struct drm_color_op {
> > +	enum color_op_block name;
> > +	enum color_op_type type;
> > +	u32 blob_id;
> > +	u32 private_flags;
> > +};
> > +
> > +The members of which will constitute:
> > +1. name: A standardised enum for the color hardware block 2. type:
> > +The type of mathematical operation done by the hardware block.
> > +         e.g. 1D Curve, 3D Curve, Matrix etc.
> > +3. blob id: Id  pointing to a blob containing information about the hardware
> > +         block advertising the respective capabilities to the userspace.
> > +         It can be an optional field depending on the members "name" and "type".
> > +4. private_flags: This can be used to provide vendor specific hints
> > +         to user space
> > +
> > +
> > +   For example to represent LUTs, we introduce the drm_color_lut_range
> > +   structure. It can represent LUTs with varied number of entries and
> > +   distributions (Multi segmented, Logarithmic etc).
> > +
> > +   struct drm_color_lut_range {
> > +		/* DRM_MODE_LUT_* */
> > +		__u32 flags;
> > +		/* number of points on the curve */
> > +		__u16 count;
> > +		/* input/output bits per component */
> > +		__u8 input_bpc, output_bpc;
> > +		/* input start/end values */
> > +		__s32 start, end;
> > +		/* output min/max values */
> > +		__s32 min, max;
> > +   };
> > +
> > +Note: More details on exact usage and implementation of this structure can be
> > +      found in the RFC. This structure is taken as is from the series.
> > +      https://patchwork.freedesktop.org/series/90825/
> > +      However, we can add more members to it to encompass all use-cases.
> > +      For example. we can add a precision field to advertise the
> > +      bitdepth of the LUTs. Similarly, we can reserve some bits in the flag
> > +      field for vendor specific use cases.
> > +
> > +      At the same time, we don't need to pass any additional information for the
> > +      CSC block as userspace and driver already agrees struct drm_color_ctm as
> > +      a uAPI to pass on data.
> > +
> > +These color operations will form the building blocks of a color
> > +pipeline which best represents the underlying Hardware. Color
> > +operations can be re-arranged, substracted or added to create
> > +distinct color pipelines to accurately describe the Hardware blocks present in the
> display engine.
> > +
> 
> What do you mean when you're talking of re-arranging, or subtracting color
> operations?

I will re-phrase this to avoid confusion. The pipeline can be defined by driver by
Putting together the hardware blocks in some order supported by respective hardware.
If any new generation changes the behaviour, a new pipeline can be defined and exposed
for that particular platform. So pipeline definition is fully flexible but done by driver.
Userspace will just be able to use that and cannot alter the same once exposed by driver.

> > +In this proposal, a color pipeline is represented as an array of
> > +struct drm_color_op.
> > +
> > +struct drm_color_op color_pipeline_1[]
> > +
> > +For example to represent the pre-blending color pipeline as described
> > +above
> > +
> > +We can create a color pipeline like this.
> > +
> > +struct drm_color_op color_pipeline_1[] = {
> > +	{
> > +		.name = DRM_CB_PRE_CSC,
> > +		.type = CURVE_1D,
> > +		.blob_id = 0; /* actual value to be populated during plane
> > +						 initialization*/
> > +	},
> > +	{
> > +		.name = DRM_CB_CSC,
> > +		.type = MATRIX,
> > +		.blob_id = 0;
> > +	},
> > +	{
> > +		.name = DRM_CB_POST_CSC,
> > +		.type = CURVE_1D,
> > +		.blob_id = 0;
> > +	},
> > +};
> > +
> > +Then, for individual color operation, we add blobs to advertise the
> > +capability of the particular Hardware block. In case of the example
> > +pipeline, we add blobs of type drm_color_lut_range for the "pre-csc" and "post-
> csc".
> > +For the "csc" block we pass no blob id to user space as it is known
> > +to both user space and driver that drm_color_ctm structure is to be
> > +used for such operation.
> > +
> > +To represent, this in a diagram.
> > +
> > +   struct drm_color_op color_pipeline_1[]
> > +      +---------------------------+
> > +      |                           |           drm_color_op
> > +      |  +---------------------+--+-----------+---------------------+
> > +      |  |                     |  |           |                     |
> > +      |  |                     |  |           | +-----------------+ |
> > +      |  |                     |  |           | |     name        | |
> > +      |  |                     |  |           | +-----------------+ |
> > +      |  |                     |  |           | |     type        | |
> > +      |  |    color_op_1       |  |           | +-----------------+ |
> > +      |  |                     |  |           | |     blob id     | +--------+
> > +      |  |                     |  |           | +-----------------+ |        |
> > +      |  |                     |  |           | |     private     | |        |
> > +      |  |                     |  |           | |      flags      | |        |
> > +      |  |                     |  |           | +-----------------+ |        |
> > +      |  |                     |  |           |                     |        |
> > +      |  +---------------------+--+-----------+---------------------+        |
> > +      |                           |                                          |
> > +      |                           |                                          |
> > +      |  +---------------------+  |                                          |
> > +      |  |                     |  |           drm_color_lut_range            |
> > +      |  |    color_op_2       |  |           +-------------------------+    |
> > +      |  |                     |  |           |                         |    |
> > +      |  |                     |  |           | +---------------------+ |    |
> > +      |  +---------------------+  |           | | segment 1 {         | |<---+
> > +      |                           |           | |  ...                | |
> > +      |  +---------------------+  |           | |  .input_bpc = 16,   | |
> > +      |  |                     |  |           | |  .output_bpc = 16,  | |
> > +      |  |    color_op_3       |  |           | |  ...                | |
> > +      |  |                     |  |           | | }                   | |
> > +      |  |                     |  |           | +---------------------+ |
> > +      |  +---------------------+  |           |                         |
> > +      |             .             |           | +---------------------+ |
> > +      |             .             |           | | segment 2 {         | |
> > +      |             .             |           | | ...                 | |
> > +      +---------------------------+           | | }                   | |
> > +                                              | |                     | |
> > +                                              | |                     | |
> > +                                              | |                     | |
> > +                                              | +---------------------+ |
> > +                                              |            .            |
> > +                                              |            .            |
> > +                                              |            .            |
> > +
> > + +-------------------------+
> > +
> > +
> > +
> > +This color pipeline is then packaged within a blob for the user space
> > +to retrieve it. Details can be found in the next section
> > +
> 
> Not sure I like blobs that contain other blob ids.

It provides flexibility and helps with just one interface to userspace. Its easy to handle and
manage once we get the hang of it 
Pekka Paalanen Aug. 30, 2023, 12:28 p.m. UTC | #3
On Wed, 30 Aug 2023 08:59:36 +0000
"Shankar, Uma" <uma.shankar@intel.com> wrote:

> > -----Original Message-----
> > From: Harry Wentland <harry.wentland@amd.com>
> > Sent: Wednesday, August 30, 2023 1:10 AM
> > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> > devel@lists.freedesktop.org
> > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; wayland-
> > devel@lists.freedesktop.org
> > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane Color
> > Pipeline
> > 
> > 
> > 
> > On 2023-08-29 12:03, Uma Shankar wrote:  
> > > Add the documentation for the new proposed Plane Color Pipeline.
> > >
> > > Co-developed-by: Chaitanya Kumar Borah
> > > <chaitanya.kumar.borah@intel.com>
> > > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > ---
> > >   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
> > >   1 file changed, 394 insertions(+)
> > >   create mode 100644 Documentation/gpu/rfc/plane_color_pipeline.rst
> > >
> > > diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > new file mode 100644
> > > index 000000000000..60ce515b6ea7
> > > --- /dev/null
> > > +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst

...

Hi Uma!

> > > +This color pipeline is then packaged within a blob for the user space
> > > +to retrieve it. Details can be found in the next section
> > > +  
> > 
> > Not sure I like blobs that contain other blob ids.  
> 
> It provides flexibility and helps with just one interface to userspace. Its easy to handle and
> manage once we get the hang of it 
Uma Shankar Sept. 4, 2023, 1:44 p.m. UTC | #4
> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Pekka
> Paalanen
> Sent: Wednesday, August 30, 2023 5:59 PM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>; dri-devel@lists.freedesktop.org; wayland-
> devel@lists.freedesktop.org
> Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane
> Color Pipeline
> 
> On Wed, 30 Aug 2023 08:59:36 +0000
> "Shankar, Uma" <uma.shankar@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Harry Wentland <harry.wentland@amd.com>
> > > Sent: Wednesday, August 30, 2023 1:10 AM
> > > To: Shankar, Uma <uma.shankar@intel.com>;
> > > intel-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org
> > > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>;
> > > wayland- devel@lists.freedesktop.org
> > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed
> > > Plane Color Pipeline
> > >
> > >
> > >
> > > On 2023-08-29 12:03, Uma Shankar wrote:
> > > > Add the documentation for the new proposed Plane Color Pipeline.
> > > >
> > > > Co-developed-by: Chaitanya Kumar Borah
> > > > <chaitanya.kumar.borah@intel.com>
> > > > Signed-off-by: Chaitanya Kumar Borah
> > > > <chaitanya.kumar.borah@intel.com>
> > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > > ---
> > > >   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
> > > >   1 file changed, 394 insertions(+)
> > > >   create mode 100644
> > > > Documentation/gpu/rfc/plane_color_pipeline.rst
> > > >
> > > > diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > new file mode 100644
> > > > index 000000000000..60ce515b6ea7
> > > > --- /dev/null
> > > > +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst
> 
> ...
> 
> Hi Uma!

Thanks Pekka for the feedback and useful inputs.

> > > > +This color pipeline is then packaged within a blob for the user
> > > > +space to retrieve it. Details can be found in the next section
> > > > +
> > >
> > > Not sure I like blobs that contain other blob ids.
> >
> > It provides flexibility and helps with just one interface to
> > userspace. Its easy to handle and manage once we get the hang of it 
Pekka Paalanen Sept. 5, 2023, 11:32 a.m. UTC | #5
On Mon, 4 Sep 2023 13:44:49 +0000
"Shankar, Uma" <uma.shankar@intel.com> wrote:

> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Pekka
> > Paalanen
> > Sent: Wednesday, August 30, 2023 5:59 PM
> > To: Shankar, Uma <uma.shankar@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar
> > <chaitanya.kumar.borah@intel.com>; dri-devel@lists.freedesktop.org; wayland-
> > devel@lists.freedesktop.org
> > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane
> > Color Pipeline
> > 
> > On Wed, 30 Aug 2023 08:59:36 +0000
> > "Shankar, Uma" <uma.shankar@intel.com> wrote:
> >   
> > > > -----Original Message-----
> > > > From: Harry Wentland <harry.wentland@amd.com>
> > > > Sent: Wednesday, August 30, 2023 1:10 AM
> > > > To: Shankar, Uma <uma.shankar@intel.com>;
> > > > intel-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org
> > > > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>;
> > > > wayland- devel@lists.freedesktop.org
> > > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed
> > > > Plane Color Pipeline
> > > >
> > > >
> > > >
> > > > On 2023-08-29 12:03, Uma Shankar wrote:  
> > > > > Add the documentation for the new proposed Plane Color Pipeline.
> > > > >
> > > > > Co-developed-by: Chaitanya Kumar Borah
> > > > > <chaitanya.kumar.borah@intel.com>
> > > > > Signed-off-by: Chaitanya Kumar Borah
> > > > > <chaitanya.kumar.borah@intel.com>
> > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > > > ---
> > > > >   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
> > > > >   1 file changed, 394 insertions(+)
> > > > >   create mode 100644
> > > > > Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > >
> > > > > diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > > b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > > new file mode 100644
> > > > > index 000000000000..60ce515b6ea7
> > > > > --- /dev/null
> > > > > +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst  
> > 
> > ...
> > 
> > Hi Uma!  
> 
> Thanks Pekka for the feedback and useful inputs.

Hi Uma,

sorry to say, but the overall feeling I get from this proposal is that
it is just the current color related KMS properties wrapped in a
pipeline blob. That is not the re-design I believe we are looking for,
and the feeling is based on several details that are just copied from
the current property design. Also the "private" stuff has to go.

All the varying LUT entries, varying LUT precision, 1D/3D LUTs, varying
LUT tap distribution, and parametrized curves are good development, but
right now we are looking at things one step higher level: the overall
color pipeline design and how to represent any operation. Most of this
series is considering details below the current attention level, hence
I'm paying attention only to the first few patches.

More below.

> > > > > +This color pipeline is then packaged within a blob for the user
> > > > > +space to retrieve it. Details can be found in the next section
> > > > > +  
> > > >
> > > > Not sure I like blobs that contain other blob ids.  
> > >
> > > It provides flexibility and helps with just one interface to
> > > userspace. Its easy to handle and manage once we get the hang of it 
Uma Shankar Sept. 7, 2023, 12:31 p.m. UTC | #6
> -----Original Message-----
> From: Pekka Paalanen <ppaalanen@gmail.com>
> Sent: Tuesday, September 5, 2023 5:03 PM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>; dri-devel@lists.freedesktop.org; wayland-
> devel@lists.freedesktop.org; Harry Wentland <harry.wentland@amd.com>;
> Sebastian Wick <sebastian.wick@redhat.com>; ville.syrjala@linux.intel.com;
> Jonas Adahl <jadahl@redhat.com>
> Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane
> Color Pipeline
> 
> On Mon, 4 Sep 2023 13:44:49 +0000
> "Shankar, Uma" <uma.shankar@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf
> > > Of Pekka Paalanen
> > > Sent: Wednesday, August 30, 2023 5:59 PM
> > > To: Shankar, Uma <uma.shankar@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar
> > > <chaitanya.kumar.borah@intel.com>; dri-devel@lists.freedesktop.org;
> > > wayland- devel@lists.freedesktop.org
> > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed
> > > Plane Color Pipeline
> > >
> > > On Wed, 30 Aug 2023 08:59:36 +0000
> > > "Shankar, Uma" <uma.shankar@intel.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Harry Wentland <harry.wentland@amd.com>
> > > > > Sent: Wednesday, August 30, 2023 1:10 AM
> > > > > To: Shankar, Uma <uma.shankar@intel.com>;
> > > > > intel-gfx@lists.freedesktop.org; dri-
> > > > > devel@lists.freedesktop.org
> > > > > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>;
> > > > > wayland- devel@lists.freedesktop.org
> > > > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for
> > > > > proposed Plane Color Pipeline
> > > > >
> > > > >
> > > > >
> > > > > On 2023-08-29 12:03, Uma Shankar wrote:
> > > > > > Add the documentation for the new proposed Plane Color Pipeline.
> > > > > >
> > > > > > Co-developed-by: Chaitanya Kumar Borah
> > > > > > <chaitanya.kumar.borah@intel.com>
> > > > > > Signed-off-by: Chaitanya Kumar Borah
> > > > > > <chaitanya.kumar.borah@intel.com>
> > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > > > > ---
> > > > > >   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
> > > > > >   1 file changed, 394 insertions(+)
> > > > > >   create mode 100644
> > > > > > Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > > >
> > > > > > diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > > > b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > > > new file mode 100644
> > > > > > index 000000000000..60ce515b6ea7
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > >
> > > ...
> > >
> > > Hi Uma!
> >
> > Thanks Pekka for the feedback and useful inputs.
> 
> Hi Uma,
> 
> sorry to say, but the overall feeling I get from this proposal is that it is just the
> current color related KMS properties wrapped in a pipeline blob. That is not the
> re-design I believe we are looking for, and the feeling is based on several details
> that are just copied from the current property design. Also the "private" stuff has
> to go.

Hi Pekka,
Ok, got the concerns in general.  We will try to evaluate in deeper detail the
property based design and come back if there are some issues or inputs.
 
At Intel we don't need private as of now, but we thought of having an option to
enable any custom hardware or vendor. But we can drop the same for now if
community doesn't feel the need for it.

> All the varying LUT entries, varying LUT precision, 1D/3D LUTs, varying LUT tap
> distribution, and parametrized curves are good development, but right now we
> are looking at things one step higher level: the overall color pipeline design and
> how to represent any operation. Most of this series is considering details below
> the current attention level, hence I'm paying attention only to the first few
> patches.

We will need to precisely describe the hardware in userspace. Number of luts, precision,
segments etc.., we can't just pass EOTF's as enum from userspace and let driver put
hardcoded values to LUT. This will be nothing but an extension of descriptive behaviour.
This will be needed as there are multiple colorspaces possible and even LUTS can be
used to perform tone mapping. So, we need userspace to be able to program luts directly.

This is something we must expose to userspace. We will check if this can be fitted in
property based approach.

Thanks for all the feedback and comments on the design. We will work on it and get back.

Regards,
Uma Shankar

> More below.
> 
> > > > > > +This color pipeline is then packaged within a blob for the
> > > > > > +user space to retrieve it. Details can be found in the next
> > > > > > +section
> > > > > > +
> > > > >
> > > > > Not sure I like blobs that contain other blob ids.
> > > >
> > > > It provides flexibility and helps with just one interface to
> > > > userspace. Its easy to handle and manage once we get the hang of it 
Christopher Braga Sept. 7, 2023, 8:08 p.m. UTC | #7
On 8/29/2023 12:03 PM, Uma Shankar wrote:
> Add the documentation for the new proposed Plane Color Pipeline.
> 
> Co-developed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
>   1 file changed, 394 insertions(+)
>   create mode 100644 Documentation/gpu/rfc/plane_color_pipeline.rst
> 
> diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst b/Documentation/gpu/rfc/plane_color_pipeline.rst
> new file mode 100644
> index 000000000000..60ce515b6ea7
> --- /dev/null
> +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst
> @@ -0,0 +1,394 @@
> +=======================================
> + Plane Color Pipeline: A UAPI proposal
> +=======================================
> +
> +To build the proposal on, lets take the premise of a color pipeline as shown
> +below.
> +
Hi Uma,
Thanks for posting this. A few comments, with some echoing the 
sentiments of other commenters on the patch set.

> + +-------------------------------------------+
> + |                RAM                        |
> + |  +------+    +---------+    +---------+   |
> + |  | FB 1 |    |  FB 2   |    | FB N    |   |
> + |  +------+    +---------+    +---------+   |
> + +-------------------------------------------+
> +       |  Plane Color Hardware Block |
> + +--------------------------------------------+
> + | +---v-----+   +---v-------+   +---v------+ |
> + | | Plane A |   | Plane B   |   | Plane N  | |
> + | | Pre-CSC |   | Pre-CSC   |   | Pre-CSC  | |
> + | +---+-----+   +---+-------+   +---+------+ |
> + |     |             |               |        |
> + | +---v-----+   +---v-------+   +---v------+ |
> + | |Plane A  |   | Plane B   |   | Plane N  | |
> + | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
> + | +---+-----+   +----+------+   +----+-----+ |
> + |     |              |               |       |
> + | +---v-----+   +----v------+   +----v-----+ |
> + | | Plane A |   | Plane B   |   | Plane N  | |
> + | |Post-CSC |   | Post-CSC  |   | Post-CSC | |
> + | +---+-----+   +----+------+   +----+-----+ |
> + |     |              |               |       |
> + +--------------------------------------------+
> ++------v--------------v---------------v-------|
> +||                                           ||
> +||           Pipe Blender                    ||
> ++--------------------+------------------------+
> +|                    |                        |
> +|        +-----------v----------+             |
> +|        |  Pipe Pre-CSC        |             |
> +|        |                      |             |
> +|        +-----------+----------+             |
> +|                    |            Pipe Color  |
> +|        +-----------v----------+ Hardware    |
> +|        |  Pipe CSC/CTM        |             |
> +|        |                      |             |
> +|        +-----------+----------+             |
> +|                    |                        |
> +|        +-----------v----------+             |
> +|        |  Pipe Post-CSC       |             |
> +|        |                      |             |
> +|        +-----------+----------+             |
> +|                    |                        |
> ++---------------------------------------------+
> +                     |
> +                     v
> +                Pipe Output
> +
> +Each plane consists of the following color blocks
> + * Pre-CSC : This block can used to linearize the input frame buffer data.
> +             The linear data then can be further acted on by the following
> +             color hardware blocks in the display hardware pipeline
> +
> + * CSC/CTM: Used to program color transformation matrix, this block is used
> +            to perform color space conversions like BT2020 to BT709 or BT601
> +            etc. This block acts on the linearized data coming from the
> +            Pre-CSC HW block.
> +
> + * Post-CSC: This HW block can be used to non-linearize frame buffer data to
> +             match the sink. Another use case of it could be to perform Tone
> +             mapping for HDR use-cases.
> +
> +Data from multiple planes will then be fed to pipe/crtc where it will get blended.
> +There is a similar set of HW blocks available at pipe/crtc level which acts on
> +this blended data.
> +
> +Below is a sample usecase fo video playback with sub-titles and playback
> +controls
> +
> +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐
> +│FB1         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │
> +│            ├───►│Linearize    ├────►│ BT709 to    ├───►│ SDR to HDR  │
> +│BT709 SDR   │    │             │     │ BT2020      │    │ Tone Mapping├─────┐
> +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘     │
> +(subtitles)                                                                  │
> +                                                                             │
> +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐     │
> +│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │     │
> +│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├───┐ │
> +│BT601 SDR   │    │             │     │ BT2020      │    │ Tone Mapping│   │ │
> +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘   │ │
> +(Playback controls UI)                                                     │ │
> +                                                                           │ │
> +┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐   │ │
> +│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │   │ │
> +│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├─┐ │ │
> +│BT2020 HDR  │    │             │     │ pass-through│    │ pass-through│ │ │ │
> +└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘ │ │ │
BT601 to pass-through seems odd here. Shouldn't this be BT2020 pass-through?
> +(video frame)                                                            │ │ │
> +                                                                         │ │ │
> +┌────────────────────────────────────────────────────────────────────────┴─┴─┘
> +│
> +│ ┌─────────────┐      ┌─────────────┐      ┌───────────────┐
> +│ │ CRTC        │      │ CRTC        │      │ CRTC          │
> +└─┤ PRE-CSC     ├─────►│ CSC/CTM     ├─────►│ POST-CSC      ├─────► TO Port
> +  │             │      │             │      │               │
> +  └─────────────┘      └─────────────┘      └───────────────┘
> +
> +This RFC is intended to propose an uAPI for the pre-blending color pipeline
> +(however, can be also extended to post blending pipeline).
> +
> +Below are the design considerations while designing the uAPI.
> +
> +1. Individual HW blocks can be muxed. (e.g. out of two HW blocks only one can be used)
> +2. Position of the HW block can be programmable
> +3. LUTs can be one dimentional or three dimentional
> +4. Number of LUT entries can vary across platforms
> +5. Precision of LUT entries can vary across platforms
> +6. Distribution of LUT entries may vary. e.g Mutli-segmented, Logarithmic,
> +   Piece-Wise Linear(PWL) etc
> +7. There can be parameterized/non-parameterized fixed function HW blocks.
> +   e.g. Just a hardware bit, to convert from one color space to another.
> +
> +Plane Color Pipeline: Design Proposal
> +=====================================
> +Each Color Hardware block will be represented by the structure drm_color_op.
> +
> +struct drm_color_op {
> +	enum color_op_block name;
> +	enum color_op_type type; > +	u32 blob_id;
> +	u32 private_flags;
> +};
> +
> +The members of which will constitute:
> +1. name: A standardised enum for the color hardware block
> +2. type: The type of mathematical operation done by the hardware block.
> +         e.g. 1D Curve, 3D Curve, Matrix etc.
Using both 'type' and 'name' to identify the block sounds like an over 
complication to me. From a UAPI perspective we mainly care about the 
functionality the block provides, and type should be able to cover this 
on its own. If name is needed to declare a divergence from the standard 
definition for the type, why wouldn't a new type just be declared?

I'm currently not seeing any benefit that name provides.

> +3. blob id: Id  pointing to a blob containing information about the hardware
> +         block advertising the respective capabilities to the userspace.
> +         It can be an optional field depending on the members "name" and "type".
> +4. private_flags: This can be used to provide vendor specific hints
> +         to user space
> +
If OEMs are declaring essentially hints in this property, how is user 
space going to handle this when they don't know how to interpret or 
handle them? To give any compositor a chance to use the color block 
these hints would have to result in no modification of the behavior 
agreed upon by the 'type', at which point I don't see what purpose they 
serve. The blob UAPI being fixed for a given color block type would also 
pose an issue here.

On the other hand, if these flags are only used for vendor specific 
hardware blocks it sounds like we are clubbing a proprietary block + 
definition into the color pipeline. I don't think this co-exists well 
with the intent of the original proposal.

> +
> +   For example to represent LUTs, we introduce the drm_color_lut_range
> +   structure. It can represent LUTs with varied number of entries and
> +   distributions (Multi segmented, Logarithmic etc).
> +
> +   struct drm_color_lut_range {
> +		/* DRM_MODE_LUT_* */
> +		__u32 flags;
> +		/* number of points on the curve */
> +		__u16 count;
> +		/* input/output bits per component */
> +		__u8 input_bpc, output_bpc;
> +		/* input start/end values */
> +		__s32 start, end;
> +		/* output min/max values */
> +		__s32 min, max;
> +   };
> +
> +Note: More details on exact usage and implementation of this structure can be
> +      found in the RFC. This structure is taken as is from the series.
> +      https://patchwork.freedesktop.org/series/90825/
> +      However, we can add more members to it to encompass all use-cases.
> +      For example. we can add a precision field to advertise the
> +      bitdepth of the LUTs. Similarly, we can reserve some bits in the flag
> +      field for vendor specific use cases.
> +
> +      At the same time, we don't need to pass any additional information for the
> +      CSC block as userspace and driver already agrees struct drm_color_ctm as
> +      a uAPI to pass on data.
> +
> +These color operations will form the building blocks of a color pipeline which
> +best represents the underlying Hardware. Color operations can be re-arranged,
> +substracted or added to create distinct color pipelines to accurately describe
> +the Hardware blocks present in the display engine.
> +
> +In this proposal, a color pipeline is represented as an array of
> +struct drm_color_op.
> +
> +struct drm_color_op color_pipeline_1[]
> +
> +For example to represent the pre-blending color pipeline as described above
> +
> +We can create a color pipeline like this.
> +
> +struct drm_color_op color_pipeline_1[] = {
> +	{
> +		.name = DRM_CB_PRE_CSC,
> +		.type = CURVE_1D,
> +		.blob_id = 0; /* actual value to be populated during plane
> +						 initialization*/
> +	},
> +	{
> +		.name = DRM_CB_CSC,
> +		.type = MATRIX,
> +		.blob_id = 0;
> +	},
> +	{
> +		.name = DRM_CB_POST_CSC,
> +		.type = CURVE_1D,
> +		.blob_id = 0;
> +	},
> +};

I'm noticing that in the shared IGT patch set these names are being used 
to determine position instead of, well, the actual position in the color 
pipeline. We definitely need to be careful to avoid coupling 
functionality with usage, and I think having a name field is going to 
cause developers to make that mistake / shortcut.

> +
> +Then, for individual color operation, we add blobs to advertise the capability
> +of the particular Hardware block. In case of the example pipeline, we add
> +blobs of type drm_color_lut_range for the "pre-csc" and "post-csc".
> +For the "csc" block we pass no blob id to user space as it is known to both
> +user space and driver that drm_color_ctm structure is to be used for such
> +operation.
> +
> +To represent, this in a diagram.
> +
> +   struct drm_color_op color_pipeline_1[]
> +      +---------------------------+
> +      |                           |           drm_color_op
> +      |  +---------------------+--+-----------+---------------------+
> +      |  |                     |  |           |                     |
> +      |  |                     |  |           | +-----------------+ |
> +      |  |                     |  |           | |     name        | |
> +      |  |                     |  |           | +-----------------+ |
> +      |  |                     |  |           | |     type        | |
> +      |  |    color_op_1       |  |           | +-----------------+ |
> +      |  |                     |  |           | |     blob id     | +--------+
> +      |  |                     |  |           | +-----------------+ |        |
> +      |  |                     |  |           | |     private     | |        |
> +      |  |                     |  |           | |      flags      | |        |
> +      |  |                     |  |           | +-----------------+ |        |
> +      |  |                     |  |           |                     |        |
> +      |  +---------------------+--+-----------+---------------------+        |
> +      |                           |                                          |
> +      |                           |                                          |
> +      |  +---------------------+  |                                          |
> +      |  |                     |  |           drm_color_lut_range            |
> +      |  |    color_op_2       |  |           +-------------------------+    |
> +      |  |                     |  |           |                         |    |
> +      |  |                     |  |           | +---------------------+ |    |
> +      |  +---------------------+  |           | | segment 1 {         | |<---+
> +      |                           |           | |  ...                | |
> +      |  +---------------------+  |           | |  .input_bpc = 16,   | |
> +      |  |                     |  |           | |  .output_bpc = 16,  | |
> +      |  |    color_op_3       |  |           | |  ...                | |
> +      |  |                     |  |           | | }                   | |
> +      |  |                     |  |           | +---------------------+ |
> +      |  +---------------------+  |           |                         |
> +      |             .             |           | +---------------------+ |
> +      |             .             |           | | segment 2 {         | |
> +      |             .             |           | | ...                 | |
> +      +---------------------------+           | | }                   | |
> +                                              | |                     | |
> +                                              | |                     | |
> +                                              | |                     | |
> +                                              | +---------------------+ |
> +                                              |            .            |
> +                                              |            .            |
> +                                              |            .            |
> +                                              +-------------------------+
> +
> +
> +
> +This color pipeline is then packaged within a blob for the user space to
> +retrieve it. Details can be found in the next section
While I get the intent of the blob nesting, as others have mentioned 
this does feel like a recreation of what we already have with DRM 
objects. DRM objects would also have the advantage of allowing for 
easier OEM extension. Instead of private flags, an OEM could introduce 
behavior modifiers or additional features on a color block object via 
additional properties (these properties would obviously have to default 
to a disabled state to ensure default behavior is what is defined by the 
base color block type).

> +
> +Exposing a color pipeline to user space
> +=======================================
> +
> +To advertise the available color pipelines, an immutable ENUM property
> +"GET_COLOR_PIPELINE" is introduced.
> +This enum property has blob id's as values. With each blob id representing
> +a distinct color pipeline based on underlying HW capabilities and their
> +respective combinations.
> +
> +The following output of drm_info [1], shows how color pipelines are visible
> +to userspace.
> +
> +├───Plane 0
> +    │   ├───Object ID: 31
> +    │   ├───CRTCs: {0}
> +    │   ├───Legacy info
> +        ...
> +    │       ├───"GET_COLOR_PIPELINE" (immutable): enum {no color pipeline,
> +						color pipeline 1, color pipeline 2}= no color pipeline
> +
> +To understand the capabilities of individual pipelines, first the userspace
> +need to retrieve the pipeline blob with the blob ids retrieved by reading the
> +enum property.
> +
> +Once the color pipeline is retrieved, user can then parse through
> +individual drm_color_op blocks to understand the capabilities of each
> +hardware block.
> +
> +Check IGT series to see how user space can parse through color pipelines.
> +Refer the IGT series here: https://patchwork.freedesktop.org/series/123018/
> +
> +Setting up a color pipeline
> +===========================
> +
> +Once the user space decides on a color pipeline, it can set the pipeline and
> +the corresponding data for the hardware blocks within the pipeline with
> +the BLOB property "SET_COLOR_PIPELINE".
> +
> +To achieve this two structures are introduced
> +
> +1.	struct drm_color_op_data: It represents data to be passed onto individual
> +							  color hardware blocks. It contains three members
> +                      a) name: to identify the color operation block
> +                      b) blob_id: pointing to the blob with data for the
> +                                  corresponding hardware block
> +
> +	struct drm_color_op_data {
> +		enum color_op_block name;
> +		u32 blob_id;
> +	};
> +
> +2.	struct drm_color_pipeline: This structure represents the aggregate
> +                                   pipeline to be set. it contains the following  members
> +					  a) num: pipeline number to be selected
> +					  b) size: size of the data to be passed onto the driver
> +					  c) data: array of struct drm_color_op_data with data for
> +                                                   the hardware block/s that userspace wants to
> +                                                   set values for.
> +
> +	struct drm_color_pipeline {
> +		int num;
> +		int size;
> +		struct drm_color_op_data *data;
> +	};
> +
> +	User can either:
> +	1. send data for all the color operations in a pipeline as shown in [2].
> +	   The color operation data need not be in order that the pipeline advertises
> +	   however, it should not contain data for any
> +	   color operation that is not present in the pipeline.
> +
> +	   Note: This check for pipeline is not yet implemented but if the
> +	   wider proposal is accepted we have few solutions in mind.
> +
> +	2. send data for a subset of color operations as shown in [3].For the
> +	   color operation that userspace does not send data will retain their
> +	   older state.
> +
> +	3. reset/disable the pipeline by setting the "SET_COLOR_PIPELINE" blob
> +	   property to NULL as shown in both [2] and [3]
> +
> +	4. change the color pipeline as demonstrated in [3].
> +	   On the new pipeline, the user is expected to setup all the color hardware block
> +	   Once the user requests a pipeline change, the driver will provide it a clean slate
> +           which means that all the data previously set by the user will be discarded even if
> +           there are common hardware blocks between the two(previous and current) pipelines.
> +
> +IGT implementation can be found here [4]
> +
> +Representing Fixed function hardware
> +====================================
> +
> +To provide support for fixed function hardware, the driver could expose vendor
> +specific struct drm_color_op with parameters that both the userspace and
> +driver agrees on. To demonstrate, let's consider a hyphothetical fixed
> +function hardware block that converts BT601 to BT2020.
> +The driver can choose to advertise the block as such.
> +
> +struct drm_color_op color_pipeline_X[] = {
> +	{
> +		.name = DRM_CB_PRIVATE,
> +		.type = FIXED_FUNCTION,
> +		.blob_id = 45;
> +	},
> +}
> +
> +Where the blob represents some vendor specific enums, strings or any other
> +appropriate data types which both the user-space and drivers are aligned on.
> +
> +blob:45 {
> +	VENDORXXX_BT602_TO_BT2020,
> +};

As far as I see it, one of the primary advantages of moving to this new 
color pipeline is the standardization of the color block definition. 
User space looks at the 'type', and if it supports it then it can look 
at other expected information such as 'lut_size', 'bit_depth', etc to 
determine the programming considerations.

The second we start having color blocks fully defined by private_flags 
and vendor specific blobs we end up with the same scenario we have today 
with vendor specific properties on our DRM CRTC and DRM PLANE objects. 
While we aren't going to make all the different OEM hardware match, we 
still want to encourage common approaches to block declaration so user 
space can easily identify functionality it supports across a wide range 
of hardware.


> +
> +For enabling or passing parameters to such blocks, the user can send data
> +to the driver wrapped within a blob like any other color operation block.
> +
> +	struct drm_color_op_data {
> +		.name = DRM_CB_PRIVATE;
> +		.blob_id = 46;
> +	} ;
> +
> +where blob with id 46 contains data to enable the fixed function hardware(s).
> +

All that said, thanks again for working on this. I'm definitely curious 
to see how this will all come together.

- Christopher

> +References
> +==========
> +
> +[1] https://gitlab.freedesktop.org/emersion/drm_info
> +[2] https://patchwork.freedesktop.org/patch/554827/?series=123018&rev=1
> +[3] https://patchwork.freedesktop.org/patch/554826/?series=123018&rev=1
> +[4] https://patchwork.freedesktop.org/series/123018/
Pekka Paalanen Sept. 8, 2023, 8:31 a.m. UTC | #8
On Thu, 7 Sep 2023 12:31:47 +0000
"Shankar, Uma" <uma.shankar@intel.com> wrote:

> > -----Original Message-----
> > From: Pekka Paalanen <ppaalanen@gmail.com>
> > Sent: Tuesday, September 5, 2023 5:03 PM
> > To: Shankar, Uma <uma.shankar@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar
> > <chaitanya.kumar.borah@intel.com>; dri-devel@lists.freedesktop.org; wayland-
> > devel@lists.freedesktop.org; Harry Wentland <harry.wentland@amd.com>;
> > Sebastian Wick <sebastian.wick@redhat.com>; ville.syrjala@linux.intel.com;
> > Jonas Adahl <jadahl@redhat.com>
> > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane
> > Color Pipeline
> > 
> > On Mon, 4 Sep 2023 13:44:49 +0000
> > "Shankar, Uma" <uma.shankar@intel.com> wrote:
> >   
> > > > -----Original Message-----
> > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf
> > > > Of Pekka Paalanen
> > > > Sent: Wednesday, August 30, 2023 5:59 PM
> > > > To: Shankar, Uma <uma.shankar@intel.com>
> > > > Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar
> > > > <chaitanya.kumar.borah@intel.com>; dri-devel@lists.freedesktop.org;
> > > > wayland- devel@lists.freedesktop.org
> > > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed
> > > > Plane Color Pipeline
> > > >
> > > > On Wed, 30 Aug 2023 08:59:36 +0000
> > > > "Shankar, Uma" <uma.shankar@intel.com> wrote:
> > > >  
> > > > > > -----Original Message-----
> > > > > > From: Harry Wentland <harry.wentland@amd.com>
> > > > > > Sent: Wednesday, August 30, 2023 1:10 AM
> > > > > > To: Shankar, Uma <uma.shankar@intel.com>;
> > > > > > intel-gfx@lists.freedesktop.org; dri-
> > > > > > devel@lists.freedesktop.org
> > > > > > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>;
> > > > > > wayland- devel@lists.freedesktop.org
> > > > > > Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for
> > > > > > proposed Plane Color Pipeline
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 2023-08-29 12:03, Uma Shankar wrote:  
> > > > > > > Add the documentation for the new proposed Plane Color Pipeline.
> > > > > > >
> > > > > > > Co-developed-by: Chaitanya Kumar Borah
> > > > > > > <chaitanya.kumar.borah@intel.com>
> > > > > > > Signed-off-by: Chaitanya Kumar Borah
> > > > > > > <chaitanya.kumar.borah@intel.com>
> > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > > > > > ---
> > > > > > >   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
> > > > > > >   1 file changed, 394 insertions(+)
> > > > > > >   create mode 100644
> > > > > > > Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > > > >
> > > > > > > diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > > > > b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..60ce515b6ea7
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst  
> > > >
> > > > ...
> > > >
> > > > Hi Uma!  
> > >
> > > Thanks Pekka for the feedback and useful inputs.  
> > 
> > Hi Uma,
> > 
> > sorry to say, but the overall feeling I get from this proposal is that it is just the
> > current color related KMS properties wrapped in a pipeline blob. That is not the
> > re-design I believe we are looking for, and the feeling is based on several details
> > that are just copied from the current property design. Also the "private" stuff has
> > to go.  
> 
> Hi Pekka,
> Ok, got the concerns in general.  We will try to evaluate in deeper detail the
> property based design and come back if there are some issues or inputs.
>  
> At Intel we don't need private as of now, but we thought of having an option to
> enable any custom hardware or vendor. But we can drop the same for now if
> community doesn't feel the need for it.
> 
> > All the varying LUT entries, varying LUT precision, 1D/3D LUTs, varying LUT tap
> > distribution, and parametrized curves are good development, but right now we
> > are looking at things one step higher level: the overall color pipeline design and
> > how to represent any operation. Most of this series is considering details below
> > the current attention level, hence I'm paying attention only to the first few
> > patches.  
> 
> We will need to precisely describe the hardware in userspace. Number of luts, precision,
> segments etc.., we can't just pass EOTF's as enum from userspace and let driver put
> hardcoded values to LUT. This will be nothing but an extension of descriptive behaviour.
> This will be needed as there are multiple colorspaces possible and even LUTS can be
> used to perform tone mapping. So, we need userspace to be able to program luts directly.

Hi Uma,

yes, we do need to expose freely programmable LUTs when hardware has
them. That's why I say it is good development.

However, this is not an either-or situation.

We must also be able to expose fixed-function curve blocks when
hardware has them. Please, do not confuse this with a descriptive
design. This is not about saying "this FB is using PQ encoding, convert
it to NNN for me".

This is about defining an operation, that is mathematically defined as
"the PQ EOTF with normalized domain and range", for example. This is
prescriptive, because the exact mathematical formula of the operation
is defined, and it does not depend on any properties of the block's
input or output. It contains no opinions on how to convert something to
another, like a conversion from a color system and viewing environment
to another does (e.g. sRGB<->BT.2100/PQ). Every driver exposing this
operation must implement it exactly the same, with a small allowed error
tolerance.

There are no limitations on how it can be used. Userspace can choose to
apply that formula on anything it likes, and use the result in any way
it likes, even if it is an utterly non-standard pipeline not making any
sense *to us*.

You could argue that an operation to "convert PQ to HLG" is also
prescriptive for example, and yes, it is if implementations have to
adhere to a single specific formula for it. But if implementations are
allowed choose any formula they believe is the best to implement that
operation, then it is descriptive: "I have PQ content, I want HLG out,
do whatever, I don't care".

If you really have a fixed-function hardware block that literally uses
one specific formula to convert PQ to HLG, you can and probably should
expose that as a colorop. It is prescriptive, because the formula is
fully known to userspace, and userspace will choose to use it for its
formula, and not because it converts PQ to HLG.

However, if you had to combine multiple hardware blocks to achieve the
PQ-to-HLG formula, then that does not make sense to expose as a colorop
(other than for backward compatibility if your previous hardware
generation had it as a single fixed-function block). In practise, it
would be too rigid to be useful in more than few specific situations.
It would be much more flexible to expose the actual hardware blocks you
have, and let userspace use them any way it likes. This is about
finding the right balance in abstraction for UAPI.

> This is something we must expose to userspace. We will check if this can be fitted in
> property based approach.

I'm sure it can be exposed. The trivial option is to define a colorop
with the specific operation type that defines a property that will hold
the LUT configuration blob you have designed.

You do not need to force literally everything into a million
properties, you can still use blobs where they make sense, like needing
a variable or great number of elements of some type.

As such, the overall color pipeline UAPI design has little effect
on how you would design your LUT UAPI structures.


Thanks,
pq
Uma Shankar Oct. 13, 2023, 5:46 a.m. UTC | #9
> -----Original Message-----
> From: Christopher Braga <quic_cbraga@quicinc.com>
> Sent: Friday, September 8, 2023 1:39 AM
> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; wayland-
> devel@lists.freedesktop.org
> Subject: Re: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane
> Color Pipeline
> 
> 
> 
> On 8/29/2023 12:03 PM, Uma Shankar wrote:
> > Add the documentation for the new proposed Plane Color Pipeline.
> >
> > Co-developed-by: Chaitanya Kumar Borah
> > <chaitanya.kumar.borah@intel.com>
> > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >   .../gpu/rfc/plane_color_pipeline.rst          | 394 ++++++++++++++++++
> >   1 file changed, 394 insertions(+)
> >   create mode 100644 Documentation/gpu/rfc/plane_color_pipeline.rst
> >
> > diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst
> > b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > new file mode 100644
> > index 000000000000..60ce515b6ea7
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/plane_color_pipeline.rst
> > @@ -0,0 +1,394 @@
> > +=======================================
> > + Plane Color Pipeline: A UAPI proposal
> > +=======================================
> > +
> > +To build the proposal on, lets take the premise of a color pipeline
> > +as shown below.
> > +
> Hi Uma,
> Thanks for posting this. A few comments, with some echoing the sentiments of
> other commenters on the patch set.

Hi Christopher,
Thanks for your inputs on the series and my apologies for a late reply.

> > + +-------------------------------------------+
> > + |                RAM                        |
> > + |  +------+    +---------+    +---------+   |
> > + |  | FB 1 |    |  FB 2   |    | FB N    |   |
> > + |  +------+    +---------+    +---------+   |
> > + +-------------------------------------------+
> > +       |  Plane Color Hardware Block |
> > + +--------------------------------------------+
> > + | +---v-----+   +---v-------+   +---v------+ |
> > + | | Plane A |   | Plane B   |   | Plane N  | |
> > + | | Pre-CSC |   | Pre-CSC   |   | Pre-CSC  | |
> > + | +---+-----+   +---+-------+   +---+------+ |
> > + |     |             |               |        |
> > + | +---v-----+   +---v-------+   +---v------+ |
> > + | |Plane A  |   | Plane B   |   | Plane N  | |
> > + | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
> > + | +---+-----+   +----+------+   +----+-----+ |
> > + |     |              |               |       |
> > + | +---v-----+   +----v------+   +----v-----+ |
> > + | | Plane A |   | Plane B   |   | Plane N  | |
> > + | |Post-CSC |   | Post-CSC  |   | Post-CSC | |
> > + | +---+-----+   +----+------+   +----+-----+ |
> > + |     |              |               |       |
> > + +--------------------------------------------+
> > ++------v--------------v---------------v-------|
> > +||                                           ||
> > +||           Pipe Blender                    ||
> > ++--------------------+------------------------+
> > +|                    |                        |
> > +|        +-----------v----------+             |
> > +|        |  Pipe Pre-CSC        |             |
> > +|        |                      |             |
> > +|        +-----------+----------+             |
> > +|                    |            Pipe Color  |
> > +|        +-----------v----------+ Hardware    |
> > +|        |  Pipe CSC/CTM        |             |
> > +|        |                      |             |
> > +|        +-----------+----------+             |
> > +|                    |                        |
> > +|        +-----------v----------+             |
> > +|        |  Pipe Post-CSC       |             |
> > +|        |                      |             |
> > +|        +-----------+----------+             |
> > +|                    |                        |
> > ++---------------------------------------------+
> > +                     |
> > +                     v
> > +                Pipe Output
> > +
> > +Each plane consists of the following color blocks
> > + * Pre-CSC : This block can used to linearize the input frame buffer data.
> > +             The linear data then can be further acted on by the following
> > +             color hardware blocks in the display hardware pipeline
> > +
> > + * CSC/CTM: Used to program color transformation matrix, this block is used
> > +            to perform color space conversions like BT2020 to BT709 or BT601
> > +            etc. This block acts on the linearized data coming from the
> > +            Pre-CSC HW block.
> > +
> > + * Post-CSC: This HW block can be used to non-linearize frame buffer data to
> > +             match the sink. Another use case of it could be to perform Tone
> > +             mapping for HDR use-cases.
> > +
> > +Data from multiple planes will then be fed to pipe/crtc where it will get
> blended.
> > +There is a similar set of HW blocks available at pipe/crtc level
> > +which acts on this blended data.
> > +
> > +Below is a sample usecase fo video playback with sub-titles and
> > +playback controls
> > +
> > +┌────────────┐    ┌─────────────┐     ┌─────────────┐
> ┌─────────────┐
> > +│FB1         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │
> > +│            ├───►│Linearize    ├────►│ BT709 to    ├───►│ SDR to HDR  │
> > +│BT709 SDR   │    │             │     │ BT2020      │    │ Tone Mapping├─────┐
> > +└────────────┘    └─────────────┘     └─────────────┘
> └─────────────┘     │
> > +(subtitles)                                                                  │
> > +                                                                             │
> > +┌────────────┐    ┌─────────────┐     ┌─────────────┐
> ┌─────────────┐     │
> > +│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │     │
> > +│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR
> ├───┐ │
> > +│BT601 SDR   │    │             │     │ BT2020      │    │ Tone Mapping│   │ │
> > +└────────────┘    └─────────────┘     └─────────────┘
> └─────────────┘   │ │
> > +(Playback controls UI)                                                     │ │
> > +                                                                           │ │
> > +┌────────────┐    ┌─────────────┐     ┌─────────────┐
> ┌─────────────┐   │ │
> > +│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │   │ │
> > +│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├─┐
> │ │
> > +│BT2020 HDR  │    │             │     │ pass-through│    │ pass-through│ │ │ │
> > +└────────────┘    └─────────────┘     └─────────────┘
> └─────────────┘ │ │ │
> BT601 to pass-through seems odd here. Shouldn't this be BT2020 pass-through?
> > +(video frame)                                                            │ │ │
> > +
> > +│ │ │
> >
> +┌───────────────────────────────────────────────────────────────
> ─────
> > +────┴─┴─┘
> > +│
> > +│ ┌─────────────┐      ┌─────────────┐      ┌───────────────┐
> > +│ │ CRTC        │      │ CRTC        │      │ CRTC          │
> > +└─┤ PRE-CSC     ├─────►│ CSC/CTM     ├─────►│ POST-CSC      ├─────►
> TO Port
> > +  │             │      │             │      │               │
> > +  └─────────────┘      └─────────────┘      └───────────────┘
> > +
> > +This RFC is intended to propose an uAPI for the pre-blending color
> > +pipeline (however, can be also extended to post blending pipeline).
> > +
> > +Below are the design considerations while designing the uAPI.
> > +
> > +1. Individual HW blocks can be muxed. (e.g. out of two HW blocks only
> > +one can be used) 2. Position of the HW block can be programmable 3.
> > +LUTs can be one dimentional or three dimentional 4. Number of LUT
> > +entries can vary across platforms 5. Precision of LUT entries can
> > +vary across platforms 6. Distribution of LUT entries may vary. e.g
> > +Mutli-segmented, Logarithmic,
> > +   Piece-Wise Linear(PWL) etc
> > +7. There can be parameterized/non-parameterized fixed function HW blocks.
> > +   e.g. Just a hardware bit, to convert from one color space to another.
> > +
> > +Plane Color Pipeline: Design Proposal
> > +=====================================
> > +Each Color Hardware block will be represented by the structure drm_color_op.
> > +
> > +struct drm_color_op {
> > +	enum color_op_block name;
> > +	enum color_op_type type; > +	u32 blob_id;
> > +	u32 private_flags;
> > +};
> > +
> > +The members of which will constitute:
> > +1. name: A standardised enum for the color hardware block 2. type:
> > +The type of mathematical operation done by the hardware block.
> > +         e.g. 1D Curve, 3D Curve, Matrix etc.
> Using both 'type' and 'name' to identify the block sounds like an over complication
> to me. From a UAPI perspective we mainly care about the functionality the block
> provides, and type should be able to cover this on its own. If name is needed to
> declare a divergence from the standard definition for the type, why wouldn't a
> new type just be declared?
> 
> I'm currently not seeing any benefit that name provides.

We kept name just to give some logical identification to the hardware block with a
human readable name. But I agree given the combinations which could arise, its
hard to come up with a fixed name for a block. We can drop it.

> > +3. blob id: Id  pointing to a blob containing information about the hardware
> > +         block advertising the respective capabilities to the userspace.
> > +         It can be an optional field depending on the members "name" and "type".
> > +4. private_flags: This can be used to provide vendor specific hints
> > +         to user space
> > +
> If OEMs are declaring essentially hints in this property, how is user space going to
> handle this when they don't know how to interpret or handle them? To give any
> compositor a chance to use the color block these hints would have to result in no
> modification of the behavior agreed upon by the 'type', at which point I don't see
> what purpose they serve. The blob UAPI being fixed for a given color block type
> would also pose an issue here.
> 
> On the other hand, if these flags are only used for vendor specific hardware
> blocks it sounds like we are clubbing a proprietary block + definition into the color
> pipeline. I don't think this co-exists well with the intent of the original proposal.

Our idea was to have an option to allow some custom hardware implementation which
can't be represented with generic design. However with the feedback received, we can surely
drop this from the generic structure. For future a custom PRIVATE_TYPE can be made
for which respective driver can implement the relevant data structures and blob contained
within the driver and supported by its custom userspace. This way generic userspace can
totally ignore it and implementation will be standardized.

> > +
> > +   For example to represent LUTs, we introduce the drm_color_lut_range
> > +   structure. It can represent LUTs with varied number of entries and
> > +   distributions (Multi segmented, Logarithmic etc).
> > +
> > +   struct drm_color_lut_range {
> > +		/* DRM_MODE_LUT_* */
> > +		__u32 flags;
> > +		/* number of points on the curve */
> > +		__u16 count;
> > +		/* input/output bits per component */
> > +		__u8 input_bpc, output_bpc;
> > +		/* input start/end values */
> > +		__s32 start, end;
> > +		/* output min/max values */
> > +		__s32 min, max;
> > +   };
> > +
> > +Note: More details on exact usage and implementation of this structure can be
> > +      found in the RFC. This structure is taken as is from the series.
> > +      https://patchwork.freedesktop.org/series/90825/
> > +      However, we can add more members to it to encompass all use-cases.
> > +      For example. we can add a precision field to advertise the
> > +      bitdepth of the LUTs. Similarly, we can reserve some bits in the flag
> > +      field for vendor specific use cases.
> > +
> > +      At the same time, we don't need to pass any additional information for the
> > +      CSC block as userspace and driver already agrees struct drm_color_ctm as
> > +      a uAPI to pass on data.
> > +
> > +These color operations will form the building blocks of a color
> > +pipeline which best represents the underlying Hardware. Color
> > +operations can be re-arranged, substracted or added to create
> > +distinct color pipelines to accurately describe the Hardware blocks present in
> the display engine.
> > +
> > +In this proposal, a color pipeline is represented as an array of
> > +struct drm_color_op.
> > +
> > +struct drm_color_op color_pipeline_1[]
> > +
> > +For example to represent the pre-blending color pipeline as described
> > +above
> > +
> > +We can create a color pipeline like this.
> > +
> > +struct drm_color_op color_pipeline_1[] = {
> > +	{
> > +		.name = DRM_CB_PRE_CSC,
> > +		.type = CURVE_1D,
> > +		.blob_id = 0; /* actual value to be populated during plane
> > +						 initialization*/
> > +	},
> > +	{
> > +		.name = DRM_CB_CSC,
> > +		.type = MATRIX,
> > +		.blob_id = 0;
> > +	},
> > +	{
> > +		.name = DRM_CB_POST_CSC,
> > +		.type = CURVE_1D,
> > +		.blob_id = 0;
> > +	},
> > +};
> 
> I'm noticing that in the shared IGT patch set these names are being used to
> determine position instead of, well, the actual position in the color pipeline. We
> definitely need to be careful to avoid coupling functionality with usage, and I think
> having a name field is going to cause developers to make that mistake / shortcut.

Yeah we can drop the idea of name and go with type only instead to avoid any confusion

> > +
> > +Then, for individual color operation, we add blobs to advertise the capability
> > +of the particular Hardware block. In case of the example pipeline, we add
> > +blobs of type drm_color_lut_range for the "pre-csc" and "post-csc".
> > +For the "csc" block we pass no blob id to user space as it is known to both
> > +user space and driver that drm_color_ctm structure is to be used for such
> > +operation.
> > +
> > +To represent, this in a diagram.
> > +
> > +   struct drm_color_op color_pipeline_1[]
> > +      +---------------------------+
> > +      |                           |           drm_color_op
> > +      |  +---------------------+--+-----------+---------------------+
> > +      |  |                     |  |           |                     |
> > +      |  |                     |  |           | +-----------------+ |
> > +      |  |                     |  |           | |     name        | |
> > +      |  |                     |  |           | +-----------------+ |
> > +      |  |                     |  |           | |     type        | |
> > +      |  |    color_op_1       |  |           | +-----------------+ |
> > +      |  |                     |  |           | |     blob id     | +--------+
> > +      |  |                     |  |           | +-----------------+ |        |
> > +      |  |                     |  |           | |     private     | |        |
> > +      |  |                     |  |           | |      flags      | |        |
> > +      |  |                     |  |           | +-----------------+ |        |
> > +      |  |                     |  |           |                     |        |
> > +      |  +---------------------+--+-----------+---------------------+        |
> > +      |                           |                                          |
> > +      |                           |                                          |
> > +      |  +---------------------+  |                                          |
> > +      |  |                     |  |           drm_color_lut_range            |
> > +      |  |    color_op_2       |  |           +-------------------------+    |
> > +      |  |                     |  |           |                         |    |
> > +      |  |                     |  |           | +---------------------+ |    |
> > +      |  +---------------------+  |           | | segment 1 {         | |<---+
> > +      |                           |           | |  ...                | |
> > +      |  +---------------------+  |           | |  .input_bpc = 16,   | |
> > +      |  |                     |  |           | |  .output_bpc = 16,  | |
> > +      |  |    color_op_3       |  |           | |  ...                | |
> > +      |  |                     |  |           | | }                   | |
> > +      |  |                     |  |           | +---------------------+ |
> > +      |  +---------------------+  |           |                         |
> > +      |             .             |           | +---------------------+ |
> > +      |             .             |           | | segment 2 {         | |
> > +      |             .             |           | | ...                 | |
> > +      +---------------------------+           | | }                   | |
> > +                                              | |                     | |
> > +                                              | |                     | |
> > +                                              | |                     | |
> > +                                              | +---------------------+ |
> > +                                              |            .            |
> > +                                              |            .            |
> > +                                              |            .            |
> > +                                              +-------------------------+
> > +
> > +
> > +
> > +This color pipeline is then packaged within a blob for the user space to
> > +retrieve it. Details can be found in the next section
> While I get the intent of the blob nesting, as others have mentioned
> this does feel like a recreation of what we already have with DRM
> objects. DRM objects would also have the advantage of allowing for
> easier OEM extension. Instead of private flags, an OEM could introduce
> behavior modifiers or additional features on a color block object via
> additional properties (these properties would obviously have to default
> to a disabled state to ensure default behavior is what is defined by the
> base color block type).

There are pros and cons to both the approaches, however we respect the
feedback of community and will try to adhere to a universally acceptable and
scalable design. We will work with Harry Wentland and other community developers
to come up with good color implementation in upstream, which meets the needs
of all stakeholders and is generic and scalable.


> > +
> > +Exposing a color pipeline to user space
> > +=======================================
> > +
> > +To advertise the available color pipelines, an immutable ENUM property
> > +"GET_COLOR_PIPELINE" is introduced.
> > +This enum property has blob id's as values. With each blob id representing
> > +a distinct color pipeline based on underlying HW capabilities and their
> > +respective combinations.
> > +
> > +The following output of drm_info [1], shows how color pipelines are visible
> > +to userspace.
> > +
> > +├───Plane 0
> > +    │   ├───Object ID: 31
> > +    │   ├───CRTCs: {0}
> > +    │   ├───Legacy info
> > +        ...
> > +    │       ├───"GET_COLOR_PIPELINE" (immutable): enum {no color pipeline,
> > +						color pipeline 1, color pipeline
> 2}= no color pipeline
> > +
> > +To understand the capabilities of individual pipelines, first the userspace
> > +need to retrieve the pipeline blob with the blob ids retrieved by reading the
> > +enum property.
> > +
> > +Once the color pipeline is retrieved, user can then parse through
> > +individual drm_color_op blocks to understand the capabilities of each
> > +hardware block.
> > +
> > +Check IGT series to see how user space can parse through color pipelines.
> > +Refer the IGT series here: https://patchwork.freedesktop.org/series/123018/
> > +
> > +Setting up a color pipeline
> > +===========================
> > +
> > +Once the user space decides on a color pipeline, it can set the pipeline and
> > +the corresponding data for the hardware blocks within the pipeline with
> > +the BLOB property "SET_COLOR_PIPELINE".
> > +
> > +To achieve this two structures are introduced
> > +
> > +1.	struct drm_color_op_data: It represents data to be passed onto
> individual
> > +							  color hardware
> blocks. It contains three members
> > +                      a) name: to identify the color operation block
> > +                      b) blob_id: pointing to the blob with data for the
> > +                                  corresponding hardware block
> > +
> > +	struct drm_color_op_data {
> > +		enum color_op_block name;
> > +		u32 blob_id;
> > +	};
> > +
> > +2.	struct drm_color_pipeline: This structure represents the aggregate
> > +                                   pipeline to be set. it contains the following  members
> > +					  a) num: pipeline number to be
> selected
> > +					  b) size: size of the data to be passed
> onto the driver
> > +					  c) data: array of struct
> drm_color_op_data with data for
> > +                                                   the hardware block/s that userspace wants to
> > +                                                   set values for.
> > +
> > +	struct drm_color_pipeline {
> > +		int num;
> > +		int size;
> > +		struct drm_color_op_data *data;
> > +	};
> > +
> > +	User can either:
> > +	1. send data for all the color operations in a pipeline as shown in [2].
> > +	   The color operation data need not be in order that the pipeline
> advertises
> > +	   however, it should not contain data for any
> > +	   color operation that is not present in the pipeline.
> > +
> > +	   Note: This check for pipeline is not yet implemented but if the
> > +	   wider proposal is accepted we have few solutions in mind.
> > +
> > +	2. send data for a subset of color operations as shown in [3].For the
> > +	   color operation that userspace does not send data will retain their
> > +	   older state.
> > +
> > +	3. reset/disable the pipeline by setting the "SET_COLOR_PIPELINE" blob
> > +	   property to NULL as shown in both [2] and [3]
> > +
> > +	4. change the color pipeline as demonstrated in [3].
> > +	   On the new pipeline, the user is expected to setup all the color
> hardware block
> > +	   Once the user requests a pipeline change, the driver will provide it a
> clean slate
> > +           which means that all the data previously set by the user will be
> discarded even if
> > +           there are common hardware blocks between the two(previous and
> current) pipelines.
> > +
> > +IGT implementation can be found here [4]
> > +
> > +Representing Fixed function hardware
> > +====================================
> > +
> > +To provide support for fixed function hardware, the driver could expose vendor
> > +specific struct drm_color_op with parameters that both the userspace and
> > +driver agrees on. To demonstrate, let's consider a hyphothetical fixed
> > +function hardware block that converts BT601 to BT2020.
> > +The driver can choose to advertise the block as such.
> > +
> > +struct drm_color_op color_pipeline_X[] = {
> > +	{
> > +		.name = DRM_CB_PRIVATE,
> > +		.type = FIXED_FUNCTION,
> > +		.blob_id = 45;
> > +	},
> > +}
> > +
> > +Where the blob represents some vendor specific enums, strings or any other
> > +appropriate data types which both the user-space and drivers are aligned on.
> > +
> > +blob:45 {
> > +	VENDORXXX_BT602_TO_BT2020,
> > +};
> 
> As far as I see it, one of the primary advantages of moving to this new
> color pipeline is the standardization of the color block definition.
> User space looks at the 'type', and if it supports it then it can look
> at other expected information such as 'lut_size', 'bit_depth', etc to
> determine the programming considerations.
> 
> The second we start having color blocks fully defined by private_flags
> and vendor specific blobs we end up with the same scenario we have today
> with vendor specific properties on our DRM CRTC and DRM PLANE objects.
> While we aren't going to make all the different OEM hardware match, we
> still want to encourage common approaches to block declaration so user
> space can easily identify functionality it supports across a wide range
> of hardware.

Yes we can skip this PRIVATE flag to avoid such situations. For Intel, we don’t need
it and generic types should suffice.

> 
> > +
> > +For enabling or passing parameters to such blocks, the user can send data
> > +to the driver wrapped within a blob like any other color operation block.
> > +
> > +	struct drm_color_op_data {
> > +		.name = DRM_CB_PRIVATE;
> > +		.blob_id = 46;
> > +	} ;
> > +
> > +where blob with id 46 contains data to enable the fixed function hardware(s).
> > +
> 
> All that said, thanks again for working on this. I'm definitely curious
> to see how this will all come together.

Thanks Christopher for your review and constructive feedback. We will work
with rest of the community to make this happen.

Apologies again for being late on my reply.

Regards,
Uma Shankar

> - Christopher
> 
> > +References
> > +==========
> > +
> > +[1] https://gitlab.freedesktop.org/emersion/drm_info
> > +[2] https://patchwork.freedesktop.org/patch/554827/?series=123018&rev=1
> > +[3] https://patchwork.freedesktop.org/patch/554826/?series=123018&rev=1
> > +[4] https://patchwork.freedesktop.org/series/123018/
diff mbox series

Patch

diff --git a/Documentation/gpu/rfc/plane_color_pipeline.rst b/Documentation/gpu/rfc/plane_color_pipeline.rst
new file mode 100644
index 000000000000..60ce515b6ea7
--- /dev/null
+++ b/Documentation/gpu/rfc/plane_color_pipeline.rst
@@ -0,0 +1,394 @@ 
+=======================================
+ Plane Color Pipeline: A UAPI proposal
+=======================================
+
+To build the proposal on, lets take the premise of a color pipeline as shown
+below.
+
+ +-------------------------------------------+
+ |                RAM                        |
+ |  +------+    +---------+    +---------+   |
+ |  | FB 1 |    |  FB 2   |    | FB N    |   |
+ |  +------+    +---------+    +---------+   |
+ +-------------------------------------------+
+       |  Plane Color Hardware Block |
+ +--------------------------------------------+
+ | +---v-----+   +---v-------+   +---v------+ |
+ | | Plane A |   | Plane B   |   | Plane N  | |
+ | | Pre-CSC |   | Pre-CSC   |   | Pre-CSC  | |
+ | +---+-----+   +---+-------+   +---+------+ |
+ |     |             |               |        |
+ | +---v-----+   +---v-------+   +---v------+ |
+ | |Plane A  |   | Plane B   |   | Plane N  | |
+ | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
+ | +---+-----+   +----+------+   +----+-----+ |
+ |     |              |               |       |
+ | +---v-----+   +----v------+   +----v-----+ |
+ | | Plane A |   | Plane B   |   | Plane N  | |
+ | |Post-CSC |   | Post-CSC  |   | Post-CSC | |
+ | +---+-----+   +----+------+   +----+-----+ |
+ |     |              |               |       |
+ +--------------------------------------------+
++------v--------------v---------------v-------|
+||                                           ||
+||           Pipe Blender                    ||
++--------------------+------------------------+
+|                    |                        |
+|        +-----------v----------+             |
+|        |  Pipe Pre-CSC        |             |
+|        |                      |             |
+|        +-----------+----------+             |
+|                    |            Pipe Color  |
+|        +-----------v----------+ Hardware    |
+|        |  Pipe CSC/CTM        |             |
+|        |                      |             |
+|        +-----------+----------+             |
+|                    |                        |
+|        +-----------v----------+             |
+|        |  Pipe Post-CSC       |             |
+|        |                      |             |
+|        +-----------+----------+             |
+|                    |                        |
++---------------------------------------------+
+                     |
+                     v
+                Pipe Output
+
+Each plane consists of the following color blocks
+ * Pre-CSC : This block can used to linearize the input frame buffer data.
+             The linear data then can be further acted on by the following
+             color hardware blocks in the display hardware pipeline
+
+ * CSC/CTM: Used to program color transformation matrix, this block is used
+            to perform color space conversions like BT2020 to BT709 or BT601
+            etc. This block acts on the linearized data coming from the
+            Pre-CSC HW block.
+
+ * Post-CSC: This HW block can be used to non-linearize frame buffer data to
+             match the sink. Another use case of it could be to perform Tone
+             mapping for HDR use-cases.
+
+Data from multiple planes will then be fed to pipe/crtc where it will get blended.
+There is a similar set of HW blocks available at pipe/crtc level which acts on
+this blended data.
+
+Below is a sample usecase fo video playback with sub-titles and playback
+controls
+
+┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐
+│FB1         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │
+│            ├───►│Linearize    ├────►│ BT709 to    ├───►│ SDR to HDR  │
+│BT709 SDR   │    │             │     │ BT2020      │    │ Tone Mapping├─────┐
+└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘     │
+(subtitles)                                                                  │
+                                                                             │
+┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐     │
+│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │     │
+│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├───┐ │
+│BT601 SDR   │    │             │     │ BT2020      │    │ Tone Mapping│   │ │
+└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘   │ │
+(Playback controls UI)                                                     │ │
+                                                                           │ │
+┌────────────┐    ┌─────────────┐     ┌─────────────┐    ┌─────────────┐   │ │
+│FB2         │    │PRE-CSC      │     │ CTM Matrix  │    │ POST-CSC    │   │ │
+│            ├───►│Linearize    ├────►│ BT601 to    ├───►│ SDR to HDR  ├─┐ │ │
+│BT2020 HDR  │    │             │     │ pass-through│    │ pass-through│ │ │ │
+└────────────┘    └─────────────┘     └─────────────┘    └─────────────┘ │ │ │
+(video frame)                                                            │ │ │
+                                                                         │ │ │
+┌────────────────────────────────────────────────────────────────────────┴─┴─┘
+│
+│ ┌─────────────┐      ┌─────────────┐      ┌───────────────┐
+│ │ CRTC        │      │ CRTC        │      │ CRTC          │
+└─┤ PRE-CSC     ├─────►│ CSC/CTM     ├─────►│ POST-CSC      ├─────► TO Port
+  │             │      │             │      │               │
+  └─────────────┘      └─────────────┘      └───────────────┘
+
+This RFC is intended to propose an uAPI for the pre-blending color pipeline
+(however, can be also extended to post blending pipeline).
+
+Below are the design considerations while designing the uAPI.
+
+1. Individual HW blocks can be muxed. (e.g. out of two HW blocks only one can be used)
+2. Position of the HW block can be programmable
+3. LUTs can be one dimentional or three dimentional
+4. Number of LUT entries can vary across platforms
+5. Precision of LUT entries can vary across platforms
+6. Distribution of LUT entries may vary. e.g Mutli-segmented, Logarithmic,
+   Piece-Wise Linear(PWL) etc
+7. There can be parameterized/non-parameterized fixed function HW blocks.
+   e.g. Just a hardware bit, to convert from one color space to another.
+
+Plane Color Pipeline: Design Proposal
+=====================================
+Each Color Hardware block will be represented by the structure drm_color_op.
+
+struct drm_color_op {
+	enum color_op_block name;
+	enum color_op_type type;
+	u32 blob_id;
+	u32 private_flags;
+};
+
+The members of which will constitute:
+1. name: A standardised enum for the color hardware block
+2. type: The type of mathematical operation done by the hardware block.
+         e.g. 1D Curve, 3D Curve, Matrix etc.
+3. blob id: Id  pointing to a blob containing information about the hardware
+         block advertising the respective capabilities to the userspace.
+         It can be an optional field depending on the members "name" and "type".
+4. private_flags: This can be used to provide vendor specific hints
+         to user space
+
+
+   For example to represent LUTs, we introduce the drm_color_lut_range
+   structure. It can represent LUTs with varied number of entries and
+   distributions (Multi segmented, Logarithmic etc).
+
+   struct drm_color_lut_range {
+		/* DRM_MODE_LUT_* */
+		__u32 flags;
+		/* number of points on the curve */
+		__u16 count;
+		/* input/output bits per component */
+		__u8 input_bpc, output_bpc;
+		/* input start/end values */
+		__s32 start, end;
+		/* output min/max values */
+		__s32 min, max;
+   };
+
+Note: More details on exact usage and implementation of this structure can be
+      found in the RFC. This structure is taken as is from the series.
+      https://patchwork.freedesktop.org/series/90825/
+      However, we can add more members to it to encompass all use-cases.
+      For example. we can add a precision field to advertise the
+      bitdepth of the LUTs. Similarly, we can reserve some bits in the flag
+      field for vendor specific use cases.
+
+      At the same time, we don't need to pass any additional information for the
+      CSC block as userspace and driver already agrees struct drm_color_ctm as
+      a uAPI to pass on data.
+
+These color operations will form the building blocks of a color pipeline which
+best represents the underlying Hardware. Color operations can be re-arranged,
+substracted or added to create distinct color pipelines to accurately describe
+the Hardware blocks present in the display engine.
+
+In this proposal, a color pipeline is represented as an array of
+struct drm_color_op.
+
+struct drm_color_op color_pipeline_1[]
+
+For example to represent the pre-blending color pipeline as described above
+
+We can create a color pipeline like this.
+
+struct drm_color_op color_pipeline_1[] = {
+	{
+		.name = DRM_CB_PRE_CSC,
+		.type = CURVE_1D,
+		.blob_id = 0; /* actual value to be populated during plane
+						 initialization*/
+	},
+	{
+		.name = DRM_CB_CSC,
+		.type = MATRIX,
+		.blob_id = 0;
+	},
+	{
+		.name = DRM_CB_POST_CSC,
+		.type = CURVE_1D,
+		.blob_id = 0;
+	},
+};
+
+Then, for individual color operation, we add blobs to advertise the capability
+of the particular Hardware block. In case of the example pipeline, we add
+blobs of type drm_color_lut_range for the "pre-csc" and "post-csc".
+For the "csc" block we pass no blob id to user space as it is known to both
+user space and driver that drm_color_ctm structure is to be used for such
+operation.
+
+To represent, this in a diagram.
+
+   struct drm_color_op color_pipeline_1[]
+      +---------------------------+
+      |                           |           drm_color_op
+      |  +---------------------+--+-----------+---------------------+
+      |  |                     |  |           |                     |
+      |  |                     |  |           | +-----------------+ |
+      |  |                     |  |           | |     name        | |
+      |  |                     |  |           | +-----------------+ |
+      |  |                     |  |           | |     type        | |
+      |  |    color_op_1       |  |           | +-----------------+ |
+      |  |                     |  |           | |     blob id     | +--------+
+      |  |                     |  |           | +-----------------+ |        |
+      |  |                     |  |           | |     private     | |        |
+      |  |                     |  |           | |      flags      | |        |
+      |  |                     |  |           | +-----------------+ |        |
+      |  |                     |  |           |                     |        |
+      |  +---------------------+--+-----------+---------------------+        |
+      |                           |                                          |
+      |                           |                                          |
+      |  +---------------------+  |                                          |
+      |  |                     |  |           drm_color_lut_range            |
+      |  |    color_op_2       |  |           +-------------------------+    |
+      |  |                     |  |           |                         |    |
+      |  |                     |  |           | +---------------------+ |    |
+      |  +---------------------+  |           | | segment 1 {         | |<---+
+      |                           |           | |  ...                | |
+      |  +---------------------+  |           | |  .input_bpc = 16,   | |
+      |  |                     |  |           | |  .output_bpc = 16,  | |
+      |  |    color_op_3       |  |           | |  ...                | |
+      |  |                     |  |           | | }                   | |
+      |  |                     |  |           | +---------------------+ |
+      |  +---------------------+  |           |                         |
+      |             .             |           | +---------------------+ |
+      |             .             |           | | segment 2 {         | |
+      |             .             |           | | ...                 | |
+      +---------------------------+           | | }                   | |
+                                              | |                     | |
+                                              | |                     | |
+                                              | |                     | |
+                                              | +---------------------+ |
+                                              |            .            |
+                                              |            .            |
+                                              |            .            |
+                                              +-------------------------+
+
+
+
+This color pipeline is then packaged within a blob for the user space to
+retrieve it. Details can be found in the next section
+
+Exposing a color pipeline to user space
+=======================================
+
+To advertise the available color pipelines, an immutable ENUM property
+"GET_COLOR_PIPELINE" is introduced.
+This enum property has blob id's as values. With each blob id representing
+a distinct color pipeline based on underlying HW capabilities and their
+respective combinations.
+
+The following output of drm_info [1], shows how color pipelines are visible
+to userspace.
+
+├───Plane 0
+    │   ├───Object ID: 31
+    │   ├───CRTCs: {0}
+    │   ├───Legacy info
+        ...
+    │       ├───"GET_COLOR_PIPELINE" (immutable): enum {no color pipeline,
+						color pipeline 1, color pipeline 2}= no color pipeline
+
+To understand the capabilities of individual pipelines, first the userspace
+need to retrieve the pipeline blob with the blob ids retrieved by reading the
+enum property.
+
+Once the color pipeline is retrieved, user can then parse through
+individual drm_color_op blocks to understand the capabilities of each
+hardware block.
+
+Check IGT series to see how user space can parse through color pipelines.
+Refer the IGT series here: https://patchwork.freedesktop.org/series/123018/
+
+Setting up a color pipeline
+===========================
+
+Once the user space decides on a color pipeline, it can set the pipeline and
+the corresponding data for the hardware blocks within the pipeline with
+the BLOB property "SET_COLOR_PIPELINE".
+
+To achieve this two structures are introduced
+
+1.	struct drm_color_op_data: It represents data to be passed onto individual
+							  color hardware blocks. It contains three members
+                      a) name: to identify the color operation block
+                      b) blob_id: pointing to the blob with data for the
+                                  corresponding hardware block
+
+	struct drm_color_op_data {
+		enum color_op_block name;
+		u32 blob_id;
+	};
+
+2.	struct drm_color_pipeline: This structure represents the aggregate
+                                   pipeline to be set. it contains the following  members
+					  a) num: pipeline number to be selected
+					  b) size: size of the data to be passed onto the driver
+					  c) data: array of struct drm_color_op_data with data for
+                                                   the hardware block/s that userspace wants to
+                                                   set values for.
+
+	struct drm_color_pipeline {
+		int num;
+		int size;
+		struct drm_color_op_data *data;
+	};
+
+	User can either:
+	1. send data for all the color operations in a pipeline as shown in [2].
+	   The color operation data need not be in order that the pipeline advertises
+	   however, it should not contain data for any
+	   color operation that is not present in the pipeline.
+
+	   Note: This check for pipeline is not yet implemented but if the
+	   wider proposal is accepted we have few solutions in mind.
+
+	2. send data for a subset of color operations as shown in [3].For the
+	   color operation that userspace does not send data will retain their
+	   older state.
+
+	3. reset/disable the pipeline by setting the "SET_COLOR_PIPELINE" blob
+	   property to NULL as shown in both [2] and [3]
+
+	4. change the color pipeline as demonstrated in [3].
+	   On the new pipeline, the user is expected to setup all the color hardware block
+	   Once the user requests a pipeline change, the driver will provide it a clean slate
+           which means that all the data previously set by the user will be discarded even if
+           there are common hardware blocks between the two(previous and current) pipelines.
+
+IGT implementation can be found here [4]
+
+Representing Fixed function hardware
+====================================
+
+To provide support for fixed function hardware, the driver could expose vendor
+specific struct drm_color_op with parameters that both the userspace and
+driver agrees on. To demonstrate, let's consider a hyphothetical fixed
+function hardware block that converts BT601 to BT2020.
+The driver can choose to advertise the block as such.
+
+struct drm_color_op color_pipeline_X[] = {
+	{
+		.name = DRM_CB_PRIVATE,
+		.type = FIXED_FUNCTION,
+		.blob_id = 45;
+	},
+}
+
+Where the blob represents some vendor specific enums, strings or any other
+appropriate data types which both the user-space and drivers are aligned on.
+
+blob:45 {
+	VENDORXXX_BT602_TO_BT2020,
+};
+
+For enabling or passing parameters to such blocks, the user can send data
+to the driver wrapped within a blob like any other color operation block.
+
+	struct drm_color_op_data {
+		.name = DRM_CB_PRIVATE;
+		.blob_id = 46;
+	} ;
+
+where blob with id 46 contains data to enable the fixed function hardware(s).
+
+References
+==========
+
+[1] https://gitlab.freedesktop.org/emersion/drm_info
+[2] https://patchwork.freedesktop.org/patch/554827/?series=123018&rev=1
+[3] https://patchwork.freedesktop.org/patch/554826/?series=123018&rev=1
+[4] https://patchwork.freedesktop.org/series/123018/