Regards
Shashank
On 6/6/2015 6:30 AM, Matt Roper wrote:
On Thu, Jun 04, 2015 at 07:12:35PM +0530, Kausal Malladi wrote:
From: Kausal Malladi <kausal.mall...@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
applied on Pipe/plane)
I'm not sure I understand the need for this one...you're getting the
set_property call against a specific DRM object, so I don't think there
should be any confusion at that point about what the values apply to?
Actually that is for the get_property path. If you have a look at the
design document, there is a provision to query the current applied gamma
which can be pipe/plane level. Hence keeping this.
__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)
I feel like the precision would work better as a separate enum property
rather than being part of your blob; I think it would be cleaner if your
blob only held the actual values if possible.
Again, this would be required for the get_property path. If we create a
separate property for this, the design might be very complex.
This is the current implementation:
1. Pack the correction values and configurations in blob()
2. call a create_blob() and get the blob_id
3. do a set_porperty() with the blob_id
4. set_property will find this blob, with the blob_id and apply
corrections.
Adding a separate property for gamma_level will add one more state here,
which will make it further complex. Do you agree ?
__u32 num_samples;
(The num_samples indicates the number of Gamma correction
coefficients)
__u32 reserved;
__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.sha...@intel.com>
Signed-off-by: Kausal Malladi <kausal.mall...@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;
+
+ /* 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;
The flags aren't described in your commit message...what are they used
for? I guess it will become more clear as I read farther through your
patch series.
Matt
We are currently using this to define gamma/degamma as such, but yes I
agree that we have added this for future / undefined last minute usages,
and can be removed.
+ __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;
--
2.4.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
