[PATCH v2 07/10] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Damien Lespiau]

On Sat, Jun 06, 2015 at 05:39:23PM +0530, Sharma, Shashank wrote:
> >>+    if (gamma_data->gamma_precision == I915_GAMMA_PRECISION_UNKNOWN) {
> >Switch/case instead?
> Yep, got it.
> >
> >Also, is UNKNOWN for disabling? Why not rename it to DISABLE then?
> Actually unknown is valid in case of get_property() when we want to query
> about the capabilities, just want to reuse the same, to avoid need for
> another one. Else we have to handle one extra case in each get_prop
> (disable) and set_prop(unknown)

Is it? the code isn't telling that story, at least in the current form.

get_property() should primarily be for retrieving the current value of
that property, so I'd suggest having a precision field of 0 means
what is CURRENT today (that'd be the default expected behaviour of
get_property()).

Then, 2 other "needs":

 - Query interface: how useful is the query interface in this present
   form? a query for the precision only isn't that useful as we need
   per-platform knowledge beyond that anyway (say we can't encode things
   like split gamma configurations sharing the same LUT storage)

 - Some other value to disable the function (set_property() with
   precision = 0 looks fine to me.

The get_property() path is missing from this patch set AFAICT.

-- 
Damien