Hi Sonika,

Please find my responses inline.

Thanks, Kausal

On Friday 05 June 2015 05:30 PM, Jindal, Sonika wrote:

On 6/4/2015 7:12 PM, Kausal Malladi wrote:

...

From: Kausal Malladi Kausal.Malladi@intel.com

This patch adds a new structure in DRM layer for Gamma color correction. This structure will be used by all user space agents to configure appropriate Gamma precision and Gamma level.

struct drm_intel_gamma { __u32 gamma_level; (The gamma_level variable indicates if the Gamma correction is to be

Do you have any macro defines for these levels? Maybe an enum will be better?

Yes, we have macro defines for these. There are only two possible levels (pipe/plane).

...

applied on Pipe/plane)
    __u32 gamma_precision;
(The Gamma precision indicates the Gamma mode to be applied)

Supported precisions are -
#define I915_GAMMA_PRECISION_UNKNOWN    0
#define I915_GAMMA_PRECISION_CURRENT    0xFFFFFFFF
#define I915_GAMMA_PRECISION_LEGACY    (1 << 0)
#define I915_GAMMA_PRECISION_10BIT    (1 << 1)
#define I915_GAMMA_PRECISION_12BIT    (1 << 2)
#define I915_GAMMA_PRECISION_14BIT    (1 << 3)
#define I915_GAMMA_PRECISION_16BIT    (1 << 4)

__u32 num_samples;
(The num_samples indicates the number of Gamma correction
coefficients)
__u32 reserved;

You need this reserved?

In case any future platforms require any additional checks, we can use this member. Having said that, yes.. we are currently not using it.

...

__u16 values[0];
(An array of size 0, to accommodate the "num_samples" number of
R16G16B16 pixels, dynamically)

};

v2: Addressing Daniel Stone's comment, added a variable sized array to carry Gamma correction values as blob property.

Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Kausal Malladi Kausal.Malladi@intel.com

---

include/drm/drm_crtc.h | 3 +++ include/uapi/drm/drm.h | 10 ++++++++++ 2 files changed, 13 insertions(+)

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 2a75d7d..bc44f27 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -483,6 +483,9 @@ struct drm_crtc { * acquire context. */ struct drm_modeset_acquire_ctx *acquire_ctx;

Just trying to understand, why do we need to store the blob id separately?

Saving Blob ID allows us to do a lookup of the blob using ID. This will be useful in get_property call.

...

• /* Color Management Blob IDs */

• u32 gamma_blob_id; };

  /**

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 3801584..fc2661c 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -829,6 +829,16 @@ struct drm_event_vblank { __u32 reserved; };

+/* Color Management structure for Gamma */ +struct drm_gamma {

• __u32 flags;
• __u32 gamma_level;
• __u32 gamma_precision;
• __u32 num_samples;
• __u32 reserved;
• __u16 values[0];

+};

• /* typedef area */ #ifndef __KERNEL__ typedef struct drm_clip_rect drm_clip_rect_t;