[PATCH 0/2] drm/amd/display: Add HDR output metadata support for amdgpu

[PATCH 0/2] drm/amd/display: Add HDR output metadata support for amdgpu

[Nicholas Kazlauskas]

This patch series enables HDR output metadata support in amdgpu using the
DRM HDR interface merged in drm-misc-next. Enabled for DCE and DCN ASICs
over DP and HDMI.

It's limited to static HDR metadata support for now since that's all the
DRM interface supports. It requires a modeset for entering and exiting HDR
but the metadata can be changed without one.

Cc: Harry Wentland <harry.wentland@amd.com>

Nicholas Kazlauskas (2):
  drm/amd/display: Expose HDR output metadata for supported connectors
  drm/amd/display: Only force modesets when toggling HDR

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 155 +++++++++++++++++-
 1 file changed, 151 insertions(+), 4 deletions(-)

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/amd/display: Expose HDR output metadata for supported connectors

[Nicholas Kazlauskas]

[Why]
For userspace to send static HDR metadata to the display we need to
attach the property on the connector and send it to DC.

[How]
The property is attached to HDMI and DP connectors. Since the metadata
isn't actually available when creating the connector this isn't a
property we can dynamically support based on the extension block
being available or not.

When the HDR metadata is changed a modeset will be forced for now.
We need to switch from 8bpc to 10bpc in most cases anyway, and we want
to fully exit HDR mode when userspace gives us a NULL metadata, so this
isn't completely unnecessary.

The requirement can later be reduced to just entering and exiting HDR
or switching max bpc.

Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 125 ++++++++++++++++++
 1 file changed, 125 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 995f9df66142..eb31acca7ed6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3871,6 +3871,121 @@ enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector *connec
 	return result;
 }
 
+static int fill_hdr_info_packet(const struct drm_connector_state *state,
+				struct dc_info_packet *out)
+{
+	struct hdmi_drm_infoframe frame;
+	unsigned char buf[30]; /* 26 + 4 */
+	ssize_t len;
+	int ret, i;
+
+	memset(out, 0, sizeof(*out));
+
+	if (!state->hdr_output_metadata)
+		return 0;
+
+	ret = drm_hdmi_infoframe_set_hdr_metadata(&frame, state);
+	if (ret)
+		return ret;
+
+	len = hdmi_drm_infoframe_pack_only(&frame, buf, sizeof(buf));
+	if (len < 0)
+		return (int)len;
+
+	/* Static metadata is a fixed 26 bytes + 4 byte header. */
+	if (len != 30)
+		return -EINVAL;
+
+	/* Prepare the infopacket for DC. */
+	switch (state->connector->connector_type) {
+	case DRM_MODE_CONNECTOR_HDMIA:
+		out->hb0 = 0x87; /* type */
+		out->hb1 = 0x01; /* version */
+		out->hb2 = 0x1A; /* length */
+		out->sb[0] = buf[3]; /* checksum */
+		i = 1;
+		break;
+
+	case DRM_MODE_CONNECTOR_DisplayPort:
+	case DRM_MODE_CONNECTOR_eDP:
+		out->hb0 = 0x00; /* sdp id, zero */
+		out->hb1 = 0x87; /* type */
+		out->hb2 = 0x1D; /* payload len - 1 */
+		out->hb3 = (0x13 << 2); /* sdp version */
+		out->sb[0] = 0x01; /* version */
+		out->sb[1] = 0x1A; /* length */
+		i = 2;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	memcpy(&out->sb[i], &buf[4], 26);
+	out->valid = true;
+
+	print_hex_dump(KERN_DEBUG, "HDR SB:", DUMP_PREFIX_NONE, 16, 1, out->sb,
+		       sizeof(out->sb), false);
+
+	return 0;
+}
+
+static bool
+is_hdr_metadata_different(const struct drm_connector_state *old_state,
+			  const struct drm_connector_state *new_state)
+{
+	struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
+	struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
+
+	if (old_blob != new_blob) {
+		if (old_blob && new_blob &&
+		    old_blob->length == new_blob->length)
+			return memcmp(old_blob->data, new_blob->data,
+				      old_blob->length);
+
+		return true;
+	}
+
+	return false;
+}
+
+static int
+amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
+				 struct drm_connector_state *new_con_state)
+{
+	struct drm_atomic_state *state = new_con_state->state;
+	struct drm_connector_state *old_con_state =
+		drm_atomic_get_old_connector_state(state, conn);
+	struct drm_crtc *crtc = new_con_state->crtc;
+	struct drm_crtc_state *new_crtc_state;
+	int ret;
+
+	if (!crtc)
+		return 0;
+
+	if (is_hdr_metadata_different(old_con_state, new_con_state)) {
+		struct dc_info_packet hdr_infopacket;
+
+		ret = fill_hdr_info_packet(new_con_state, &hdr_infopacket);
+		if (ret)
+			return ret;
+
+		new_crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(new_crtc_state))
+			return PTR_ERR(new_crtc_state);
+
+		/*
+		 * DC considers the stream backends changed if the
+		 * static metadata changes. Forcing the modeset also
+		 * gives a simple way for userspace to switch from
+		 * 8bpc to 10bpc when setting the metadata.
+		 */
+		new_crtc_state->mode_changed = true;
+	}
+
+	return 0;
+}
+
 static const struct drm_connector_helper_funcs
 amdgpu_dm_connector_helper_funcs = {
 	/*
@@ -3881,6 +3996,7 @@ amdgpu_dm_connector_helper_funcs = {
 	 */
 	.get_modes = get_modes,
 	.mode_valid = amdgpu_dm_connector_mode_valid,
+	.atomic_check = amdgpu_dm_connector_atomic_check,
 };
 
 static void dm_crtc_helper_disable(struct drm_crtc *crtc)
@@ -4677,6 +4793,10 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 	if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
 	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
 	    connector_type == DRM_MODE_CONNECTOR_eDP) {
+		drm_object_attach_property(
+			&aconnector->base.base,
+			dm->ddev->mode_config.hdr_output_metadata_property, 0);
+
 		drm_connector_attach_vrr_capable_property(
 			&aconnector->base);
 	}
@@ -6141,6 +6261,11 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
 
 		dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
 
+		ret = fill_hdr_info_packet(drm_new_conn_state,
+					   &new_stream->hdr_static_metadata);
+		if (ret)
+			goto fail;
+
 		if (dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
 		    dc_is_stream_scaling_unchanged(new_stream, dm_old_crtc_state->stream)) {
 			new_crtc_state->mode_changed = false;
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/2] drm/amd/display: Add HDR output metadata support for amdgpu

[Harry Wentland]

On 2019-05-28 3:08 p.m., Nicholas Kazlauskas wrote:
> This patch series enables HDR output metadata support in amdgpu using the
> DRM HDR interface merged in drm-misc-next. Enabled for DCE and DCN ASICs
> over DP and HDMI.
> 
> It's limited to static HDR metadata support for now since that's all the
> DRM interface supports. It requires a modeset for entering and exiting HDR
> but the metadata can be changed without one.
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> 

Series is
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> Nicholas Kazlauskas (2):
>   drm/amd/display: Expose HDR output metadata for supported connectors
>   drm/amd/display: Only force modesets when toggling HDR
> 
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 155 +++++++++++++++++-
>  1 file changed, 151 insertions(+), 4 deletions(-)
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amd/display: Only force modesets when toggling HDR

[Nicholas Kazlauskas]

[Why]
We can issue HDR static metadata as part of stream updates for
non-modesets as long as we force a modeset when entering or exiting HDR.

This avoids unnecessary blanking for simple metadata updates.

[How]
When changing scaling and abm for the stream also check if HDR has
changed and send the stream update. This will only happen in non-modeset
cases.

Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 34 +++++++++++++++----
 1 file changed, 28 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 eb31acca7ed6..443b13ec268d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3978,9 +3978,16 @@ amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
 		 * DC considers the stream backends changed if the
 		 * static metadata changes. Forcing the modeset also
 		 * gives a simple way for userspace to switch from
-		 * 8bpc to 10bpc when setting the metadata.
+		 * 8bpc to 10bpc when setting the metadata to enter
+		 * or exit HDR.
+		 *
+		 * Changing the static metadata after it's been
+		 * set is permissible, however. So only force a
+		 * modeset if we're entering or exiting HDR.
 		 */
-		new_crtc_state->mode_changed = true;
+		new_crtc_state->mode_changed =
+			!old_con_state->hdr_output_metadata ||
+			!new_con_state->hdr_output_metadata;
 	}
 
 	return 0;
@@ -5881,7 +5888,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 		struct amdgpu_crtc *acrtc = to_amdgpu_crtc(dm_new_con_state->base.crtc);
 		struct dc_surface_update dummy_updates[MAX_SURFACES];
 		struct dc_stream_update stream_update;
+		struct dc_info_packet hdr_packet;
 		struct dc_stream_status *status = NULL;
+		bool abm_changed, hdr_changed, scaling_changed;
 
 		memset(&dummy_updates, 0, sizeof(dummy_updates));
 		memset(&stream_update, 0, sizeof(stream_update));
@@ -5898,11 +5907,19 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 		dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
 		dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
 
-		if (!is_scaling_state_different(dm_new_con_state, dm_old_con_state) &&
-				(dm_new_crtc_state->abm_level == dm_old_crtc_state->abm_level))
+		scaling_changed = is_scaling_state_different(dm_new_con_state,
+							     dm_old_con_state);
+
+		abm_changed = dm_new_crtc_state->abm_level !=
+			      dm_old_crtc_state->abm_level;
+
+		hdr_changed =
+			is_hdr_metadata_different(old_con_state, new_con_state);
+
+		if (!scaling_changed && !abm_changed && !hdr_changed)
 			continue;
 
-		if (is_scaling_state_different(dm_new_con_state, dm_old_con_state)) {
+		if (scaling_changed) {
 			update_stream_scaling_settings(&dm_new_con_state->base.crtc->mode,
 					dm_new_con_state, (struct dc_stream_state *)dm_new_crtc_state->stream);
 
@@ -5910,12 +5927,17 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 			stream_update.dst = dm_new_crtc_state->stream->dst;
 		}
 
-		if (dm_new_crtc_state->abm_level != dm_old_crtc_state->abm_level) {
+		if (abm_changed) {
 			dm_new_crtc_state->stream->abm_level = dm_new_crtc_state->abm_level;
 
 			stream_update.abm_level = &dm_new_crtc_state->abm_level;
 		}
 
+		if (hdr_changed) {
+			fill_hdr_info_packet(new_con_state, &hdr_packet);
+			stream_update.hdr_static_metadata = &hdr_packet;
+		}
+
 		status = dc_stream_get_status(dm_new_crtc_state->stream);
 		WARN_ON(!status);
 		WARN_ON(!status->plane_count);
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel