Re: [Intel-gfx] [PATCH 02/11] drm/i915: Register pipe level color properties

2014-07-24 Thread Matt Roper
On Wed, Jul 23, 2014 at 11:34:56PM +0530, shashank.sha...@intel.com wrote:
 From: Shashank Sharma shashank.sha...@intel.com
 
 In valleyview we have two pipe level color correction
 properties:
 1. CSC correction (wide gamut)
 2. Gamma correction
 
 What this patch does:
 1. This patch adds software infrastructure to register pipe level
color correction properties per CRTC. Adding a new function,
intel_attach_pipe_color_correction to register the pipe level
color correction properties with the given CRTC.
 2. Adding a pointer in intel_crtc structure to store this property.
 3. Adding structure gen6_pipe_color_corrections, which contains different
pipe level correction values for VLV.
 
 Signed-off-by: Shashank Sharma shashank.sha...@intel.com
...snip...
 +struct drm_property *intel_clrmgr_register(struct drm_device *dev,
 + struct drm_mode_object *obj, struct clrmgr_property *cp)
 +{
 + struct drm_property *property;
 +
 + /* Create drm property */
 + switch (cp-type) {
 + case DRM_MODE_PROP_BLOB:
 + property = drm_property_create(dev, DRM_MODE_PROP_BLOB,
 + cp-name, cp-len);
 + if (!property) {
 + DRM_ERROR(Failed to create property %s\n, cp-name);
 + goto error;
 + }
 + break;
 +
 + case DRM_MODE_PROP_RANGE:
 + property = drm_property_create_range(dev, DRM_MODE_PROP_RANGE,
 + cp-name, cp-min, cp-max);
 + if (!property) {
 + DRM_ERROR(Failed to create property %s\n, cp-name);
 + goto error;
 + }
 + break;
 +
 + default:
 + DRM_ERROR(Unsupported type for property %s\n, cp-name);
 + goto error;
 + }
 + /* Attach property to object */
 + drm_object_attach_property(obj, property, 0);
 + DRM_DEBUG_DRIVER(Registered property %s\n, property-name);
 + return property;
 +
 +error:
 + DRM_ERROR(Failed to create property %s\n, cp-name);
 + return NULL;
 +}

If I'm reading this right, you're creating a unique property for each
DRM object (crtc in this case) that you work with.  I.e., if you have
three pipes, then you wind up creating three CSC and three gamma
properties total.  This isn't really the way properties are meant to be
used...  A property describes a type of data that you want to store on a
per-object basis, but doesn't actually store anything itself.  You can
then attach that single property to multiple objects and all of those
objects will track their own value of the quantity described by the
property.  It's sort of like the difference between classes and objects
in OOP --- a DRM property is sort of like a class (just describes
something you plan to store) and attaching that property to a CRTC (or
plane, or connector) is like instantiating a new object of the class.

You should really only have two properties being created by the driver
here...one for CSC and one for Gamma.  Once you've created a property
once, you'll attach it to all of your CRTC's and they'll all manage
their own values.


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling  Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 02/11] drm/i915: Register pipe level color properties

2014-07-23 Thread shashank . sharma
From: Shashank Sharma shashank.sha...@intel.com

In valleyview we have two pipe level color correction
properties:
1. CSC correction (wide gamut)
2. Gamma correction

What this patch does:
1. This patch adds software infrastructure to register pipe level
   color correction properties per CRTC. Adding a new function,
   intel_attach_pipe_color_correction to register the pipe level
   color correction properties with the given CRTC.
2. Adding a pointer in intel_crtc structure to store this property.
3. Adding structure gen6_pipe_color_corrections, which contains different
   pipe level correction values for VLV.

Signed-off-by: Shashank Sharma shashank.sha...@intel.com
---
 drivers/gpu/drm/i915/intel_clrmgr.c | 148 
 drivers/gpu/drm/i915/intel_clrmgr.h |  64 
 drivers/gpu/drm/i915/intel_drv.h|   3 +
 3 files changed, 215 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_clrmgr.c 
b/drivers/gpu/drm/i915/intel_clrmgr.c
index 09a168d..8d02a62 100644
--- a/drivers/gpu/drm/i915/intel_clrmgr.c
+++ b/drivers/gpu/drm/i915/intel_clrmgr.c
@@ -33,6 +33,154 @@
 #include i915_reg.h
 #include intel_clrmgr.h
 
+/*
+  * Gen 6 SOC allows following color correction values:
+  *- CSC(wide gamut) with 3x3 matrix = 9 csc correction values.
+  *- Gamma correction with 128 gamma values.
+  */
+struct clrmgr_property gen6_pipe_color_corrections[] = {
+   {
+   .tweak_id = csc,
+   .type = DRM_MODE_PROP_BLOB,
+   .len = VLV_CSC_MATRIX_MAX_VALS,
+   .name = csc-correction,
+   },
+   {
+   .tweak_id = gamma,
+   .type = DRM_MODE_PROP_BLOB,
+   .len = VLV_10BIT_GAMMA_MAX_VALS,
+   .name = gamma-correction,
+   },
+};
+
+struct drm_property *intel_clrmgr_register(struct drm_device *dev,
+   struct drm_mode_object *obj, struct clrmgr_property *cp)
+{
+   struct drm_property *property;
+
+   /* Create drm property */
+   switch (cp-type) {
+   case DRM_MODE_PROP_BLOB:
+   property = drm_property_create(dev, DRM_MODE_PROP_BLOB,
+   cp-name, cp-len);
+   if (!property) {
+   DRM_ERROR(Failed to create property %s\n, cp-name);
+   goto error;
+   }
+   break;
+
+   case DRM_MODE_PROP_RANGE:
+   property = drm_property_create_range(dev, DRM_MODE_PROP_RANGE,
+   cp-name, cp-min, cp-max);
+   if (!property) {
+   DRM_ERROR(Failed to create property %s\n, cp-name);
+   goto error;
+   }
+   break;
+
+   default:
+   DRM_ERROR(Unsupported type for property %s\n, cp-name);
+   goto error;
+   }
+   /* Attach property to object */
+   drm_object_attach_property(obj, property, 0);
+   DRM_DEBUG_DRIVER(Registered property %s\n, property-name);
+   return property;
+
+error:
+   DRM_ERROR(Failed to create property %s\n, cp-name);
+   return NULL;
+}
+
+bool intel_clrmgr_register_pipe_property(struct intel_crtc *intel_crtc,
+   struct clrmgr_reg_request *features)
+
+{
+   u32 count = 0;
+   struct clrmgr_property *cp;
+   struct clrmgr_regd_prop *regd_property;
+   struct drm_property *property;
+   struct drm_device *dev = intel_crtc-base.dev;
+   struct drm_mode_object *obj = intel_crtc-base.base;
+   struct clrmgr_status *status = intel_crtc-color_status;
+
+   /* Color manager initialized? */
+   if (!status) {
+   DRM_ERROR(Register request without pipe init ?\n);
+   return false;
+   }
+
+   /* Validate input */
+   if (!features || !features-no_of_properties) {
+   DRM_ERROR(Invalid input to color manager register\n);
+   return false;
+   }
+
+   /* Create drm property */
+   while (count  features-no_of_properties) {
+   cp = features-cp[count++];
+   property = intel_clrmgr_register(dev, obj, cp);
+   if (!property) {
+   DRM_ERROR(Failed to register property %s\n,
+   property-name);
+   goto error;
+   }
+
+   /* Add the property in global pipe status */
+   regd_property = kzalloc(sizeof(struct clrmgr_regd_prop),
+   GFP_KERNEL);
+   regd_property-property = property;
+   regd_property-enabled = false;
+   regd_property-set_property = cp-set_property;
+   status-cp[status-no_of_properties++] = regd_property;
+   }
+   /* Successfully registered all */
+   DRM_DEBUG_DRIVER(Registered color properties on pipe %c\n,
+   pipe_name(intel_crtc-pipe));
+   return true;
+
+error:
+   if