[v8 00/10] Add HDR Metadata Parsing and handling in DRM layer

[v8 00/10] Add HDR Metadata Parsing and handling in DRM layer

[Uma Shankar]

This patch series enables HDR support in drm. It basically defines
HDR metadata structures, property to pass content (after blending)
metadata from user space compositors to driver.

Dynamic Range and Mastering infoframe creation and sending.

ToDo:
1. We need to get the color framework in place for all planes
   which support HDR content in hardware. This is already in progres
   and patches are out for review in mailing list.
2. UserSpace/Compositors: Blending policies and metadata blob
   creation and passing to driver. Work is already in progress
   by Intel's middleware teams on wayland and the patches for
   the same are in review.

Please review and share your feedbacks/suggestions.

Note: The intention for these patches is to get a design feedback on
the uapi changes, generic property design and infoframe handling.
This cannot get merged as of now without the userspace support in place.

A POC has already been developed by Ville based on wayland. Please refer
below link to see the component interactions and usage:
https://lists.freedesktop.org/archives/wayland-devel/2017-December/036403.html

v2: Updated Ville's POC changes to the patch series.Incorporated cleanups
and fixes from Ville. Rebase on latest drm-tip.

v3: Fixed a warning causing builds to break on CI. No major change.

v4: Addressed Shashank's review comments.

v5: Rebase on top of Ville's infoframe refactoring changes. Fixed non modeset
case for HDR metadata update. Dropped a redundant patch.

v6: Addressed Shashank's review comments and added RB's received.

v7: Squashed 2 patches, dropped 1 change and addressed Brian Starkey's and
Shashank's review comments.

v8: Addressed Jonas Karlman review comments. Added Shashank's RB to the series,
fixed a WARN_ON on BYT/CHT.

Note: Media driver and VAAPI changes for HDR are already out, with compositors
changes also expected to land soon. Weston changes already floated and reviews
started in community and is in active development along with GL efforts.

Uma Shankar (8):
  drm: Add HDR source metadata property
  drm: Parse HDR metadata info from EDID
  drm: Enable HDR infoframe support
  drm/i915: Attach HDR metadata property to connector
  drm/i915: Write HDR infoframe and send to panel
  drm/i915:Enabled Modeset when HDR Infoframe changes
  drm/i915: Set Infoframe for non modeset case for HDR
  drm/i915: Added DRM Infoframe handling for BYT/CHT

Ville Syrjälä (2):
  drm/i915: Add HLG EOTF
  drm/i915: Enable infoframes on GLK+ for HDR

 drivers/gpu/drm/drm_atomic.c        |   2 +
 drivers/gpu/drm/drm_atomic_uapi.c   |  13 +++
 drivers/gpu/drm/drm_connector.c     |   6 ++
 drivers/gpu/drm/drm_edid.c          | 101 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h     |   4 +
 drivers/gpu/drm/i915/intel_atomic.c |  14 ++-
 drivers/gpu/drm/i915/intel_ddi.c    |  13 +++
 drivers/gpu/drm/i915/intel_drv.h    |   1 +
 drivers/gpu/drm/i915/intel_hdmi.c   | 105 ++++++++++++++++++--
 drivers/video/hdmi.c                | 186 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h         |  11 +++
 include/drm/drm_edid.h              |   5 +
 include/drm/drm_mode_config.h       |   6 ++
 include/linux/hdmi.h                |  38 ++++++++
 include/uapi/drm/drm_mode.h         |  39 ++++++++
 15 files changed, 537 insertions(+), 7 deletions(-)

-- 
1.9.1

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

[v8 01/10] drm: Add HDR source metadata property

[Uma Shankar]

This patch adds a blob property to get HDR metadata
information from userspace. This will be send as part
of AVI Infoframe to panel.

It also implements get() and set() functions for HDR output
metadata property.The blob data is received from userspace and
saved in connector state, the same is returned as blob in get
property call to userspace.

v2: Rebase and modified the metadata structure elements
as per Ville's POC changes.

v3: No Change

v4: Addressed Shashank's review comments

v5: Rebase.

v6: Addressed Brian Starkey's review comments, defined
new structure with header for dynamic metadata scalability.
Merge get/set property functions for metadata in this patch.

v7: Addressed Jonas Karlman review comments and defined separate
structure for infoframe to better align with CTA 861.G spec. Added
Shashank's RB.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_atomic.c      |  2 ++
 drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
 drivers/gpu/drm/drm_connector.c   |  6 ++++++
 include/drm/drm_connector.h       | 11 +++++++++++
 include/drm/drm_mode_config.h     |  6 ++++++
 include/linux/hdmi.h              | 10 ++++++++++
 include/uapi/drm/drm_mode.h       | 39 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 87 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5eb4013..8b9c126 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -881,6 +881,8 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
 
 	drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name);
 	drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)");
+	drm_printf(p, "\thdr_metadata_changed=%d\n",
+		   state->hdr_metadata_changed);
 
 	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
 		if (state->writeback_job && state->writeback_job->fb)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index ea797d4..6d0d161 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -673,6 +673,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_mode_config *config = &dev->mode_config;
+	bool replaced = false;
+	int ret;
 
 	if (property == config->prop_crtc_id) {
 		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val);
@@ -721,6 +723,14 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 		 */
 		if (state->link_status != DRM_LINK_STATUS_GOOD)
 			state->link_status = val;
+	} else if (property == config->hdr_output_metadata_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+				&state->hdr_output_metadata_blob_ptr,
+				val,
+				-1, sizeof(struct hdr_output_metadata),
+				&replaced);
+		state->hdr_metadata_changed |= replaced;
+		return ret;
 	} else if (property == config->aspect_ratio_property) {
 		state->picture_aspect_ratio = val;
 	} else if (property == config->content_type_property) {
@@ -807,6 +817,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 		*val = state->colorspace;
 	} else if (property == connector->scaling_mode_property) {
 		*val = state->scaling_mode;
+	} else if (property == config->hdr_output_metadata_property) {
+		*val = (state->hdr_output_metadata_blob_ptr) ?
+			state->hdr_output_metadata_blob_ptr->base.id : 0;
 	} else if (property == connector->content_protection_property) {
 		*val = state->content_protection;
 	} else if (property == config->writeback_fb_id_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2355124..0bdae90 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.non_desktop_property = prop;
 
+	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
+				   "HDR_OUTPUT_METADATA", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.hdr_output_metadata_property = prop;
+
 	return 0;
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 02a1312..5343f60 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -599,6 +599,13 @@ struct drm_connector_state {
 	 * and the connector bpc limitations obtained from edid.
 	 */
 	u8 max_bpc;
+
+	/**
+	 * @metadata_blob_ptr:
+	 * DRM blob property for HDR output metadata
+	 */
+	struct drm_property_blob *hdr_output_metadata_blob_ptr;
+	u8 hdr_metadata_changed : 1;
 };
 
 /**
@@ -1239,6 +1246,10 @@ struct drm_connector {
 	 * &drm_mode_config.connector_free_work.
 	 */
 	struct llist_node free_node;
+
+	/* HDR metdata */
+	struct hdr_output_metadata hdr_output_metadata;
+	struct hdr_sink_metadata hdr_sink_metadata;
 };
 
 #define obj_to_connector(x) container_of(x, struct drm_connector, base)
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 7f60e8e..ef2656b 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -836,6 +836,12 @@ struct drm_mode_config {
 	 */
 	struct drm_property *writeback_out_fence_ptr_property;
 
+	/*
+	 * hdr_metadata_property: Connector property containing hdr metatda
+	 * This will be provided by userspace compositors based on HDR content
+	 */
+	struct drm_property *hdr_output_metadata_property;
+
 	/* dumb ioctl parameters */
 	uint32_t preferred_depth, prefer_shadow;
 
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 927ad64..a065194 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -152,6 +152,16 @@ enum hdmi_content_type {
 	HDMI_CONTENT_TYPE_GAME,
 };
 
+enum hdmi_metadata_type {
+	HDMI_STATIC_METADATA_TYPE1 = 1,
+};
+
+enum hdmi_eotf {
+	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
+	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
+	HDMI_EOTF_SMPTE_ST2084,
+};
+
 struct hdmi_avi_infoframe {
 	enum hdmi_infoframe_type type;
 	unsigned char version;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 83cd163..7b6a519 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -630,6 +630,45 @@ struct drm_color_lut {
 	__u16 reserved;
 };
 
+/* HDR Metadata Infoframe as per 861.G spec */
+struct hdr_metadata_infoframe {
+	__u8 eotf;
+	__u8 metadata_type;
+	struct {
+		__u16 x, y;
+		} display_primaries[3];
+	struct {
+		__u16 x, y;
+		} white_point;
+	__u16 max_display_mastering_luminance;
+	__u16 min_display_mastering_luminance;
+	__u16 max_cll;
+	__u16 max_fall;
+};
+
+struct hdr_output_metadata {
+	__u32 metadata_type;
+	union {
+		struct hdr_metadata_infoframe hdmi_metadata_type1;
+	};
+};
+
+/* HDR Metadata as per 861.G spec */
+struct hdr_static_metadata {
+	__u8 eotf;
+	__u8 metadata_type;
+	__u16 max_cll;
+	__u16 max_fall;
+	__u16 min_cll;
+};
+
+struct hdr_sink_metadata {
+	__u32 metadata_type;
+	union {
+		struct hdr_static_metadata hdmi_type1;
+	};
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-- 
1.9.1

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

[v8 02/10] drm: Parse HDR metadata info from EDID

[Uma Shankar]

HDR metadata block is introduced in CEA-861.3 spec.
Parsing the same to get the panel's HDR metadata.

v2: Rebase and added Ville's POC changes to the patch.

v3: No Change

v4: Addressed Shashank's review comments

v5: Addressed Shashank's comment and added his RB.

v6: Addressed Jonas Karlman review comments.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 2c22ea4..1fc371b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2830,6 +2830,7 @@ static int drm_cvt_modes(struct drm_connector *connector,
 #define VIDEO_BLOCK     0x02
 #define VENDOR_BLOCK    0x03
 #define SPEAKER_BLOCK	0x04
+#define HDR_STATIC_METADATA_BLOCK	0x6
 #define USE_EXTENDED_TAG 0x07
 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
 #define EXT_VIDEO_DATA_BLOCK_420	0x0E
@@ -3577,6 +3578,12 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
 }
 
 static int
+cea_db_payload_len_ext(const u8 *db)
+{
+	return (db[0] & 0x1f) - 1;
+}
+
+static int
 cea_db_extended_tag(const u8 *db)
 {
 	return db[1];
@@ -3812,6 +3819,49 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
 	mode->clock = clock;
 }
 
+static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
+{
+	if (cea_db_tag(db) != USE_EXTENDED_TAG)
+		return false;
+
+	if (db[1] != HDR_STATIC_METADATA_BLOCK)
+		return false;
+
+	return true;
+}
+
+static uint8_t eotf_supported(const u8 *edid_ext)
+{
+	return edid_ext[2] &
+		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
+		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
+		 BIT(HDMI_EOTF_SMPTE_ST2084));
+}
+
+static uint8_t hdr_metadata_type(const u8 *edid_ext)
+{
+	return edid_ext[3] &
+		BIT(HDMI_STATIC_METADATA_TYPE1);
+}
+
+static void
+drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db)
+{
+	u16 len;
+
+	len = cea_db_payload_len_ext(db);
+	connector->hdr_sink_metadata.hdmi_type1.eotf = eotf_supported(db);
+	connector->hdr_sink_metadata.hdmi_type1.metadata_type =
+					hdr_metadata_type(db);
+
+	if (len >= 4)
+		connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4];
+	if (len >= 5)
+		connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5];
+	if (len >= 6)
+		connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6];
+}
+
 static void
 drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
 {
@@ -4439,6 +4489,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 			drm_parse_y420cmdb_bitmap(connector, db);
 		if (cea_db_is_vcdb(db))
 			drm_parse_vcdb(connector, db);
+		if (cea_db_is_hdmi_hdr_metadata_block(db))
+			drm_parse_hdr_metadata_block(connector, db);
 	}
 }
 
-- 
1.9.1

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

[v8 03/10] drm: Enable HDR infoframe support

[Uma Shankar]

Enable Dynamic Range and Mastering Infoframe for HDR
content, which is defined in CEA 861.3 spec.

The metadata will be computed based on blending
policy in userspace compositors and passed as a connector
property blob to driver. The same will be sent as infoframe
to panel which support HDR.

Added the const version of infoframe for DRM metadata
for HDR.

v2: Rebase and added Ville's POC changes.

v3: No Change

v4: Addressed Shashank's review comments and merged the
patch making drm infoframe function arguments as constant.

v5: Rebase

v6: Fixed checkpatch warnings with --strict option. Addressed
Shashank's review comments and added his RB.

v7: Addressed Brian Starkey's review comments. Merged 2 patches
into one.

v8: Addressed Jonas Karlman review comments.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c |  48 ++++++++++++
 drivers/video/hdmi.c       | 186 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     |   5 ++
 include/linux/hdmi.h       |  27 +++++++
 4 files changed, 266 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1fc371b..9b71a64 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4884,6 +4884,54 @@ static bool is_hdmi2_sink(struct drm_connector *connector)
 }
 
 /**
+ * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI AVI infoframe with
+ *                                         HDR metadata from userspace
+ * @frame: HDMI AVI infoframe
+ * @hdr_source_metadata: hdr_source_metadata info from userspace
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
+				    struct hdr_output_metadata *hdr_metadata)
+{
+	int err;
+
+	if (!frame || !hdr_metadata)
+		return true;
+
+	err = hdmi_drm_infoframe_init(frame);
+	if (err < 0)
+		return err;
+
+	DRM_DEBUG_KMS("type = %x\n", frame->type);
+
+	frame->length = sizeof(struct hdr_metadata_infoframe);
+
+	frame->eotf = hdr_metadata->hdmi_metadata_type1.eotf;
+	frame->metadata_type = hdr_metadata->hdmi_metadata_type1.metadata_type;
+
+	memcpy(&frame->display_primaries,
+	       &hdr_metadata->hdmi_metadata_type1.display_primaries, 12);
+
+	memcpy(&frame->white_point,
+	       &hdr_metadata->hdmi_metadata_type1.white_point, 4);
+
+	frame->max_display_mastering_luminance =
+		hdr_metadata->hdmi_metadata_type1.max_display_mastering_luminance;
+	frame->min_display_mastering_luminance =
+		hdr_metadata->hdmi_metadata_type1.min_display_mastering_luminance;
+	frame->max_fall = hdr_metadata->hdmi_metadata_type1.max_fall;
+	frame->max_cll = hdr_metadata->hdmi_metadata_type1.max_cll;
+
+	hdmi_infoframe_log(KERN_CRIT, NULL,
+			   (union hdmi_infoframe *)frame);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
+
+/**
  * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
  *                                              data from a DRM display mode
  * @frame: HDMI AVI infoframe
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 799ae49..717ed7fb 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -650,6 +650,146 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
 	return 0;
 }
 
+/**
+ * hdmi_drm_infoframe_init() - initialize an HDMI Dynaminc Range and
+ * mastering infoframe
+ * @frame: HDMI DRM infoframe
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame)
+{
+	memset(frame, 0, sizeof(*frame));
+
+	frame->type = HDMI_INFOFRAME_TYPE_DRM;
+	frame->version = 1;
+
+	return 0;
+}
+EXPORT_SYMBOL(hdmi_drm_infoframe_init);
+
+static int hdmi_drm_infoframe_check_only(const struct hdmi_drm_infoframe *frame)
+{
+	if (frame->type != HDMI_INFOFRAME_TYPE_DRM ||
+	    frame->version != 1)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * hdmi_drm_infoframe_check() - check a HDMI DRM infoframe
+ * @frame: HDMI DRM infoframe
+ *
+ * Validates that the infoframe is consistent.
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_drm_infoframe_check(struct hdmi_drm_infoframe *frame)
+{
+	return hdmi_drm_infoframe_check_only(frame);
+}
+EXPORT_SYMBOL(hdmi_drm_infoframe_check);
+
+/**
+ * hdmi_drm_infoframe_pack() - write HDMI DRM infoframe to binary buffer
+ * @frame: HDMI DRM infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Packs the information contained in the @frame structure into a binary
+ * representation that can be written into the corresponding controller
+ * registers. Also computes the checksum as required by section 5.3.5 of
+ * the HDMI 1.4 specification.
+ *
+ * Returns the number of bytes packed into the binary buffer or a negative
+ * error code on failure.
+ */
+ssize_t hdmi_drm_infoframe_pack_only(const struct hdmi_drm_infoframe *frame,
+				     void *buffer, size_t size)
+{
+	u8 *ptr = buffer;
+	size_t length;
+	int i;
+
+	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
+
+	if (size < length)
+		return -ENOSPC;
+
+	memset(buffer, 0, size);
+
+	ptr[0] = frame->type;
+	ptr[1] = frame->version;
+	ptr[2] = frame->length;
+	ptr[3] = 0; /* checksum */
+
+	/* start infoframe payload */
+	ptr += HDMI_INFOFRAME_HEADER_SIZE;
+
+	*ptr++ = frame->eotf;
+	*ptr++ = frame->metadata_type;
+
+	for (i = 0; i < 3; i++) {
+		*ptr++ = frame->display_primaries[i].x;
+		*ptr++ = frame->display_primaries[i].x >> 8;
+		*ptr++ = frame->display_primaries[i].y;
+		*ptr++ = frame->display_primaries[i].y >> 8;
+	}
+
+	*ptr++ = frame->white_point.x;
+	*ptr++ = frame->white_point.x >> 8;
+
+	*ptr++ = frame->white_point.y;
+	*ptr++ = frame->white_point.y >> 8;
+
+	*ptr++ = frame->max_display_mastering_luminance;
+	*ptr++ = frame->max_display_mastering_luminance >> 8;
+
+	*ptr++ = frame->min_display_mastering_luminance;
+	*ptr++ = frame->min_display_mastering_luminance >> 8;
+
+	*ptr++ = frame->max_cll;
+	*ptr++ = frame->max_cll >> 8;
+
+	*ptr++ = frame->max_fall;
+	*ptr++ = frame->max_fall >> 8;
+
+	hdmi_infoframe_set_checksum(buffer, length);
+
+	return length;
+}
+EXPORT_SYMBOL(hdmi_drm_infoframe_pack_only);
+
+/**
+ * hdmi_drm_infoframe_pack() - check a HDMI DRM infoframe,
+ *                             and write it to binary buffer
+ * @frame: HDMI DRM infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Validates that the infoframe is consistent and updates derived fields
+ * (eg. length) based on other fields, after which it packs the information
+ * contained in the @frame structure into a binary representation that
+ * can be written into the corresponding controller registers. This function
+ * also computes the checksum as required by section 5.3.5 of the HDMI 1.4
+ * specification.
+ *
+ * Returns the number of bytes packed into the binary buffer or a negative
+ * error code on failure.
+ */
+ssize_t hdmi_drm_infoframe_pack(struct hdmi_drm_infoframe *frame,
+				void *buffer, size_t size)
+{
+	int ret;
+
+	ret = hdmi_drm_infoframe_check(frame);
+	if (ret)
+		return ret;
+
+	return hdmi_drm_infoframe_pack_only(frame, buffer, size);
+}
+EXPORT_SYMBOL(hdmi_drm_infoframe_pack);
+
 /*
  * hdmi_vendor_any_infoframe_check() - check a vendor infoframe
  */
@@ -758,6 +898,10 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
 		length = hdmi_avi_infoframe_pack_only(&frame->avi,
 						      buffer, size);
 		break;
+	case HDMI_INFOFRAME_TYPE_DRM:
+		length = hdmi_drm_infoframe_pack_only(&frame->drm,
+						      buffer, size);
+		break;
 	case HDMI_INFOFRAME_TYPE_SPD:
 		length = hdmi_spd_infoframe_pack_only(&frame->spd,
 						      buffer, size);
@@ -806,6 +950,9 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
 	case HDMI_INFOFRAME_TYPE_AVI:
 		length = hdmi_avi_infoframe_pack(&frame->avi, buffer, size);
 		break;
+	case HDMI_INFOFRAME_TYPE_DRM:
+		length = hdmi_drm_infoframe_pack(&frame->drm, buffer, size);
+		break;
 	case HDMI_INFOFRAME_TYPE_SPD:
 		length = hdmi_spd_infoframe_pack(&frame->spd, buffer, size);
 		break;
@@ -838,6 +985,8 @@ static const char *hdmi_infoframe_type_get_name(enum hdmi_infoframe_type type)
 		return "Source Product Description (SPD)";
 	case HDMI_INFOFRAME_TYPE_AUDIO:
 		return "Audio";
+	case HDMI_INFOFRAME_TYPE_DRM:
+		return "Dynamic Range and Mastering";
 	}
 	return "Reserved";
 }
@@ -1284,6 +1433,40 @@ static void hdmi_audio_infoframe_log(const char *level,
 			frame->downmix_inhibit ? "Yes" : "No");
 }
 
+/**
+ * hdmi_drm_infoframe_log() - log info of HDMI DRM infoframe
+ * @level: logging level
+ * @dev: device
+ * @frame: HDMI DRM infoframe
+ */
+static void hdmi_drm_infoframe_log(const char *level,
+				   struct device *dev,
+				   const struct hdmi_drm_infoframe *frame)
+{
+	int i;
+
+	hdmi_infoframe_log_header(level, dev,
+				  (struct hdmi_any_infoframe *)frame);
+	hdmi_log("length: %d\n", frame->length);
+	hdmi_log("metadata type: %d\n", frame->metadata_type);
+	hdmi_log("eotf: %d\n", frame->eotf);
+	for (i = 0; i < 3; i++) {
+		hdmi_log("x[%d]: %d\n", i, frame->display_primaries[i].x);
+		hdmi_log("y[%d]: %d\n", i, frame->display_primaries[i].y);
+	}
+
+	hdmi_log("white point x: %d\n", frame->white_point.x);
+	hdmi_log("white point y: %d\n", frame->white_point.y);
+
+	hdmi_log("max_display_mastering_luminance: %d\n",
+		 frame->max_display_mastering_luminance);
+	hdmi_log("min_display_mastering_luminance: %d\n",
+		 frame->min_display_mastering_luminance);
+
+	hdmi_log("max_cll: %d\n", frame->max_cll);
+	hdmi_log("max_fall: %d\n", frame->max_fall);
+}
+
 static const char *
 hdmi_3d_structure_get_name(enum hdmi_3d_structure s3d_struct)
 {
@@ -1372,6 +1555,9 @@ void hdmi_infoframe_log(const char *level,
 	case HDMI_INFOFRAME_TYPE_VENDOR:
 		hdmi_vendor_any_infoframe_log(level, dev, &frame->vendor);
 		break;
+	case HDMI_INFOFRAME_TYPE_DRM:
+		hdmi_drm_infoframe_log(level, dev, &frame->drm);
+		break;
 	}
 }
 EXPORT_SYMBOL(hdmi_infoframe_log);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 9d3b5b9..d81c5c2 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -25,6 +25,7 @@
 
 #include <linux/types.h>
 #include <linux/hdmi.h>
+#include <drm/drm_mode.h>
 
 struct drm_device;
 struct i2c_adapter;
@@ -370,6 +371,10 @@ int drm_av_sync_delay(struct drm_connector *connector,
 				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range);
 
+int
+drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
+				    struct hdr_output_metadata *hdr_metadata);
+
 /**
  * drm_eld_mnl - Get ELD monitor name length in bytes.
  * @eld: pointer to an eld memory structure with mnl set
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index a065194..3927d88 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -47,6 +47,7 @@ enum hdmi_infoframe_type {
 	HDMI_INFOFRAME_TYPE_AVI = 0x82,
 	HDMI_INFOFRAME_TYPE_SPD = 0x83,
 	HDMI_INFOFRAME_TYPE_AUDIO = 0x84,
+	HDMI_INFOFRAME_TYPE_DRM = 0x87,
 };
 
 #define HDMI_IEEE_OUI 0x000c03
@@ -185,12 +186,37 @@ struct hdmi_avi_infoframe {
 	unsigned short right_bar;
 };
 
+/* DRM Infoframe as per CTA 861.G spec */
+struct hdmi_drm_infoframe {
+	enum hdmi_infoframe_type type;
+	unsigned char version;
+	unsigned char length;
+	enum hdmi_eotf eotf;
+	enum hdmi_metadata_type metadata_type;
+	struct {
+		u16 x, y;
+	} display_primaries[3];
+	struct {
+		u16 x, y;
+	} white_point;
+	u16 max_display_mastering_luminance;
+	u16 min_display_mastering_luminance;
+	u16 max_cll;
+	u16 max_fall;
+};
+
 int hdmi_avi_infoframe_init(struct hdmi_avi_infoframe *frame);
 ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer,
 				size_t size);
 ssize_t hdmi_avi_infoframe_pack_only(const struct hdmi_avi_infoframe *frame,
 				     void *buffer, size_t size);
 int hdmi_avi_infoframe_check(struct hdmi_avi_infoframe *frame);
+int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame);
+ssize_t hdmi_drm_infoframe_pack(struct hdmi_drm_infoframe *frame, void *buffer,
+				size_t size);
+ssize_t hdmi_drm_infoframe_pack_only(const struct hdmi_drm_infoframe *frame,
+				     void *buffer, size_t size);
+int hdmi_drm_infoframe_check(struct hdmi_drm_infoframe *frame);
 
 enum hdmi_spd_sdi {
 	HDMI_SPD_SDI_UNKNOWN,
@@ -365,6 +391,7 @@ ssize_t hdmi_vendor_infoframe_pack_only(const struct hdmi_vendor_infoframe *fram
 	struct hdmi_spd_infoframe spd;
 	union hdmi_vendor_any_infoframe vendor;
 	struct hdmi_audio_infoframe audio;
+	struct hdmi_drm_infoframe drm;
 };
 
 ssize_t hdmi_infoframe_pack(union hdmi_infoframe *frame, void *buffer,
-- 
1.9.1

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

[v8 04/10] drm/i915: Attach HDR metadata property to connector

[Uma Shankar]

Attach HDR metadata property to connector object.

v2: Rebase

v3: Updated the property name as per updated name
while creating hdr metadata property

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e1005d7..d9851da 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2730,6 +2730,8 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
 
 	drm_connector_attach_content_type_property(connector);
 	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+	drm_object_attach_property(&connector->base,
+				   connector->dev->mode_config.hdr_output_metadata_property, 0);
 
 	if (!HAS_GMCH(dev_priv))
 		drm_connector_attach_max_bpc_property(connector, 8, 12);
-- 
1.9.1

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

[v8 05/10] drm/i915: Write HDR infoframe and send to panel

[Uma Shankar]

Enable writing of HDR metadata infoframe to panel.
The data will be provid by usersapace compositors, based
on blending policies and passsed to driver through a blob
property.

v2: Rebase

v3: Fixed a warning message

v4: Addressed Shashank's review comments

v5: Rebase. Added infoframe calculation in compute config.

v6: Addressed Shashank's review comment. Added HDR metadata
support from GEN10 onwards as per Shashank's recommendation.

v7: Addressed Shashank's review comments

v8: Added Shashank's RB.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 48 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a38b9cf..6a9d132 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1051,6 +1051,7 @@ struct intel_crtc_state {
 		union hdmi_infoframe avi;
 		union hdmi_infoframe spd;
 		union hdmi_infoframe hdmi;
+		union hdmi_infoframe drm;
 	} infoframes;
 
 	/* HDMI scrambling status */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index d9851da..40d55b0 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -566,6 +566,7 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
 	HDMI_INFOFRAME_TYPE_AVI,
 	HDMI_INFOFRAME_TYPE_SPD,
 	HDMI_INFOFRAME_TYPE_VENDOR,
+	HDMI_INFOFRAME_TYPE_DRM,
 };
 
 u32 intel_hdmi_infoframe_enable(unsigned int type)
@@ -788,6 +789,30 @@ void intel_read_infoframe(struct intel_encoder *encoder,
 	return true;
 }
 
+static bool
+intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
+				 struct intel_crtc_state *crtc_state,
+				 struct drm_connector_state *conn_state)
+{
+	struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm;
+	struct hdr_output_metadata *hdr_metadata;
+	int ret;
+
+	hdr_metadata = (struct hdr_output_metadata *)
+			conn_state->hdr_output_metadata_blob_ptr->data;
+
+	ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
+	if (ret < 0) {
+		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
+		return false;
+	}
+
+	crtc_state->infoframes.enable |=
+		intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);
+
+	return true;
+}
+
 static void g4x_set_infoframes(struct intel_encoder *encoder,
 			       bool enable,
 			       const struct intel_crtc_state *crtc_state,
@@ -1186,6 +1211,16 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_VENDOR,
 			      &crtc_state->infoframes.hdmi);
+
+	/*
+	 * Support HDR Metadata from Gen10 onwards
+	 * ToDo: Gen9 also can support HDR with LSPCON.
+	 * Support for the same to be enabled later.
+	 */
+	if (INTEL_GEN(dev_priv) >= 10)
+		intel_write_infoframe(encoder, crtc_state,
+				      HDMI_INFOFRAME_TYPE_DRM,
+				      &crtc_state->infoframes.drm);
 }
 
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable)
@@ -2392,6 +2427,19 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 		return -EINVAL;
 	}
 
+	/*
+	 * Support HDR Metadata from Gen10 onwards
+	 * ToDo: Gen9 also can support HDR with LSPCON.
+	 * Support for the same to be enabled later.
+	 */
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+		if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config,
+						      conn_state)) {
+			DRM_DEBUG_KMS("bad DRM infoframe\n");
+			return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 
-- 
1.9.1

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

[v8 06/10] drm/i915: Add HLG EOTF

[Uma Shankar]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

ADD HLG EOTF to the list of EOTF transfer functions supported.
Hybrid Log-Gamma (HLG) is a high dynamic range (HDR) standard.
HLG defines a nonlinear transfer function in which the lower
half of the signal values use a gamma curve and the upper half
of the signal values use a logarithmic curve.

v2: Rebase

v3: Fixed a warning message

v4: Addressed Shashank's review comments

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 3 ++-
 include/linux/hdmi.h       | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9b71a64..f88c0c7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3835,7 +3835,8 @@ static uint8_t eotf_supported(const u8 *edid_ext)
 	return edid_ext[2] &
 		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
 		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
-		 BIT(HDMI_EOTF_SMPTE_ST2084));
+		 BIT(HDMI_EOTF_SMPTE_ST2084) |
+		 BIT(HDMI_EOTF_BT_2100_HLG));
 }
 
 static uint8_t hdr_metadata_type(const u8 *edid_ext)
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 3927d88..daf7a37 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -161,6 +161,7 @@ enum hdmi_eotf {
 	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
 	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
 	HDMI_EOTF_SMPTE_ST2084,
+	HDMI_EOTF_BT_2100_HLG,
 };
 
 struct hdmi_avi_infoframe {
-- 
1.9.1

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

[v8 07/10] drm/i915: Enable infoframes on GLK+ for HDR

[Uma Shankar]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This patch enables infoframes on GLK+ to be
used to send HDR metadata to HDMI sink.

v2: Addressed Shashank's review comment.

v3: Addressed Shashank's review comment.

v4: Added Shashank's RB.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |  4 ++++
 drivers/gpu/drm/i915/intel_hdmi.c | 22 +++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c206e8..6cd250d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4692,6 +4692,7 @@ enum {
 #define   VIDEO_DIP_FREQ_MASK		(3 << 16)
 /* HSW and later: */
 #define   DRM_DIP_ENABLE		(1 << 28)
+#define   VIDEO_DIP_ENABLE_DRM_GLK	(1 << 28)
 #define   PSR_VSC_BIT_7_SET		(1 << 27)
 #define   VSC_SELECT_MASK		(0x3 << 25)
 #define   VSC_SELECT_SHIFT		25
@@ -8143,6 +8144,7 @@ enum {
 #define _HSW_VIDEO_DIP_SPD_DATA_A	0x602A0
 #define _HSW_VIDEO_DIP_GMP_DATA_A	0x602E0
 #define _HSW_VIDEO_DIP_VSC_DATA_A	0x60320
+#define _GLK_VIDEO_DIP_DRM_DATA_A	0x60440
 #define _HSW_VIDEO_DIP_AVI_ECC_A	0x60240
 #define _HSW_VIDEO_DIP_VS_ECC_A		0x60280
 #define _HSW_VIDEO_DIP_SPD_ECC_A	0x602C0
@@ -8156,6 +8158,7 @@ enum {
 #define _HSW_VIDEO_DIP_SPD_DATA_B	0x612A0
 #define _HSW_VIDEO_DIP_GMP_DATA_B	0x612E0
 #define _HSW_VIDEO_DIP_VSC_DATA_B	0x61320
+#define _GLK_VIDEO_DIP_DRM_DATA_B	0x61440
 #define _HSW_VIDEO_DIP_BVI_ECC_B	0x61240
 #define _HSW_VIDEO_DIP_VS_ECC_B		0x61280
 #define _HSW_VIDEO_DIP_SPD_ECC_B	0x612C0
@@ -8181,6 +8184,7 @@ enum {
 #define HSW_TVIDEO_DIP_SPD_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_SPD_DATA_A + (i) * 4)
 #define HSW_TVIDEO_DIP_GMP_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_GMP_DATA_A + (i) * 4)
 #define HSW_TVIDEO_DIP_VSC_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_VSC_DATA_A + (i) * 4)
+#define GLK_TVIDEO_DIP_DRM_DATA(trans, i)	_MMIO_TRANS2(trans, _GLK_VIDEO_DIP_DRM_DATA_A + (i) * 4)
 #define ICL_VIDEO_DIP_PPS_DATA(trans, i)	_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_DATA_A + (i) * 4)
 #define ICL_VIDEO_DIP_PPS_ECC(trans, i)		_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_ECC_A + (i) * 4)
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 40d55b0..0ecfda0 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -145,6 +145,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
 		return VIDEO_DIP_ENABLE_SPD_HSW;
 	case HDMI_INFOFRAME_TYPE_VENDOR:
 		return VIDEO_DIP_ENABLE_VS_HSW;
+	case HDMI_INFOFRAME_TYPE_DRM:
+		return VIDEO_DIP_ENABLE_DRM_GLK;
 	default:
 		MISSING_CASE(type);
 		return 0;
@@ -170,6 +172,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
 		return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder, i);
 	case HDMI_INFOFRAME_TYPE_VENDOR:
 		return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder, i);
+	case HDMI_INFOFRAME_TYPE_DRM:
+		return GLK_TVIDEO_DIP_DRM_DATA(cpu_transcoder, i);
 	default:
 		MISSING_CASE(type);
 		return INVALID_MMIO_REG;
@@ -553,10 +557,16 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 val = I915_READ(HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder));
+	u32 mask;
 
-	return val & (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
-		      VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
-		      VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
+	mask = (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
+		VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
+		VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
+
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+		mask |= VIDEO_DIP_ENABLE_DRM_GLK;
+
+	return val & mask;
 }
 
 static const u8 infoframe_type_to_idx[] = {
@@ -1188,7 +1198,8 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 
 	val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
 		 VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
-		 VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
+		 VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
+		 VIDEO_DIP_ENABLE_DRM_GLK);
 
 	if (!enable) {
 		I915_WRITE(reg, val);
@@ -1217,7 +1228,8 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 	 * ToDo: Gen9 also can support HDR with LSPCON.
 	 * Support for the same to be enabled later.
 	 */
-	if (INTEL_GEN(dev_priv) >= 10)
+	if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+	    conn_state->hdr_metadata_changed)
 		intel_write_infoframe(encoder, crtc_state,
 				      HDMI_INFOFRAME_TYPE_DRM,
 				      &crtc_state->infoframes.drm);
-- 
1.9.1

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

[v8 08/10] drm/i915:Enabled Modeset when HDR Infoframe changes

[Uma Shankar]

This patch enables modeset whenever HDR metadata
needs to be updated to sink.

v2: Addressed Shashank's review comments.

v3: Added Shashank's RB.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c | 14 +++++++++++++-
 drivers/gpu/drm/i915/intel_hdmi.c   | 26 ++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 8c8fae3..e8b5f84 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -104,6 +104,16 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
 	return -EINVAL;
 }
 
+static bool blob_equal(const struct drm_property_blob *a,
+		       const struct drm_property_blob *b)
+{
+	if (a && b)
+		return a->length == b->length &&
+			!memcmp(a->data, b->data, a->length);
+
+	return !a == !b;
+}
+
 int intel_digital_connector_atomic_check(struct drm_connector *conn,
 					 struct drm_connector_state *new_state)
 {
@@ -131,7 +141,9 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
 	    new_conn_state->base.colorspace != old_conn_state->base.colorspace ||
 	    new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio ||
 	    new_conn_state->base.content_type != old_conn_state->base.content_type ||
-	    new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode)
+	    new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode ||
+	    !blob_equal(new_conn_state->base.hdr_output_metadata_blob_ptr,
+			old_conn_state->base.hdr_output_metadata_blob_ptr))
 		crtc_state->mode_changed = true;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 0ecfda0..85333a7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -799,6 +799,20 @@ void intel_read_infoframe(struct intel_encoder *encoder,
 	return true;
 }
 
+static bool is_eotf_supported(u8 output_eotf, u8 sink_eotf)
+{
+	if (output_eotf == 0)
+		return (sink_eotf & (1 << 0));
+	if (output_eotf == 1)
+		return (sink_eotf & (1 << 1));
+	if (output_eotf == 2)
+		return (sink_eotf & (1 << 2));
+	if (output_eotf == 3)
+		return (sink_eotf & (1 << 3));
+
+	return false;
+}
+
 static bool
 intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
 				 struct intel_crtc_state *crtc_state,
@@ -806,11 +820,23 @@ void intel_read_infoframe(struct intel_encoder *encoder,
 {
 	struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm;
 	struct hdr_output_metadata *hdr_metadata;
+	struct drm_connector *connector = conn_state->connector;
 	int ret;
 
+	if (!conn_state->hdr_output_metadata_blob_ptr ||
+	    conn_state->hdr_output_metadata_blob_ptr->length == 0)
+		return true;
+
 	hdr_metadata = (struct hdr_output_metadata *)
 			conn_state->hdr_output_metadata_blob_ptr->data;
 
+	/* Sink EOTF is Bit map while infoframe is absolute values */
+	if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf,
+			       connector->hdr_sink_metadata.hdmi_type1.eotf)) {
+		DRM_ERROR("EOTF Not Supported\n");
+		return true;
+	}
+
 	ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
 	if (ret < 0) {
 		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
-- 
1.9.1

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

[v8 09/10] drm/i915: Set Infoframe for non modeset case for HDR

[Uma Shankar]

HDR metadata requires a infoframe to be set. Due to fastset,
full modeset is not performed hence adding it to update_pipe
to handle that.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_hdmi.c |  7 +++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0ab3a8a..49b384d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3545,6 +3545,10 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *intel_dig_port =
+			enc_to_dig_port(&encoder->base);
+
 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
 
@@ -3554,6 +3558,15 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 	else if (conn_state->content_protection ==
 		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
 		intel_hdcp_disable(to_intel_connector(conn_state->connector));
+
+	/* Set the infoframe for NON modeset cases as well */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
+		if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+		    conn_state->hdr_metadata_changed)
+			intel_dig_port->set_infoframes(encoder,
+						       crtc_state->has_infoframe,
+						       crtc_state, conn_state);
+	}
 }
 
 static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 85333a7..56a36b5 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1219,8 +1219,11 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 	i915_reg_t reg = HSW_TVIDEO_DIP_CTL(crtc_state->cpu_transcoder);
 	u32 val = I915_READ(reg);
 
