[Intel-gfx] [PATCH v2 00/10] Color Manager Implementation

[Daniel Vetter]

On Tue, Jun 09, 2015 at 01:50:48PM +0100, Damien Lespiau wrote:
> On Thu, Jun 04, 2015 at 07:12:31PM +0530, Kausal Malladi wrote:
> > From: Kausal Malladi <Kausal.Malladi at intel.com>
> > 
> > This patch set adds color manager implementation in drm/i915 layer.
> > Color Manager is an extension in i915 driver to support color 
> > correction/enhancement. Various Intel platforms support several
> > color correction capabilities. Color Manager provides abstraction
> > of these properties and allows a user space UI agent to 
> > correct/enhance the display.
> 
> So I did a first rough pass on the API itself. The big question that
> isn't solved at the moment is: do we want to try to do generic KMS
> properties for pre-LUT + matrix + post-LUT or not. "Generic" has 3 levels:
> 
>   1/ Generic for all KMS drivers
>   2/ Generic for i915 supported platfoms
>   3/ Specific to each platform
> 
> At this point, I'm quite tempted to say we should give 1/ a shot. We
> should be able to have pre-LUT + matrix + post-LUT on CRTC objects and
> guarantee that, when the drivers expose such properties, user space can
> at least give 8 bits LUT + 3x3 matrix + 8 bits LUT.
> 
> It may be possible to use the "try" version of the atomic ioctl to
> explore the space of possibilities from a generic user space to use
> bigger LUTs as well. A HAL layer (which is already there in some but not
> all OSes) would still be able to use those generic properties to load
> "precision optimized" LUTs with some knowledge of the hardware.

Yeah, imo 1/ should be doable. For the matrix we should be able to be
fully generic with a 16.16 format. For gamma one option would be to have
an enum property listing all the supported gamma table formats, of which
8bit 256 entry (the current standard) would be a one. This enum space
would need to be drm-wide ofc. Then the gamma blob would just contain the
table. This way we can allow funky stuff like the 1025th entry for 1.0+
values some intel tables have, and similar things.

Wrt pre-post and plan/crtc I guess we'd just add the properties to all the
objects where they're possible on a given platform and then the driver
must check if there's constraints (e.g. post-lut gamma only on 1 plane or
the crtc or similar stuff).

Also there's the legacy gamma ioctl. That should forward to the crtc gamma
(and there probably pick post lut and pre-lut only if there's no post
lut). For names I'd suggest

"pre-gamma-type", "pre-gamma-data", "post-gamma-type" and
"post-gamma-data" but I don't care terrible much about them.
-Daniel

> 
> Option 3/ is, IMHO, a no-go, we should really try hard to limit the work
> we need to do per-platform, which means defining a common format for the
> values we give to the kernel. As stated in various places, 16.16 seems
> the format of choice, even for the LUTs as we have wide gamut support in
> some of the LUTs where we can map values > 1.0 to other values > 1.0.
> 
> Another thing, the documentation of the interface needs to be a bit more
> crisp. For instance, we don't currently define the order in which the
> CSC and LUT transforms of this patch set are applied: is this a de-gamma
> LUT to do the CSC in linear space? but then that means the display is
> linear, oops. So it must be a post-CSC lut, but then we don't de-gamma
> sRGB (not technically a single gamma power curve for sRGB, but details,
> details) before applying a linear transform. So with this interface, we
> have to enforce the fbs are linear, losing dynamic range. I'm sure later
> patches would expose more properties, but as a stand-alone patch set, it
> would seem we can't do anything useful?
> 
> -- 
> Damien
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch