RE: [v2 3/7] drm: Add gamma mode property

2019-04-08 Thread Shankar, Uma


>-Original Message-
>From: Sam Ravnborg [mailto:s...@ravnborg.org]
>Sent: Tuesday, April 2, 2019 12:07 AM
>To: Shankar, Uma 
>Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>dcasta...@chromium.org; emil.l.veli...@gmail.com; seanp...@chromium.org;
>Syrjala, Ville ; Lankhorst, Maarten
>
>Subject: Re: [v2 3/7] drm: Add gamma mode property
>
>Hi Uma.
>
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -256,6 +256,13 @@ struct drm_crtc_state {
>>  struct drm_property_blob *gamma_mode_caps;
>>
>>  /**
>> + * @gamma_mode:
>> + *
>> + * FIXME
>> + */
>> +struct drm_property_blob *gamma_mode;
>
>HEre the name matches, but please add documentation too.

Sure, will add documentation explaining how this should get used once we
have the agreement on design and approach. 

>>
>> +struct drm_color_mode_lut {
>> +/* DRM_MODE_LUT_* */
>> +__u32 flags;
>> +/* number of points on the curve */
>> +__u32 count;
>> +/* Name of Gamma Mode */
>> +char name[DRM_PROP_NAME_LEN];
>> +/* Pointer to Lut elements */
>> +__u64 lut;
>> +};
>From an alignment point of view the __u64 should come before the char name[].
>But the above may be fine depending on DRM_PROP_NAME_LEN

Yeah, but can re-order this. Thanks for the review.

Regards,
Uma Shankar

>
>   Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 3/7] drm: Add gamma mode property

2019-04-01 Thread Sam Ravnborg
Hi Uma.

> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -256,6 +256,13 @@ struct drm_crtc_state {
>   struct drm_property_blob *gamma_mode_caps;
>  
>   /**
> +  * @gamma_mode:
> +  *
> +  * FIXME
> +  */
> + struct drm_property_blob *gamma_mode;

HEre the name matches, but please add documentation too.

>  
> +struct drm_color_mode_lut {
> + /* DRM_MODE_LUT_* */
> + __u32 flags;
> + /* number of points on the curve */
> + __u32 count;
> + /* Name of Gamma Mode */
> + char name[DRM_PROP_NAME_LEN];
> + /* Pointer to Lut elements */
> + __u64 lut;
> +};
From an alignment point of view the __u64 should come before the char name[].
But the above may be fine depending on DRM_PROP_NAME_LEN

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[v2 3/7] drm: Add gamma mode property

2019-04-01 Thread Uma Shankar
Add Gamma Mode property to set the gamma mode
(Interploated, Split, Multi Segmented etc) from the
list obtained through the gamma mode caps property.

Create the blob and send to driver for programming
the luts to the appropriate registers and setting
the chosen gamma mode.

Signed-off-by: Uma Shankar 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 10 ++
 drivers/gpu/drm/drm_color_mgmt.c  | 32 
 include/drm/drm_color_mgmt.h  |  3 +++
 include/drm/drm_crtc.h|  7 +++
 include/drm/drm_mode_config.h |  5 +
 include/uapi/drm/drm_mode.h   | 11 +++
 6 files changed, 68 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 03df2a4..d3008ea 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -459,6 +459,14 @@ static int drm_atomic_crtc_set_property(struct drm_crtc 
*crtc,
);
state->color_mgmt_changed |= replaced;
return ret;
+   } else if (property == config->gamma_mode_property) {
+   ret = drm_atomic_replace_property_blob_from_id(dev,
+   >gamma_mode,
+   val,
+   -1, sizeof(struct drm_color_mode_lut),
+   );
+   state->color_mgmt_changed |= replaced;
+   return ret;
} else if (property == config->prop_out_fence_ptr) {
s32 __user *fence_ptr = u64_to_user_ptr(val);
 
@@ -498,6 +506,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc 
*crtc,
else if (property == config->gamma_mode_caps_property)
*val = (state->gamma_mode_caps) ?
state->gamma_mode_caps->base.id : 0;
+   else if (property == config->gamma_mode_property)
+   *val = (state->gamma_mode) ? state->gamma_mode->base.id : 0;
else if (property == config->degamma_lut_property)
*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
else if (property == config->ctm_property)
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 054f0ed..cba1d6d 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -176,6 +176,38 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 }
 EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
 
+void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc)
+{
+   struct drm_device *dev = crtc->dev;
+   struct drm_mode_config *config = >mode_config;
+
+   if (!config->gamma_mode_property)
+   return;
+
+   drm_object_attach_property(>base,
+  config->gamma_mode_property, 0);
+}
+EXPORT_SYMBOL(drm_crtc_attach_gamma_mode_property);
+
+int drm_color_create_gamma_mode_property(struct drm_device *dev,
+int num_values)
+{
+   struct drm_mode_config *config = >mode_config;
+   struct drm_property *prop;
+
+   prop = drm_property_create(dev,
+  DRM_MODE_PROP_BLOB |
+  DRM_MODE_PROP_ATOMIC,
+  "GAMMA_MODE", num_values);
+   if (!prop)
+   return -ENOMEM;
+
+   config->gamma_mode_property = prop;
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_color_create_gamma_mode_property);
+
 void drm_crtc_attach_gamma_mode_caps_property(struct drm_crtc *crtc)
 {
struct drm_device *dev = crtc->dev;
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index e0f94db..4306e07 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -54,6 +54,9 @@ static inline int drm_color_lut_size(const struct 
drm_property_blob *blob)
 int drm_color_create_gamma_mode_caps_property(struct drm_device *dev,
  int num_values);
 void drm_crtc_attach_gamma_mode_caps_property(struct drm_crtc *crtc);
+int drm_color_create_gamma_mode_property(struct drm_device *dev,
+int num_values);
+void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc);
 int drm_color_add_gamma_mode_range(struct drm_device *dev,
   const char *name,
   const struct drm_color_lut_range *ranges,
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index cdfda90..bc8a2e7 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -256,6 +256,13 @@ struct drm_crtc_state {
struct drm_property_blob *gamma_mode_caps;
 
/**
+* @gamma_mode:
+*
+* FIXME
+*/
+   struct drm_property_blob *gamma_mode;
+
+   /**
 * @degamma_lut:
 *
 * Lookup table for converting framebuffer pixel data before apply the