[PATCH 10/22] drm/i915: Register color correction capabilities

[Emil Velikov]

On 10 October 2015 at 06:01, Sharma, Shashank <shashank.sharma at intel.com> wrote:
> On 10/10/2015 3:51 AM, Emil Velikov wrote:
>>
>> Hi Shashank,
>>
>> On 9 October 2015 at 20:29, Shashank Sharma <shashank.sharma at intel.com>
>> wrote:
>>>
>>>  From DRM color management:
>>> ============================
>>> DRM color manager supports these color properties:
>>> 1. "ctm": Color transformation matrix property, where a
>>>     color transformation matrix of 9 correction values gets
>>>     applied as correction.
>>> 2. "palette_before_ctm": for corrections which get applied
>>>     beore color transformation matrix correction.
>>> 3. "palette_after_ctm": for corrections which get applied
>>>     after color transformation matrix correction.
>>>
>>> These color correction capabilities may differ per platform, supporting
>>> various different no. of correction coefficients. So DRM color manager
>>> support few properties using which a user space can query the platform's
>>> capability, and prepare color correction accordingly.
>>> These query properties are:
>>> 1. cm_coeff_after_ctm_property
>>> 2. cm_coeff_before_ctm_property
>>> (CTM is fix to 9 coefficients across industry)
>>>
>>> Now, Intel color manager registers:
>>> ======================================
>>> 1. Gamma correction property as "palette_after_ctm" property
>>> 2. Degamma correction capability as "palette_bafore_ctm" property
>>>     capability as "palette_after_ctm" DRM color property hook.
>>> 3. CSC as "ctm" property.
>>>
>>> So finally, This patch does the following:
>>> 1. Add a function which loads the platform's color correction
>>>     capabilities in the cm_crtc_palette_capabilities_property structure.
>>> 2. Attaches the cm_crtc_palette_capabilities_property to every CRTC
>>>     getting initiaized.
>>> 3. Adds two new parameters "num_samples_after_ctm" and
>>>     "num_samples_before_ctm" in intel_device_info as gamma and
>>>     degamma coefficients vary per platform basis.
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>>> Signed-off-by: Kausal Malladi <kausalmalladi at gmail.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h            |  2 ++
>>>   drivers/gpu/drm/i915/intel_color_manager.c | 33
>>> +++++++++++++++++++++++++++++-
>>>   2 files changed, 34 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index ad37b25..8bf1d6f 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -798,6 +798,8 @@ struct intel_device_info {
>>>          u8 num_sprites[I915_MAX_PIPES];
>>>          u8 gen;
>>>          u8 ring_mask; /* Rings supported by the HW */
>>> +       u16 num_samples_after_ctm;
>>> +       u16 num_samples_before_ctm;
>>>          DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
>>>          /* Register offsets for the various display pipes and
>>> transcoders */
>>>          int pipe_offsets[I915_MAX_TRANSCODERS];
>>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
>>> b/drivers/gpu/drm/i915/intel_color_manager.c
>>> index 7357d99..e466748 100644
>>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>>> @@ -28,6 +28,37 @@
>>>   #include "intel_color_manager.h"
>>>
>>>   void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>>> -               struct drm_mode_object *mode_obj)
>>> +               struct drm_crtc *crtc)
>>>   {
>>> +       struct drm_mode_config *config = &dev->mode_config;
>>> +       struct drm_mode_object *mode_obj = &crtc->base;
>>> +
>>> +       /*
>>> +        * Register:
>>> +        * =========
>>> +        * Gamma correction as palette_after_ctm property
>>> +        * Degamma correction as palette_before_ctm property
>>> +        *
>>> +        * Load:
>>> +        * =====
>>> +        * no. of coefficients supported on this platform for gamma
>>> +        * and degamma with the query properties. A user
>>> +        * space agent should read these query property, and prepare
>>> +        * the color correction values accordingly. Its expected from the
>>> +        * driver to load the right number of coefficients during the
>>> init
>>> +        * phase.
>>> +        */
>>> +       if (config->cm_coeff_after_ctm_property) {
>>> +               drm_object_attach_property(mode_obj,
>>> +                       config->cm_coeff_after_ctm_property,
>>> +                               INTEL_INFO(dev)->num_samples_after_ctm);
>>> +               DRM_DEBUG_DRIVER("Gamma query property initialized\n");
>>> +       }
>>> +
>>> +       if (config->cm_coeff_before_ctm_property) {
>>> +               drm_object_attach_property(mode_obj,
>>> +                       config->cm_coeff_before_ctm_property,
>>> +                               INTEL_INFO(dev)->num_samples_before_ctm);
>>> +               DRM_DEBUG_DRIVER("Degamma query property initialized\n");
>>> +       }
>>
>> Shouldn't this commit be squashed with the previous ? You're also
>> missing the function declaration.
>>
> Please see the history of the review comments, initially this patch was like
> as you suggested. But one of the previous review comments, suggested to
> split that into two, as it was 'overdoing' stuff. So I had split it into two
> separate ones, so I think this is ok :)
>
Sorry did not see it. The revision history should be within the patch
either above or below the --- line.

> So this is just the body of the
> function, and definition is coming up in next patch, where this function is
> being called from intel_crtc_init (patch 17/22 to be specific).
>
This is not how things work I'm afraid. The function declaration and
definition must _always_ be in the same patch.

Regards,
Emil