What I understood from the reviews comments from the experts, is having a central color management at DRM kernel layer is not a good idea, and we should create individual DRM properties for the color correction methods, and let the control be there in the user space level, where an atomic modeset call will take decisions and figure out what and how to be done.
I will change my design accordingly, and make them all DRM properties so that this can be directly clubbed with atomic modeset.
Please note that the color correction methods changes per platform and what's valid for one Intel platform may not be valid for other. So the atomic modeset should have a clear idea of what is supported on which platforms.
Thanks for your time and review.
Regards Shashank On 5/14/2014 9:24 PM, Thierry Reding wrote:
On Tue, May 13, 2014 at 09:18:45AM +0530, Sharma, Shashank wrote:
Daniel, Please find my comments inline.
Regards Shashank On 5/12/2014 8:58 PM, Daniel Vetter wrote:
On Mon, May 12, 2014 at 05:35:13PM +0530, Sharma, Shashank wrote:
Thanks for your time and the comments David. please find mine inline.
Regards Shashank On 5/12/2014 5:20 PM, David Herrmann wrote:
Hi
On Mon, May 12, 2014 at 12:26 PM, Sharma, Shashank shashank.sharma@intel.com wrote:
Gamma correction lut is already supported. For the other stuff we can use SET_BLOB (or fix it if it doesn't work).
Current gamma correction supports only 8 bit mode, which cant do a real gamma correction. This is only to initialize the LUT. Actual gamma correction needs 10 bit support.
As discussed in design, the idea is same, ie to fix (implement) SET_BLOB. But see some of the requirements on LUT size of VLV:
- Gamma correction: 256 values
- CSC : 9 values in form of 6 register
- Hue : 1 value (Plane level)
- Saturation: 1 value (Plane level)
- Contrast: 1 value (Plane level)
- Brightness: 1 value (Plane level)
For CHV, the requirement is again different. There are different values, which vary from platform to platform and property-by-property. Now, one method of supporting these values is create a DRM property for each, some blob, some single valued, set individual interface and set them all at random. IMHO, this looks the non-systematic way of doing it.
That's exactly what atomic modeset/pageflip is meant to address. You get the flexibility of individual properties and on top of that a way to apply them all atomically.
The same thing has to be done differently for different platfroms, with some new color corrections added, some removed, and some no of coefficients changed. I can clearly see a requirement here.
Having them separated into individual properties will make it easy for userspace to determine at runtime which of them are available and which aren't. Also it seems to me that all of these properties should have a unified userspace interface. Drivers would then be free to implement the kernel side with the hardware-specific details.
AFAIK color management is not a part of atomic modeset, but once we create such an interface, it would be really easy to club that in the atomic modeset.
See above, this is a reason to _not_ add a separate color manager. -Daniel
As I mentioned above, color manager is designed to be clubbed with atomic modeset, and will not be any blockage there.
I think the point here is that once we have atomic modesetting/pageflip then there's no longer a need to have an "atomic" color manager property since there will be a mechanism to atomically apply any number of properties.
Thierry