On 06/15/2015 08:53 AM, Daniel Vetter wrote: > 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@xxxxxxxxx> >>> >>> 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 I know I am late replying, apologies for that. I've been working on CSC support for V4L2 as well (still work in progress) and I would like to at least end up with the same low-level fixed point format as DRM so we can share matrix/vector calculations. Based on my experiences I have concerns about the 16.16 format: the precision is quite low which can be a problem when such values are used in matrix multiplications. In addition, while the precision may be sufficient for 8 bit color component values, I'm pretty sure it will be insufficient when dealing with 12 or 16 bit color components. In earlier versions of my CSC code I used a 12.20 format, but in the latest I switched to 32.32. This fits nicely in a u64 and it's easy to extract the integer and fractional parts. If this is going to be a generic and future proof API, then my suggestion would be to increase the precision of the underlying data type. Regards, Hans > 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@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
