Re: [v2 1/2] drm: Add colorspace property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/02/2018 03:29 PM, Jonas Karlman wrote:
On 2018-11-02 15:13, Shankar, Uma wrote:

-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx]
Sent: Friday, November 2, 2018 5:00 PM
To: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Cc: Shankar, Uma <uma.shankar@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>;
Lankhorst, Maarten <maarten.lankhorst@xxxxxxxxx>; Hans Verkuil
<hansverk@xxxxxxxxx>
Subject: Re:  [v2 1/2] drm: Add colorspace property

On Fri, Nov 02, 2018 at 10:19:10AM +0100, Maarten Lankhorst wrote:
Op 31-10-18 om 13:05 schreef Uma Shankar:
This patch adds a colorspace property enabling userspace to switch
to various supported colorspaces.
This will help enable BT2020 along with other colorspaces.

v2: Addressed Maarten and Ville's review comments. Enhanced the
colorspace enum to incorporate both HDMI and DP supported
colorspaces. Also, added a default option for colorspace.

Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
---
  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
  drivers/gpu/drm/drm_connector.c   | 44
+++++++++++++++++++++++++++++++++++++++
  include/drm/drm_connector.h       |  7 +++++++
  include/drm/drm_mode_config.h     |  6 ++++++
  include/uapi/drm/drm_mode.h       | 24 +++++++++++++++++++++
  5 files changed, 85 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index d5b7f31..9e1d46b 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -721,6 +721,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
  		state->picture_aspect_ratio = val;
  	} else if (property == config->content_type_property) {
  		state->content_type = val;
+	} else if (property == config->colorspace_property) {
+		state->colorspace = val;
  	} else if (property == connector->scaling_mode_property) {
  		state->scaling_mode = val;
  	} else if (property == connector->content_protection_property) {
@@ -795,6 +797,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
  		*val = state->picture_aspect_ratio;
  	} else if (property == config->content_type_property) {
  		*val = state->content_type;
+	} else if (property == config->colorspace_property) {
+		*val = state->colorspace;
  	} else if (property == connector->scaling_mode_property) {
  		*val = state->scaling_mode;
  	} else if (property == connector->content_protection_property) {
diff --git a/drivers/gpu/drm/drm_connector.c
b/drivers/gpu/drm/drm_connector.c index aa18b1d..5ad52a0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -826,6 +826,38 @@ int drm_display_info_set_bus_formats(struct
drm_display_info *info,  };
DRM_ENUM_NAME_FN(drm_get_content_protection_name,
drm_cp_enum_list)
+static const struct drm_prop_enum_list colorspace[] = {
+	/* Standard Definition Colorimetry based on CEA 861 */
+	{ COLORIMETRY_ITU_601, "601" },
+	{ COLORIMETRY_ITU_709, "709" },
+	/* Standard Definition Colorimetry based on IEC 61966-2-4 */
+	{ COLORIMETRY_XV_YCC_601, "601" },
+	/* High Definition Colorimetry based on IEC 61966-2-4 */
+	{ COLORIMETRY_XV_YCC_709, "709" },
2 definitions that share the same name?
All in all, I think the enum strings need to be more descriptive and self-
consistent.
+1
Sure, will modify this.

+	/* Colorimetry based on IEC 61966-2-1/Amendment 1 */
+	{ COLORIMETRY_S_YCC_601, "s601" },
+	/* Colorimetry based on IEC 61966-2-5 [33] */
+	{ COLORIMETRY_ADOBE_YCC_601, "adobe601" },
Hans (cc:d) recetly noted that these things aren't called Adobe<something>
anymore in the CTA-861 spec. There is some new name for them, which I now
forget.
EC2 EC1 EC0 Extended Colorimetry
0       1      1      AdobeYCC601
This is the bit format mentioned in CEA861.F while defining the AVI infoframe, so looks
like they are still keeping it that way. Not sure if its updated as part of any latest spec
update.

New names is opRGB and opYCC601 according to the notice on the first page of CTA-861-G [1]

Updated CTA-861-E/F/G can be found at https://standards.cta.tech/kwspub/published_docs/

[1] https://standards.cta.tech/kwspub/published_docs/CTA-861-G_FINAL_revised_2017.pdf

Correct. The names were updated since Adobe complained to the CTA about trademark issues. And in all fairness, it makes sense to refer to the official international standard name instead of a company standard, even
though they are effectively identical.

Basically, just avoid the name 'Adobe' :-)

Regards,

	Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux