>-----Original Message-----
>From: dri-devel [mailto:[email protected]] On Behalf Of
>Brian Starkey
>Sent: Tuesday, November 7, 2017 11:10 PM
>To: Shankar, Uma <[email protected]>
>Cc: [email protected]; Syrjala, Ville <[email protected]>;
>Lankhorst, Maarten <[email protected]>; dri-
>[email protected]
>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 <[email protected]>
>>---
>> 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
>>[email protected]
>>https://lists.freedesktop.org/mailman/listinfo/dri-devel
>_______________________________________________
>dri-devel mailing list
>[email protected]
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to