[PATCH v3 00/14] New uAPI drm properties for color management

List overview All Threads
Download

newer

older

[PATCH v6] dmabuf: Add the...

[PATCH 0/3] Update firmware to...

Werner Sembach

15 Jun 2021 15 Jun '21

2:14 p.m.

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.

Show replies by date

Werner Sembach

15 Jun 15 Jun

2:14 p.m.

New subject: [PATCH v3 01/14] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 02/14] drm/amd/display: Add missing cases convert_dc_color_depth_into_bpc

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property

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

Werner Sembach

5:33 p.m.

New subject: [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property

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.

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 04/14] drm/amd/display: Add handling for new "active bpc" property

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 05/14] drm/i915/display: Add handling for new "active bpc" property

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 06/14] drm/uAPI: Add "active color format" drm property as feedback for userspace

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 07/14] drm/amd/display: Add handling for new "active color format" property

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 08/14] drm/i915/display: Add handling for new "active color format" property

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 09/14] drm/uAPI: Add "active color range" drm property as feedback for userspace

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 10/14] drm/amd/display: Add handling for new "active color range" property

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 11/14] drm/i915/display: Add handling for new "active color range" property

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 12/14] drm/uAPI: Add "preferred color format" drm property as setting for userspace

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 13/14] drm/amd/display: Add handling for new "preferred color format" property

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

Werner Sembach

2:14 p.m.

New subject: [PATCH v3 14/14] drm/i915/display: Add handling for new "preferred color format" property

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

1416

Age (days ago)

1416

Last active (days ago)

dri-devel@lists.freedesktop.org

15 comments

1 participants

tags (0)

participants (1)

Werner Sembach