dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/14] New uAPI drm properties for color management
@ 2021-06-15 14:14 Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 01/14] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check Werner Sembach
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx

I started work on my proposal for better color handling in Linux display
drivers: https://lkml.org/lkml/2021/5/12/764

In this 3rd revision everything except the generalised Broadcast RGB
implementation is included. I did however not yet include everything suggested
in the feedback for v1 and v2.

I rebased the patch series on drm-tip to have the latest changes in i915's
YCbCr420 handling and to make the intel-gfx ci not fail on merge anymore.

The read only properties are now correctly marked as immutable.

Some questions I already have:

I think Broadcast RGB is really no good name for the property as, at least in
theory, YCbCr can also be "Limited" or "Full". Should the new implementation
have a different name and make "Broadcast RGB" an alias for it? I propose
"preferred color range" as the new name.

I have not tested dp mst (both on AMD and Intel) as i have no adapter for it at
hand. If one can test it, please let me know if things break or not.

I found the DRM_MODE_PROP_ATOMIC flag and from the documentation it sounds like
"max bpc" (and "preferred color format" and "Broadcast RGB") should have it. Is
there a reason it doesn't?

I have not yet looked into dsc and dithering behaviour.

I have already submitted the first two patches separately to the lkml as they fix
potential bugs and don't introduce new uAPI.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 01/14] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 02/14] drm/amd/display: Add missing cases convert_dc_color_depth_into_bpc Werner Sembach
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check that was performed in the
drm_mode_is_420_only() case, but not in the drm_mode_is_420_also() &&
force_yuv420_output case.

Without further knowledge if YCbCr 4:2:0 is supported outside of HDMI, there is
no reason to use RGB when the display reports drm_mode_is_420_only() even on a
non HDMI connection.

This patch also moves both checks in the same if-case. This  eliminates an extra
else-if-case.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6fda0dfb78f8..44757720b15f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5353,10 +5353,7 @@ static void fill_stream_properties_from_drm_display_mode(
 	timing_out->v_border_bottom = 0;
 	/* TODO: un-hardcode */
 	if (drm_mode_is_420_only(info, mode_in)
-			&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
-		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
-	else if (drm_mode_is_420_also(info, mode_in)
-			&& aconnector->force_yuv420_output)
+			|| (drm_mode_is_420_also(info, mode_in) && aconnector->force_yuv420_output))
 		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
 	else if ((connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB444)
 			&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 02/14] drm/amd/display: Add missing cases convert_dc_color_depth_into_bpc
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 01/14] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property Werner Sembach
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

convert_dc_color_depth_into_bpc() that converts the enum dc_color_depth to an
integer had the casses for COLOR_DEPTH_999 and COLOR_DEPTH_111111 missing.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 44757720b15f..cd1df5cf4815 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6705,6 +6705,10 @@ static int convert_dc_color_depth_into_bpc (enum dc_color_depth display_color_de
 			return 14;
 		case COLOR_DEPTH_161616:
 			return 16;
+		case COLOR_DEPTH_999:
+			return 9;
+		case COLOR_DEPTH_111111:
+			return 11;
 		default:
 			break;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 01/14] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 02/14] drm/amd/display: Add missing cases convert_dc_color_depth_into_bpc Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 17:33   ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 04/14] drm/amd/display: Add handling for new "active bpc" property Werner Sembach
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

Add a new general drm property "active bpc" which can be used by graphic drivers
to report the applied bit depth per pixel back to userspace.

While "max bpc" can be used to change the color depth, there was no way to check
which one actually got used. While in theory the driver chooses the best/highest
color depth within the max bpc setting a user might not be fully aware what his
hardware is or isn't capable off. This is meant as a quick way to double check
the setup.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/drm_connector.c | 50 +++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h     |  8 ++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da39e7ff6965..02c310c1ac2d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1197,6 +1197,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
  *	drm_connector_attach_max_bpc_property() to create and attach the
  *	property to the connector during initialization.
  *
+ * active bpc:
+ *	This read-only range property tells userspace the pixel color bit depth
+ *	actually used by the hardware display engine on "the cable" on a
+ *	connector. The chosen value depends on hardware capabilities, both
+ *	display engine and connected monitor, and the "max bpc" property.
+ *	Drivers shall use drm_connector_attach_active_bpc_property() to install
+ *	this property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2152,6 +2160,48 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
 
+/**
+ * drm_connector_attach_active_bpc_property - attach "active bpc" property
+ * @connector: connector to attach active bpc property on.
+ * @min: The minimum bit depth supported by the connector.
+ * @max: The maximum bit depth supported by the connector.
+ *
+ * This is used to check the applied bit depth on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_bpc_property(struct drm_connector *connector, int min, int max)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	if (!connector->active_bpc_property) {
+		prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE, "active bpc", min, max);
+		if (!prop)
+			return -ENOMEM;
+
+		connector->active_bpc_property = prop;
+		drm_object_attach_property(&connector->base, prop, 0);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_bpc_property);
+
+/**
+ * drm_connector_set_active_bpc_property - sets the active bits per color property for a connector
+ * @connector: drm connector
+ * @active_bpc: bits per color for the connector currently active on "the cable"
+ *
+ * Should be used by atomic drivers to update the active bits per color over a connector.
+ */
+void drm_connector_set_active_bpc_property(struct drm_connector *connector, int active_bpc)
+{
+	drm_object_property_set_value(&connector->base, connector->active_bpc_property, active_bpc);
+}
+EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 714d1a01c065..eee86de62a5f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1380,6 +1380,12 @@ struct drm_connector {
 	 */
 	struct drm_property *max_bpc_property;
 
+	/**
+	 * @active_bpc_property: Default connector property for the active bpc
+	 * to be driven out of the connector.
+	 */
+	struct drm_property *active_bpc_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1702,6 +1708,8 @@ int drm_connector_set_panel_orientation_with_quirk(
 	int width, int height);
 int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 					  int min, int max);
+int drm_connector_attach_active_bpc_property(struct drm_connector *connector, int min, int max);
+void drm_connector_set_active_bpc_property(struct drm_connector *connector, int active_bpc);
 
 /**
  * struct drm_tile_group - Tile group metadata
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 04/14] drm/amd/display: Add handling for new "active bpc" property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (2 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 05/14] drm/i915/display: " Werner Sembach
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

This commit implements the "active bpc" drm property for the AMD GPU driver.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++++++++++++++-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 ++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index cd1df5cf4815..f31bbcb11f03 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7713,8 +7713,10 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 				adev->mode_info.underscan_vborder_property,
 				0);
 
-	if (!aconnector->mst_port)
+	if (!aconnector->mst_port) {
 		drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
+		drm_connector_attach_active_bpc_property(&aconnector->base, 8, 16);
+	}
 
 	/* This defaults to the max in the range, but we want 8bpc for non-edp. */
 	aconnector->base.state->max_bpc = (connector_type == DRM_MODE_CONNECTOR_eDP) ? 16 : 8;
