Re: Design review request: DRM color manager

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> 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:

1. Gamma correction: 256 values
2. CSC : 9 values in form of 6 register
3. Hue : 1 value (Plane level)
4. Saturation: 1 value (Plane level)
5. Contrast: 1 value (Plane level)
6. 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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux