diff mbox series

[v2,1/7] drm: Add gamma mode caps property

Message ID 1554139811-13280-2-git-send-email-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Add Multi Segment Gamma Support | expand

Commit Message

Shankar, Uma April 1, 2019, 5:30 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a gamma mode capability property to enable various kind of
gamma modes supported by platforms like: Interpolated, Split,
Multi Segmented etc. Userspace can get this property and
should be able to get the platform capabilties wrt various
gamma modes possible and the possible ranges.

It can then create the LUT and send it to driver using
another gamma mode property as blob.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  3 ++
 drivers/gpu/drm/drm_color_mgmt.c  | 78 +++++++++++++++++++++++++++++++++++++++
 include/drm/drm_color_mgmt.h      |  8 ++++
 include/drm/drm_crtc.h            |  7 ++++
 include/drm/drm_mode_config.h     |  5 +++
 include/uapi/drm/drm_mode.h       | 38 +++++++++++++++++++
 6 files changed, 139 insertions(+)

Comments

Sam Ravnborg April 1, 2019, 6:33 p.m. UTC | #1
Hi Uma.

> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 58ad983..cdfda90 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -249,6 +249,13 @@ struct drm_crtc_state {
>  	struct drm_property_blob *mode_blob;
>  
>  	/**
> +	 * @gamma_mode:
> +	 *
> +	 * FIXME
> +	 */
> +	struct drm_property_blob *gamma_mode_caps;

This looks wrong as "gamma_mode" != "gamma_mode_caps".
Could you fix it, and also add some documentation to help
the next reader.

> +/*
> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
> + * can be used for either purpose, but not simultaneously. To expose
> + * modes that support gamma and degamma simultaneously the gamma mode
> + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
> + * ranges.
> + */
> +/* LUT is for gamma (after CTM) */
> +#define DRM_MODE_LUT_GAMMA (1 << 0)
> +/* LUT is for degamma (before CTM) */
> +#define DRM_MODE_LUT_DEGAMMA (1 << 1)
> +/* linearly interpolate between the points */
> +#define DRM_MODE_LUT_INTERPOLATE (1 << 2)

Use BIT(2), and same for the other bits above and below?

> +/*
> + * the last value of the previous range is the
> + * first value of the current range.
> + */
> +#define DRM_MODE_LUT_REUSE_LAST (1 << 3)
> +/* the curve must be non-decreasing */
> +#define DRM_MODE_LUT_NON_DECREASING (1 << 4)
> +/* the curve is reflected across origin for negative inputs */
> +#define DRM_MODE_LUT_REFLECT_NEGATIVE (1 << 5)
> +/* the same curve (red) is used for blue and green channels as well */
> +#define DRM_MODE_LUT_SINGLE_CHANNEL (1 << 6)

	Sam
Shankar, Uma April 8, 2019, 2:45 p.m. UTC | #2
>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Sam
>Ravnborg
>Sent: Tuesday, April 2, 2019 12:03 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: dcastagna@chromium.org; intel-gfx@lists.freedesktop.org;
>emil.l.velikov@gmail.com; dri-devel@lists.freedesktop.org;
>seanpaul@chromium.org; Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 1/7] drm: Add gamma mode caps property
>
>Hi Uma.
>
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
>> 58ad983..cdfda90 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -249,6 +249,13 @@ struct drm_crtc_state {
>>  	struct drm_property_blob *mode_blob;
>>
>>  	/**
>> +	 * @gamma_mode:
>> +	 *
>> +	 * FIXME
>> +	 */
>> +	struct drm_property_blob *gamma_mode_caps;
>
>This looks wrong as "gamma_mode" != "gamma_mode_caps".
>Could you fix it, and also add some documentation to help the next reader.

Hi Sam,
Thanks for the review.

Sure will update the naming and add more documentation once we have the
design agreement on this. Idea is to converge to just one property.

Regards,
Uma Shankar


>
>> +/*
>> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means
>the LUT
>> + * can be used for either purpose, but not simultaneously. To expose
>> + * modes that support gamma and degamma simultaneously the gamma mode
>> + * must declare distinct DRM_MODE_LUT_GAMMA and
>DRM_MODE_LUT_DEGAMMA
>> + * ranges.
>> + */
>> +/* LUT is for gamma (after CTM) */
>> +#define DRM_MODE_LUT_GAMMA (1 << 0)
>> +/* LUT is for degamma (before CTM) */ #define DRM_MODE_LUT_DEGAMMA (1
>> +<< 1)
>> +/* linearly interpolate between the points */ #define
>> +DRM_MODE_LUT_INTERPOLATE (1 << 2)
>
>Use BIT(2), and same for the other bits above and below?

Sure, will update this.

>> +/*
>> + * the last value of the previous range is the
>> + * first value of the current range.
>> + */
>> +#define DRM_MODE_LUT_REUSE_LAST (1 << 3)
>> +/* the curve must be non-decreasing */ #define
>> +DRM_MODE_LUT_NON_DECREASING (1 << 4)
>> +/* the curve is reflected across origin for negative inputs */
>> +#define DRM_MODE_LUT_REFLECT_NEGATIVE (1 << 5)
>> +/* the same curve (red) is used for blue and green channels as well
>> +*/ #define DRM_MODE_LUT_SINGLE_CHANNEL (1 << 6)
>
>	Sam
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index ea797d4..03df2a4 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -495,6 +495,9 @@  static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
 	else if (property == config->prop_vrr_enabled)
 		*val = state->vrr_enabled;
+	else if (property == config->gamma_mode_caps_property)
+		*val = (state->gamma_mode_caps) ?
+			state->gamma_mode_caps->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 d5d34d0..054f0ed 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -176,6 +176,84 @@  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 }
 EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
 
+void drm_crtc_attach_gamma_mode_caps_property(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (!config->gamma_mode_caps_property)
+		return;
+
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_mode_caps_property, 0);
+}
+EXPORT_SYMBOL(drm_crtc_attach_gamma_mode_caps_property);
+
+int drm_color_create_gamma_mode_caps_property(struct drm_device *dev,
+					      int num_values)
+{
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_property *prop;
+
+	prop = drm_property_create(dev,
+				   DRM_MODE_PROP_ENUM |
+				   DRM_MODE_PROP_IMMUTABLE,
+				   "GAMMA_MODE_CAPS", num_values);
+	if (!prop)
+		return -ENOMEM;
+
+	config->gamma_mode_caps_property = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_color_create_gamma_mode_caps_property);
+
+int drm_color_add_gamma_mode_range(struct drm_device *dev,
+				   const char *name,
+				   const struct drm_color_lut_range *ranges,
+				   size_t length)
+{
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_property_blob *blob;
+	struct drm_property *prop;
+	int num_ranges = length / sizeof(ranges[0]);
+	int i, ret, num_types_0;
+
+	if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
+		return -EINVAL;
+
+	num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
+						  DRM_MODE_LUT_DEGAMMA));
+	if (num_types_0 == 0)
+		return -EINVAL;
+
+	for (i = 1; i < num_ranges; i++) {
+		int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |
+							    DRM_MODE_LUT_DEGAMMA));
+
+		/* either all ranges have DEGAMMA|GAMMA or none have it */
+		if (num_types_0 != num_types)
+			return -EINVAL;
+	}
+
+	prop = config->gamma_mode_caps_property;
+	if (!prop)
+		return -EINVAL;
+
+	blob = drm_property_create_blob(dev, length, ranges);
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
+
+	ret = drm_property_add_enum(prop, blob->base.id, name);
+	if (ret) {
+		drm_property_blob_put(blob);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_color_add_gamma_mode_range);
+
 /**
  * drm_mode_crtc_set_gamma_size - set the gamma table size
  * @crtc: CRTC to set the gamma table size for
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index d1c662d..e0f94db 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -51,6 +51,14 @@  static inline int drm_color_lut_size(const struct drm_property_blob *blob)
 	return blob->length / sizeof(struct drm_color_lut);
 }
 
+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_add_gamma_mode_range(struct drm_device *dev,
+				   const char *name,
+				   const struct drm_color_lut_range *ranges,
+				   size_t length);
+
 enum drm_color_encoding {
 	DRM_COLOR_YCBCR_BT601,
 	DRM_COLOR_YCBCR_BT709,
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 58ad983..cdfda90 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -249,6 +249,13 @@  struct drm_crtc_state {
 	struct drm_property_blob *mode_blob;
 
 	/**
+	 * @gamma_mode:
+	 *
+	 * FIXME
+	 */
+	struct drm_property_blob *gamma_mode_caps;
+
+	/**
 	 * @degamma_lut:
 	 *
 	 * Lookup table for converting framebuffer pixel data before apply the
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 7f60e8e..7b20355 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -761,6 +761,11 @@  struct drm_mode_config {
 	 */
 	struct drm_property *content_type_property;
 	/**
+	 * @gamma_mode_property: Optional CRTC property to enumerate and
+	 * select the mode of the crtc gamma/degmama LUTs.
+	 */
+	struct drm_property *gamma_mode_caps_property;
+	/**
 	 * @degamma_lut_property: Optional CRTC property to set the LUT used to
 	 * convert the framebuffer's colors to linear gamma.
 	 */
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 09d7296..e475f4a 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -629,6 +629,44 @@  struct drm_color_lut {
 	__u16 reserved;
 };
 
+/*
+ * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
+ * can be used for either purpose, but not simultaneously. To expose
+ * modes that support gamma and degamma simultaneously the gamma mode
+ * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
+ * ranges.
+ */
+/* LUT is for gamma (after CTM) */
+#define DRM_MODE_LUT_GAMMA (1 << 0)
+/* LUT is for degamma (before CTM) */
+#define DRM_MODE_LUT_DEGAMMA (1 << 1)
+/* linearly interpolate between the points */
+#define DRM_MODE_LUT_INTERPOLATE (1 << 2)
+/*
+ * the last value of the previous range is the
+ * first value of the current range.
+ */
+#define DRM_MODE_LUT_REUSE_LAST (1 << 3)
+/* the curve must be non-decreasing */
+#define DRM_MODE_LUT_NON_DECREASING (1 << 4)
+/* the curve is reflected across origin for negative inputs */
+#define DRM_MODE_LUT_REFLECT_NEGATIVE (1 << 5)
+/* the same curve (red) is used for blue and green channels as well */
+#define DRM_MODE_LUT_SINGLE_CHANNEL (1 << 6)
+
+struct drm_color_lut_range {
+	/* DRM_MODE_LUT_* */
+	__u32 flags;
+	/* number of points on the curve */
+	__u16 count;
+	/* input/output bits per component */
+	__u8 input_bpc, output_bpc;
+	/* input start/end values */
+	__s32 start, end;
+	/* output min/max values */
+	__s32 min, max;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4