@@ -9083,6 +9085,21 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 		mutex_unlock(&dm->dc_lock);
 	}
 
+	/* Extract information from crtc to communicate it to userspace as connector properties */
+	for_each_new_connector_in_state(state, connector, new_con_state, i) {
+		struct drm_crtc *crtc = new_con_state->crtc;
+		if (crtc) {
+			new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+			dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+			if (dm_new_crtc_state->stream)
+				drm_connector_set_active_bpc_property(connector,
+					convert_dc_color_depth_into_bpc(
+					dm_new_crtc_state->stream->timing.display_color_depth));
+		}
+		else
+			drm_connector_set_active_bpc_property(connector, 0);
+	}
+
 	/* Count number of newly disabled CRTCs for dropping PM refs later. */
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
 				      new_crtc_state, i) {
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 5568d4e518e6..0cf38743ec47 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -409,6 +409,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	if (connector->max_bpc_property)
 		drm_connector_attach_max_bpc_property(connector, 8, 16);
 
+	connector->active_bpc_property = master->base.active_bpc_property;
+	if (connector->active_bpc_property)
+		drm_connector_attach_active_bpc_property(&aconnector->base, 8, 16);
+
 	connector->vrr_capable_property = master->base.vrr_capable_property;
 	if (connector->vrr_capable_property)
 		drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 05/14] drm/i915/display: Add handling for new "active bpc" property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (3 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 04/14] drm/amd/display: Add handling for new "active bpc" property Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 06/14] drm/uAPI: Add "active color format" drm property as feedback for userspace Werner Sembach
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

This commit implements the "active bpc" drm property for the Intel GPU driver.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c      |  8 ++++++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  5 +++++
 drivers/gpu/drm/i915/display/intel_hdmi.c    |  4 +++-
 4 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6be1b31af07b..ee3669bd4662 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10839,6 +10839,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 {
 	struct intel_atomic_state *state = to_intel_atomic_state(_state);
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_connector *connector;
+	struct drm_connector_state *new_conn_state;
+	int i;
 	int ret = 0;
 
 	state->wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
@@ -10907,6 +10910,17 @@ static int intel_atomic_commit(struct drm_device *dev,
 	intel_shared_dpll_swap_state(state);
 	intel_atomic_track_fbs(state);
 
+	/* Extract information from crtc to communicate it to userspace as connector properties */
+	for_each_new_connector_in_state(&state->base, connector, new_conn_state, i) {
+		struct intel_crtc *crtc = to_intel_crtc(new_conn_state->crtc);
+		if (crtc) {
+			struct intel_crtc_state *new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
+			drm_connector_set_active_bpc_property(connector, new_crtc_state->pipe_bpp / 3);
+		}
+		else
+			drm_connector_set_active_bpc_property(connector, 0);
+	}
+
 	drm_atomic_state_get(&state->base);
 	INIT_WORK(&state->base.commit_work, intel_atomic_commit_work);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 5c9222283044..404a27e56ceb 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4688,10 +4688,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 		intel_attach_force_audio_property(connector);
 
 	intel_attach_broadcast_rgb_property(connector);
-	if (HAS_GMCH(dev_priv))
+	if (HAS_GMCH(dev_priv)) {
 		drm_connector_attach_max_bpc_property(connector, 6, 10);
-	else if (DISPLAY_VER(dev_priv) >= 5)
+		drm_connector_attach_active_bpc_property(connector, 6, 10);
+	}
+	else if (DISPLAY_VER(dev_priv) >= 5) {
 		drm_connector_attach_max_bpc_property(connector, 6, 12);
+		drm_connector_attach_active_bpc_property(connector, 6, 12);
+	}
 
 	/* Register HDMI colorspace for case of lspcon */
 	if (intel_bios_is_lspcon_present(dev_priv, port)) {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index b170e272bdee..16bfc59570a5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -851,6 +851,11 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	if (connector->max_bpc_property)
 		drm_connector_attach_max_bpc_property(connector, 6, 12);
 
+	connector->active_bpc_property =
+		intel_dp->attached_connector->base.active_bpc_property;
+	if (connector->active_bpc_property)
+		drm_connector_attach_active_bpc_property(connector, 6, 12);
+
 	return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 7e51c98c475e..9160e21ac9d6 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2513,8 +2513,10 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	if (DISPLAY_VER(dev_priv) >= 10)
 		drm_connector_attach_hdr_output_metadata_property(connector);
 
-	if (!HAS_GMCH(dev_priv))
+	if (!HAS_GMCH(dev_priv)) {
 		drm_connector_attach_max_bpc_property(connector, 8, 12);
+		drm_connector_attach_active_bpc_property(connector, 8, 12);
+	}
 }
 
 /*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 06/14] drm/uAPI: Add "active color format" drm property as feedback for userspace
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (4 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 05/14] drm/i915/display: " Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 07/14] drm/amd/display: Add handling for new "active color format" property Werner Sembach
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

Add a new general drm property "active color format" which can be used by
graphic drivers to report the used color format back to userspace.

There was no way to check which color format got actually used on a given
monitor. To surely predict this, one must know the exact capabilities of the
monitor, the GPU, and the connection used and what the default behaviour of the
used driver is (e.g. amdgpu prefers YCbCr 4:4:4 while i915 prefers RGB). This
property helps eliminating the guessing on this point.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/drm_connector.c | 56 +++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h     |  8 +++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 02c310c1ac2d..2411b964c342 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -889,6 +889,14 @@ static const struct drm_prop_enum_list drm_dp_subconnector_enum_list[] = {
 	{ DRM_MODE_SUBCONNECTOR_Native,	     "Native"    }, /* DP */
 };
 
+static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
+	{ 0, "unknown" },
+	{ DRM_COLOR_FORMAT_RGB444, "rgb" },
+	{ DRM_COLOR_FORMAT_YCRCB444, "ycbcr444" },
+	{ DRM_COLOR_FORMAT_YCRCB422, "ycbcr422" },
+	{ DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
+};
+
 DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
 		 drm_dp_subconnector_enum_list)
 
@@ -1205,6 +1213,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
  *	Drivers shall use drm_connector_attach_active_bpc_property() to install
  *	this property.
  *
+ * active color format:
+ *	This read-only property tells userspace the color format actually used
+ *	by the hardware display engine on "the cable" on a connector. The chosen
+ *	value depends on hardware capabilities, both display engine and
+ *	connected monitor. Drivers shall use
+ *	drm_connector_attach_active_color_format_property() to install this
+ *	property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2202,6 +2218,46 @@ void drm_connector_set_active_bpc_property(struct drm_connector *connector, int
 }
 EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
 
+/**
+ * drm_connector_attach_active_color_format_property - attach "active color format" property
+ * @connector: connector to attach active color format property on.
+ *
+ * This is used to check the applied color format on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_color_format_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	if (!connector->active_color_format_property) {
+		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE, "active color format", drm_active_color_format_enum_list, ARRAY_SIZE(drm_active_color_format_enum_list));
+		if (!prop)
+			return -ENOMEM;
+
+		connector->active_color_format_property = prop;
+		drm_object_attach_property(&connector->base, prop, 0);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_color_format_property);
+
+/**
+ * drm_connector_set_active_color_format_property - sets the active color format property for a connector
+ * @connector: drm connector
+ * @active_color_format: color format for the connector currently active on "the cable"
+ *
+ * Should be used by atomic drivers to update the active color format over a connector.
+ */
+void drm_connector_set_active_color_format_property(struct drm_connector *connector, u32 active_color_format)
+{
+	drm_object_property_set_value(&connector->base, connector->active_color_format_property, active_color_format);
+}
+EXPORT_SYMBOL(drm_connector_set_active_color_format_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index eee86de62a5f..99e4989dd6b3 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1386,6 +1386,12 @@ struct drm_connector {
 	 */
 	struct drm_property *active_bpc_property;
 
+	/**
+	 * @active_color_format_property: Default connector property for the
+	 * active color format to be driven out of the connector.
+	 */
+	struct drm_property *active_color_format_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1710,6 +1716,8 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 					  int min, int max);
 int drm_connector_attach_active_bpc_property(struct drm_connector *connector, int min, int max);
 void drm_connector_set_active_bpc_property(struct drm_connector *connector, int active_bpc);
+int drm_connector_attach_active_color_format_property(struct drm_connector *connector);
+void drm_connector_set_active_color_format_property(struct drm_connector *connector, u32 active_color_format);
 
 /**
  * struct drm_tile_group - Tile group metadata
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 07/14] drm/amd/display: Add handling for new "active color format" property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (5 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 06/14] drm/uAPI: Add "active color format" drm property as feedback for userspace Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 08/14] drm/i915/display: " Werner Sembach
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

This commit implements the "active color format" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 28 +++++++++++++++++--
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 +++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f31bbcb11f03..b66336a1403e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6715,6 +6715,23 @@ static int convert_dc_color_depth_into_bpc (enum dc_color_depth display_color_de
 	return 0;
 }
 
+static int convert_dc_pixel_encoding_into_drm_color_format(enum dc_pixel_encoding display_pixel_encoding)
+{
+	switch (display_pixel_encoding) {
+		case PIXEL_ENCODING_RGB:
+			return DRM_COLOR_FORMAT_RGB444;
+		case PIXEL_ENCODING_YCBCR422:
+			return DRM_COLOR_FORMAT_YCRCB422;
+		case PIXEL_ENCODING_YCBCR444:
+			return DRM_COLOR_FORMAT_YCRCB444;
+		case PIXEL_ENCODING_YCBCR420:
+			return DRM_COLOR_FORMAT_YCRCB420;
+		default:
+			break;
+		}
+	return 0;
+}
+
 static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
 					  struct drm_crtc_state *crtc_state,
 					  struct drm_connector_state *conn_state)
@@ -7716,6 +7733,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 	if (!aconnector->mst_port) {
 		drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
 		drm_connector_attach_active_bpc_property(&aconnector->base, 8, 16);
+		drm_connector_attach_active_color_format_property(&aconnector->base);
 	}
 
 	/* This defaults to the max in the range, but we want 8bpc for non-edp. */
@@ -9091,13 +9109,19 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 		if (crtc) {
 			new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 			dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-			if (dm_new_crtc_state->stream)
+			if (dm_new_crtc_state->stream) {
 				drm_connector_set_active_bpc_property(connector,
 					convert_dc_color_depth_into_bpc(
 					dm_new_crtc_state->stream->timing.display_color_depth));
+				drm_connector_set_active_color_format_property(connector,
+					convert_dc_pixel_encoding_into_drm_color_format(
+					dm_new_crtc_state->stream->timing.pixel_encoding));
+			}
 		}
-		else
+		else {
 			drm_connector_set_active_bpc_property(connector, 0);
+			drm_connector_set_active_color_format_property(connector, 0);
+		}
 	}
 
 	/* Count number of newly disabled CRTCs for dropping PM refs later. */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 0cf38743ec47..13151d13aa73 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -413,6 +413,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	if (connector->active_bpc_property)
 		drm_connector_attach_active_bpc_property(&aconnector->base, 8, 16);
 
+	connector->active_color_format_property = master->base.active_color_format_property;
+	if (connector->active_color_format_property)
+		drm_connector_attach_active_color_format_property(&aconnector->base);
+
 	connector->vrr_capable_property = master->base.vrr_capable_property;
 	if (connector->vrr_capable_property)
 		drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 08/14] drm/i915/display: Add handling for new "active color format" property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (6 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 07/14] drm/amd/display: Add handling for new "active color format" property Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 09/14] drm/uAPI: Add "active color range" drm property as feedback for userspace Werner Sembach
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

This commit implements the "active color format" drm property for the Intel GPU
driver.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 21 +++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_dp.c      |  2 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  5 +++++
 drivers/gpu/drm/i915/display/intel_hdmi.c    |  1 +
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ee3669bd4662..2cf09599ddc3 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10609,6 +10609,21 @@ static void intel_atomic_prepare_plane_clear_colors(struct intel_atomic_state *s
 	}
 }
 
+static int convert_intel_output_format_into_drm_color_format(enum intel_output_format output_format)
+{
+	switch (output_format) {
+		case INTEL_OUTPUT_FORMAT_RGB:
+			return DRM_COLOR_FORMAT_RGB444;
+		case INTEL_OUTPUT_FORMAT_YCBCR420:
+			return DRM_COLOR_FORMAT_YCRCB420;
+		case INTEL_OUTPUT_FORMAT_YCBCR444:
+			return DRM_COLOR_FORMAT_YCRCB444;
+		default:
+			break;
+		}
+	return 0;
+}
+
 static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 {
 	struct drm_device *dev = state->base.dev;
@@ -10916,9 +10931,13 @@ static int intel_atomic_commit(struct drm_device *dev,
 		if (crtc) {
 			struct intel_crtc_state *new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
 			drm_connector_set_active_bpc_property(connector, new_crtc_state->pipe_bpp / 3);
+			drm_connector_set_active_color_format_property(connector,
+				convert_intel_output_format_into_drm_color_format(new_crtc_state->output_format));
 		}
-		else
+		else {
 			drm_connector_set_active_bpc_property(connector, 0);
+			drm_connector_set_active_color_format_property(connector, 0);
+		}
 	}
 
 	drm_atomic_state_get(&state->base);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 404a27e56ceb..01fdb9141592 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4691,10 +4691,12 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	if (HAS_GMCH(dev_priv)) {
 		drm_connector_attach_max_bpc_property(connector, 6, 10);
 		drm_connector_attach_active_bpc_property(connector, 6, 10);
+		drm_connector_attach_active_color_format_property(connector);
 	}
 	else if (DISPLAY_VER(dev_priv) >= 5) {
 		drm_connector_attach_max_bpc_property(connector, 6, 12);
 		drm_connector_attach_active_bpc_property(connector, 6, 12);
+		drm_connector_attach_active_color_format_property(connector);
 	}
 
 	/* Register HDMI colorspace for case of lspcon */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 16bfc59570a5..3e4237df3360 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -856,6 +856,11 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	if (connector->active_bpc_property)
 		drm_connector_attach_active_bpc_property(connector, 6, 12);
 
+	connector->active_color_format_property =
+		intel_dp->attached_connector->base.active_color_format_property;
+	if (connector->active_color_format_property)
+		drm_connector_attach_active_color_format_property(connector);
+
 	return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9160e21ac9d6..367aba57b55f 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2516,6 +2516,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	if (!HAS_GMCH(dev_priv)) {
 		drm_connector_attach_max_bpc_property(connector, 8, 12);
 		drm_connector_attach_active_bpc_property(connector, 8, 12);
+		drm_connector_attach_active_color_format_property(connector);
 	}
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 09/14] drm/uAPI: Add "active color range" drm property as feedback for userspace
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (7 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 08/14] drm/i915/display: " Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 10/14] drm/amd/display: Add handling for new "active color range" property Werner Sembach
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

Add a new general drm property "active color range" which can be used by graphic
drivers to report the used color range back to userspace.

There was no way to check which color range got actually used on a given
monitor. To surely predict this, one must know the exact capabilities of the
monitor and what the default behaviour of the used driver is. This property
helps eliminating the guessing at this point.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/drm_connector.c | 54 +++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h     | 26 ++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2411b964c342..b4aff7d6910f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -897,6 +897,12 @@ static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
 	{ DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
 };
 
+static const struct drm_prop_enum_list drm_active_color_range_enum_list[] = {
+	{ DRM_MODE_COLOR_RANGE_UNSET, "Unknown" },
+	{ DRM_MODE_COLOR_RANGE_FULL, "Full" },
+	{ DRM_MODE_COLOR_RANGE_LIMITED_16_235, "Limited 16:235" },
+};
+
 DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
 		 drm_dp_subconnector_enum_list)
 
@@ -1221,6 +1227,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
  *	drm_connector_attach_active_color_format_property() to install this
  *	property.
  *
+ * active color range:
+ *	This read-only property tells userspace the color range actually used by
+ *	the hardware display engine on "the cable" on a connector. The chosen
+ *	value depends on hardware capabilities of the monitor and the used color
+ *	format. Drivers shall use
+ *	drm_connector_attach_active_color_range_property() to install this
+ *	property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2258,6 +2272,46 @@ void drm_connector_set_active_color_format_property(struct drm_connector *connec
 }
 EXPORT_SYMBOL(drm_connector_set_active_color_format_property);
 
+/**
+ * drm_connector_attach_active_color_range_property - attach "active color range" property
+ * @connector: connector to attach active color range property on.
+ *
+ * This is used to check the applied color range on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_color_range_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	if (!connector->active_color_range_property) {
+		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE, "active color range", drm_active_color_range_enum_list, ARRAY_SIZE(drm_active_color_range_enum_list));
+		if (!prop)
+			return -ENOMEM;
+
+		connector->active_color_range_property = prop;
+		drm_object_attach_property(&connector->base, prop, DRM_MODE_COLOR_RANGE_UNSET);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_color_range_property);
+
+/**
+ * drm_connector_set_active_color_range_property - sets the active color range property for a connector
+ * @connector: drm connector
+ * @active_color_range: color range for the connector currently active on "the cable"
+ *
+ * Should be used by atomic drivers to update the active color range over a connector.
+ */
+void drm_connector_set_active_color_range_property(struct drm_connector *connector, enum drm_mode_color_range active_color_range)
+{
+	drm_object_property_set_value(&connector->base, connector->active_color_range_property, active_color_range);
+}
+EXPORT_SYMBOL(drm_connector_set_active_color_range_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 99e4989dd6b3..5cb122812727 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -648,6 +648,24 @@ struct drm_tv_connector_state {
 	unsigned int hue;
 };
 
+/**
+ * enum drm_mode_color_range - color_range info for &drm_connector
+ *
+ * This enum is used to represent full or limited color range on the display
+ * connector signal.
+ *
+ * @DRM_MODE_COLOR_RANGE_UNSET:		Color range is unspecified/default.
+ * @DRM_MODE_COLOR_RANGE_FULL:		Color range is full range, 0-255 for
+ * 					8-Bit color depth.
+ * DRM_MODE_COLOR_RANGE_LIMITED_16_235:	Color range is limited range, 16-235 for
+ * 					8-Bit color depth.
+ */
+enum drm_mode_color_range {
+	DRM_MODE_COLOR_RANGE_UNSET,
+	DRM_MODE_COLOR_RANGE_FULL,
+	DRM_MODE_COLOR_RANGE_LIMITED_16_235,
+};
+
 /**
  * struct drm_connector_state - mutable connector state
  */
@@ -1392,6 +1410,12 @@ struct drm_connector {
 	 */
 	struct drm_property *active_color_format_property;
 
+	/**
+	 * @active_color_range_property: Default connector property for the
+	 * active color range to be driven out of the connector.
+	 */
+	struct drm_property *active_color_range_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1718,6 +1742,8 @@ int drm_connector_attach_active_bpc_property(struct drm_connector *connector, in
 void drm_connector_set_active_bpc_property(struct drm_connector *connector, int active_bpc);
 int drm_connector_attach_active_color_format_property(struct drm_connector *connector);
 void drm_connector_set_active_color_format_property(struct drm_connector *connector, u32 active_color_format);
+int drm_connector_attach_active_color_range_property(struct drm_connector *connector);
+void drm_connector_set_active_color_range_property(struct drm_connector *connector, enum drm_mode_color_range active_color_range);
 
 /**
  * struct drm_tile_group - Tile group metadata
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 10/14] drm/amd/display: Add handling for new "active color range" property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (8 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 09/14] drm/uAPI: Add "active color range" drm property as feedback for userspace Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 11/14] drm/i915/display: " Werner Sembach
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

This commit implements the "active color range" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 +++++++++++++++++++
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 +++
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b66336a1403e..705cd2e015af 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6732,6 +6732,33 @@ static int convert_dc_pixel_encoding_into_drm_color_format(enum dc_pixel_encodin
 	return 0;
 }
 
+static int convert_dc_color_space_into_drm_mode_color_range(enum dc_color_space color_space)
+{
+	if (color_space == COLOR_SPACE_SRGB ||
+	    color_space == COLOR_SPACE_XR_RGB ||
+	    color_space == COLOR_SPACE_MSREF_SCRGB ||
+	    color_space == COLOR_SPACE_2020_RGB_FULLRANGE ||
+	    color_space == COLOR_SPACE_ADOBERGB ||
+	    color_space == COLOR_SPACE_DCIP3 ||
+	    color_space == COLOR_SPACE_DOLBYVISION ||
+	    color_space == COLOR_SPACE_YCBCR601 ||
+	    color_space == COLOR_SPACE_XV_YCC_601 ||
+	    color_space == COLOR_SPACE_YCBCR709 ||
+	    color_space == COLOR_SPACE_XV_YCC_709 ||
+	    color_space == COLOR_SPACE_2020_YCBCR ||
+	    color_space == COLOR_SPACE_YCBCR709_BLACK ||
+	    color_space == COLOR_SPACE_DISPLAYNATIVE ||
+	    color_space == COLOR_SPACE_APPCTRL ||
+	    color_space == COLOR_SPACE_CUSTOMPOINTS)
+		return DRM_MODE_COLOR_RANGE_FULL;
+	if (color_space == COLOR_SPACE_SRGB_LIMITED ||
+	    color_space == COLOR_SPACE_2020_RGB_LIMITEDRANGE ||
+	    color_space == COLOR_SPACE_YCBCR601_LIMITED ||
+	    color_space == COLOR_SPACE_YCBCR709_LIMITED)
+		return DRM_MODE_COLOR_RANGE_LIMITED_16_235;
+	return DRM_MODE_COLOR_RANGE_UNSET;
+}
+
 static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
 					  struct drm_crtc_state *crtc_state,
 					  struct drm_connector_state *conn_state)
@@ -7734,6 +7761,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 		drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
 		drm_connector_attach_active_bpc_property(&aconnector->base, 8, 16);
 		drm_connector_attach_active_color_format_property(&aconnector->base);
+		drm_connector_attach_active_color_range_property(&aconnector->base);
 	}
 
 	/* This defaults to the max in the range, but we want 8bpc for non-edp. */
@@ -9116,11 +9144,15 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 				drm_connector_set_active_color_format_property(connector,
 					convert_dc_pixel_encoding_into_drm_color_format(
 					dm_new_crtc_state->stream->timing.pixel_encoding));
+				drm_connector_set_active_color_range_property(connector,
+					convert_dc_color_space_into_drm_mode_color_range(
+					dm_new_crtc_state->stream->output_color_space));
 			}
 		}
 		else {
 			drm_connector_set_active_bpc_property(connector, 0);
 			drm_connector_set_active_color_format_property(connector, 0);
+			drm_connector_set_active_color_range_property(connector, DRM_MODE_COLOR_RANGE_UNSET);
 		}
 	}
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 13151d13aa73..b5d57bbbdd20 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -417,6 +417,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	if (connector->active_color_format_property)
 		drm_connector_attach_active_color_format_property(&aconnector->base);
 
+	connector->active_color_range_property = master->base.active_color_range_property;
+	if (connector->active_color_range_property)
+		drm_connector_attach_active_color_range_property(&aconnector->base);
+
 	connector->vrr_capable_property = master->base.vrr_capable_property;
 	if (connector->vrr_capable_property)
 		drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 11/14] drm/i915/display: Add handling for new "active color range" property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (9 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 10/14] drm/amd/display: Add handling for new "active color range" property Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 12/14] drm/uAPI: Add "preferred color format" drm property as setting for userspace Werner Sembach
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

This commit implements the "active color range" drm property for the Intel GPU
driver.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 4 ++++
 drivers/gpu/drm/i915/display/intel_dp.c      | 2 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c  | 5 +++++
 drivers/gpu/drm/i915/display/intel_hdmi.c    | 1 +
 4 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2cf09599ddc3..09d7c3da105a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10933,10 +10933,14 @@ static int intel_atomic_commit(struct drm_device *dev,
 			drm_connector_set_active_bpc_property(connector, new_crtc_state->pipe_bpp / 3);
 			drm_connector_set_active_color_format_property(connector,
 				convert_intel_output_format_into_drm_color_format(new_crtc_state->output_format));
+			drm_connector_set_active_color_range_property(connector,
+				new_crtc_state->limited_color_range? DRM_MODE_COLOR_RANGE_LIMITED_16_235
+								   : DRM_MODE_COLOR_RANGE_FULL);
 		}
 		else {
 			drm_connector_set_active_bpc_property(connector, 0);
 			drm_connector_set_active_color_format_property(connector, 0);
+			drm_connector_set_active_color_range_property(connector, DRM_MODE_COLOR_RANGE_UNSET);
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 01fdb9141592..d648582d0786 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4692,11 +4692,13 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 		drm_connector_attach_max_bpc_property(connector, 6, 10);
 		drm_connector_attach_active_bpc_property(connector, 6, 10);
 		drm_connector_attach_active_color_format_property(connector);
+		drm_connector_attach_active_color_range_property(connector);
 	}
 	else if (DISPLAY_VER(dev_priv) >= 5) {
 		drm_connector_attach_max_bpc_property(connector, 6, 12);
 		drm_connector_attach_active_bpc_property(connector, 6, 12);
 		drm_connector_attach_active_color_format_property(connector);
+		drm_connector_attach_active_color_range_property(connector);
 	}
 
 	/* Register HDMI colorspace for case of lspcon */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 3e4237df3360..cb876175258f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -861,6 +861,11 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	if (connector->active_color_format_property)
 		drm_connector_attach_active_color_format_property(connector);
 
+	connector->active_color_range_property =
+		intel_dp->attached_connector->base.active_color_range_property;
+	if (connector->active_color_range_property)
+		drm_connector_attach_active_color_range_property(connector);
+
 	return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 367aba57b55f..dacac23a6c30 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2517,6 +2517,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 		drm_connector_attach_max_bpc_property(connector, 8, 12);
 		drm_connector_attach_active_bpc_property(connector, 8, 12);
 		drm_connector_attach_active_color_format_property(connector);
+		drm_connector_attach_active_color_range_property(connector);
 	}
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 12/14] drm/uAPI: Add "preferred color format" drm property as setting for userspace
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (10 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 11/14] drm/i915/display: " Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 13/14] drm/amd/display: Add handling for new "preferred color format" property Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 14/14] drm/i915/display: " Werner Sembach
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

Add a new general drm property "preferred color format" which can be used by
userspace to tell the graphic drivers to which color format to use.

Possible options are:
    - auto (default/current behaviour)
    - rgb
    - ycbcr444
    - ycbcr422 (not supported by both amdgpu and i915)
    - ycbcr420

In theory the auto option should choose the best available option for the
current setup, but because of bad internal conversion some monitors look better
with rgb and some with ycbcr444.

Also, because of bad shielded connectors and/or cables, it might be preferable
to use the less bandwidth heavy ycbcr422 and ycbcr420 formats for a signal that
is less deceptible to interference.

In the future, automatic color calibration for screens might also depend on this
option being available.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/drm_atomic_helper.c |  4 +++
 drivers/gpu/drm/drm_atomic_uapi.c   |  4 +++
 drivers/gpu/drm/drm_connector.c     | 46 ++++++++++++++++++++++++++++-
 include/drm/drm_connector.h         | 17 +++++++++++
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index bc3487964fb5..90d62f305257 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -687,6 +687,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 			if (old_connector_state->max_requested_bpc !=
 			    new_connector_state->max_requested_bpc)
 				new_crtc_state->connectors_changed = true;
+
+			if (old_connector_state->preferred_color_format !=
+			    new_connector_state->preferred_color_format)
+				new_crtc_state->connectors_changed = true;
 		}
 
 		if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 438e9585b225..c536f5e22016 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -796,6 +796,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 						   fence_ptr);
 	} else if (property == connector->max_bpc_property) {
 		state->max_requested_bpc = val;
+	} else if (property == connector->preferred_color_format_property) {
+		state->preferred_color_format = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -873,6 +875,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = 0;
 	} else if (property == connector->max_bpc_property) {
 		*val = state->max_requested_bpc;
+	} else if (property == connector->preferred_color_format_property) {
+		*val = state->preferred_color_format;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b4aff7d6910f..1dc537514c71 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -889,6 +889,14 @@ static const struct drm_prop_enum_list drm_dp_subconnector_enum_list[] = {
 	{ DRM_MODE_SUBCONNECTOR_Native,	     "Native"    }, /* DP */
 };
 
+static const struct drm_prop_enum_list drm_preferred_color_format_enum_list[] = {
+	{ 0, "auto" },
+	{ DRM_COLOR_FORMAT_RGB444, "rgb" },
+	{ DRM_COLOR_FORMAT_YCRCB444, "ycbcr444" },
+	{ DRM_COLOR_FORMAT_YCRCB422, "ycbcr422" },
+	{ DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
+};
+
 static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
 	{ 0, "unknown" },
 	{ DRM_COLOR_FORMAT_RGB444, "rgb" },
@@ -1219,11 +1227,19 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
  *	Drivers shall use drm_connector_attach_active_bpc_property() to install
  *	this property.
  *
+ * preferred color format:
+ *	This property is used by userspace to change the used color format. When
+ *	used the driver will use the selected format if valid for the hardware,
+ *	sink, and current resolution and refresh rate combination. Drivers to
+ *	use the function drm_connector_attach_preferred_color_format_property()
+ *	to create and attach the property to the connector during
+ *	initialization.
+ *
  * active color format:
  *	This read-only property tells userspace the color format actually used
  *	by the hardware display engine on "the cable" on a connector. The chosen
  *	value depends on hardware capabilities, both display engine and
- *	connected monitor. Drivers shall use
+ *	connected monitor, and the "preferred color format". Drivers shall use
  *	drm_connector_attach_active_color_format_property() to install this
  *	property.
  *
@@ -2232,6 +2248,34 @@ void drm_connector_set_active_bpc_property(struct drm_connector *connector, int
 }
 EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
 
+/**
+ * drm_connector_attach_preferred_color_format_property - attach "preferred color format" property
+ * @connector: connector to attach active color format property on.
+ *
+ * This is used to add support for selecting a color format on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_preferred_color_format_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	if (!connector->preferred_color_format_property) {
+		prop = drm_property_create_enum(dev, 0, "preferred color format", drm_preferred_color_format_enum_list, ARRAY_SIZE(drm_preferred_color_format_enum_list));
+		if (!prop)
+			return -ENOMEM;
+
+		connector->preferred_color_format_property = prop;
+		drm_object_attach_property(&connector->base, prop, 0);
+		connector->state->preferred_color_format = 0;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_preferred_color_format_property);
+
 /**
  * drm_connector_attach_active_color_format_property - attach "active color format" property
  * @connector: connector to attach active color format property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 5cb122812727..e52e0d2c3c4e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -799,6 +799,16 @@ struct drm_connector_state {
 	 */
 	u8 max_bpc;
 
+	/**
+	 * preferred_color_format: Property set by userspace to tell the GPU
+	 * driver which color format to use. It only gets applied if hardware,
+	 * meaning both the computer and the monitor, and the driver support the
+	 * given format at the current resolution and refresh rate. Userspace
+	 * can check for (un-)successful application via the active_color_format
+	 * property.
+	 */
+	u32 preferred_color_format;
+
 	/**
 	 * @hdr_output_metadata:
 	 * DRM blob property for HDR output metadata
@@ -1404,6 +1414,12 @@ struct drm_connector {
 	 */
 	struct drm_property *active_bpc_property;
 
+	/**
+	 * @preferred_color_format_property: Default connector property for the
+	 * preferred color format to be driven out of the connector.
+	 */
+	struct drm_property *preferred_color_format_property;
+
 	/**
 	 * @active_color_format_property: Default connector property for the
 	 * active color format to be driven out of the connector.
@@ -1740,6 +1756,7 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 					  int min, int max);
 int drm_connector_attach_active_bpc_property(struct drm_connector *connector, int min, int max);
 void drm_connector_set_active_bpc_property(struct drm_connector *connector, int active_bpc);
+int drm_connector_attach_preferred_color_format_property(struct drm_connector *connector);
 int drm_connector_attach_active_color_format_property(struct drm_connector *connector);
 void drm_connector_set_active_color_format_property(struct drm_connector *connector, u32 active_color_format);
 int drm_connector_attach_active_color_range_property(struct drm_connector *connector);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 13/14] drm/amd/display: Add handling for new "preferred color format" property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (11 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 12/14] drm/uAPI: Add "preferred color format" drm property as setting for userspace Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  2021-06-15 14:14 ` [PATCH v3 14/14] drm/i915/display: " Werner Sembach
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

This commit implements the "preferred color format" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ++++++++++++++-----
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 ++++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 705cd2e015af..ead246b2ec57 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5351,15 +5351,26 @@ static void fill_stream_properties_from_drm_display_mode(
 	timing_out->h_border_right = 0;
 	timing_out->v_border_top = 0;
 	timing_out->v_border_bottom = 0;
-	/* TODO: un-hardcode */
-	if (drm_mode_is_420_only(info, mode_in)
-			|| (drm_mode_is_420_also(info, mode_in) && aconnector->force_yuv420_output))
+
+	if (connector_state && (connector_state->preferred_color_format == DRM_COLOR_FORMAT_YCRCB420
+			|| aconnector->force_yuv420_output) && drm_mode_is_420(info, mode_in))
 		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
-	else if ((connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB444)
-			&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
+	else if (connector_state && connector_state->preferred_color_format == DRM_COLOR_FORMAT_YCRCB444
+			&& connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB444)
 		timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
-	else
+	else if (connector_state && connector_state->preferred_color_format == DRM_COLOR_FORMAT_RGB444
+			&& !drm_mode_is_420_only(info, mode_in))
 		timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
+	else /* connector_state->preferred_color_format not possible
+			|| connector_state->preferred_color_format == 0 (auto)
+			|| connector_state->preferred_color_format == DRM_COLOR_FORMAT_YCRCB422 */
+		if (drm_mode_is_420_only(info, mode_in))
+			timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
+		else if ((connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB444)
+				&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
+			timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
+		else
+			timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
 
 	timing_out->timing_3d_format = TIMING_3D_FORMAT_NONE;
 	timing_out->display_color_depth = convert_color_depth_from_display_info(
@@ -7760,6 +7771,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 	if (!aconnector->mst_port) {
 		drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
 		drm_connector_attach_active_bpc_property(&aconnector->base, 8, 16);
+		drm_connector_attach_preferred_color_format_property(&aconnector->base);
 		drm_connector_attach_active_color_format_property(&aconnector->base);
 		drm_connector_attach_active_color_range_property(&aconnector->base);
 	}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index b5d57bbbdd20..2563788ba95a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -413,6 +413,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	if (connector->active_bpc_property)
 		drm_connector_attach_active_bpc_property(&aconnector->base, 8, 16);
 
+	connector->preferred_color_format_property = master->base.preferred_color_format_property;
+	if (connector->preferred_color_format_property)
+		drm_connector_attach_preferred_color_format_property(&aconnector->base);
+
 	connector->active_color_format_property = master->base.active_color_format_property;
 	if (connector->active_color_format_property)
 		drm_connector_attach_active_color_format_property(&aconnector->base);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 14/14] drm/i915/display: Add handling for new "preferred color format" property
  2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
                   ` (12 preceding siblings ...)
  2021-06-15 14:14 ` [PATCH v3 13/14] drm/amd/display: Add handling for new "preferred color format" property Werner Sembach
@ 2021-06-15 14:14 ` Werner Sembach
  13 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 14:14 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx
  Cc: Werner Sembach

This commit implements the "preferred color format" drm property for the Intel GPU
driver.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 7 ++++++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++++
 drivers/gpu/drm/i915/display/intel_hdmi.c   | 5 +++++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index d648582d0786..5c83ae624ad1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -616,9 +616,12 @@ intel_dp_output_format(struct drm_connector *connector,
 {
 	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
 	const struct drm_display_info *info = &connector->display_info;
+	const struct drm_connector_state *connector_state = connector->state;
 
 	if (!connector->ycbcr_420_allowed ||
-	    !drm_mode_is_420_only(info, mode))
+	    !(drm_mode_is_420_only(info, mode) ||
+	    (drm_mode_is_420_also(info, mode) && connector_state &&
+	    connector_state->preferred_color_format == DRM_COLOR_FORMAT_YCRCB420)))
 		return INTEL_OUTPUT_FORMAT_RGB;
 
 	if (intel_dp->dfp.rgb_to_ycbcr &&
@@ -4691,12 +4694,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	if (HAS_GMCH(dev_priv)) {
 		drm_connector_attach_max_bpc_property(connector, 6, 10);
 		drm_connector_attach_active_bpc_property(connector, 6, 10);
+		drm_connector_attach_preferred_color_format_property(connector);
 		drm_connector_attach_active_color_format_property(connector);
 		drm_connector_attach_active_color_range_property(connector);
 	}
 	else if (DISPLAY_VER(dev_priv) >= 5) {
 		drm_connector_attach_max_bpc_property(connector, 6, 12);
 		drm_connector_attach_active_bpc_property(connector, 6, 12);
+		drm_connector_attach_preferred_color_format_property(connector);
 		drm_connector_attach_active_color_format_property(connector);
 		drm_connector_attach_active_color_range_property(connector);
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index cb876175258f..67f0fb649876 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -856,6 +856,11 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	if (connector->active_bpc_property)
 		drm_connector_attach_active_bpc_property(connector, 6, 12);
 
+	connector->preferred_color_format_property =
+		intel_dp->attached_connector->base.preferred_color_format_property;
+	if (connector->preferred_color_format_property)
+		drm_connector_attach_preferred_color_format_property(connector);
+
 	connector->active_color_format_property =
 		intel_dp->attached_connector->base.active_color_format_property;
 	if (connector->active_color_format_property)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index dacac23a6c30..0d917d61482d 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2153,6 +2153,10 @@ static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
 		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	}
 
+	if (connector->ycbcr_420_allowed && conn_state->preferred_color_format == DRM_COLOR_FORMAT_YCRCB420 &&
+	    drm_mode_is_420_also(&connector->display_info, adjusted_mode))
+		crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+
 	ret = intel_hdmi_compute_clock(encoder, crtc_state);
 	if (ret) {
 		if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
@@ -2516,6 +2520,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	if (!HAS_GMCH(dev_priv)) {
 		drm_connector_attach_max_bpc_property(connector, 8, 12);
 		drm_connector_attach_active_bpc_property(connector, 8, 12);
+		drm_connector_attach_preferred_color_format_property(connector);
 		drm_connector_attach_active_color_format_property(connector);
 		drm_connector_attach_active_color_range_property(connector);
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property
  2021-06-15 14:14 ` [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property Werner Sembach
@ 2021-06-15 17:33   ` Werner Sembach
  0 siblings, 0 replies; 16+ messages in thread
From: Werner Sembach @ 2021-06-15 17:33 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	jani.nikula, joonas.lahtinen, rodrigo.vivi, amd-gfx, dri-devel,
	linux-kernel, intel-gfx

Am 15.06.21 um 16:14 schrieb Werner Sembach:
> Add a new general drm property "active bpc" which can be used by graphic drivers
> to report the applied bit depth per pixel back to userspace.
>
> While "max bpc" can be used to change the color depth, there was no way to check
> which one actually got used. While in theory the driver chooses the best/highest
> color depth within the max bpc setting a user might not be fully aware what his
> hardware is or isn't capable off. This is meant as a quick way to double check
> the setup.
>
> In the future, automatic color calibration for screens might also depend on this
> information being available.
>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>   drivers/gpu/drm/drm_connector.c | 50 +++++++++++++++++++++++++++++++++
>   include/drm/drm_connector.h     |  8 ++++++
>   2 files changed, 58 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index da39e7ff6965..02c310c1ac2d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1197,6 +1197,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
>    *	drm_connector_attach_max_bpc_property() to create and attach the
>    *	property to the connector during initialization.
>    *
> + * active bpc:
> + *	This read-only range property tells userspace the pixel color bit depth
> + *	actually used by the hardware display engine on "the cable" on a
> + *	connector. The chosen value depends on hardware capabilities, both
> + *	display engine and connected monitor, and the "max bpc" property.
> + *	Drivers shall use drm_connector_attach_active_bpc_property() to install
> + *	this property.
> + *
>    * Connectors also have one standardized atomic property:
>    *
>    * CRTC_ID:
> @@ -2152,6 +2160,48 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>   }
>   EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
>   
> +/**
> + * drm_connector_attach_active_bpc_property - attach "active bpc" property
> + * @connector: connector to attach active bpc property on.
> + * @min: The minimum bit depth supported by the connector.
> + * @max: The maximum bit depth supported by the connector.
> + *
> + * This is used to check the applied bit depth on a connector.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_active_bpc_property(struct drm_connector *connector, int min, int max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	if (!connector->active_bpc_property) {
> +		prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE, "active bpc", min, max);
> +		if (!prop)
> +			return -ENOMEM;
> +
> +		connector->active_bpc_property = prop;
> +		drm_object_attach_property(&connector->base, prop, 0);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_active_bpc_property);
> +
> +/**
> + * drm_connector_set_active_bpc_property - sets the active bits per color property for a connector
> + * @connector: drm connector
> + * @active_bpc: bits per color for the connector currently active on "the cable"
> + *
> + * Should be used by atomic drivers to update the active bits per color over a connector.
> + */
> +void drm_connector_set_active_bpc_property(struct drm_connector *connector, int active_bpc)
> +{
> +	drm_object_property_set_value(&connector->base, connector->active_bpc_property, active_bpc);
> +}
> +EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
> +
>   /**
>    * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property
>    * @connector: connector to attach the property on.
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 714d1a01c065..eee86de62a5f 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1380,6 +1380,12 @@ struct drm_connector {
>   	 */
>   	struct drm_property *max_bpc_property;
>   
> +	/**
> +	 * @active_bpc_property: Default connector property for the active bpc
> +	 * to be driven out of the connector.
> +	 */
> +	struct drm_property *active_bpc_property;
> +
>   #define DRM_CONNECTOR_POLL_HPD (1 << 0)
>   #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
>   #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> @@ -1702,6 +1708,8 @@ int drm_connector_set_panel_orientation_with_quirk(
>   	int width, int height);
>   int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>   					  int min, int max);
> +int drm_connector_attach_active_bpc_property(struct drm_connector *connector, int min, int max);
> +void drm_connector_set_active_bpc_property(struct drm_connector *connector, int active_bpc);
>   
>   /**
>    * struct drm_tile_group - Tile group metadata

I looked into DSC:

In both the amd and intel implementation it's:

max_bpc >= uncompressed bpc >= dsc bpc

Currently the patch is setting active bpc to the uncompressed bpc.

I gave the DSC specification a (very brief) look and did not find any 
obvious mention of some kind bpc upsampling, so I guess the dsc bpc is 
what the user actually sees in the end.

=> In the next version, active bpc will be set to dsc bpc in the case 
dsc is active.

I did not look into dithering yet, but it's next on my list.


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-06-15 17:34 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 14:14 [PATCH v3 00/14] New uAPI drm properties for color management Werner Sembach
2021-06-15 14:14 ` [PATCH v3 01/14] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check Werner Sembach
2021-06-15 14:14 ` [PATCH v3 02/14] drm/amd/display: Add missing cases convert_dc_color_depth_into_bpc Werner Sembach
2021-06-15 14:14 ` [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property Werner Sembach
2021-06-15 17:33   ` Werner Sembach
2021-06-15 14:14 ` [PATCH v3 04/14] drm/amd/display: Add handling for new "active bpc" property Werner Sembach
2021-06-15 14:14 ` [PATCH v3 05/14] drm/i915/display: " Werner Sembach
2021-06-15 14:14 ` [PATCH v3 06/14] drm/uAPI: Add "active color format" drm property as feedback for userspace Werner Sembach
2021-06-15 14:14 ` [PATCH v3 07/14] drm/amd/display: Add handling for new "active color format" property Werner Sembach
2021-06-15 14:14 ` [PATCH v3 08/14] drm/i915/display: " Werner Sembach
2021-06-15 14:14 ` [PATCH v3 09/14] drm/uAPI: Add "active color range" drm property as feedback for userspace Werner Sembach
2021-06-15 14:14 ` [PATCH v3 10/14] drm/amd/display: Add handling for new "active color range" property Werner Sembach
2021-06-15 14:14 ` [PATCH v3 11/14] drm/i915/display: " Werner Sembach
2021-06-15 14:14 ` [PATCH v3 12/14] drm/uAPI: Add "preferred color format" drm property as setting for userspace Werner Sembach
2021-06-15 14:14 ` [PATCH v3 13/14] drm/amd/display: Add handling for new "preferred color format" property Werner Sembach
2021-06-15 14:14 ` [PATCH v3 14/14] drm/i915/display: " Werner Sembach

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).