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 at 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 at intel.com> >> Signed-off-by: Kausal Malladi <Kausal.Malladi at 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; >>