From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sharma, Shashank" Subject: Re: [PATCH 0/6] Intel Color Manager Framework Date: Tue, 25 Feb 2014 03:56:59 +0000 Message-ID: References: <1392899847-2641-1-git-send-email-shashank.sharma@intel.com> <20140220131129.GG3852@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0649799833==" Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: =?iso-8859-1?Q?St=E9phane_Marchesin?= Cc: Intel Graphics Development , "Shankar, Uma" , "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org --===============0649799833== Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_FF3DDC77922A8A4BB08A3BC48A1EA8CB0165948FBGSMSX101garcor_" --_000_FF3DDC77922A8A4BB08A3BC48A1EA8CB0165948FBGSMSX101garcor_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Thanks for your comments St=E9phane Please find mine inline. In general, I got the overall recommendations that if this implementation c= omes from generic DRM property, it would be easy to club with general inter= faces, and atomic modeset calls. I will work on this, and will come back with modified patches. Regards Shashank From: St=E9phane Marchesin [mailto:stephane.marchesin@gmail.com] Sent: Monday, February 24, 2014 9:35 AM To: Sharma, Shashank Cc: Ville Syrj=E4l=E4; Intel Graphics Development; Shankar, Uma; dri-devel@= lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 0/6] Intel Color Manager Framework >+1. We'e looking into hooking up color correction controls, and if the int= erface isn't standard our user space won't be portable across drivers. Ther= e are multiple reasons for using drm properties: >- the KMS interface already provides a way to set the gamma ramp, which th= is code seems to replicate. >The current KMS interface just initializes the gamma soft LUT palette reg= isters, in 8 bit mode corresponding to unit gamma. It's impossible to apply= accurate values corresponding to gamma=3D2.2 or 1.5 from KMS >Because for that we need to program palette registers in 10.6 bit mode of = hardware. >Then the existing interface should be extended. Otherwise you have two way= s to do the same thing... Agree. >- the KMS interface allows us to name properties independently and enumera= te them. It seems like right now you can't enumerate properties or guess wh= at "property 0" is. I'd rather set the "Color conversion matrix" than remem= ber to set >"property 0" (and even then, I'm not really sure it exists). All the properties are getting enumerated in color manager register functio= n. The framework defines proper identifiers and mapping for each property, = and every property is having a corresponding soft-lut to be loaded with cor= rection values. Correct me if I'm wrong, but I don't see a way for user space to query the = presence/absence of a given property. KMS allows that. The color manager read function dumps the no of properties, and current sta= tus of the property. But I agree its better interface to have it in form of= property, as far as the central control is concerned. - you can reuse the get/set infrastructure which is already in place >Another thing that came out of the discussion on irc is that we should sta= ndardize the properties. For example we could use a text file describing th= e name of the controls and the format of the data (something similar to the= device tree >bindings). That way user space can expect "color conversion m= atrix" to mean the same thing everywhere, to get the same data as input, an= d to work the same way on all platforms. If you can please have a look on the header file, we are almost doing the s= ame thing, in form of a protocol. >This protocol however is not extensible. With the KMS interface I can alre= ady do the following from user space: >- query the existence of a given property >- set each property in a portable fashion (for example the same gamma ramp= code works on all DRM drivers) >- easily match properties to a given crtc Actually each of this is possible from color manager read/write, read dumps= information per pipe basis. --_000_FF3DDC77922A8A4BB08A3BC48A1EA8CB0165948FBGSMSX101garcor_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

Thanks for your comments St=E9phane

Please find mine inline.

 

 

In general, I got the overall recommenda= tions that if this implementation comes from generic DRM property, it would= be easy to club with general interfaces, and atomic modeset calls.

I will work on this, and will come back = with modified patches.

 

Regards

Shashank

From: St=E9pha= ne Marchesin [mailto:stephane.marchesin@gmail.com]
Sent: Monday, February 24, 2014 9:35 AM
To: Sharma, Shashank
Cc: Ville Syrj=E4l=E4; Intel Graphics Development; Shankar, Uma; dri= -devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 0/6] Intel Color Manager Framework

 

>+1. We'e looking into= hooking up color correction controls, and if the interface isn't standard = our user space won't be portable across drivers. There are multiple reasons for using drm properties:

>- the KMS interface alrea= dy provides a way to set the gamma ramp, which this code seems to replicate= .

 >The current KMS interface just initializes the gamma soft LUT palette register= s, in 8 bit mode corresponding to unit gamma. It’s impossible to appl= y accurate values corresponding to gamma=3D2.2 or 1.5 from KMS

>B= ecause for that we need to program palette registers in 10.6 bit mode of hardware.

>Then the ex= isting interface should be extended. Otherwise you have two ways to do the = same thing...

 

Agree.

 

>- the KMS interface allow= s us to name properties independently and enumerate them. It seems like rig= ht now you can't enumerate properties or guess what "property 0" is. I'd rather set the "Color conve= rsion matrix" than remember to set >"property 0" (and even t= hen, I'm not really sure it exists).

 

All the properties are getting enumerated in col= or manager register function. The framework defines proper identifiers and = mapping for each property, and every property is having a corresponding soft-lut to be loaded with correction values.  

Correct me if I'm wrong, but I don't see a way for u= ser space to query the presence/absence of a given property. KMS allows tha= t.

The color manager read= function dumps the no of properties, and current status of the property. B= ut I agree its better interface to have it in form of property, as far as t= he central control is concerned.  

 

- you can reuse the get/set infrastructure which is already in pla= ce

 

 

>Another thing that came o= ut of the discussion on irc is that we should standardize the properties. F= or example we could use a text file describing the name of the controls and the format of the data (something similar to = the device tree >bindings). That way user space can= expect "color conversion matrix" to mean the same thing everywhe= re, to get the same data as input, and to work the same way on all platform= s.

If you can please have a look on the header f= ile, we are almost doing the same thing, in form of a protocol.

>This protoc= ol however is not extensible. With the KMS interface I can already do the f= ollowing from user space:

>- query the= existence of a given property

>- set each = property in a portable fashion (for example the same gamma ramp code works = on all DRM drivers)

>- easily ma= tch properties to a given crtc

 <= /p>

Actually each of this is = possible from color manager read/write, read dumps information per pipe bas= is.

 <= /p>

 

--_000_FF3DDC77922A8A4BB08A3BC48A1EA8CB0165948FBGSMSX101garcor_-- --===============0649799833== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0649799833==--