-	assert_hdmi_transcoder_func_disabled(dev_priv,
-					     crtc_state->cpu_transcoder);
+	/* DRM Infoframe can be send with transcoder enabled */
+	if (!((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+	      conn_state->hdr_metadata_changed))
+		assert_hdmi_transcoder_func_disabled(dev_priv,
+						     crtc_state->cpu_transcoder);
 
 	val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
 		 VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
-- 
1.9.1

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

[v8 10/10] drm/i915: Added DRM Infoframe handling for BYT/CHT

[Uma Shankar]

BYT/CHT doesn't support DRM Infoframe. This caused
a WARN_ON due to a missing CASE while executing
intel_hdmi_infoframes_enabled function. This patch
fixes the same.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 56a36b5..34f9805 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -122,6 +122,8 @@ static u32 g4x_infoframe_enable(unsigned int type)
 		return VIDEO_DIP_ENABLE_SPD;
 	case HDMI_INFOFRAME_TYPE_VENDOR:
 		return VIDEO_DIP_ENABLE_VENDOR;
+	case HDMI_INFOFRAME_TYPE_DRM:
+		return 0;
 	default:
 		MISSING_CASE(type);
 		return 0;
-- 
1.9.1

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

✗ Fi.CI.CHECKPATCH: warning for Add HDR Metadata Parsing and handling in DRM layer (rev8)

[Patchwork]

== Series Details ==

Series: Add HDR Metadata Parsing and handling in DRM layer (rev8)
URL   : https://patchwork.freedesktop.org/series/25091/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
6f71d6db5493 drm: Add HDR source metadata property
-:67: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#67: FILE: drivers/gpu/drm/drm_atomic_uapi.c:728:
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+				&state->hdr_output_metadata_blob_ptr,

total: 0 errors, 0 warnings, 1 checks, 147 lines checked
ef782728fcdc drm: Parse HDR metadata info from EDID
93c6dce2ad71 drm: Enable HDR infoframe support
40ad900cccee drm/i915: Attach HDR metadata property to connector
a48b441df6b7 drm/i915: Write HDR infoframe and send to panel
ca7165d040c9 drm/i915: Add HLG EOTF
a0b33e2a5181 drm/i915: Enable infoframes on GLK+ for HDR
-:54: WARNING:LONG_LINE: line over 100 characters
#54: FILE: drivers/gpu/drm/i915/i915_reg.h:8187:
+#define GLK_TVIDEO_DIP_DRM_DATA(trans, i)	_MMIO_TRANS2(trans, _GLK_VIDEO_DIP_DRM_DATA_A + (i) * 4)

total: 0 errors, 1 warnings, 0 checks, 81 lines checked
1350e7754205 drm/i915:Enabled Modeset when HDR Infoframe changes
969c71e2025d drm/i915: Set Infoframe for non modeset case for HDR
d3c9dc75502d drm/i915: Added DRM Infoframe handling for BYT/CHT

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

✓ Fi.CI.BAT: success for Add HDR Metadata Parsing and handling in DRM layer (rev8)

[Patchwork]

== Series Details ==

Series: Add HDR Metadata Parsing and handling in DRM layer (rev8)
URL   : https://patchwork.freedesktop.org/series/25091/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5897 -> Patchwork_12743
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/25091/revisions/8/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12743 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@sync-fork-compute0:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@gtt-bsd1:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@gem_exec_store@basic-bsd1:
    - fi-kbl-r:           NOTRUN -> SKIP [fdo#109271] +41

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_busy@basic-flip-a:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@basic-flip-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] +62
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] +52

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
    - fi-glk-dsi:         PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +48

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@gem_ctx_exec@basic:
    - fi-icl-u3:          INCOMPLETE [fdo#107713] -> PASS

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (44 -> 43)
------------------------------

  Additional (3): fi-byt-j1900 fi-kbl-r fi-bsw-n3050 
  Missing    (4): fi-bsw-cyan fi-byt-squawks fi-icl-dsi fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5897 -> Patchwork_12743

  CI_DRM_5897: 7d07e025e78603d6270bc115fdb6c1efea6e66a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4934: dc4f45eb6874331daec870dc1e4cfc3ac5c49311 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12743: d3c9dc75502d87094374ac0b5173c0df2e17d31b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d3c9dc75502d drm/i915: Added DRM Infoframe handling for BYT/CHT
969c71e2025d drm/i915: Set Infoframe for non modeset case for HDR
1350e7754205 drm/i915:Enabled Modeset when HDR Infoframe changes
a0b33e2a5181 drm/i915: Enable infoframes on GLK+ for HDR
ca7165d040c9 drm/i915: Add HLG EOTF
a48b441df6b7 drm/i915: Write HDR infoframe and send to panel
40ad900cccee drm/i915: Attach HDR metadata property to connector
93c6dce2ad71 drm: Enable HDR infoframe support
ef782728fcdc drm: Parse HDR metadata info from EDID
6f71d6db5493 drm: Add HDR source metadata property

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12743/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Add HDR Metadata Parsing and handling in DRM layer (rev8)

[Patchwork]

== Series Details ==

Series: Add HDR Metadata Parsing and handling in DRM layer (rev8)
URL   : https://patchwork.freedesktop.org/series/25091/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5897_full -> Patchwork_12743_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12743_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_pread@pagefault-pread:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277] / [fdo#109960]

  * igt@gem_tiled_fence_blits@normal:
    - shard-iclb:         PASS -> TIMEOUT [fdo#109673]

  * igt@gem_wait@busy-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276]

  * igt@i915_pm_rpm@gem-execbuf-stress-pc8:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109506] / [fdo#109960]

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
    - shard-apl:          NOTRUN -> FAIL [fdo#109660]

  * igt@kms_atomic_transition@6x-modeset-transitions:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] / [fdo#109960]

  * igt@kms_busy@extended-modeset-hang-oldfb-render-d:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278]

  * igt@kms_content_protection@atomic:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109300] / [fdo#109960]

  * igt@kms_content_protection@legacy:
    - shard-apl:          NOTRUN -> FAIL [fdo#110321] / [fdo#110336]

  * igt@kms_crtc_background_color:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109305] / [fdo#109960]

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          PASS -> FAIL [fdo#106509] / [fdo#107409]

  * igt@kms_flip@2x-plain-flip-ts-check:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] / [fdo#109960]

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#109507]

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-kbl:          PASS -> FAIL [fdo#103060]

  * igt@kms_flip_tiling@flip-to-x-tiled:
    - shard-iclb:         PASS -> FAIL [fdo#108134]

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +146

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +15

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         PASS -> FAIL [fdo#103167] / [fdo#109960]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-render:
    - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-blt:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +7

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-draw-mmap-gtt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] / [fdo#109960] +1

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-apl:          NOTRUN -> FAIL [fdo#110279]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-e:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +17

  * igt@kms_pipe_crc_basic@read-crc-pipe-d:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +11

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145] +5

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +1

  * igt@kms_psr@sprite_mmap_cpu:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +2

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-apl:          NOTRUN -> FAIL [fdo#104894]

  * igt@perf_pmu@busy-accuracy-50-vcs1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] / [fdo#109960] +1

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-apl:          NOTRUN -> FAIL [fdo#105010]

  * igt@prime_vgem@sync-bsd1:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +241

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@forked-big-copy-xy:
    - shard-iclb:         TIMEOUT [fdo#109673] -> PASS

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-iclb:         INCOMPLETE [fdo#109801] -> PASS

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-glk:          DMESG-WARN [fdo#109513] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-kbl:          DMESG-WARN [fdo#108566] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
    - shard-iclb:         FAIL [fdo#103355] -> PASS +2

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          FAIL [fdo#102887] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS
    - shard-glk:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_flip@flip-vs-rmfb:
    - shard-iclb:         INCOMPLETE [fdo#107713] -> PASS

  * igt@kms_flip@flip-vs-suspend:
    - shard-iclb:         FAIL [fdo#103375] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +19

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          SKIP [fdo#109271] -> PASS

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] -> PASS

  * igt@kms_psr@cursor_plane_onoff:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS

  * igt@kms_psr@no_drrs:
    - shard-iclb:         FAIL [fdo#108341] -> PASS

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS +1

  * igt@perf@blocking:
    - shard-iclb:         FAIL [fdo#108587] -> PASS

  
#### Warnings ####

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> FAIL [fdo#110270]

  * igt@runner@aborted:
    - shard-glk:          ( 2 FAIL ) [fdo#109373] / [k.org#202321] -> FAIL [fdo#109373] / [k.org#202321]

  
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108587]: https://bugs.freedesktop.org/show_bug.cgi?id=108587
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#109305]: https://bugs.freedesktop.org/show_bug.cgi?id=109305
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109513]: https://bugs.freedesktop.org/show_bug.cgi?id=109513
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#109960]: https://bugs.freedesktop.org/show_bug.cgi?id=109960
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110270]: https://bugs.freedesktop.org/show_bug.cgi?id=110270
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5897 -> Patchwork_12743

  CI_DRM_5897: 7d07e025e78603d6270bc115fdb6c1efea6e66a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4934: dc4f45eb6874331daec870dc1e4cfc3ac5c49311 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12743: d3c9dc75502d87094374ac0b5173c0df2e17d31b @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12743/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v8 01/10] drm: Add HDR source metadata property

[Jonas Karlman]

On 2019-04-09 18:44, Uma Shankar wrote:
> This patch adds a blob property to get HDR metadata
> information from userspace. This will be send as part
> of AVI Infoframe to panel.
>
> It also implements get() and set() functions for HDR output
> metadata property.The blob data is received from userspace and
> saved in connector state, the same is returned as blob in get
> property call to userspace.
>
> v2: Rebase and modified the metadata structure elements
> as per Ville's POC changes.
>
> v3: No Change
>
> v4: Addressed Shashank's review comments
>
> v5: Rebase.
>
> v6: Addressed Brian Starkey's review comments, defined
> new structure with header for dynamic metadata scalability.
> Merge get/set property functions for metadata in this patch.
>
> v7: Addressed Jonas Karlman review comments and defined separate
> structure for infoframe to better align with CTA 861.G spec. Added
> Shashank's RB.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c      |  2 ++
>  drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
>  drivers/gpu/drm/drm_connector.c   |  6 ++++++
>  include/drm/drm_connector.h       | 11 +++++++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  include/linux/hdmi.h              | 10 ++++++++++
>  include/uapi/drm/drm_mode.h       | 39 +++++++++++++++++++++++++++++++++++++++
>  7 files changed, 87 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5eb4013..8b9c126 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -881,6 +881,8 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
>  
>  	drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name);
>  	drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)");
> +	drm_printf(p, "\thdr_metadata_changed=%d\n",
> +		   state->hdr_metadata_changed);
>  
>  	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>  		if (state->writeback_job && state->writeback_job->fb)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index ea797d4..6d0d161 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -673,6 +673,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> +	bool replaced = false;
> +	int ret;
>  
>  	if (property == config->prop_crtc_id) {
>  		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val);
> @@ -721,6 +723,14 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		 */
>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
>  			state->link_status = val;
> +	} else if (property == config->hdr_output_metadata_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +				&state->hdr_output_metadata_blob_ptr,
> +				val,
> +				-1, sizeof(struct hdr_output_metadata),
> +				&replaced);
> +		state->hdr_metadata_changed |= replaced;
> +		return ret;
>  	} else if (property == config->aspect_ratio_property) {
>  		state->picture_aspect_ratio = val;
>  	} else if (property == config->content_type_property) {
> @@ -807,6 +817,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		*val = state->colorspace;
>  	} else if (property == connector->scaling_mode_property) {
>  		*val = state->scaling_mode;
> +	} else if (property == config->hdr_output_metadata_property) {
> +		*val = (state->hdr_output_metadata_blob_ptr) ?
> +			state->hdr_output_metadata_blob_ptr->base.id : 0;
>  	} else if (property == connector->content_protection_property) {
>  		*val = state->content_protection;
>  	} else if (property == config->writeback_fb_id_property) {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124..0bdae90 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.non_desktop_property = prop;
>  
> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
> +				   "HDR_OUTPUT_METADATA", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.hdr_output_metadata_property = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 02a1312..5343f60 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -599,6 +599,13 @@ struct drm_connector_state {
>  	 * and the connector bpc limitations obtained from edid.
>  	 */
>  	u8 max_bpc;
> +
> +	/**
> +	 * @metadata_blob_ptr:
> +	 * DRM blob property for HDR output metadata
> +	 */
> +	struct drm_property_blob *hdr_output_metadata_blob_ptr;
> +	u8 hdr_metadata_changed : 1;
>  };
>  
>  /**
> @@ -1239,6 +1246,10 @@ struct drm_connector {
>  	 * &drm_mode_config.connector_free_work.
>  	 */
>  	struct llist_node free_node;
> +
> +	/* HDR metdata */
> +	struct hdr_output_metadata hdr_output_metadata;
> +	struct hdr_sink_metadata hdr_sink_metadata;
>  };
>  
>  #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 7f60e8e..ef2656b 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -836,6 +836,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *writeback_out_fence_ptr_property;
>  
> +	/*
> +	 * hdr_metadata_property: Connector property containing hdr metatda
> +	 * This will be provided by userspace compositors based on HDR content
> +	 */
> +	struct drm_property *hdr_output_metadata_property;
> +
>  	/* dumb ioctl parameters */
>  	uint32_t preferred_depth, prefer_shadow;
>  
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 927ad64..a065194 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -152,6 +152,16 @@ enum hdmi_content_type {
>  	HDMI_CONTENT_TYPE_GAME,
>  };
>  
> +enum hdmi_metadata_type {
> +	HDMI_STATIC_METADATA_TYPE1 = 1,
> +};
> +
> +enum hdmi_eotf {
> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
> +	HDMI_EOTF_SMPTE_ST2084,
> +};
> +
>  struct hdmi_avi_infoframe {
>  	enum hdmi_infoframe_type type;
>  	unsigned char version;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 83cd163..7b6a519 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -630,6 +630,45 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/* HDR Metadata Infoframe as per 861.G spec */
> +struct hdr_metadata_infoframe {
> +	__u8 eotf;
> +	__u8 metadata_type;
> +	struct {
> +		__u16 x, y;
> +		} display_primaries[3];
> +	struct {
> +		__u16 x, y;
> +		} white_point;
> +	__u16 max_display_mastering_luminance;
> +	__u16 min_display_mastering_luminance;
> +	__u16 max_cll;
> +	__u16 max_fall;
> +};
> +
> +struct hdr_output_metadata {
> +	__u32 metadata_type;
> +	union {
> +		struct hdr_metadata_infoframe hdmi_metadata_type1;
> +	};
> +};
> +
> +/* HDR Metadata as per 861.G spec */
> +struct hdr_static_metadata {
> +	__u8 eotf;
> +	__u8 metadata_type;
> +	__u16 max_cll;
> +	__u16 max_fall;
> +	__u16 min_cll;
> +};
> +
> +struct hdr_sink_metadata {
> +	__u32 metadata_type;
> +	union {
> +		struct hdr_static_metadata hdmi_type1;
> +	};
> +};

These two structs (hdr_static_metadata and hdr_sink_metadata) should probably not be part of uapi,
unless the metadata is intended to be exposed in a HDR_SINK_METADATA property.

The commit "drm/i915: Add HLG EOTF" should probably not use a i915 in prefix as it affects drm core.

The blob_ptr should probably be reference counted in duplicate_state/destroy_state helper similar to [1].
I know too little about drm core in order to know if that is correct, please feel free to pick/squash if this is correct.

[1] https://github.com/Kwiboo/linux-rockchip/commit/5f065ff0bbcca145ee46e9466bb8ca048c7a7b1e

I have tested this patchset on rockchip / dw-hdmi using dw-hdmi [2] and Kodi [3] patches with a successful result of enabling HDR mode on the sink.
There is more work needed to get a full 10-bit pipeline for dw-hdmi in order to make full use of HDR,
but for the purpose of enabling HDR on the sink this patchset seems ready.

[2] https://github.com/Kwiboo/linux-rockchip/commit/d63e38d1cb905e5d885c903286402b202be8541e
[3] https://github.com/Kwiboo/xbmc/commit/c41f85ddaa48995659786ba6d7df6b61c7276aa0

Regards,
Jonas

> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4

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

RE: [v8 01/10] drm: Add HDR source metadata property

[Shankar, Uma]

>-----Original Message-----
>From: Jonas Karlman [mailto:jonas@kwiboo.se]
>Sent: Saturday, May 4, 2019 3:48 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
>devel@lists.freedesktop.org
>Cc: dcastagna@chromium.org; emil.l.velikov@gmail.com; seanpaul@chromium.org;
>Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v8 01/10] drm: Add HDR source metadata property
>
>On 2019-04-09 18:44, Uma Shankar wrote:
>> This patch adds a blob property to get HDR metadata information from
>> userspace. This will be send as part of AVI Infoframe to panel.
>>
>> It also implements get() and set() functions for HDR output metadata
>> property.The blob data is received from userspace and saved in
>> connector state, the same is returned as blob in get property call to
>> userspace.
>>
>> v2: Rebase and modified the metadata structure elements as per Ville's
>> POC changes.
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments
>>
>> v5: Rebase.
>>
>> v6: Addressed Brian Starkey's review comments, defined new structure
>> with header for dynamic metadata scalability.
>> Merge get/set property functions for metadata in this patch.
>>
>> v7: Addressed Jonas Karlman review comments and defined separate
>> structure for infoframe to better align with CTA 861.G spec. Added
>> Shashank's RB.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic.c      |  2 ++
>>  drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
>>  drivers/gpu/drm/drm_connector.c   |  6 ++++++
>>  include/drm/drm_connector.h       | 11 +++++++++++
>>  include/drm/drm_mode_config.h     |  6 ++++++
>>  include/linux/hdmi.h              | 10 ++++++++++
>>  include/uapi/drm/drm_mode.h       | 39
>+++++++++++++++++++++++++++++++++++++++
>>  7 files changed, 87 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c
>> b/drivers/gpu/drm/drm_atomic.c index 5eb4013..8b9c126 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -881,6 +881,8 @@ static void
>> drm_atomic_connector_print_state(struct drm_printer *p,
>>
>>  	drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector-
>>name);
>>  	drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
>> "(null)");
>> +	drm_printf(p, "\thdr_metadata_changed=%d\n",
>> +		   state->hdr_metadata_changed);
>>
>>  	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>>  		if (state->writeback_job && state->writeback_job->fb) diff --git
>> a/drivers/gpu/drm/drm_atomic_uapi.c
>> b/drivers/gpu/drm/drm_atomic_uapi.c
>> index ea797d4..6d0d161 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -673,6 +673,8 @@ static int
>> drm_atomic_connector_set_property(struct drm_connector *connector,  {
>>  	struct drm_device *dev = connector->dev;
>>  	struct drm_mode_config *config = &dev->mode_config;
>> +	bool replaced = false;
>> +	int ret;
>>
>>  	if (property == config->prop_crtc_id) {
>>  		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val); @@ -721,6
>> +723,14 @@ static int drm_atomic_connector_set_property(struct drm_connector
>*connector,
>>  		 */
>>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
>>  			state->link_status = val;
>> +	} else if (property == config->hdr_output_metadata_property) {
>> +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> +				&state->hdr_output_metadata_blob_ptr,
>> +				val,
>> +				-1, sizeof(struct hdr_output_metadata),
>> +				&replaced);
>> +		state->hdr_metadata_changed |= replaced;
>> +		return ret;
>>  	} else if (property == config->aspect_ratio_property) {
>>  		state->picture_aspect_ratio = val;
>>  	} else if (property == config->content_type_property) { @@ -807,6
>> +817,9 @@ static int drm_atomic_connector_set_property(struct drm_connector
>*connector,
>>  		*val = state->colorspace;
>>  	} else if (property == connector->scaling_mode_property) {
>>  		*val = state->scaling_mode;
>> +	} else if (property == config->hdr_output_metadata_property) {
>> +		*val = (state->hdr_output_metadata_blob_ptr) ?
>> +			state->hdr_output_metadata_blob_ptr->base.id : 0;
>>  	} else if (property == connector->content_protection_property) {
>>  		*val = state->content_protection;
>>  	} else if (property == config->writeback_fb_id_property) { diff
>> --git a/drivers/gpu/drm/drm_connector.c
>> b/drivers/gpu/drm/drm_connector.c index 2355124..0bdae90 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct
>drm_device *dev)
>>  		return -ENOMEM;
>>  	dev->mode_config.non_desktop_property = prop;
>>
>> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
>> +				   "HDR_OUTPUT_METADATA", 0);
>> +	if (!prop)
>> +		return -ENOMEM;
>> +	dev->mode_config.hdr_output_metadata_property = prop;
>> +
>>  	return 0;
>>  }
>>
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 02a1312..5343f60 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -599,6 +599,13 @@ struct drm_connector_state {
>>  	 * and the connector bpc limitations obtained from edid.
>>  	 */
>>  	u8 max_bpc;
>> +
>> +	/**
>> +	 * @metadata_blob_ptr:
>> +	 * DRM blob property for HDR output metadata
>> +	 */
>> +	struct drm_property_blob *hdr_output_metadata_blob_ptr;
>> +	u8 hdr_metadata_changed : 1;
>>  };
>>
>>  /**
>> @@ -1239,6 +1246,10 @@ struct drm_connector {
>>  	 * &drm_mode_config.connector_free_work.
>>  	 */
>>  	struct llist_node free_node;
>> +
>> +	/* HDR metdata */
>> +	struct hdr_output_metadata hdr_output_metadata;
>> +	struct hdr_sink_metadata hdr_sink_metadata;
>>  };
>>
>>  #define obj_to_connector(x) container_of(x, struct drm_connector,
>> base) diff --git a/include/drm/drm_mode_config.h
>> b/include/drm/drm_mode_config.h index 7f60e8e..ef2656b 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -836,6 +836,12 @@ struct drm_mode_config {
>>  	 */
>>  	struct drm_property *writeback_out_fence_ptr_property;
>>
>> +	/*
>> +	 * hdr_metadata_property: Connector property containing hdr metatda
>> +	 * This will be provided by userspace compositors based on HDR content
>> +	 */
>> +	struct drm_property *hdr_output_metadata_property;
>> +
>>  	/* dumb ioctl parameters */
>>  	uint32_t preferred_depth, prefer_shadow;
>>
>> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index
>> 927ad64..a065194 100644
>> --- a/include/linux/hdmi.h
>> +++ b/include/linux/hdmi.h
>> @@ -152,6 +152,16 @@ enum hdmi_content_type {
>>  	HDMI_CONTENT_TYPE_GAME,
>>  };
>>
>> +enum hdmi_metadata_type {
>> +	HDMI_STATIC_METADATA_TYPE1 = 1,
>> +};
>> +
>> +enum hdmi_eotf {
>> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
>> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
>> +	HDMI_EOTF_SMPTE_ST2084,
>> +};
>> +
>>  struct hdmi_avi_infoframe {
>>  	enum hdmi_infoframe_type type;
>>  	unsigned char version;
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index 83cd163..7b6a519 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -630,6 +630,45 @@ struct drm_color_lut {
>>  	__u16 reserved;
>>  };
>>
>> +/* HDR Metadata Infoframe as per 861.G spec */ struct
>> +hdr_metadata_infoframe {
>> +	__u8 eotf;
>> +	__u8 metadata_type;
>> +	struct {
>> +		__u16 x, y;
>> +		} display_primaries[3];
>> +	struct {
>> +		__u16 x, y;
>> +		} white_point;
>> +	__u16 max_display_mastering_luminance;
>> +	__u16 min_display_mastering_luminance;
>> +	__u16 max_cll;
>> +	__u16 max_fall;
>> +};
>> +
>> +struct hdr_output_metadata {
>> +	__u32 metadata_type;
>> +	union {
>> +		struct hdr_metadata_infoframe hdmi_metadata_type1;
>> +	};
>> +};
>> +
>> +/* HDR Metadata as per 861.G spec */
>> +struct hdr_static_metadata {
>> +	__u8 eotf;
>> +	__u8 metadata_type;
>> +	__u16 max_cll;
>> +	__u16 max_fall;
>> +	__u16 min_cll;
>> +};
>> +
>> +struct hdr_sink_metadata {
>> +	__u32 metadata_type;
>> +	union {
>> +		struct hdr_static_metadata hdmi_type1;
>> +	};
>> +};
>
>These two structs (hdr_static_metadata and hdr_sink_metadata) should probably not
>be part of uapi, unless the metadata is intended to be exposed in a
>HDR_SINK_METADATA property.

Hmm yeah, currently we are not planning it to expose that. It was just to help userspace
get and parse the EDID and store the details. But yes this doesn't need to be UAPI and
userspace should be free to use whatever way they want to parse and store sink metadata.
Will move it outside of uapi headers.


>The commit "drm/i915: Add HLG EOTF" should probably not use a i915 in prefix as it
>affects drm core.

Ok, will update this.

>
>The blob_ptr should probably be reference counted in duplicate_state/destroy_state
>helper similar to [1].
>I know too little about drm core in order to know if that is correct, please feel free to
>pick/squash if this is correct.

Sure, I will add these changes to the series.

>[1] https://github.com/Kwiboo/linux-
>rockchip/commit/5f065ff0bbcca145ee46e9466bb8ca048c7a7b1e
>
>I have tested this patchset on rockchip / dw-hdmi using dw-hdmi [2] and Kodi [3]
>patches with a successful result of enabling HDR mode on the sink.
>There is more work needed to get a full 10-bit pipeline for dw-hdmi in order to make
>full use of HDR, but for the purpose of enabling HDR on the sink this patchset seems
>ready.

Thanks Jonas for your comments and more importantly for testing and enabling with kodi.

I will rebase the series and send out next version. With the changes already in place in kodi branch,
we should probably be good for merge.

Regards,
Uma Shankar

>[2] https://github.com/Kwiboo/linux-
>rockchip/commit/d63e38d1cb905e5d885c903286402b202be8541e
>[3]
>https://github.com/Kwiboo/xbmc/commit/c41f85ddaa48995659786ba6d7df6b61c72
>76aa0
>
>Regards,
>Jonas
>
>> +
>>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01  #define
>> DRM_MODE_PAGE_FLIP_ASYNC 0x02  #define
>> DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4

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

Re: [v8 01/10] drm: Add HDR source metadata property

[Ville Syrjälä]

On Tue, May 07, 2019 at 09:03:45AM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Jonas Karlman [mailto:jonas@kwiboo.se]
> >Sent: Saturday, May 4, 2019 3:48 PM
> >To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> >devel@lists.freedesktop.org
> >Cc: dcastagna@chromium.org; emil.l.velikov@gmail.com; seanpaul@chromium.org;
> >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
> ><maarten.lankhorst@intel.com>
> >Subject: Re: [v8 01/10] drm: Add HDR source metadata property
> >
> >On 2019-04-09 18:44, Uma Shankar wrote:
> >> This patch adds a blob property to get HDR metadata information from
> >> userspace. This will be send as part of AVI Infoframe to panel.
> >>
> >> It also implements get() and set() functions for HDR output metadata
> >> property.The blob data is received from userspace and saved in
> >> connector state, the same is returned as blob in get property call to
> >> userspace.
> >>
> >> v2: Rebase and modified the metadata structure elements as per Ville's
> >> POC changes.
> >>
> >> v3: No Change
> >>
> >> v4: Addressed Shashank's review comments
> >>
> >> v5: Rebase.
> >>
> >> v6: Addressed Brian Starkey's review comments, defined new structure
> >> with header for dynamic metadata scalability.
> >> Merge get/set property functions for metadata in this patch.
> >>
> >> v7: Addressed Jonas Karlman review comments and defined separate
> >> structure for infoframe to better align with CTA 861.G spec. Added
> >> Shashank's RB.
> >>
> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >>  drivers/gpu/drm/drm_atomic.c      |  2 ++
> >>  drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
> >>  drivers/gpu/drm/drm_connector.c   |  6 ++++++
> >>  include/drm/drm_connector.h       | 11 +++++++++++
> >>  include/drm/drm_mode_config.h     |  6 ++++++
> >>  include/linux/hdmi.h              | 10 ++++++++++
> >>  include/uapi/drm/drm_mode.h       | 39
> >+++++++++++++++++++++++++++++++++++++++
> >>  7 files changed, 87 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic.c
> >> b/drivers/gpu/drm/drm_atomic.c index 5eb4013..8b9c126 100644
> >> --- a/drivers/gpu/drm/drm_atomic.c
> >> +++ b/drivers/gpu/drm/drm_atomic.c
> >> @@ -881,6 +881,8 @@ static void
> >> drm_atomic_connector_print_state(struct drm_printer *p,
> >>
> >>  	drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector-
> >>name);
> >>  	drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
> >> "(null)");
> >> +	drm_printf(p, "\thdr_metadata_changed=%d\n",
> >> +		   state->hdr_metadata_changed);
> >>
> >>  	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
> >>  		if (state->writeback_job && state->writeback_job->fb) diff --git
> >> a/drivers/gpu/drm/drm_atomic_uapi.c
> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index ea797d4..6d0d161 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -673,6 +673,8 @@ static int
> >> drm_atomic_connector_set_property(struct drm_connector *connector,  {
> >>  	struct drm_device *dev = connector->dev;
> >>  	struct drm_mode_config *config = &dev->mode_config;
> >> +	bool replaced = false;
> >> +	int ret;
> >>
> >>  	if (property == config->prop_crtc_id) {
> >>  		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val); @@ -721,6
> >> +723,14 @@ static int drm_atomic_connector_set_property(struct drm_connector
> >*connector,
> >>  		 */
> >>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
> >>  			state->link_status = val;
> >> +	} else if (property == config->hdr_output_metadata_property) {
> >> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> >> +				&state->hdr_output_metadata_blob_ptr,
> >> +				val,
> >> +				-1, sizeof(struct hdr_output_metadata),
> >> +				&replaced);
> >> +		state->hdr_metadata_changed |= replaced;
> >> +		return ret;
> >>  	} else if (property == config->aspect_ratio_property) {
> >>  		state->picture_aspect_ratio = val;
> >>  	} else if (property == config->content_type_property) { @@ -807,6
> >> +817,9 @@ static int drm_atomic_connector_set_property(struct drm_connector
> >*connector,
> >>  		*val = state->colorspace;
> >>  	} else if (property == connector->scaling_mode_property) {
> >>  		*val = state->scaling_mode;
> >> +	} else if (property == config->hdr_output_metadata_property) {
> >> +		*val = (state->hdr_output_metadata_blob_ptr) ?
> >> +			state->hdr_output_metadata_blob_ptr->base.id : 0;
> >>  	} else if (property == connector->content_protection_property) {
> >>  		*val = state->content_protection;
> >>  	} else if (property == config->writeback_fb_id_property) { diff
> >> --git a/drivers/gpu/drm/drm_connector.c
> >> b/drivers/gpu/drm/drm_connector.c index 2355124..0bdae90 100644
> >> --- a/drivers/gpu/drm/drm_connector.c
> >> +++ b/drivers/gpu/drm/drm_connector.c
> >> @@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct
> >drm_device *dev)
> >>  		return -ENOMEM;
> >>  	dev->mode_config.non_desktop_property = prop;
> >>
> >> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
> >> +				   "HDR_OUTPUT_METADATA", 0);
> >> +	if (!prop)
> >> +		return -ENOMEM;
> >> +	dev->mode_config.hdr_output_metadata_property = prop;
> >> +
> >>  	return 0;
> >>  }
> >>
> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> >> index 02a1312..5343f60 100644
> >> --- a/include/drm/drm_connector.h
> >> +++ b/include/drm/drm_connector.h
> >> @@ -599,6 +599,13 @@ struct drm_connector_state {
> >>  	 * and the connector bpc limitations obtained from edid.
> >>  	 */
> >>  	u8 max_bpc;
> >> +
> >> +	/**
> >> +	 * @metadata_blob_ptr:
> >> +	 * DRM blob property for HDR output metadata
> >> +	 */
> >> +	struct drm_property_blob *hdr_output_metadata_blob_ptr;
> >> +	u8 hdr_metadata_changed : 1;
> >>  };
> >>
> >>  /**
> >> @@ -1239,6 +1246,10 @@ struct drm_connector {
> >>  	 * &drm_mode_config.connector_free_work.
> >>  	 */
> >>  	struct llist_node free_node;
> >> +
> >> +	/* HDR metdata */
> >> +	struct hdr_output_metadata hdr_output_metadata;
> >> +	struct hdr_sink_metadata hdr_sink_metadata;
> >>  };
> >>
> >>  #define obj_to_connector(x) container_of(x, struct drm_connector,
> >> base) diff --git a/include/drm/drm_mode_config.h
> >> b/include/drm/drm_mode_config.h index 7f60e8e..ef2656b 100644
> >> --- a/include/drm/drm_mode_config.h
> >> +++ b/include/drm/drm_mode_config.h
> >> @@ -836,6 +836,12 @@ struct drm_mode_config {
> >>  	 */
> >>  	struct drm_property *writeback_out_fence_ptr_property;
> >>
> >> +	/*
> >> +	 * hdr_metadata_property: Connector property containing hdr metatda
> >> +	 * This will be provided by userspace compositors based on HDR content
> >> +	 */
> >> +	struct drm_property *hdr_output_metadata_property;
> >> +
> >>  	/* dumb ioctl parameters */
> >>  	uint32_t preferred_depth, prefer_shadow;
> >>
> >> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index
> >> 927ad64..a065194 100644
> >> --- a/include/linux/hdmi.h
> >> +++ b/include/linux/hdmi.h
> >> @@ -152,6 +152,16 @@ enum hdmi_content_type {
> >>  	HDMI_CONTENT_TYPE_GAME,
> >>  };
> >>
> >> +enum hdmi_metadata_type {
> >> +	HDMI_STATIC_METADATA_TYPE1 = 1,
> >> +};
> >> +
> >> +enum hdmi_eotf {
> >> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
> >> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
> >> +	HDMI_EOTF_SMPTE_ST2084,
> >> +};
> >> +
> >>  struct hdmi_avi_infoframe {
> >>  	enum hdmi_infoframe_type type;
> >>  	unsigned char version;
> >> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> >> index 83cd163..7b6a519 100644
> >> --- a/include/uapi/drm/drm_mode.h
> >> +++ b/include/uapi/drm/drm_mode.h
> >> @@ -630,6 +630,45 @@ struct drm_color_lut {
> >>  	__u16 reserved;
> >>  };
> >>
> >> +/* HDR Metadata Infoframe as per 861.G spec */ struct
> >> +hdr_metadata_infoframe {
> >> +	__u8 eotf;
> >> +	__u8 metadata_type;
> >> +	struct {
> >> +		__u16 x, y;
> >> +		} display_primaries[3];
> >> +	struct {
> >> +		__u16 x, y;
> >> +		} white_point;
> >> +	__u16 max_display_mastering_luminance;
> >> +	__u16 min_display_mastering_luminance;
> >> +	__u16 max_cll;
> >> +	__u16 max_fall;
> >> +};
> >> +
> >> +struct hdr_output_metadata {
> >> +	__u32 metadata_type;
> >> +	union {
> >> +		struct hdr_metadata_infoframe hdmi_metadata_type1;
> >> +	};
> >> +};
> >> +
> >> +/* HDR Metadata as per 861.G spec */
> >> +struct hdr_static_metadata {
> >> +	__u8 eotf;
> >> +	__u8 metadata_type;
> >> +	__u16 max_cll;
> >> +	__u16 max_fall;
> >> +	__u16 min_cll;
> >> +};
> >> +
> >> +struct hdr_sink_metadata {
> >> +	__u32 metadata_type;
> >> +	union {
> >> +		struct hdr_static_metadata hdmi_type1;
> >> +	};
> >> +};
> >
> >These two structs (hdr_static_metadata and hdr_sink_metadata) should probably not
> >be part of uapi, unless the metadata is intended to be exposed in a
> >HDR_SINK_METADATA property.
> 
> Hmm yeah, currently we are not planning it to expose that. It was just to help userspace
> get and parse the EDID and store the details. But yes this doesn't need to be UAPI and
> userspace should be free to use whatever way they want to parse and store sink metadata.
> Will move it outside of uapi headers.
> 
> 
> >The commit "drm/i915: Add HLG EOTF" should probably not use a i915 in prefix as it
> >affects drm core.
> 
> Ok, will update this.
> 
> >
> >The blob_ptr should probably be reference counted in duplicate_state/destroy_state
> >helper similar to [1].
> >I know too little about drm core in order to know if that is correct, please feel free to
> >pick/squash if this is correct.
> 
> Sure, I will add these changes to the series.
> 
> >[1] https://github.com/Kwiboo/linux-
> >rockchip/commit/5f065ff0bbcca145ee46e9466bb8ca048c7a7b1e
> >
> >I have tested this patchset on rockchip / dw-hdmi using dw-hdmi [2] and Kodi [3]
> >patches with a successful result of enabling HDR mode on the sink.
> >There is more work needed to get a full 10-bit pipeline for dw-hdmi in order to make
> >full use of HDR, but for the purpose of enabling HDR on the sink this patchset seems
> >ready.
> 
> Thanks Jonas for your comments and more importantly for testing and enabling with kodi.
> 
> I will rebase the series and send out next version. With the changes already in place in kodi branch,
> we should probably be good for merge.

Patch 9 is not ready IMO. I don't see any analysis on why it's OK to
update infoframes without a full modeset. I would just drop that patch
from the initial set and work on it as a followup.

Also did someone analysze how the new dynamic HDR metadata fits in with
the new uapi?

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v8 08/10] drm/i915:Enabled Modeset when HDR Infoframe changes

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 10:14:42PM +0530, Uma Shankar wrote:
> This patch enables modeset whenever HDR metadata
> needs to be updated to sink.
> 
> v2: Addressed Shashank's review comments.
> 
> v3: Added Shashank's RB.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c | 14 +++++++++++++-
>  drivers/gpu/drm/i915/intel_hdmi.c   | 26 ++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 8c8fae3..e8b5f84 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -104,6 +104,16 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
>  	return -EINVAL;
>  }
>  
> +static bool blob_equal(const struct drm_property_blob *a,
> +		       const struct drm_property_blob *b)
> +{
> +	if (a && b)
> +		return a->length == b->length &&
> +			!memcmp(a->data, b->data, a->length);
> +
> +	return !a == !b;
> +}

I have a feeling the memcmp() is overkill. We could just check for
whether the blob is the same or not. If userspace is an idiot and
creates a new blob with identical content so be it.

> +
>  int intel_digital_connector_atomic_check(struct drm_connector *conn,
>  					 struct drm_connector_state *new_state)
>  {
> @@ -131,7 +141,9 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
>  	    new_conn_state->base.colorspace != old_conn_state->base.colorspace ||
>  	    new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio ||
>  	    new_conn_state->base.content_type != old_conn_state->base.content_type ||
> -	    new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode)
> +	    new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode ||
> +	    !blob_equal(new_conn_state->base.hdr_output_metadata_blob_ptr,
> +			old_conn_state->base.hdr_output_metadata_blob_ptr))
>  		crtc_state->mode_changed = true;
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 0ecfda0..85333a7 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -799,6 +799,20 @@ void intel_read_infoframe(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static bool is_eotf_supported(u8 output_eotf, u8 sink_eotf)
> +{
> +	if (output_eotf == 0)
> +		return (sink_eotf & (1 << 0));
> +	if (output_eotf == 1)
> +		return (sink_eotf & (1 << 1));
> +	if (output_eotf == 2)
> +		return (sink_eotf & (1 << 2));
> +	if (output_eotf == 3)
> +		return (sink_eotf & (1 << 3));
> +
> +	return false;

return sink_eotf & BIT(output_eotf);

> +}
> +
>  static bool
>  intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
>  				 struct intel_crtc_state *crtc_state,
> @@ -806,11 +820,23 @@ void intel_read_infoframe(struct intel_encoder *encoder,
>  {
>  	struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm;
>  	struct hdr_output_metadata *hdr_metadata;
> +	struct drm_connector *connector = conn_state->connector;
>  	int ret;
>  
> +	if (!conn_state->hdr_output_metadata_blob_ptr ||
> +	    conn_state->hdr_output_metadata_blob_ptr->length == 0)
> +		return true;
> +
>  	hdr_metadata = (struct hdr_output_metadata *)
>  			conn_state->hdr_output_metadata_blob_ptr->data;
>  
> +	/* Sink EOTF is Bit map while infoframe is absolute values */
> +	if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf,
> +			       connector->hdr_sink_metadata.hdmi_type1.eotf)) {
> +		DRM_ERROR("EOTF Not Supported\n");
> +		return true;
> +	}
> +
>  	ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
>  	if (ret < 0) {
>  		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
> -- 
> 1.9.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v8 01/10] drm: Add HDR source metadata property

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 10:14:35PM +0530, Uma Shankar wrote:
> This patch adds a blob property to get HDR metadata
> information from userspace. This will be send as part
> of AVI Infoframe to panel.
> 
> It also implements get() and set() functions for HDR output
> metadata property.The blob data is received from userspace and
> saved in connector state, the same is returned as blob in get
> property call to userspace.
> 
> v2: Rebase and modified the metadata structure elements
> as per Ville's POC changes.
> 
> v3: No Change
> 
> v4: Addressed Shashank's review comments
> 
> v5: Rebase.
> 
> v6: Addressed Brian Starkey's review comments, defined
> new structure with header for dynamic metadata scalability.
> Merge get/set property functions for metadata in this patch.
> 
> v7: Addressed Jonas Karlman review comments and defined separate
> structure for infoframe to better align with CTA 861.G spec. Added
> Shashank's RB.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c      |  2 ++
>  drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
>  drivers/gpu/drm/drm_connector.c   |  6 ++++++
>  include/drm/drm_connector.h       | 11 +++++++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  include/linux/hdmi.h              | 10 ++++++++++
>  include/uapi/drm/drm_mode.h       | 39 +++++++++++++++++++++++++++++++++++++++
>  7 files changed, 87 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5eb4013..8b9c126 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -881,6 +881,8 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
>  
>  	drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name);
>  	drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)");
> +	drm_printf(p, "\thdr_metadata_changed=%d\n",
> +		   state->hdr_metadata_changed);

Is that flag actually useful?

>  
>  	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>  		if (state->writeback_job && state->writeback_job->fb)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index ea797d4..6d0d161 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -673,6 +673,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> +	bool replaced = false;
> +	int ret;
>  
>  	if (property == config->prop_crtc_id) {
>  		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val);
> @@ -721,6 +723,14 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		 */
>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
>  			state->link_status = val;
> +	} else if (property == config->hdr_output_metadata_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +				&state->hdr_output_metadata_blob_ptr,
> +				val,
> +				-1, sizeof(struct hdr_output_metadata),
> +				&replaced);
> +		state->hdr_metadata_changed |= replaced;
> +		return ret;
>  	} else if (property == config->aspect_ratio_property) {
>  		state->picture_aspect_ratio = val;
>  	} else if (property == config->content_type_property) {
> @@ -807,6 +817,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		*val = state->colorspace;
>  	} else if (property == connector->scaling_mode_property) {
>  		*val = state->scaling_mode;
> +	} else if (property == config->hdr_output_metadata_property) {
> +		*val = (state->hdr_output_metadata_blob_ptr) ?

Pointless parens.

> +			state->hdr_output_metadata_blob_ptr->base.id : 0;
>  	} else if (property == connector->content_protection_property) {
>  		*val = state->content_protection;
>  	} else if (property == config->writeback_fb_id_property) {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124..0bdae90 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.non_desktop_property = prop;
>  
> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
> +				   "HDR_OUTPUT_METADATA", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.hdr_output_metadata_property = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 02a1312..5343f60 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -599,6 +599,13 @@ struct drm_connector_state {
>  	 * and the connector bpc limitations obtained from edid.
>  	 */
>  	u8 max_bpc;
> +
> +	/**
> +	 * @metadata_blob_ptr:
> +	 * DRM blob property for HDR output metadata
> +	 */
> +	struct drm_property_blob *hdr_output_metadata_blob_ptr;

I would drop the _blob_ptr suffix. We don't have that for any other
state blob.

> +	u8 hdr_metadata_changed : 1;
>  };
>  
>  /**
> @@ -1239,6 +1246,10 @@ struct drm_connector {
>  	 * &drm_mode_config.connector_free_work.
>  	 */
>  	struct llist_node free_node;
> +
> +	/* HDR metdata */
> +	struct hdr_output_metadata hdr_output_metadata;
> +	struct hdr_sink_metadata hdr_sink_metadata;
>  };
>  
>  #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 7f60e8e..ef2656b 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -836,6 +836,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *writeback_out_fence_ptr_property;
>  
> +	/*
> +	 * hdr_metadata_property: Connector property containing hdr metatda
> +	 * This will be provided by userspace compositors based on HDR content
> +	 */
> +	struct drm_property *hdr_output_metadata_property;
> +
>  	/* dumb ioctl parameters */
>  	uint32_t preferred_depth, prefer_shadow;
>  
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 927ad64..a065194 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -152,6 +152,16 @@ enum hdmi_content_type {
>  	HDMI_CONTENT_TYPE_GAME,
>  };
>  
> +enum hdmi_metadata_type {
> +	HDMI_STATIC_METADATA_TYPE1 = 1,
> +};
> +
> +enum hdmi_eotf {
> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
> +	HDMI_EOTF_SMPTE_ST2084,
> +};
> +
>  struct hdmi_avi_infoframe {
>  	enum hdmi_infoframe_type type;
>  	unsigned char version;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 83cd163..7b6a519 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -630,6 +630,45 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/* HDR Metadata Infoframe as per 861.G spec */
> +struct hdr_metadata_infoframe {
> +	__u8 eotf;
> +	__u8 metadata_type;
> +	struct {
> +		__u16 x, y;
> +		} display_primaries[3];
> +	struct {
> +		__u16 x, y;
> +		} white_point;
> +	__u16 max_display_mastering_luminance;
> +	__u16 min_display_mastering_luminance;
> +	__u16 max_cll;
> +	__u16 max_fall;
> +};
> +
> +struct hdr_output_metadata {
> +	__u32 metadata_type;
> +	union {
> +		struct hdr_metadata_infoframe hdmi_metadata_type1;
> +	};
> +};
> +
> +/* HDR Metadata as per 861.G spec */
> +struct hdr_static_metadata {
> +	__u8 eotf;
> +	__u8 metadata_type;
> +	__u16 max_cll;
> +	__u16 max_fall;
> +	__u16 min_cll;
> +};
> +
> +struct hdr_sink_metadata {
> +	__u32 metadata_type;
> +	union {
> +		struct hdr_static_metadata hdmi_type1;
> +	};
> +};
> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v8 01/10] drm: Add HDR source metadata property

[Ville Syrjälä]

On Tue, May 07, 2019 at 01:25:42PM +0300, Ville Syrjälä wrote:
> On Tue, May 07, 2019 at 09:03:45AM +0000, Shankar, Uma wrote:
> > 
> > 
> > >-----Original Message-----
> > >From: Jonas Karlman [mailto:jonas@kwiboo.se]
> > >Sent: Saturday, May 4, 2019 3:48 PM
> > >To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> > >devel@lists.freedesktop.org
> > >Cc: dcastagna@chromium.org; emil.l.velikov@gmail.com; seanpaul@chromium.org;
> > >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
> > ><maarten.lankhorst@intel.com>
> > >Subject: Re: [v8 01/10] drm: Add HDR source metadata property
> > >
> > >On 2019-04-09 18:44, Uma Shankar wrote:
> > >> This patch adds a blob property to get HDR metadata information from
> > >> userspace. This will be send as part of AVI Infoframe to panel.
> > >>
> > >> It also implements get() and set() functions for HDR output metadata
> > >> property.The blob data is received from userspace and saved in
> > >> connector state, the same is returned as blob in get property call to
> > >> userspace.
> > >>
> > >> v2: Rebase and modified the metadata structure elements as per Ville's
> > >> POC changes.
> > >>
> > >> v3: No Change
> > >>
> > >> v4: Addressed Shashank's review comments
> > >>
> > >> v5: Rebase.
> > >>
> > >> v6: Addressed Brian Starkey's review comments, defined new structure
> > >> with header for dynamic metadata scalability.
> > >> Merge get/set property functions for metadata in this patch.
> > >>
> > >> v7: Addressed Jonas Karlman review comments and defined separate
> > >> structure for infoframe to better align with CTA 861.G spec. Added
> > >> Shashank's RB.
> > >>
> > >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> > >> ---
> > >>  drivers/gpu/drm/drm_atomic.c      |  2 ++
> > >>  drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
> > >>  drivers/gpu/drm/drm_connector.c   |  6 ++++++
> > >>  include/drm/drm_connector.h       | 11 +++++++++++
> > >>  include/drm/drm_mode_config.h     |  6 ++++++
> > >>  include/linux/hdmi.h              | 10 ++++++++++
> > >>  include/uapi/drm/drm_mode.h       | 39
> > >+++++++++++++++++++++++++++++++++++++++
> > >>  7 files changed, 87 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_atomic.c
> > >> b/drivers/gpu/drm/drm_atomic.c index 5eb4013..8b9c126 100644
> > >> --- a/drivers/gpu/drm/drm_atomic.c
> > >> +++ b/drivers/gpu/drm/drm_atomic.c
> > >> @@ -881,6 +881,8 @@ static void
> > >> drm_atomic_connector_print_state(struct drm_printer *p,
> > >>
> > >>  	drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector-
> > >>name);
> > >>  	drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
> > >> "(null)");
> > >> +	drm_printf(p, "\thdr_metadata_changed=%d\n",
> > >> +		   state->hdr_metadata_changed);
> > >>
> > >>  	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
> > >>  		if (state->writeback_job && state->writeback_job->fb) diff --git
> > >> a/drivers/gpu/drm/drm_atomic_uapi.c
> > >> b/drivers/gpu/drm/drm_atomic_uapi.c
> > >> index ea797d4..6d0d161 100644
> > >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > >> @@ -673,6 +673,8 @@ static int
> > >> drm_atomic_connector_set_property(struct drm_connector *connector,  {
> > >>  	struct drm_device *dev = connector->dev;
> > >>  	struct drm_mode_config *config = &dev->mode_config;
> > >> +	bool replaced = false;
> > >> +	int ret;
> > >>
> > >>  	if (property == config->prop_crtc_id) {
> > >>  		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val); @@ -721,6
> > >> +723,14 @@ static int drm_atomic_connector_set_property(struct drm_connector
> > >*connector,
> > >>  		 */
> > >>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
> > >>  			state->link_status = val;
> > >> +	} else if (property == config->hdr_output_metadata_property) {
> > >> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> > >> +				&state->hdr_output_metadata_blob_ptr,
> > >> +				val,
> > >> +				-1, sizeof(struct hdr_output_metadata),
> > >> +				&replaced);
> > >> +		state->hdr_metadata_changed |= replaced;
> > >> +		return ret;
> > >>  	} else if (property == config->aspect_ratio_property) {
> > >>  		state->picture_aspect_ratio = val;
> > >>  	} else if (property == config->content_type_property) { @@ -807,6
> > >> +817,9 @@ static int drm_atomic_connector_set_property(struct drm_connector
> > >*connector,
> > >>  		*val = state->colorspace;
> > >>  	} else if (property == connector->scaling_mode_property) {
> > >>  		*val = state->scaling_mode;
> > >> +	} else if (property == config->hdr_output_metadata_property) {
> > >> +		*val = (state->hdr_output_metadata_blob_ptr) ?
> > >> +			state->hdr_output_metadata_blob_ptr->base.id : 0;
> > >>  	} else if (property == connector->content_protection_property) {
> > >>  		*val = state->content_protection;
> > >>  	} else if (property == config->writeback_fb_id_property) { diff
> > >> --git a/drivers/gpu/drm/drm_connector.c
> > >> b/drivers/gpu/drm/drm_connector.c index 2355124..0bdae90 100644
> > >> --- a/drivers/gpu/drm/drm_connector.c
> > >> +++ b/drivers/gpu/drm/drm_connector.c
> > >> @@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct
> > >drm_device *dev)
> > >>  		return -ENOMEM;
> > >>  	dev->mode_config.non_desktop_property = prop;
> > >>
> > >> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
> > >> +				   "HDR_OUTPUT_METADATA", 0);
> > >> +	if (!prop)
> > >> +		return -ENOMEM;
> > >> +	dev->mode_config.hdr_output_metadata_property = prop;
> > >> +
> > >>  	return 0;
> > >>  }
> > >>
> > >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > >> index 02a1312..5343f60 100644
> > >> --- a/include/drm/drm_connector.h
> > >> +++ b/include/drm/drm_connector.h
> > >> @@ -599,6 +599,13 @@ struct drm_connector_state {
> > >>  	 * and the connector bpc limitations obtained from edid.
> > >>  	 */
> > >>  	u8 max_bpc;
> > >> +
> > >> +	/**
> > >> +	 * @metadata_blob_ptr:
> > >> +	 * DRM blob property for HDR output metadata
> > >> +	 */
> > >> +	struct drm_property_blob *hdr_output_metadata_blob_ptr;
> > >> +	u8 hdr_metadata_changed : 1;
> > >>  };
> > >>
> > >>  /**
> > >> @@ -1239,6 +1246,10 @@ struct drm_connector {
> > >>  	 * &drm_mode_config.connector_free_work.
> > >>  	 */
> > >>  	struct llist_node free_node;
> > >> +
> > >> +	/* HDR metdata */
> > >> +	struct hdr_output_metadata hdr_output_metadata;
> > >> +	struct hdr_sink_metadata hdr_sink_metadata;
> > >>  };
> > >>
> > >>  #define obj_to_connector(x) container_of(x, struct drm_connector,
> > >> base) diff --git a/include/drm/drm_mode_config.h
> > >> b/include/drm/drm_mode_config.h index 7f60e8e..ef2656b 100644
> > >> --- a/include/drm/drm_mode_config.h
> > >> +++ b/include/drm/drm_mode_config.h
> > >> @@ -836,6 +836,12 @@ struct drm_mode_config {
> > >>  	 */
> > >>  	struct drm_property *writeback_out_fence_ptr_property;
> > >>
> > >> +	/*
> > >> +	 * hdr_metadata_property: Connector property containing hdr metatda
> > >> +	 * This will be provided by userspace compositors based on HDR content
> > >> +	 */
> > >> +	struct drm_property *hdr_output_metadata_property;
> > >> +
> > >>  	/* dumb ioctl parameters */
> > >>  	uint32_t preferred_depth, prefer_shadow;
> > >>
> > >> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index
> > >> 927ad64..a065194 100644
> > >> --- a/include/linux/hdmi.h
> > >> +++ b/include/linux/hdmi.h
> > >> @@ -152,6 +152,16 @@ enum hdmi_content_type {
> > >>  	HDMI_CONTENT_TYPE_GAME,
> > >>  };
> > >>
> > >> +enum hdmi_metadata_type {
> > >> +	HDMI_STATIC_METADATA_TYPE1 = 1,
> > >> +};
> > >> +
> > >> +enum hdmi_eotf {
> > >> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
> > >> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
> > >> +	HDMI_EOTF_SMPTE_ST2084,
> > >> +};
> > >> +
> > >>  struct hdmi_avi_infoframe {
> > >>  	enum hdmi_infoframe_type type;
> > >>  	unsigned char version;
> > >> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > >> index 83cd163..7b6a519 100644
> > >> --- a/include/uapi/drm/drm_mode.h
> > >> +++ b/include/uapi/drm/drm_mode.h
> > >> @@ -630,6 +630,45 @@ struct drm_color_lut {
> > >>  	__u16 reserved;
> > >>  };
> > >>
> > >> +/* HDR Metadata Infoframe as per 861.G spec */ struct
> > >> +hdr_metadata_infoframe {
> > >> +	__u8 eotf;
> > >> +	__u8 metadata_type;
> > >> +	struct {
> > >> +		__u16 x, y;
> > >> +		} display_primaries[3];
> > >> +	struct {
> > >> +		__u16 x, y;
> > >> +		} white_point;
> > >> +	__u16 max_display_mastering_luminance;
> > >> +	__u16 min_display_mastering_luminance;
> > >> +	__u16 max_cll;
> > >> +	__u16 max_fall;
> > >> +};
> > >> +
> > >> +struct hdr_output_metadata {
> > >> +	__u32 metadata_type;
> > >> +	union {
> > >> +		struct hdr_metadata_infoframe hdmi_metadata_type1;
> > >> +	};
> > >> +};
> > >> +
> > >> +/* HDR Metadata as per 861.G spec */
> > >> +struct hdr_static_metadata {
> > >> +	__u8 eotf;
> > >> +	__u8 metadata_type;
> > >> +	__u16 max_cll;
> > >> +	__u16 max_fall;
> > >> +	__u16 min_cll;
> > >> +};
> > >> +
> > >> +struct hdr_sink_metadata {
> > >> +	__u32 metadata_type;
> > >> +	union {
> > >> +		struct hdr_static_metadata hdmi_type1;
> > >> +	};
> > >> +};
> > >
> > >These two structs (hdr_static_metadata and hdr_sink_metadata) should probably not
> > >be part of uapi, unless the metadata is intended to be exposed in a
> > >HDR_SINK_METADATA property.
> > 
> > Hmm yeah, currently we are not planning it to expose that. It was just to help userspace
> > get and parse the EDID and store the details. But yes this doesn't need to be UAPI and
> > userspace should be free to use whatever way they want to parse and store sink metadata.
> > Will move it outside of uapi headers.
> > 
> > 
> > >The commit "drm/i915: Add HLG EOTF" should probably not use a i915 in prefix as it
> > >affects drm core.
> > 
> > Ok, will update this.
> > 
> > >
> > >The blob_ptr should probably be reference counted in duplicate_state/destroy_state
> > >helper similar to [1].
> > >I know too little about drm core in order to know if that is correct, please feel free to
> > >pick/squash if this is correct.
> > 
> > Sure, I will add these changes to the series.
> > 
> > >[1] https://github.com/Kwiboo/linux-
> > >rockchip/commit/5f065ff0bbcca145ee46e9466bb8ca048c7a7b1e
> > >
> > >I have tested this patchset on rockchip / dw-hdmi using dw-hdmi [2] and Kodi [3]
> > >patches with a successful result of enabling HDR mode on the sink.
> > >There is more work needed to get a full 10-bit pipeline for dw-hdmi in order to make
> > >full use of HDR, but for the purpose of enabling HDR on the sink this patchset seems
> > >ready.
> > 
> > Thanks Jonas for your comments and more importantly for testing and enabling with kodi.
> > 
> > I will rebase the series and send out next version. With the changes already in place in kodi branch,
> > we should probably be good for merge.
> 
> Patch 9 is not ready IMO. I don't see any analysis on why it's OK to
> update infoframes without a full modeset. I would just drop that patch
> from the initial set and work on it as a followup.
> 
> Also did someone analysze how the new dynamic HDR metadata fits in with
> the new uapi?

Also I can't see readout + state checker for the HDR infoframe. Am I blind
or is it missing?

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v8 02/10] drm: Parse HDR metadata info from EDID

[Kazlauskas, Nicholas]

On 4/9/19 12:44 PM, Uma Shankar wrote:
> HDR metadata block is introduced in CEA-861.3 spec.
> Parsing the same to get the panel's HDR metadata.
> 
> v2: Rebase and added Ville's POC changes to the patch.
> 
> v3: No Change
> 
> v4: Addressed Shashank's review comments
> 
> v5: Addressed Shashank's comment and added his RB.
> 
> v6: Addressed Jonas Karlman review comments.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 2c22ea4..1fc371b 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2830,6 +2830,7 @@ static int drm_cvt_modes(struct drm_connector *connector,
>   #define VIDEO_BLOCK     0x02
>   #define VENDOR_BLOCK    0x03
>   #define SPEAKER_BLOCK	0x04
> +#define HDR_STATIC_METADATA_BLOCK	0x6
>   #define USE_EXTENDED_TAG 0x07
>   #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
>   #define EXT_VIDEO_DATA_BLOCK_420	0x0E
> @@ -3577,6 +3578,12 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
>   }
>   
>   static int
> +cea_db_payload_len_ext(const u8 *db)
> +{
> +	return (db[0] & 0x1f) - 1;
> +}
> +
> +static int
>   cea_db_extended_tag(const u8 *db)
>   {
>   	return db[1];
> @@ -3812,6 +3819,49 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
>   	mode->clock = clock;
>   }
>   
> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
> +{
> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
> +		return false;
> +
> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
> +		return false;

Shouldn't this just be cea_db_extended_tag(db) != HDR_STATIC_METADATA_BLOCK?

Also, the other functions all check that we're given a valid here here with:

if (!cea_db_payload_len_ext(db))
     return false;

Is there any reason this isn't done here?


> +
> +	return true;
> +}
> +
> +static uint8_t eotf_supported(const u8 *edid_ext)
> +{
> +	return edid_ext[2] &
> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
> +}
> +
> +static uint8_t hdr_metadata_type(const u8 *edid_ext)
> +{
> +	return edid_ext[3] &
> +		BIT(HDMI_STATIC_METADATA_TYPE1);
> +}
> +
> +static void
> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db)
> +{
> +	u16 len;
> +
> +	len = cea_db_payload_len_ext(db);
> +	connector->hdr_sink_metadata.hdmi_type1.eotf = eotf_supported(db);
> +	connector->hdr_sink_metadata.hdmi_type1.metadata_type =
> +					hdr_metadata_type(db);

While metadata_type is assigned here like it should be, it isn't 
assigned to the outer metadata_type in the hdr_sink_metadata. I also 
can't see anything in the other patches that assigns this anywhere.

Shouldn't it also be set here?

> +
> +	if (len >= 4)
> +		connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4];
> +	if (len >= 5)
> +		connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5];
> +	if (len >= 6)
> +		connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6];
> +}
> +
>   static void
>   drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
>   {
> @@ -4439,6 +4489,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>   			drm_parse_y420cmdb_bitmap(connector, db);
>   		if (cea_db_is_vcdb(db))
>   			drm_parse_vcdb(connector, db);
> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
> +			drm_parse_hdr_metadata_block(connector, db);
>   	}
>   }
>   
> 

Nicholas Kazlauskas

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

Re: [v8 01/10] drm: Add HDR source metadata property

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, May 7, 2019 5:40 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: Jonas Karlman <jonas@kwiboo.se>; intel-gfx@lists.freedesktop.org; dri-
>devel@lists.freedesktop.org; seanpaul@chromium.org; emil.l.velikov@gmail.com;
>dcastagna@chromium.org; Lankhorst, Maarten <maarten.lankhorst@intel.com>;
>Syrjala, Ville <ville.syrjala@intel.com>
>Subject: Re: [v8 01/10] drm: Add HDR source metadata property
>
>On Tue, May 07, 2019 at 01:25:42PM +0300, Ville Syrjälä wrote:
>> On Tue, May 07, 2019 at 09:03:45AM +0000, Shankar, Uma wrote:
>> >
>> >
>> > >-----Original Message-----
>> > >From: Jonas Karlman [mailto:jonas@kwiboo.se]
>> > >Sent: Saturday, May 4, 2019 3:48 PM
>> > >To: Shankar, Uma <uma.shankar@intel.com>;
>> > >intel-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org
>> > >Cc: dcastagna@chromium.org; emil.l.velikov@gmail.com;
>> > >seanpaul@chromium.org; Syrjala, Ville <ville.syrjala@intel.com>;
>> > >Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> > >Subject: Re: [v8 01/10] drm: Add HDR source metadata property
>> > >
>> > >On 2019-04-09 18:44, Uma Shankar wrote:
>> > >> This patch adds a blob property to get HDR metadata information
>> > >> from userspace. This will be send as part of AVI Infoframe to panel.
>> > >>
>> > >> It also implements get() and set() functions for HDR output
>> > >> metadata property.The blob data is received from userspace and
>> > >> saved in connector state, the same is returned as blob in get
>> > >> property call to userspace.
>> > >>
>> > >> v2: Rebase and modified the metadata structure elements as per
>> > >> Ville's POC changes.
>> > >>
>> > >> v3: No Change
>> > >>
>> > >> v4: Addressed Shashank's review comments
>> > >>
>> > >> v5: Rebase.
>> > >>
>> > >> v6: Addressed Brian Starkey's review comments, defined new
>> > >> structure with header for dynamic metadata scalability.
>> > >> Merge get/set property functions for metadata in this patch.
>> > >>
>> > >> v7: Addressed Jonas Karlman review comments and defined separate
>> > >> structure for infoframe to better align with CTA 861.G spec.
>> > >> Added Shashank's RB.
>> > >>
>> > >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> > >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>> > >> ---
>> > >>  drivers/gpu/drm/drm_atomic.c      |  2 ++
>> > >>  drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
>> > >>  drivers/gpu/drm/drm_connector.c   |  6 ++++++
>> > >>  include/drm/drm_connector.h       | 11 +++++++++++
>> > >>  include/drm/drm_mode_config.h     |  6 ++++++
>> > >>  include/linux/hdmi.h              | 10 ++++++++++
>> > >>  include/uapi/drm/drm_mode.h       | 39
>> > >+++++++++++++++++++++++++++++++++++++++
>> > >>  7 files changed, 87 insertions(+)
>> > >>
>> > >> diff --git a/drivers/gpu/drm/drm_atomic.c
>> > >> b/drivers/gpu/drm/drm_atomic.c index 5eb4013..8b9c126 100644
>> > >> --- a/drivers/gpu/drm/drm_atomic.c
>> > >> +++ b/drivers/gpu/drm/drm_atomic.c
>> > >> @@ -881,6 +881,8 @@ static void
>> > >> drm_atomic_connector_print_state(struct drm_printer *p,
>> > >>
>> > >>  	drm_printf(p, "connector[%u]: %s\n", connector->base.id,
>> > >>connector- name);
>> > >>  	drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
>> > >> "(null)");
>> > >> +	drm_printf(p, "\thdr_metadata_changed=%d\n",
>> > >> +		   state->hdr_metadata_changed);
>> > >>
>> > >>  	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>> > >>  		if (state->writeback_job && state->writeback_job->fb) diff
>> > >> --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> > >> b/drivers/gpu/drm/drm_atomic_uapi.c
>> > >> index ea797d4..6d0d161 100644
>> > >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> > >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> > >> @@ -673,6 +673,8 @@ static int
>> > >> drm_atomic_connector_set_property(struct drm_connector *connector,  {
>> > >>  	struct drm_device *dev = connector->dev;
>> > >>  	struct drm_mode_config *config = &dev->mode_config;
>> > >> +	bool replaced = false;
>> > >> +	int ret;
>> > >>
>> > >>  	if (property == config->prop_crtc_id) {
>> > >>  		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val); @@
>> > >> -721,6
>> > >> +723,14 @@ static int drm_atomic_connector_set_property(struct
>> > >> +drm_connector
>> > >*connector,
>> > >>  		 */
>> > >>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
>> > >>  			state->link_status = val;
>> > >> +	} else if (property == config->hdr_output_metadata_property) {
>> > >> +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> > >> +				&state->hdr_output_metadata_blob_ptr,
>> > >> +				val,
>> > >> +				-1, sizeof(struct hdr_output_metadata),
>> > >> +				&replaced);
>> > >> +		state->hdr_metadata_changed |= replaced;
>> > >> +		return ret;
>> > >>  	} else if (property == config->aspect_ratio_property) {
>> > >>  		state->picture_aspect_ratio = val;
>> > >>  	} else if (property == config->content_type_property) { @@
>> > >> -807,6
>> > >> +817,9 @@ static int drm_atomic_connector_set_property(struct
>> > >> +drm_connector
>> > >*connector,
>> > >>  		*val = state->colorspace;
>> > >>  	} else if (property == connector->scaling_mode_property) {
>> > >>  		*val = state->scaling_mode;
>> > >> +	} else if (property == config->hdr_output_metadata_property) {
>> > >> +		*val = (state->hdr_output_metadata_blob_ptr) ?
>> > >> +			state->hdr_output_metadata_blob_ptr->base.id : 0;
>> > >>  	} else if (property == connector->content_protection_property) {
>> > >>  		*val = state->content_protection;
>> > >>  	} else if (property == config->writeback_fb_id_property) { diff
>> > >> --git a/drivers/gpu/drm/drm_connector.c
>> > >> b/drivers/gpu/drm/drm_connector.c index 2355124..0bdae90 100644
>> > >> --- a/drivers/gpu/drm/drm_connector.c
>> > >> +++ b/drivers/gpu/drm/drm_connector.c
>> > >> @@ -1058,6 +1058,12 @@ int
>> > >> drm_connector_create_standard_properties(struct
>> > >drm_device *dev)
>> > >>  		return -ENOMEM;
>> > >>  	dev->mode_config.non_desktop_property = prop;
>> > >>
>> > >> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
>> > >> +				   "HDR_OUTPUT_METADATA", 0);
>> > >> +	if (!prop)
>> > >> +		return -ENOMEM;
>> > >> +	dev->mode_config.hdr_output_metadata_property = prop;
>> > >> +
>> > >>  	return 0;
>> > >>  }
>> > >>
>> > >> diff --git a/include/drm/drm_connector.h
>> > >> b/include/drm/drm_connector.h index 02a1312..5343f60 100644
>> > >> --- a/include/drm/drm_connector.h
>> > >> +++ b/include/drm/drm_connector.h
>> > >> @@ -599,6 +599,13 @@ struct drm_connector_state {
>> > >>  	 * and the connector bpc limitations obtained from edid.
>> > >>  	 */
>> > >>  	u8 max_bpc;
>> > >> +
>> > >> +	/**
>> > >> +	 * @metadata_blob_ptr:
>> > >> +	 * DRM blob property for HDR output metadata
>> > >> +	 */
>> > >> +	struct drm_property_blob *hdr_output_metadata_blob_ptr;
>> > >> +	u8 hdr_metadata_changed : 1;
>> > >>  };
>> > >>
>> > >>  /**
>> > >> @@ -1239,6 +1246,10 @@ struct drm_connector {
>> > >>  	 * &drm_mode_config.connector_free_work.
>> > >>  	 */
>> > >>  	struct llist_node free_node;
>> > >> +
>> > >> +	/* HDR metdata */
>> > >> +	struct hdr_output_metadata hdr_output_metadata;
>> > >> +	struct hdr_sink_metadata hdr_sink_metadata;
>> > >>  };
>> > >>
>> > >>  #define obj_to_connector(x) container_of(x, struct
>> > >> drm_connector,
>> > >> base) diff --git a/include/drm/drm_mode_config.h
>> > >> b/include/drm/drm_mode_config.h index 7f60e8e..ef2656b 100644
>> > >> --- a/include/drm/drm_mode_config.h
>> > >> +++ b/include/drm/drm_mode_config.h
>> > >> @@ -836,6 +836,12 @@ struct drm_mode_config {
>> > >>  	 */
>> > >>  	struct drm_property *writeback_out_fence_ptr_property;
>> > >>
>> > >> +	/*
>> > >> +	 * hdr_metadata_property: Connector property containing hdr metatda
>> > >> +	 * This will be provided by userspace compositors based on HDR content
>> > >> +	 */
>> > >> +	struct drm_property *hdr_output_metadata_property;
>> > >> +
>> > >>  	/* dumb ioctl parameters */
>> > >>  	uint32_t preferred_depth, prefer_shadow;
>> > >>
>> > >> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index
>> > >> 927ad64..a065194 100644
>> > >> --- a/include/linux/hdmi.h
>> > >> +++ b/include/linux/hdmi.h
>> > >> @@ -152,6 +152,16 @@ enum hdmi_content_type {
>> > >>  	HDMI_CONTENT_TYPE_GAME,
>> > >>  };
>> > >>
>> > >> +enum hdmi_metadata_type {
>> > >> +	HDMI_STATIC_METADATA_TYPE1 = 1, };
>> > >> +
>> > >> +enum hdmi_eotf {
>> > >> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
>> > >> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
>> > >> +	HDMI_EOTF_SMPTE_ST2084,
>> > >> +};
>> > >> +
>> > >>  struct hdmi_avi_infoframe {
>> > >>  	enum hdmi_infoframe_type type;
>> > >>  	unsigned char version;
>> > >> diff --git a/include/uapi/drm/drm_mode.h
>> > >> b/include/uapi/drm/drm_mode.h index 83cd163..7b6a519 100644
>> > >> --- a/include/uapi/drm/drm_mode.h
>> > >> +++ b/include/uapi/drm/drm_mode.h
>> > >> @@ -630,6 +630,45 @@ struct drm_color_lut {
>> > >>  	__u16 reserved;
>> > >>  };
>> > >>
>> > >> +/* HDR Metadata Infoframe as per 861.G spec */ struct
>> > >> +hdr_metadata_infoframe {
>> > >> +	__u8 eotf;
>> > >> +	__u8 metadata_type;
>> > >> +	struct {
>> > >> +		__u16 x, y;
>> > >> +		} display_primaries[3];
>> > >> +	struct {
>> > >> +		__u16 x, y;
>> > >> +		} white_point;
>> > >> +	__u16 max_display_mastering_luminance;
>> > >> +	__u16 min_display_mastering_luminance;
>> > >> +	__u16 max_cll;
>> > >> +	__u16 max_fall;
>> > >> +};
>> > >> +
>> > >> +struct hdr_output_metadata {
>> > >> +	__u32 metadata_type;
>> > >> +	union {
>> > >> +		struct hdr_metadata_infoframe hdmi_metadata_type1;
>> > >> +	};
>> > >> +};
>> > >> +
>> > >> +/* HDR Metadata as per 861.G spec */ struct hdr_static_metadata
>> > >> +{
>> > >> +	__u8 eotf;
>> > >> +	__u8 metadata_type;
>> > >> +	__u16 max_cll;
>> > >> +	__u16 max_fall;
>> > >> +	__u16 min_cll;
>> > >> +};
>> > >> +
>> > >> +struct hdr_sink_metadata {
>> > >> +	__u32 metadata_type;
>> > >> +	union {
>> > >> +		struct hdr_static_metadata hdmi_type1;
>> > >> +	};
>> > >> +};
>> > >
>> > >These two structs (hdr_static_metadata and hdr_sink_metadata)
>> > >should probably not be part of uapi, unless the metadata is
>> > >intended to be exposed in a HDR_SINK_METADATA property.
>> >
>> > Hmm yeah, currently we are not planning it to expose that. It was
>> > just to help userspace get and parse the EDID and store the details.
>> > But yes this doesn't need to be UAPI and userspace should be free to use whatever
>way they want to parse and store sink metadata.
>> > Will move it outside of uapi headers.
>> >
>> >
>> > >The commit "drm/i915: Add HLG EOTF" should probably not use a i915
>> > >in prefix as it affects drm core.
>> >
>> > Ok, will update this.
>> >
>> > >
>> > >The blob_ptr should probably be reference counted in
>> > >duplicate_state/destroy_state helper similar to [1].
>> > >I know too little about drm core in order to know if that is
>> > >correct, please feel free to pick/squash if this is correct.
>> >
>> > Sure, I will add these changes to the series.
>> >
>> > >[1] https://github.com/Kwiboo/linux-
>> > >rockchip/commit/5f065ff0bbcca145ee46e9466bb8ca048c7a7b1e
>> > >
>> > >I have tested this patchset on rockchip / dw-hdmi using dw-hdmi [2]
>> > >and Kodi [3] patches with a successful result of enabling HDR mode on the sink.
>> > >There is more work needed to get a full 10-bit pipeline for dw-hdmi
>> > >in order to make full use of HDR, but for the purpose of enabling
>> > >HDR on the sink this patchset seems ready.
>> >
>> > Thanks Jonas for your comments and more importantly for testing and enabling
>with kodi.
>> >
>> > I will rebase the series and send out next version. With the changes
>> > already in place in kodi branch, we should probably be good for merge.
>>
>> Patch 9 is not ready IMO. I don't see any analysis on why it's OK to
>> update infoframes without a full modeset. I would just drop that patch
>> from the initial set and work on it as a followup.

If the timings are not changed, driver goes for fastest and if we don't have
DRM infoframe handled there, we may not be able to send metadata to sink.
Hence its needed here, also it may help later for dynamic metadata cases as well
where we don't need modeset/blanking and just update metadata via infoframe.

>> Also did someone analysze how the new dynamic HDR metadata fits in
>> with the new uapi?

Currently, we have added the metadata as union so we should be able to extend
it later for dynamic metadata. 

>Also I can't see readout + state checker for the HDR infoframe. Am I blind or is it
>missing?

Yeah it's not there, will add the readout and state check.

Thanks Ville for your comments and inputs. Will send out the next version with those fixed.

Regards,
Uma Shankar

>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

RE: [v8 08/10] drm/i915:Enabled Modeset when HDR Infoframe changes

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, May 7, 2019 5:32 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Lankhorst,
>Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>; emil.l.velikov@gmail.com;
>brian.starkey@arm.com; dcastagna@chromium.org; seanpaul@chromium.org;
>Roper, Matthew D <matthew.d.roper@intel.com>; jonas@kwiboo.se
>Subject: Re: [v8 08/10] drm/i915:Enabled Modeset when HDR Infoframe changes
>
>On Tue, Apr 09, 2019 at 10:14:42PM +0530, Uma Shankar wrote:
>> This patch enables modeset whenever HDR metadata needs to be updated
>> to sink.
>>
>> v2: Addressed Shashank's review comments.
>>
>> v3: Added Shashank's RB.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_atomic.c | 14 +++++++++++++-
>>  drivers/gpu/drm/i915/intel_hdmi.c   | 26 ++++++++++++++++++++++++++
>>  2 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index 8c8fae3..e8b5f84 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -104,6 +104,16 @@ int intel_digital_connector_atomic_set_property(struct
>drm_connector *connector,
>>  	return -EINVAL;
>>  }
>>
>> +static bool blob_equal(const struct drm_property_blob *a,
>> +		       const struct drm_property_blob *b) {
>> +	if (a && b)
>> +		return a->length == b->length &&
>> +			!memcmp(a->data, b->data, a->length);
>> +
>> +	return !a == !b;
>> +}
>
>I have a feeling the memcmp() is overkill. We could just check for whether the blob is
>the same or not. If userspace is an idiot and creates a new blob with identical content
>so be it.

I feel it's good to have such check in kernel and avoid extra infoframe programming.
More like how we suppress modeset request if same mode is requested as part of
modeset.  Hope its ok.

>> +
>>  int intel_digital_connector_atomic_check(struct drm_connector *conn,
>>  					 struct drm_connector_state *new_state)  {
>@@ -131,7 +141,9 @@
>> int intel_digital_connector_atomic_check(struct drm_connector *conn,
>>  	    new_conn_state->base.colorspace != old_conn_state->base.colorspace ||
>>  	    new_conn_state->base.picture_aspect_ratio != old_conn_state-
>>base.picture_aspect_ratio ||
>>  	    new_conn_state->base.content_type != old_conn_state-
>>base.content_type ||
>> -	    new_conn_state->base.scaling_mode != old_conn_state-
>>base.scaling_mode)
>> +	    new_conn_state->base.scaling_mode != old_conn_state-
>>base.scaling_mode ||
>> +	    !blob_equal(new_conn_state->base.hdr_output_metadata_blob_ptr,
>> +			old_conn_state->base.hdr_output_metadata_blob_ptr))
>>  		crtc_state->mode_changed = true;
>>
>>  	return 0;
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 0ecfda0..85333a7 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -799,6 +799,20 @@ void intel_read_infoframe(struct intel_encoder *encoder,
>>  	return true;
>>  }
>>
>> +static bool is_eotf_supported(u8 output_eotf, u8 sink_eotf) {
>> +	if (output_eotf == 0)
>> +		return (sink_eotf & (1 << 0));
>> +	if (output_eotf == 1)
>> +		return (sink_eotf & (1 << 1));
>> +	if (output_eotf == 2)
>> +		return (sink_eotf & (1 << 2));
>> +	if (output_eotf == 3)
>> +		return (sink_eotf & (1 << 3));
>> +
>> +	return false;
>
>return sink_eotf & BIT(output_eotf);

Nice suggestion. Will update this.

Regards,
Uma Shankar

>> +}
>> +
>>  static bool
>>  intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
>>  				 struct intel_crtc_state *crtc_state, @@ -806,11
>+820,23 @@ void
>> intel_read_infoframe(struct intel_encoder *encoder,  {
>>  	struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm;
>>  	struct hdr_output_metadata *hdr_metadata;
>> +	struct drm_connector *connector = conn_state->connector;
>>  	int ret;
>>
>> +	if (!conn_state->hdr_output_metadata_blob_ptr ||
>> +	    conn_state->hdr_output_metadata_blob_ptr->length == 0)
>> +		return true;
>> +
>>  	hdr_metadata = (struct hdr_output_metadata *)
>>  			conn_state->hdr_output_metadata_blob_ptr->data;
>>
>> +	/* Sink EOTF is Bit map while infoframe is absolute values */
>> +	if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf,
>> +			       connector->hdr_sink_metadata.hdmi_type1.eotf)) {
>> +		DRM_ERROR("EOTF Not Supported\n");
>> +		return true;
>> +	}
>> +
>>  	ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
>>  	if (ret < 0) {
>>  		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
>> --
>> 1.9.1
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel