Re: [v7 00/16] Add Plane Color Properties

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jun 19, 2019 at 12:33:33PM -0300, Ezequiel Garcia wrote:
> On Wed, 2019-06-19 at 18:03 +0300, Ville Syrjälä wrote:
> > On Wed, Jun 19, 2019 at 10:18:18AM -0300, Ezequiel Garcia wrote:
> > > On Wed, 2019-06-19 at 06:20 +0000, Shankar, Uma wrote:
> > > > > -----Original Message-----
> > > > > From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
> > > > > Ezequiel Garcia
> > > > > Sent: Friday, June 14, 2019 9:48 PM
> > > > > To: Shankar, Uma <uma.shankar@intel.com>
> > > > > Cc: Emil Velikov <emil.l.velikov@gmail.com>; intel-gfx@lists.freedesktop.org; Syrjala,
> > > > > Ville <ville.syrjala@intel.com>; Lankhorst, Maarten <maarten.lankhorst@intel.com>;
> > > > > dri-devel <dri-devel@lists.freedesktop.org>
> > > > > Subject: Re: [v7 00/16] Add Plane Color Properties
> > > > > 
> > > > > On Thu, 28 Mar 2019 at 16:50, Uma Shankar <uma.shankar@intel.com> wrote:
> > > > > > This is how a typical display color hardware pipeline looks like:
> > > > > >  +-------------------------------------------+
> > > > > >  |                RAM                        |
> > > > > >  |  +------+    +---------+    +---------+   |
> > > > > >  |  | FB 1 |    |  FB 2   |    | FB N    |   |
> > > > > >  |  +------+    +---------+    +---------+   |
> > > > > >  +-------------------------------------------+
> > > > > >        |  Plane Color Hardware Block |
> > > > > > +--------------------------------------------+
> > > > > >  | +---v-----+   +---v-------+   +---v------+ |
> > > > > >  | | Plane A |   | Plane B   |   | Plane N  | |
> > > > > >  | | DeGamma |   | Degamma   |   | Degamma  | |
> > > > > >  | +---+-----+   +---+-------+   +---+------+ |
> > > > > >  |     |             |               |        |
> > > > > >  | +---v-----+   +---v-------+   +---v------+ |
> > > > > >  | |Plane A  |   | Plane B   |   | Plane N  | |
> > > > > >  | |CSC/CTM  |   | CSC/CTM   |   | CSC/CTM  | |
> > > > > >  | +---+-----+   +----+------+   +----+-----+ |
> > > > > >  |     |              |               |       |
> > > > > >  | +---v-----+   +----v------+   +----v-----+ |
> > > > > >  | | Plane A |   | Plane B   |   | Plane N  | |
> > > > > >  | | Gamma   |   | Gamma     |   | Gamma    | |
> > > > > >  | +---+-----+   +----+------+   +----+-----+ |
> > > > > >  |     |              |               |       |
> > > > > >  +--------------------------------------------+
> > > > > > +------v--------------v---------------v-------|
> > > > > > > >                                           ||
> > > > > > > >           Pipe Blender                    ||
> > > > > > +--------------------+------------------------+
> > > > > > >                    |                        |
> > > > > > >        +-----------v----------+             |
> > > > > > >        |  Pipe DeGamma        |             |
> > > > > > >        |                      |             |
> > > > > > >        +-----------+----------+             |
> > > > > > >                    |            Pipe Color  |
> > > > > > >        +-----------v----------+ Hardware    |
> > > > > > >        |  Pipe CSC/CTM        |             |
> > > > > > >        |                      |             |
> > > > > > >        +-----------+----------+             |
> > > > > > >                    |                        |
> > > > > > >        +-----------v----------+             |
> > > > > > >        |  Pipe Gamma          |             |
> > > > > > >        |                      |             |
> > > > > > >        +-----------+----------+             |
> > > > > > >                    |                        |
> > > > > > +---------------------------------------------+
> > > > > >                      |
> > > > > >                      v
> > > > > >                Pipe Output
> > > > > > 
> > > > > > This patch series adds properties for plane color features. It adds
> > > > > > properties for degamma used to linearize data, CSC used for gamut
> > > > > > conversion, and gamma used to again non-linearize data as per panel
> > > > > > supported color space. These can be utilize by user space to convert
> > > > > > planes from one format to another, one color space to another etc.
> > > > > > 
> > > > > > Usersapce can take smart blending decisions and utilize these hardware
> > > > > > supported plane color features to get accurate color profile. The same
> > > > > > can help in consistent color quality from source to panel taking
> > > > > > advantage of advanced color features in hardware.
> > > > > > 
> > > > > > These patches just add the property interfaces and enable helper
> > > > > > functions.
> > > > > > 
> > > > > > This series adds Intel Gen9 specific plane gamma feature. We can build
> > > > > > up and add other platform/hardware specific implementation on top of
> > > > > > this series
> > > > > > 
> > > > > > Note: This is just to get a design feedback whether these interfaces
> > > > > > look ok. Based on community feedback on interfaces, we will implement
> > > > > > IGT tests to validate plane color features. This is un-tested currently.
> > > > > > 
> > > > > > Userspace implementation using these properties have been done in drm
> > > > > > hwcomposer by "Alexandru-Cosmin Gheorghe Alexandru-
> > > > > Cosmin.Gheorghe@arm.com"
> > > > > > from ARM. A merge request has been opened by Alexandru for
> > > > > > drm_hwcomposer, implementing the property changes for the same. Please review
> > > > > that as well:
> > > > > > https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_req
> > > > > > uests/25
> > > > > > 
> > > > > > v2: Dropped legacy gamma table for plane as suggested by Maarten.
> > > > > > Added Gen9/BDW plane gamma feature and rebase on tot.
> > > > > > 
> > > > > > v3: Added a new drm_color_lut_ext structure to accommodate 32 bit
> > > > > > precision entries, pointed to by Brian, Starkey for HDR usecases.
> > > > > > Addressed Sean,Paul comments and moved plane color properties to
> > > > > > drm_plane instead of mode_config. Added property documentation as suggested by
> > > > > Daniel, Vetter.
> > > > > > Fixed a rebase fumble which occurred in v2, pointed by Emil Velikov.
> > > > > > 
> > > > > > v4: Rebase
> > > > > > 
> > > > > > v5: Added "Display Color Hardware Pipeline" flow to kernel
> > > > > > documentation as suggested by "Ville Syrjala" and "Brian Starkey".
> > > > > > Moved the property creation to drm_color_mgmt.c file to consolidate
> > > > > > all color operations at one place. Addressed Alexandru's review comments.
> > > > > > 
> > > > > > v6: Rebase. Added support for ICL Color features. Enhanced Lut
> > > > > > precision to accept input values in u32.32 format. This is needed for
> > > > > > higher precision required in HDR data processing.
> > > > > > 
> > > > > > v7: Fixed Lut roundup and extraction function in patch 1 and address
> > > > > > definitions for Degamma index in patch 10. Rest of the patches are
> > > > > > just rebased.
> > > > > > 
> > > > > 
> > > > > I can't find any comments on this one.
> > > > > 
> > > > > What's the status of this?
> > > > 
> > > > Hi Eze,
> > > > This was blocked due to lack of a userspace consumer. Since its adds a new UAPI, we need a userspace
> > > > consumer using this for it to get merge. We have some folks signed up in Intel who can help here.
> > > > Hopefully we can see some movement on this soon.
> > > > 
> > > 
> > > Hi Uma,
> > > 
> > > Thanks a lot for your reply.
> > > 
> > > We do have a userspace consumer in chromeos [1]. This is being shipped
> > > since some time now, and I have been (perhaps too silently) waiting
> > > for this series to be merged.
> > 
> > I don't think the proposed uapi will be good enough and instead we
> > need something like the gamma_mode prop for planes as well. Eg. even
> > bunch of Intel platforms have plane gamma where the input points are
> > not evenly spaced, so we again have the problem of either lying to
> > userspace and throwing away a bunch of the user provided LUT entries,
> > or we add the gamma_mode prop that actually describes the LUT
> > capabilities.
> > 
> > Another idea that was thrown around was supporting hardware that
> > supports only hardcoded gamma curves. We need to describe those
> > somehow and let userspace select the right one.
> > 
> 
> Is the uapi "not good enough" or "incorrect"? For the chromeos usage,
> it seems this PLANE_CTM property is enough, so it's not clear
> to me exactly what we should fix or change.

The ctm is probably OK. Should be the same as what we have for the crtc
I think. The degamma/gamma probably need more work.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx