diff mbox

[RFC,2/7] drm: Add Plane CTM property

Message ID 1510056391-9684-3-git-send-email-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shankar, Uma Nov. 7, 2017, 12:06 p.m. UTC
Add a blob property for plane CSC usage.

v2: Rebase

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_atomic.c        |   10 ++++++++++
 drivers/gpu/drm/drm_atomic_helper.c |    3 +++
 drivers/gpu/drm/drm_mode_config.c   |    7 +++++++
 include/drm/drm_mode_config.h       |    6 ++++++
 include/drm/drm_plane.h             |    8 ++++++++
 5 files changed, 34 insertions(+)

Comments

Brian Starkey Nov. 7, 2017, 5:39 p.m. UTC | #1
Hi Uma,

On Tue, Nov 07, 2017 at 05:36:26PM +0530, Uma Shankar wrote:
>Add a blob property for plane CSC usage.
>
>v2: Rebase
>
>Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>---
> drivers/gpu/drm/drm_atomic.c        |   10 ++++++++++
> drivers/gpu/drm/drm_atomic_helper.c |    3 +++
> drivers/gpu/drm/drm_mode_config.c   |    7 +++++++
> include/drm/drm_mode_config.h       |    6 ++++++
> include/drm/drm_plane.h             |    8 ++++++++
> 5 files changed, 34 insertions(+)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 30853c7..45aede5 100644
>--- a/drivers/gpu/drm/drm_atomic.c
>+++ b/drivers/gpu/drm/drm_atomic.c
>@@ -766,6 +766,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> 					val, -1, &replaced);
> 		state->color_mgmt_changed |= replaced;
> 		return ret;
>+	} else if (property == config->plane_ctm_property) {
>+		ret = drm_atomic_replace_property_blob_from_id(dev,
>+					&state->ctm,
>+					val,
>+					sizeof(struct drm_color_ctm),
>+					&replaced);
>+		state->color_mgmt_changed |= replaced;
>+		return ret;
> 	} else {
> 		return -EINVAL;
> 	}
>@@ -827,6 +835,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> 	} else if (property == config->plane_degamma_lut_property) {
> 		*val = (state->degamma_lut) ?
> 			state->degamma_lut->base.id : 0;
>+	} else if (property == config->plane_ctm_property) {
>+		*val = (state->ctm) ? state->ctm->base.id : 0;
> 	} else {
> 		return -EINVAL;
> 	}
>diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>index ba924cf..d3154e0 100644
>--- a/drivers/gpu/drm/drm_atomic_helper.c
>+++ b/drivers/gpu/drm/drm_atomic_helper.c
>@@ -3398,6 +3398,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>
> 	if (state->degamma_lut)
> 		drm_property_blob_get(state->degamma_lut);
>+	if (state->ctm)
>+		drm_property_blob_get(state->ctm);
> 	state->color_mgmt_changed = false;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>@@ -3445,6 +3447,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
> 		drm_crtc_commit_put(state->commit);
>
> 	drm_property_blob_put(state->degamma_lut);
>+	drm_property_blob_put(state->ctm);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
>diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>index 118f6ac..bccc70e 100644
>--- a/drivers/gpu/drm/drm_mode_config.c
>+++ b/drivers/gpu/drm/drm_mode_config.c
>@@ -362,6 +362,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> 		return -ENOMEM;
> 	dev->mode_config.plane_degamma_lut_size_property = prop;
>
>+	prop = drm_property_create(dev,
>+			DRM_MODE_PROP_BLOB,
>+			"PLANE_CTM", 0);

I do wonder if "PLANE_" is really needed here, as the property will
only ever be found on a plane (same would apply to all three property
names).

>+	if (!prop)
>+		return -ENOMEM;
>+	dev->mode_config.plane_ctm_property = prop;
>+
> 	return 0;
> }
>
>diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>index 6ee2df6..3bf7fc6 100644
>--- a/include/drm/drm_mode_config.h
>+++ b/include/drm/drm_mode_config.h
>@@ -727,6 +727,12 @@ struct drm_mode_config {
> 	 * size of the degamma LUT as supported by the driver (read-only).
> 	 */
> 	struct drm_property *plane_degamma_lut_size_property;
>+	/**
>+	 * @plane_ctm_property: Optional CRTC property to set the
>+	 * matrix used to convert colors after the lookup in the
>+	 * degamma LUT.
>+	 */

Copy-paste error - should be "Optional plane property"

Thanks,
-Brian

>+	struct drm_property *plane_ctm_property;
>
> 	/**
> 	 * @suggested_x_property: Optional connector property with a hint for
>diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>index d112c50..7fcc51e 100644
>--- a/include/drm/drm_plane.h
>+++ b/include/drm/drm_plane.h
>@@ -132,6 +132,14 @@ struct drm_plane_state {
> 	struct drm_property_blob *degamma_lut;
>
> 	/**
>+	 * @ctm:
>+	 *
>+	 * Color transformation matrix. See drm_plane_enable_color_mgmt(). The
>+	 * blob (if not NULL) is a &struct drm_color_ctm.
>+	 */
>+	struct drm_property_blob *ctm;
>+
>+	/**
> 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
> 	 * and for async plane updates.
> 	 *
>-- 
>1.7.9.5
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
Shankar, Uma Nov. 8, 2017, 9:08 a.m. UTC | #2
>-----Original Message-----

>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of

>Brian Starkey

>Sent: Tuesday, November 7, 2017 11:10 PM

>To: Shankar, Uma <uma.shankar@intel.com>

>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;

>Lankhorst, Maarten <maarten.lankhorst@intel.com>; dri-

>devel@lists.freedesktop.org

>Subject: Re: [RFC 2/7] drm: Add Plane CTM property

>

>Hi Uma,

>

>On Tue, Nov 07, 2017 at 05:36:26PM +0530, Uma Shankar wrote:

>>Add a blob property for plane CSC usage.

>>

>>v2: Rebase

>>

>>Signed-off-by: Uma Shankar <uma.shankar@intel.com>

>>---

>> drivers/gpu/drm/drm_atomic.c        |   10 ++++++++++

>> drivers/gpu/drm/drm_atomic_helper.c |    3 +++

>> drivers/gpu/drm/drm_mode_config.c   |    7 +++++++

>> include/drm/drm_mode_config.h       |    6 ++++++

>> include/drm/drm_plane.h             |    8 ++++++++

>> 5 files changed, 34 insertions(+)

>>

>>diff --git a/drivers/gpu/drm/drm_atomic.c

>>b/drivers/gpu/drm/drm_atomic.c index 30853c7..45aede5 100644

>>--- a/drivers/gpu/drm/drm_atomic.c

>>+++ b/drivers/gpu/drm/drm_atomic.c

>>@@ -766,6 +766,14 @@ static int drm_atomic_plane_set_property(struct

>drm_plane *plane,

>> 					val, -1, &replaced);

>> 		state->color_mgmt_changed |= replaced;

>> 		return ret;

>>+	} else if (property == config->plane_ctm_property) {

>>+		ret = drm_atomic_replace_property_blob_from_id(dev,

>>+					&state->ctm,

>>+					val,

>>+					sizeof(struct drm_color_ctm),

>>+					&replaced);

>>+		state->color_mgmt_changed |= replaced;

>>+		return ret;

>> 	} else {

>> 		return -EINVAL;

>> 	}

>>@@ -827,6 +835,8 @@ static int drm_atomic_plane_set_property(struct

>drm_plane *plane,

>> 	} else if (property == config->plane_degamma_lut_property) {

>> 		*val = (state->degamma_lut) ?

>> 			state->degamma_lut->base.id : 0;

>>+	} else if (property == config->plane_ctm_property) {

>>+		*val = (state->ctm) ? state->ctm->base.id : 0;

>> 	} else {

>> 		return -EINVAL;

>> 	}

>>diff --git a/drivers/gpu/drm/drm_atomic_helper.c

>>b/drivers/gpu/drm/drm_atomic_helper.c

>>index ba924cf..d3154e0 100644

>>--- a/drivers/gpu/drm/drm_atomic_helper.c

>>+++ b/drivers/gpu/drm/drm_atomic_helper.c

>>@@ -3398,6 +3398,8 @@ void

>>__drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,

>>

>> 	if (state->degamma_lut)

>> 		drm_property_blob_get(state->degamma_lut);

>>+	if (state->ctm)

>>+		drm_property_blob_get(state->ctm);

>> 	state->color_mgmt_changed = false;

>> }

>> EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);

>>@@ -3445,6 +3447,7 @@ void

>__drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)

>> 		drm_crtc_commit_put(state->commit);

>>

>> 	drm_property_blob_put(state->degamma_lut);

>>+	drm_property_blob_put(state->ctm);

>> }

>> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);

>>

>>diff --git a/drivers/gpu/drm/drm_mode_config.c

>>b/drivers/gpu/drm/drm_mode_config.c

>>index 118f6ac..bccc70e 100644

>>--- a/drivers/gpu/drm/drm_mode_config.c

>>+++ b/drivers/gpu/drm/drm_mode_config.c

>>@@ -362,6 +362,13 @@ static int

>drm_mode_create_standard_properties(struct drm_device *dev)

>> 		return -ENOMEM;

>> 	dev->mode_config.plane_degamma_lut_size_property = prop;

>>

>>+	prop = drm_property_create(dev,

>>+			DRM_MODE_PROP_BLOB,

>>+			"PLANE_CTM", 0);

>

>I do wonder if "PLANE_" is really needed here, as the property will only ever be

>found on a plane (same would apply to all three property names).


This is just to explicitly separate out the names from the crtc (pipe) properties. 
(Similar properties exist for pipe already). It will create confusion, hence explicitly called
them out appending with a  "PLANE" prefix.

>

>>+	if (!prop)

>>+		return -ENOMEM;

>>+	dev->mode_config.plane_ctm_property = prop;

>>+

>> 	return 0;

>> }

>>

>>diff --git a/include/drm/drm_mode_config.h

>>b/include/drm/drm_mode_config.h index 6ee2df6..3bf7fc6 100644

>>--- a/include/drm/drm_mode_config.h

>>+++ b/include/drm/drm_mode_config.h

>>@@ -727,6 +727,12 @@ struct drm_mode_config {

>> 	 * size of the degamma LUT as supported by the driver (read-only).

>> 	 */

>> 	struct drm_property *plane_degamma_lut_size_property;

>>+	/**

>>+	 * @plane_ctm_property: Optional CRTC property to set the

>>+	 * matrix used to convert colors after the lookup in the

>>+	 * degamma LUT.

>>+	 */

>

>Copy-paste error - should be "Optional plane property"


Yeah indeed, thanks for spotting it. Will fix in next set.

Regards,
Uma Shankar

>

>Thanks,

>-Brian

>

>>+	struct drm_property *plane_ctm_property;

>>

>> 	/**

>> 	 * @suggested_x_property: Optional connector property with a hint for

>>diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index

>>d112c50..7fcc51e 100644

>>--- a/include/drm/drm_plane.h

>>+++ b/include/drm/drm_plane.h

>>@@ -132,6 +132,14 @@ struct drm_plane_state {

>> 	struct drm_property_blob *degamma_lut;

>>

>> 	/**

>>+	 * @ctm:

>>+	 *

>>+	 * Color transformation matrix. See drm_plane_enable_color_mgmt().

>The

>>+	 * blob (if not NULL) is a &struct drm_color_ctm.

>>+	 */

>>+	struct drm_property_blob *ctm;

>>+

>>+	/**

>> 	 * @commit: Tracks the pending commit to prevent use-after-free

>conditions,

>> 	 * and for async plane updates.

>> 	 *

>>--

>>1.7.9.5

>>

>>_______________________________________________

>>dri-devel mailing list

>>dri-devel@lists.freedesktop.org

>>https://lists.freedesktop.org/mailman/listinfo/dri-devel

>_______________________________________________

>dri-devel mailing list

>dri-devel@lists.freedesktop.org

>https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 30853c7..45aede5 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -766,6 +766,14 @@  static int drm_atomic_plane_set_property(struct drm_plane *plane,
 					val, -1, &replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
+	} else if (property == config->plane_ctm_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->ctm,
+					val,
+					sizeof(struct drm_color_ctm),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else {
 		return -EINVAL;
 	}
@@ -827,6 +835,8 @@  static int drm_atomic_plane_set_property(struct drm_plane *plane,
 	} else if (property == config->plane_degamma_lut_property) {
 		*val = (state->degamma_lut) ?
 			state->degamma_lut->base.id : 0;
+	} else if (property == config->plane_ctm_property) {
+		*val = (state->ctm) ? state->ctm->base.id : 0;
 	} else {
 		return -EINVAL;
 	}
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ba924cf..d3154e0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3398,6 +3398,8 @@  void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 
 	if (state->degamma_lut)
 		drm_property_blob_get(state->degamma_lut);
+	if (state->ctm)
+		drm_property_blob_get(state->ctm);
 	state->color_mgmt_changed = false;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
@@ -3445,6 +3447,7 @@  void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 		drm_crtc_commit_put(state->commit);
 
 	drm_property_blob_put(state->degamma_lut);
+	drm_property_blob_put(state->ctm);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 118f6ac..bccc70e 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -362,6 +362,13 @@  static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.plane_degamma_lut_size_property = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"PLANE_CTM", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.plane_ctm_property = prop;
+
 	return 0;
 }
 
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 6ee2df6..3bf7fc6 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -727,6 +727,12 @@  struct drm_mode_config {
 	 * size of the degamma LUT as supported by the driver (read-only).
 	 */
 	struct drm_property *plane_degamma_lut_size_property;
+	/**
+	 * @plane_ctm_property: Optional CRTC property to set the
+	 * matrix used to convert colors after the lookup in the
+	 * degamma LUT.
+	 */
+	struct drm_property *plane_ctm_property;
 
 	/**
 	 * @suggested_x_property: Optional connector property with a hint for
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index d112c50..7fcc51e 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -132,6 +132,14 @@  struct drm_plane_state {
 	struct drm_property_blob *degamma_lut;
 
 	/**
+	 * @ctm:
+	 *
+	 * Color transformation matrix. See drm_plane_enable_color_mgmt(). The
+	 * blob (if not NULL) is a &struct drm_color_ctm.
+	 */
+	struct drm_property_blob *ctm;
+
+	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
 	 *