RE: [RFC 01/33] drm/doc/rfc: Add RFC document for proposed Plane Color Pipeline
> -Original Message- > From: dri-devel On Behalf Of Pekka > Paalanen > Sent: Wednesday, August 30, 2023 5:59 PM > To: Shankar, Uma > Cc: intel-...@lists.freedesktop.org; Borah, Chaitanya Kumar > ; dri-de...@lists.freedesktop.org; wayland- > de...@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 + > "Shankar, Uma" wrote: > > > > -Original Message- > > > From: Harry Wentland > > > Sent: Wednesday, August 30, 2023 1:10 AM > > > To: Shankar, Uma ; > > > intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org > > > Cc: Borah, Chaitanya Kumar ; > > > wayland- de...@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 > > > > > > > > Signed-off-by: Chaitanya Kumar Borah > > > > > > > > Signed-off-by: Uma Shankar > > > > --- > > > > .../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 ..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 😊. > > > > We can clearly define the steps of parsing and data structures to be > > used while interpreting and parsing the blobs. > > Don't forget extendability. Possibly every single struct will need some kind > of > versioning, and then it's not simple to parse anymore. Add to that new/old > kernel > vs. old/new userspace, and it seems a bit nightmarish to design. Structure to be used to interpret the blob should be defined as UAPI only and is not expected to change once agreed upon. It should be interpreted like a standard property. So structure to be used, say for 3dLut or 1dlut or CTM operations should be standardized and fixed. No versioning of structure should be done and same rules/restrictions as of UAPI property should be applied. New vs old userspace problem exists even today as you rightly highlighted in mail below, however we are planning to propose that we clean the hardware state once the userspace client switches or same client switches the pipeline. > Also since it's records inside a single blob, it's like a new file > format: every record needs a standard header that allows skipping it > appropriately if userspace does not understand it, or you need a standard > index > telling where everything is. Making all records the same size would waste > space, > and extendability requires variable size. The design currently implements 1 hardware block by a struct drm_color_op data structure. Multiple such blocks make the pipeline. So userspace just needs to get the pipeline and then parse blocks 1 by 1. For blocks which it doesn't understand it can just skip and move to the next one. Each block is differentiated by a unique "name" standardized by an enum which will be part of the UAPI. Thus we will have scope for variable size blob to represent the particular hardware pipeline, userspace can parse and implement whichever blocks it understands. Only rule defined by UAPI is the way the respective block is to be parsed and programmed. > I also would not assume that we can declare a standard set of blocks and that > nothing else will be needed. The existing hardware is too diverse for that > from > what I have understood. I assume that some hardware have blocks unique to > them, and they want to at least expose that functionality through a UAPI that > allows at least generic enumeration of functionality, even if it needs > specialized > userspace code to actually make use of. Yeah, this is right and for that reason we came up with an idea of DRM_CB_PRIVATE name for the hardware block. This will tell userspace that this is private hardware block for a particular hardware vendor. Generic userspace will ignore this block. Vendor specific HAL or compositor implementation can parse and use this block. To interpret the blob_id and assign to a respective data structure, private_flags will be used. These private_flags will be agreed upon by HAL and vendor implem
RE: [RFC 02/33] drm: Add color operation structure
> -Original Message-
> From: dri-devel On Behalf Of Pekka
> Paalanen
> Sent: Wednesday, August 30, 2023 6:30 PM
> To: Shankar, Uma
> Cc: intel-...@lists.freedesktop.org; Borah, Chaitanya Kumar
> ; dri-de...@lists.freedesktop.org; wayland-
> de...@lists.freedesktop.org
> Subject: Re: [RFC 02/33] drm: Add color operation structure
>
> On Tue, 29 Aug 2023 21:33:51 +0530
> Uma Shankar wrote:
>
> > From: Chaitanya Kumar Borah
> >
> > Each Color Hardware block will be represented uniquely in the color
> > pipeline. Define the structure to represent the same.
> >
> > 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.
> >
> > Co-developed-by: Uma Shankar
> > Signed-off-by: Uma Shankar
> > Signed-off-by: Chaitanya Kumar Borah
> > ---
> > include/uapi/drm/drm_mode.h | 72
> > +
> > 1 file changed, 72 insertions(+)
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index ea1b639bcb28..882479f41745 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -943,6 +943,78 @@ struct hdr_output_metadata {
> > };
> > };
> >
> > +/**
> > + * enum color_op_block
> > + *
> > + * Enums to identify hardware color blocks.
> > + *
> > + * @DRM_CB_PRE_CSC: LUT before the CTM unit
> > + * @DRM_CB_CSC: CTM hardware supporting 3x3 matrix
> > + * @DRM_CB_POST_CSC: LUT after the CTM unit
> > + * @DRM_CB_3D_LUT: LUT hardware with coefficients for all
> > + * color components
> > + * @DRM_CB_PRIVATE: Vendor specific hardware unit. Vendor
> > + * can expose a custom hardware by defining a
> > + * color operation block with this name as
> > + * identifier
>
> This naming scheme does not seem to work. It assumes a far too rigid pipeline,
> just like the old KMS property design. What if you have two other operations
> between PRE_CSC and CSC?
>
> What sense do PRE_CSC and POST_CSC make if you don't happen to have a CSC
> operation?
Sure, we can re-look at the naming. However, it will be good to define some
standard
operations common to all vendors and keep the rest as vendor private.
> What if a driver put POST_CSC before PRE_CSC in its pipeline?
>
> What if your CSC is actually a series of three independent operations, and in
> addition you have PRE_CSC and POST_CSC?
We should try to standardized the operations as much as possible and leave rest
as
vendor private. Current proposal allows us to do that.
> 3D_LUT is an operation category, not a name. The same could be said about
> private.
Sure, will fix this.
> Given that all these are also UAPI, do we also need protect old userspace from
> seeing values it does not understand?
For the values userspace doesn't understand, it can ignore the blocks. We
should ensure
that userspace always gets a clean state wrt color hardware state and no
baggage from
another client should be there. With that there is no burden of disabling that
particular
block will be there on an older userspace.
> > + */
> > +enum color_op_block {
> > + DRM_CB_INVAL = -1,
> > +
> > + DRM_CB_PRE_CSC = 0,
> > + DRM_CB_CSC,
> > + DRM_CB_POST_CSC,
> > + DRM_CB_3D_LUT,
> > +
> > + /* Any new generic hardware block can be updated here */
> > +
> > + /*
> > +* PRIVATE is kept at 255 to make it future proof and leave
> > +* scope for any new addition
> > +*/
> > + DRM_CB_PRIVATE = 255,
> > + DRM_CB_MAX = DRM_CB_PRIVATE,
> > +};
> > +
> > +/**
> > + * enum color_op_type
> > + *
> > + * These enums are to identify the mathematical operation that
> > + * a hardware block is capable of.
> > + * @CURVE_1D: It represents a one dimensional lookup table
> > + * @CURVE_3D: Represents lut value for each color component for 3d
> > +lut capable hardware
> > + * @MATRIX: It represents co-efficients for a CSC/CTM matrix hardware
> > + * @FIXED_FUNCTION: To enable and program any custom fixed function
> > +hardware unit */ enum color_op_type {
> > + CURVE_1D,
> > + CURVE_3D,
> > + MATRIX,
> > + FIXED_FUNCTION,
>
> My assumption was that a color_op_type would clearly and uniquely define the
> mathematical model of the operation and the UABI structure of the parameter
> blob. That means we need different values for uniform vs. exponentially vs.
> programmable distributed 1D LUT, etc.
In the hardware the LUTS are programmed as they are received from userspace.
So once the userspace gets to know the distribution of LUTS, segments,
precision,
Number of lut samples, it can create the lut values to be programmed.
This information on the distribution of luts in the hardware can be extracted
by the
drm_color_lut_range structure which is exposed as blob in the h
RE: [RFC 00/33] Add Support for Plane Color Pipeline
> -Original Message- > From: Sebastian Wick > Sent: Thursday, August 31, 2023 2:46 AM > To: Shankar, Uma > Cc: Harry Wentland ; intel- > g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org; wayland- > de...@lists.freedesktop.org; Ville Syrjala ; > Pekka > Paalanen ; Simon Ser ; > Melissa Wen ; Jonas Ã…dahl ; Shashank > Sharma ; Alexander Goins ; > Naseer Ahmed ; Christopher Braga > > Subject: Re: [RFC 00/33] Add Support for Plane Color Pipeline > > On Wed, Aug 30, 2023 at 08:47:37AM +, Shankar, Uma wrote: > > > > > > > -Original Message- > > > From: Harry Wentland > > > Sent: Wednesday, August 30, 2023 12:56 AM > > > To: Shankar, Uma ; > > > intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org > > > Cc: wayland-devel@lists.freedesktop.org; Ville Syrjala > > > ; Pekka Paalanen > > > ; Simon Ser ; > > > Melissa Wen ; Jonas Ã…dahl ; > > > Sebastian Wick ; Shashank Sharma > > > ; Alexander Goins ; > > > Naseer Ahmed ; Christopher Braga > > > > > > Subject: Re: [RFC 00/33] Add Support for Plane Color Pipeline > > > > > > +CC Naseer and Chris, FYI > > > > > > See https://patchwork.freedesktop.org/series/123024/ for whole series. > > > > > > On 2023-08-29 12:03, Uma Shankar wrote: > > > > Introduction > > > > > > > > > > > > Modern hardwares have various color processing capabilities both > > > > at pre-blending and post-blending phases in the color pipeline. > > > > The current drm implementation exposes only the post-blending > > > > color hardware blocks. Support for pre-blending hardware is missing. > > > > There are multiple use cases where pre-blending color hardware > > > > will be > > > > useful: > > > > a) Linearization of input buffers encoded in various transfer > > > >functions. > > > > b) Color Space conversion > > > > c) Tone mapping > > > > d) Frame buffer format conversion > > > > e) Non-linearization of buffer(apply transfer function) > > > > f) 3D Luts > > > > > > > > and other miscellaneous color operations. > > > > > > > > Hence, there is a need to expose the color capabilities of the > > > > hardware to user-space. This will help userspace/middleware to use > > > > display hardware for color processing and blending instead of > > > > doing it through GPU shaders. > > > > > > > > > > Thanks, Uma, for sending this. I've been working on something > > > similar but you beat me to it. :) > > > > Thanks Harry for the useful feedback and overall collaboration on this so > > far. > > > > > > > > > > Work done so far and relevant references > > > > > > > > > > > > Some implementation is done by Intel and AMD/Igalia to address the same. > > > > Broad consensus is there that we need a generic API at drm core to > > > > suffice the use case of various HW vendors. Below are the links > > > > capturing the discussion so far. > > > > > > > > Intel's Plane Color Implementation: > > > > https://patchwork.freedesktop.org/series/90825/ > > > > AMD's Plane Color Implementation: > > > > https://patchwork.freedesktop.org/series/116862/ > > > > > > > > > > > > Hackfest conclusions > > > > > > > > > > > > HDR/Color Hackfest was organised by Redhat to bring all the > > > > industry stakeholders together and converge on a common uapi > expectations. > > > > Participants from Intel, AMD, Nvidia, Collabora, Redhat, Igalia > > > > and other prominent user-space developers and maintainers. > > > > > > > > Discussions happened on the uapi expectations, opens, nature of > > > > hardware of multiple hardware vendors, challenges in generalizing > > > > the same and the path forward. Consensus was made that drm core > > > > should implement descriptive APIs and not go with prescriptive > > > > APIs. DRM core should just expose the hardware capabilities; > > > > enabling, customizing and programming the same should be done by > > > > the user-space. Driver should just > > > honor the user space request without doing any operations internally. > > > > > > > > Thanks to Simon Ser, for nicely documenting the design consensus > > > > and an UAPI RFC which can be referred to here: > > > > > > > > https://lore.kernel.org/dri-devel/QMers3awXvNCQlyhWdTtsPwkp5ie9bze > > > > _hD5 > > > > > > > > nAccFW7a_RXlWjYB7MoUW_8CKLT2bSQwIXVi5H6VULYIxCdgvryZoAoJnC5lZgyK1 > Q > > > Wn48 > > > > 8=@emersion.fr/ > > > > > > > > > > > > Design considerations > > > > = > > > > > > > > Following are the important aspects taken into account while > > > > designing the current RFC > > > > proposal: > > > > > > > > 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 in the pipeline can be programmable > > > > 3. LUTs can be one dimentional or three dimentional > > > > 4. Number of LUT entries can vary across platforms > > > > 5. Preci
