[v2 00/14] Add HDR Metadata Parsing and handling in DRM layer

[v2 00/14] 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 we should be getting
   the initial version pretty soon for 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.

Note: It took a while to float this version as there was no userspace
consumer for it and dependency on full userspace stack. However, things
are falling in place now. Media driver and VAAPI changes for HDR are
already out, with compositors changes also expected to land soon.

Uma Shankar (10):
  drm: Add HDR source metadata property
  drm: Add CEA extended tag blocks and HDR bitfield macros
  drm: Parse HDR metadata info from EDID
  drm: Parse Colorimetry data block from EDID
  drm/i915: Attach HDR metadata property to connector
  drm: Add HDR capability field to plane structure
  drm: Implement HDR source metadata set and get property handling
  drm: Enable HDR infoframe support
  drm/i915: Write HDR infoframe and send to panel
  drm/i915:Enabled Modeset when HDR Infoframe changes

Ville Syrjälä (4):
  drm/i915: [DO NOT MERGE] hack for glk board outputs
  drm/i915: Add HLG EOTF
  drm/i915: Enable infoframes on GLK+ for HDR
  drivers/video: Constantify function argument for HDMI infoframe log

 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          | 143 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h     |   4 +
 drivers/gpu/drm/i915/intel_atomic.c |  15 +++-
 drivers/gpu/drm/i915/intel_bios.c   |   7 ++
 drivers/gpu/drm/i915/intel_hdmi.c   |  43 ++++++++++-
 drivers/video/hdmi.c                | 129 ++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h         |  12 +++
 include/drm/drm_edid.h              |   4 +
 include/drm/drm_mode_config.h       |   6 ++
 include/drm/drm_plane.h             |   3 +
 include/linux/hdmi.h                |  33 +++++++++
 include/uapi/drm/drm_mode.h         |  16 ++++
 15 files changed, 433 insertions(+), 3 deletions(-)

-- 
1.9.1

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

[v2 01/14] 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.

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

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_connector.c |  6 ++++++
 include/drm/drm_connector.h     | 10 ++++++++++
 include/drm/drm_mode_config.h   |  6 ++++++
 include/linux/hdmi.h            | 10 ++++++++++
 include/uapi/drm/drm_mode.h     | 16 ++++++++++++++++
 5 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da8ae80..361fcda 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1027,6 +1027,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_SOURCE_METADATA", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.hdr_source_metadata_property = prop;
+
 	return 0;
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 9be2181..2ee45dc 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -520,6 +520,13 @@ struct drm_connector_state {
 	 * and the connector bpc limitations obtained from edid.
 	 */
 	u8 max_bpc;
+
+	/**
+	 * @metadata_blob_ptr:
+	 * DRM blob property for HDR metadata
+	 */
+	struct drm_property_blob *hdr_source_metadata_blob_ptr;
+	u8 hdr_metadata_changed : 1;
 };
 
 /**
@@ -1154,6 +1161,9 @@ struct drm_connector {
 	 * &drm_mode_config.connector_free_work.
 	 */
 	struct llist_node free_node;
+
+	/* HDR metdata */
+	struct hdr_static_metadata hdr_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 69ccd27..4b3211b 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_source_metadata_property;
+
 	/* dumb ioctl parameters */
 	uint32_t preferred_depth, prefer_shadow;
 
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index d2bacf5..bed3e08 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -137,6 +137,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 a439c2e..5012af2 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -630,6 +630,22 @@ struct drm_color_lut {
 	__u16 reserved;
 };
 
+/* HDR Metadata */
+struct hdr_static_metadata {
+	uint8_t eotf;
+	uint8_t metadata_type;
+	struct {
+		uint16_t x, y;
+		} display_primaries[3];
+	struct {
+		uint16_t x, y;
+		} white_point;
+	uint16_t max_mastering_display_luminance;
+	uint16_t min_mastering_display_luminance;
+	uint16_t max_fall;
+	uint16_t max_cll;
+};
+
 #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

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

[v2 02/14] drm: Add CEA extended tag blocks and HDR bitfield macros

[Uma Shankar]

Add bit field and macro for extended tag in CEA block. Also,
declare macros for HDR metadata block.

v2: Rebase

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b506e36..106fd38 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2843,6 +2843,22 @@ static int drm_cvt_modes(struct drm_connector *connector,
 #define EDID_CEA_YCRCB422	(1 << 4)
 #define EDID_CEA_VCDB_QS	(1 << 6)
 
+#define DATA_BLOCK_EXTENDED_TAG		0x07
+#define VIDEO_CAPABILITY_DATA_BLOCK	0x0
+#define VSVD_DATA_BLOCK			0x1
+#define COLORIMETRY_DATA_BLOCK		0x5
+#define HDR_STATIC_METADATA_BLOCK	0x6
+
+/* HDR Metadata Block: Bit fields */
+#define SUPPORTED_EOTF_MASK            0x3f
+#define TRADITIONAL_GAMMA_SDR          (0x1 << 0)
+#define TRADITIONAL_GAMMA_HDR          (0x1 << 1)
+#define SMPTE_ST2084                   (0x1 << 2)
+#define FUTURE_EOTF                    (0x1 << 3)
+#define RESERVED_EOTF                  (0x3 << 4)
+
+#define STATIC_METADATA_TYPE1          (0x1 << 0)
+
 /*
  * Search EDID for CEA extension block.
  */
-- 
1.9.1

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

[v2 03/14] 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.

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 106fd38..d12b74e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3818,6 +3818,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) != DATA_BLOCK_EXTENDED_TAG)
+		return false;
+
+	if (db[1] != HDR_STATIC_METADATA_BLOCK)
+		return false;
+
+	return true;
+}
+
+static uint16_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 uint16_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)
+{
+	uint16_t len;
+
+	len = cea_db_payload_len(db);
+	connector->hdr_metadata.eotf = eotf_supported(db);
+	connector->hdr_metadata.metadata_type = hdr_metadata_type(db);
+
+	if (len >= 5)
+		connector->hdr_metadata.max_fall = db[5];
+	if (len >= 4)
+		connector->hdr_metadata.max_cll = db[4];
+}
+
 static void
 drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
 {
@@ -4468,6 +4511,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 			drm_parse_hdmi_forum_vsdb(connector, db);
 		if (cea_db_is_y420cmdb(db))
 			drm_parse_y420cmdb_bitmap(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

[v2 04/14] drm: Parse Colorimetry data block from EDID

[Uma Shankar]

CEA 861.3 spec adds colorimetry data block for HDMI.
Parsing the block to get the colorimetry data from
panel.

v2: Rebase

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_edid.c  | 24 ++++++++++++++++++++++++
 include/drm/drm_connector.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index d12b74e..344d8c1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3818,6 +3818,28 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
 	mode->clock = clock;
 }
 
+static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db)
+{
+	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
+		return false;
+
+	if (db[1] != COLORIMETRY_DATA_BLOCK)
+		return false;
+
+	return true;
+}
+
+static void
+drm_parse_colorimetry_data_block(struct drm_connector *connector, const u8 *db)
+{
+	struct drm_hdmi_info *info = &connector->display_info.hdmi;
+	uint16_t len;
+
+	len = cea_db_payload_len(db);
+	info->colorimetry = db[2];
+}
+
+
 static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
 {
 	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
@@ -4513,6 +4535,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 			drm_parse_y420cmdb_bitmap(connector, db);
 		if (cea_db_is_hdmi_hdr_metadata_block(db))
 			drm_parse_hdr_metadata_block(connector, db);
+		if (cea_db_is_hdmi_colorimetry_data_block(db))
+			drm_parse_colorimetry_data_block(connector, db);
 	}
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 2ee45dc..90ce364 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -206,6 +206,8 @@ struct drm_hdmi_info {
 
 	/** @y420_dc_modes: bitmap of deep color support index */
 	u8 y420_dc_modes;
+
+	u8 colorimetry;
 };
 
 /**
-- 
1.9.1

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

[v2 05/14] drm/i915: Attach HDR metadata property to connector

[Uma Shankar]

Attach HDR metadata property to connector object.

v2: Rebase

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 07e803a..8a1e5cb 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2154,6 +2154,8 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
 	intel_attach_aspect_ratio_property(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_source_metadata_property, 0);
 
 	if (!HAS_GMCH_DISPLAY(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

[v2 06/14] drm: Add HDR capability field to plane structure

[Uma Shankar]

Hardware may have HDR capability on certain plane
engines. Enabling the same in drm plane structure
so that this can be communicated to user space.

Each drm driver should set this flag to true for planes
which support HDR.

v2: Rebase

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 include/drm/drm_plane.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 6078c70..206eefc 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -638,6 +638,9 @@ struct drm_plane {
 	/** @type: Type of plane, see &enum drm_plane_type for details. */
 	enum drm_plane_type type;
 
+	/* Value of true:1 means HDR is supported */
+	bool hdr_supported;
+
 	/**
 	 * @index: Position inside the mode_config.list, can be used as an array
 	 * index. It is invariant over the lifetime of the plane.
-- 
1.9.1

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

[v2 07/14] drm: Implement HDR source metadata set and get property handling

[Uma Shankar]

HDR source metadata set and get property implemented in this
patch. 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 added Ville's POC changes

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_atomic.c      |  2 ++
 drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5eb4013..4e71c6b 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 c408898..b721b12 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -686,6 +686,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);
@@ -734,6 +736,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_source_metadata_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+				&state->hdr_source_metadata_blob_ptr,
+				val,
+				-1, sizeof(struct hdr_static_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) {
@@ -816,6 +826,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 		*val = state->content_type;
 	} else if (property == connector->scaling_mode_property) {
 		*val = state->scaling_mode;
+	} else if (property == config->hdr_source_metadata_property) {
+		*val = (state->hdr_source_metadata_blob_ptr) ?
+			state->hdr_source_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) {
-- 
1.9.1

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

[v2 08/14] 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.

v2: Rebase and added Ville's POC changes.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_edid.c |  58 ++++++++++++++++++++
 drivers/video/hdmi.c       | 129 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     |   4 ++
 include/linux/hdmi.h       |  22 ++++++++
 4 files changed, 213 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 344d8c1..5a7fc9b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4916,6 +4916,64 @@ void drm_set_preferred_mode(struct drm_connector *connector,
 EXPORT_SYMBOL(drm_set_preferred_mode);
 
 /**
+ * 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,
+				    void *hdr_metadata)
+{
+	struct hdr_static_metadata *hdr_source_metadata;
+	int err, i;
+
+	if (!frame || !hdr_metadata)
+		return -EINVAL;
+
+	err = hdmi_drm_infoframe_init(frame);
+	if (err < 0)
+		return err;
+
+	DRM_DEBUG_KMS("type = %x\n", frame->type);
+
+	hdr_source_metadata = (struct hdr_static_metadata *)hdr_metadata;
+
+	frame->length = sizeof(struct hdr_static_metadata);
+
+
+	frame->eotf = hdr_source_metadata->eotf;
+	frame->metadata_type = hdr_source_metadata->metadata_type;
+
+	for (i = 0; i < 3; i++) {
+		frame->display_primaries[i].x =
+			hdr_source_metadata->display_primaries[i].x;
+		frame->display_primaries[i].y =
+			hdr_source_metadata->display_primaries[i].y;
+	}
+
+	frame->white_point.x = hdr_source_metadata->white_point.x;
+	frame->white_point.y = hdr_source_metadata->white_point.y;
+
+	frame->max_mastering_display_luminance =
+		hdr_source_metadata->max_mastering_display_luminance;
+	frame->min_mastering_display_luminance =
+		hdr_source_metadata->min_mastering_display_luminance;
+
+	frame->max_cll = hdr_source_metadata->max_cll;
+	frame->max_fall = hdr_source_metadata->max_fall;
+
+	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..0937c8c 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -650,6 +650,93 @@ 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);
+
+/**
+ * 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(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 & 0xff;
+		*ptr++ = frame->display_primaries[i].x >> 8;
+		*ptr++ = frame->display_primaries[i].y & 0xff;
+		*ptr++ = frame->display_primaries[i].y >> 8;
+	}
+
+	*ptr++ = frame->white_point.x & 0xff;
+	*ptr++ = frame->white_point.x >> 8;
+
+	*ptr++ = frame->white_point.y & 0xff;
+	*ptr++ = frame->white_point.y >> 8;
+
+	*ptr++ = frame->max_mastering_display_luminance & 0xff;
+	*ptr++ = frame->max_mastering_display_luminance >> 8;
+
+	*ptr++ = frame->min_mastering_display_luminance & 0xff;
+	*ptr++ = frame->min_mastering_display_luminance >> 8;
+
+	*ptr++ = frame->max_cll & 0xff;
+	*ptr++ = frame->max_cll >> 8;
+
+	*ptr++ = frame->max_fall & 0xff;
+	*ptr++ = frame->max_fall >> 8;
+
+	hdmi_infoframe_set_checksum(buffer, length);
+
+	return length;
+}
+
 /*
  * hdmi_vendor_any_infoframe_check() - check a vendor infoframe
  */
@@ -806,6 +893,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 +928,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 +1376,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,
+                                  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_mastering_display_luminance: %d\n",
+			frame->max_mastering_display_luminance);
+	hdmi_log("min_mastering_display_luminance: %d\n",
+			frame->min_mastering_display_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 +1498,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 e3c4048..0511607 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -365,6 +365,10 @@ int drm_av_sync_delay(struct drm_connector *connector,
 				   bool rgb_quant_range_selectable,
 				   bool is_hdmi2_sink);
 
+int
+drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
+				    void *hdr_source_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 bed3e08..ce00e1e 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -32,6 +32,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
@@ -170,12 +171,32 @@ struct hdmi_avi_infoframe {
 	unsigned short right_bar;
 };
 
+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 {
+		uint16_t x, y;
+	} display_primaries[3];
+	struct {
+		uint16_t x, y;
+	} white_point;
+	uint16_t max_mastering_display_luminance;
+	uint16_t min_mastering_display_luminance;
+	uint16_t max_fall;
+	uint16_t max_cll;
+	uint16_t min_cll;
+};
+
 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);
 
 enum hdmi_spd_sdi {
 	HDMI_SPD_SDI_UNKNOWN,
@@ -350,6 +371,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

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

[v2 09/14] 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

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

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 8a1e5cb..6286c4a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -475,6 +475,29 @@ static void intel_write_infoframe(struct intel_encoder *encoder,
 					frame->any.type, buffer, len);
 }
 
+/* Set Dynamic Range and Mastering Infoframe */
+static void intel_hdmi_set_drm_infoframe(struct drm_encoder *encoder,
+					 const struct intel_crtc_state
+					 *crtc_state,
+					 const struct drm_connector_state
+					 *conn_state)
+{
+	union hdmi_infoframe frame;
+	struct hdr_static_metadata *hdr_metadata;
+	int ret;
+
+	hdr_metadata = (struct hdr_static_metadata *)
+		conn_state->hdr_source_metadata_blob_ptr->data;
+
+	ret = drm_hdmi_infoframe_set_hdr_metadata(&frame.drm, hdr_metadata);
+	if (ret < 0) {
+		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
+		return;
+	}
+
+	intel_write_infoframe(encoder, crtc_state, &frame);
+}
+
 static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
 					 const struct intel_crtc_state *crtc_state,
 					 const struct drm_connector_state *conn_state)
@@ -883,6 +906,10 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 	intel_hdmi_set_avi_infoframe(encoder, crtc_state, conn_state);
 	intel_hdmi_set_spd_infoframe(encoder, crtc_state);
 	intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state);
+
+	/* Set Dynamic Range and Mastering Infoframe if supported and changed */
+	if (conn_state->hdr_metadata_changed)
+		intel_hdmi_set_drm_infoframe(encoder, crtc_state, conn_state);
 }
 
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable)
-- 
1.9.1

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

[v2 10/14] drm/i915: [DO NOT MERGE] hack for glk board outputs

[Uma Shankar]

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

This is to limit PORT C on GLK to drive only
HDMI. Not sure if this is mandatory, this is just
to test HDR on GLK HDMI.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 6d3e026..e1c48ad 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1381,6 +1381,13 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 		is_hdmi = false;
 	}
 
+	if (IS_GEMINILAKE(dev_priv) && port == PORT_C) {
+		is_hdmi = true;
+		is_dvi = true;
+		is_dp = false;
+		is_edp = false;
+	}
+
 	info->supports_dvi = is_dvi;
 	info->supports_hdmi = is_hdmi;
 	info->supports_dp = is_dp;
-- 
1.9.1

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

[v2 11/14] 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.

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5a7fc9b..fa86494 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3857,8 +3857,8 @@ static uint16_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 uint16_t hdr_metadata_type(const u8 *edid_ext)
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index ce00e1e..b5346c3 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -146,6 +146,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

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

[v2 12/14] 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.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |  4 ++++
 drivers/gpu/drm/i915/intel_hdmi.c | 12 +++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a7d605..6f44d02 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4591,6 +4591,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
@@ -8015,6 +8016,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
@@ -8028,6 +8030,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
@@ -8052,6 +8055,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_GCP(trans)		_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_GCP_A)
 #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 6286c4a..5c2e3c9 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -123,6 +123,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;
@@ -146,6 +148,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;
@@ -432,7 +436,8 @@ static bool hsw_infoframe_enabled(struct intel_encoder *encoder,
 
 	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);
+		      VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
+		      VIDEO_DIP_ENABLE_DRM_GLK);
 }
 
 /*
@@ -889,7 +894,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);
@@ -908,7 +914,7 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 	intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state);
 
 	/* Set Dynamic Range and Mastering Infoframe if supported and changed */
-	if (conn_state->hdr_metadata_changed)
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
 		intel_hdmi_set_drm_infoframe(encoder, crtc_state, conn_state);
 }
 
-- 
1.9.1

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

[v2 13/14] drm/i915:Enabled Modeset when HDR Infoframe changes

[Uma Shankar]

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

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c | 15 ++++++++++++++-
 drivers/gpu/drm/i915/intel_hdmi.c   |  4 ++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 8cb02f2..72c3fcb 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -101,6 +101,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)
 {
@@ -127,7 +137,10 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
 	    new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
 	    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_source_metadata_blob_ptr,
+			old_conn_state->base.hdr_source_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 5c2e3c9..29b4218 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -491,6 +491,10 @@ static void intel_hdmi_set_drm_infoframe(struct drm_encoder *encoder,
 	struct hdr_static_metadata *hdr_metadata;
 	int ret;
 
+	if (!conn_state->hdr_source_metadata_blob_ptr ||
+		conn_state->hdr_source_metadata_blob_ptr->length == 0)
+		return;
+
 	hdr_metadata = (struct hdr_static_metadata *)
 		conn_state->hdr_source_metadata_blob_ptr->data;
 
-- 
1.9.1

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

[v2 14/14] drivers/video: Constantify function argument for HDMI infoframe log

[Uma Shankar]

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

Function argument for hdmi_drm_infoframe_log is made constant.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/video/hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 0937c8c..7ab8086 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -1383,8 +1383,8 @@ static void hdmi_audio_infoframe_log(const char *level,
  * @frame: HDMI DRM infoframe
  */
 static void hdmi_drm_infoframe_log(const char *level,
-                                  struct device *dev,
-                                  struct hdmi_drm_infoframe *frame)
+				   struct device *dev,
+				   const struct hdmi_drm_infoframe *frame)
 {
 	int i;
 
-- 
1.9.1

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

Re: [v2 01/14] drm: Add HDR source metadata property

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, 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.
>
> v2: Rebase and modified the metadata structure elements
> as per Ville's POC changes.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/drm_connector.c |  6 ++++++
>   include/drm/drm_connector.h     | 10 ++++++++++
>   include/drm/drm_mode_config.h   |  6 ++++++
>   include/linux/hdmi.h            | 10 ++++++++++
>   include/uapi/drm/drm_mode.h     | 16 ++++++++++++++++
>   5 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index da8ae80..361fcda 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1027,6 +1027,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_SOURCE_METADATA", 0);
I see that the compositor would be using this blob to setup the output 
HDR curve post blending, which would be in most of cases, sink HDR. So 
we should ideally call it HDR_OUTPUT_METADATA or output_hdr_metadata.
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.hdr_source_metadata_property = prop;
> +
>   	return 0;
>   }
>   
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 9be2181..2ee45dc 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -520,6 +520,13 @@ struct drm_connector_state {
>   	 * and the connector bpc limitations obtained from edid.
>   	 */
>   	u8 max_bpc;
> +
> +	/**
> +	 * @metadata_blob_ptr:
> +	 * DRM blob property for HDR metadata
> +	 */
> +	struct drm_property_blob *hdr_source_metadata_blob_ptr;
Same goes again here, output_hdr_metadata_blob ?
> +	u8 hdr_metadata_changed : 1;
>   };
>   
>   /**
> @@ -1154,6 +1161,9 @@ struct drm_connector {
>   	 * &drm_mode_config.connector_free_work.
>   	 */
>   	struct llist_node free_node;
> +
> +	/* HDR metdata */
> +	struct hdr_static_metadata hdr_metadata;
I think while designing this framework, we should leave the scope for 
dynamic metadata, which would be following up soon. So we should have a 
union inside struct hdr_metedata, which will have option for both static 
and dynamic type of metadata. I will add details to the place where you 
are adding definition of hdr_static_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 69ccd27..4b3211b 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_source_metadata_property;
Again, source->output
> +
>   	/* dumb ioctl parameters */
>   	uint32_t preferred_depth, prefer_shadow;
>   
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index d2bacf5..bed3e08 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -137,6 +137,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,
I guess we are not bothering about HLG now, which is fine actually, less 
complicated for now.
> +};
> +
>   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 a439c2e..5012af2 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -630,6 +630,22 @@ struct drm_color_lut {
>   	__u16 reserved;
>   };
>   
> +/* HDR Metadata */
> +struct hdr_static_metadata {
> +	uint8_t eotf;
> +	uint8_t metadata_type;
> +	struct {
> +		uint16_t x, y;
> +		} display_primaries[3];
> +	struct {
> +		uint16_t x, y;
> +		} white_point;
> +	uint16_t max_mastering_display_luminance;
> +	uint16_t min_mastering_display_luminance;
> +	uint16_t max_fall;
> +	uint16_t max_cll;
> +};
How about defining struct hdr_metadata {
     uint8_t size;
     union {
         struct hdr_static_metadata *static;
         struct hdr_metadata_dynamic *dynamic;
     } metadata;
};
So that its easier when we are extending support for dynamic HDR ?

- Shashank
> +
>   #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: [v2 02/14] drm: Add CEA extended tag blocks and HDR bitfield macros

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> Add bit field and macro for extended tag in CEA block. Also,
> declare macros for HDR metadata block.
This should have been a part of patch, where these macros are being 
used, so that we can see it being used properly. While re-basing can you 
please merge ?
> v2: Rebase
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b506e36..106fd38 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2843,6 +2843,22 @@ static int drm_cvt_modes(struct drm_connector *connector,
>   #define EDID_CEA_YCRCB422	(1 << 4)
>   #define EDID_CEA_VCDB_QS	(1 << 6)
>   
> +#define DATA_BLOCK_EXTENDED_TAG		0x07
Alignment should be one tab back, also I think we already have added 
extended tag macro (for parsing YCBCR420 blocks)
> +#define VIDEO_CAPABILITY_DATA_BLOCK	0x0
> +#define VSVD_DATA_BLOCK			0x1
Alignment one tab back
> +#define COLORIMETRY_DATA_BLOCK		0x5
> +#define HDR_STATIC_METADATA_BLOCK	0x6
> +
> +/* HDR Metadata Block: Bit fields */
> +#define SUPPORTED_EOTF_MASK            0x3f
> +#define TRADITIONAL_GAMMA_SDR          (0x1 << 0)
> +#define TRADITIONAL_GAMMA_HDR          (0x1 << 1)
> +#define SMPTE_ST2084                   (0x1 << 2)
> +#define FUTURE_EOTF                    (0x1 << 3)
why not properly call it HLG if we are adding a bit for it already ?
> +#define RESERVED_EOTF                  (0x3 << 4)
> +
> +#define STATIC_METADATA_TYPE1          (0x1 << 0)
> +
>   /*
>    * Search EDID for CEA extension block.
>    */
- Shashank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 03/14] drm: Parse HDR metadata info from EDID

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, 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.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 106fd38..d12b74e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3818,6 +3818,49 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
>   	mode->clock = clock;
>   }
I guess the previous patch (or a art of previous patch) should be merged 
in this patch.
>   
> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
> +{
> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
> +		return false;
> +
> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
> +		return false;
> +
> +	return true;
> +}
> +
> +static uint16_t eotf_supported(const u8 *edid_ext)
Why uint16 ? why not uint8_t ? this is clearly one byte.
> +{
> +
> +	return edid_ext[2] &
> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
Should we bother about SDR bit at all ? Why not just check HDR bits ?
> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
> +
> +}
> +
> +static uint16_t hdr_metadata_type(const u8 *edid_ext)
> +{
Same uint8_t stuff
> +
> +	return edid_ext[3] &
> +		BIT(HDMI_STATIC_METADATA_TYPE1);
> +}
> +
> +static void
> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db)
> +{
> +	uint16_t len;
> +
> +	len = cea_db_payload_len(db);
Length is  incorrect here for extended tag blocks, as this function 
doesn't account the extended tag byte's size. So the payload length is 
looking for is len - 1. May be a good time to add 
cea_extended_db_payload_len() ?
> +	connector->hdr_metadata.eotf = eotf_supported(db);
> +	connector->hdr_metadata.metadata_type = hdr_metadata_type(db);
> +
> +	if (len >= 5)
> +		connector->hdr_metadata.max_fall = db[5];
> +	if (len >= 4)
> +		connector->hdr_metadata.max_cll = db[4];
> +}
> +
>   static void
>   drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
>   {
> @@ -4468,6 +4511,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>   			drm_parse_hdmi_forum_vsdb(connector, db);
>   		if (cea_db_is_y420cmdb(db))
>   			drm_parse_y420cmdb_bitmap(connector, db);
> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
> +			drm_parse_hdr_metadata_block(connector, db);
>   	}
>   }
>   
- Shashank

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

Re: [v2 04/14] drm: Parse Colorimetry data block from EDID

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> CEA 861.3 spec adds colorimetry data block for HDMI.
> Parsing the block to get the colorimetry data from
> panel.
>
> v2: Rebase
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c  | 24 ++++++++++++++++++++++++
>   include/drm/drm_connector.h |  2 ++
>   2 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index d12b74e..344d8c1 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3818,6 +3818,28 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
>   	mode->clock = clock;
>   }
>   
> +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db)
> +{
> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
> +		return false;
> +
> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
> +	
Again, the COLORIMETRY_DATA_BLOCK definition should be added into this 
patch.
> 	return false;
> +
> +	return true;
> +}
> +
> +static void
> +drm_parse_colorimetry_data_block(struct drm_connector *connector, const u8 *db)
> +{
> +	struct drm_hdmi_info *info = &connector->display_info.hdmi;
> +	uint16_t len;
> +
> +	len = cea_db_payload_len(db);
Again, the return value of this function doesn't account for extended db 
tag byte, so what we are looking for is len -1
> +	info->colorimetry = db[2];
colorimetry block is actually db[2] and db[3].bit7 (which represents 
DCI-P3 space), so we probably want to make info->colorimetryuint16_t
> +}
> +
> +
>   static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
>   {
>   	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
> @@ -4513,6 +4535,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>   			drm_parse_y420cmdb_bitmap(connector, db);
>   		if (cea_db_is_hdmi_hdr_metadata_block(db))
>   			drm_parse_hdr_metadata_block(connector, db);
> +		if (cea_db_is_hdmi_colorimetry_data_block(db))
> +			drm_parse_colorimetry_data_block(connector, db);
>   	}
>   }
>   
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 2ee45dc..90ce364 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -206,6 +206,8 @@ struct drm_hdmi_info {
>   
>   	/** @y420_dc_modes: bitmap of deep color support index */
>   	u8 y420_dc_modes;
> +
Please add a one line description about this variable.
> +	u8 colorimetry;
>   };
>   
>   /**
Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v2 05/14] drm/i915: Attach HDR metadata property to connector

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> Attach HDR metadata property to connector object.
>
> v2: Rebase
>
> 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 07e803a..8a1e5cb 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2154,6 +2154,8 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
>   	intel_attach_aspect_ratio_property(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_source_metadata_property, 0);
Alignment with line above missing.
- Shashank
>   
>   	if (!HAS_GMCH_DISPLAY(dev_priv))
>   		drm_connector_attach_max_bpc_property(connector, 8, 12);

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

Re: [v2 07/14] drm: Implement HDR source metadata set and get property handling

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> HDR source metadata set and get property implemented in this
> patch.
Again, HDR output metadata ? How about re-arranging the line like "This 
patch implements get() and set() functions for HDR output metadata 
property" just to make it more clear ?
> 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 added Ville's POC changes
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/drm_atomic.c      |  2 ++
>   drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
>   2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5eb4013..4e71c6b 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);
Alignment
>   
>   	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 c408898..b721b12 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -686,6 +686,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);
> @@ -734,6 +736,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_source_metadata_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +				&state->hdr_source_metadata_blob_ptr,
> +				val,
> +				-1, sizeof(struct hdr_static_metadata),
> +				&replaced);
Alignment, but I know it might be difficult to contain the params within 
80 chars.
> +		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) {
> @@ -816,6 +826,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>   		*val = state->content_type;
>   	} else if (property == connector->scaling_mode_property) {
>   		*val = state->scaling_mode;
> +	} else if (property == config->hdr_source_metadata_property) {
> +		*val = (state->hdr_source_metadata_blob_ptr) ?
> +			state->hdr_source_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) {
- Shashank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 08/14] drm: Enable HDR infoframe support

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> 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.
>
> v2: Rebase and added Ville's POC changes.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c |  58 ++++++++++++++++++++
>   drivers/video/hdmi.c       | 129 +++++++++++++++++++++++++++++++++++++++++++++
>   include/drm/drm_edid.h     |   4 ++
>   include/linux/hdmi.h       |  22 ++++++++
>   4 files changed, 213 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 344d8c1..5a7fc9b 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4916,6 +4916,64 @@ void drm_set_preferred_mode(struct drm_connector *connector,
>   EXPORT_SYMBOL(drm_set_preferred_mode);
>   
>   /**
> + * 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,
> +				    void *hdr_metadata)
> +{
> +	struct hdr_static_metadata *hdr_source_metadata;
> +	int err, i;
> +
> +	if (!frame || !hdr_metadata)
> +		return -EINVAL;
> +
> +	err = hdmi_drm_infoframe_init(frame);
> +	if (err < 0)
> +		return err;
> +
> +	DRM_DEBUG_KMS("type = %x\n", frame->type);
> +
> +	hdr_source_metadata = (struct hdr_static_metadata *)hdr_metadata;
> +
> +	frame->length = sizeof(struct hdr_static_metadata);
> +
> +
> +	frame->eotf = hdr_source_metadata->eotf;
> +	frame->metadata_type = hdr_source_metadata->metadata_type;
> +
> +	for (i = 0; i < 3; i++) {
> +		frame->display_primaries[i].x =
> +			hdr_source_metadata->display_primaries[i].x;
> +		frame->display_primaries[i].y =
> +			hdr_source_metadata->display_primaries[i].y;
> +	}
> +
> +	frame->white_point.x = hdr_source_metadata->white_point.x;
> +	frame->white_point.y = hdr_source_metadata->white_point.y;
> +
> +	frame->max_mastering_display_luminance =
> +		hdr_source_metadata->max_mastering_display_luminance;
> +	frame->min_mastering_display_luminance =
> +		hdr_source_metadata->min_mastering_display_luminance;
> +
> +	frame->max_cll = hdr_source_metadata->max_cll;
> +	frame->max_fall = hdr_source_metadata->max_fall;
> +
> +	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..0937c8c 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -650,6 +650,93 @@ 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);
> +
> +/**
> + * 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(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 & 0xff;
As ptr is a u8 ptr, & 0xff is not required, it will take lower 8 bits only.
> +		*ptr++ = frame->display_primaries[i].x >> 8;
> +		*ptr++ = frame->display_primaries[i].y & 0xff;
Same here and below all the places.
> +		*ptr++ = frame->display_primaries[i].y >> 8;
> +	}
> +
> +	*ptr++ = frame->white_point.x & 0xff;
> +	*ptr++ = frame->white_point.x >> 8;
> +
> +	*ptr++ = frame->white_point.y & 0xff;
> +	*ptr++ = frame->white_point.y >> 8;
> +
> +	*ptr++ = frame->max_mastering_display_luminance & 0xff;
> +	*ptr++ = frame->max_mastering_display_luminance >> 8;
> +
> +	*ptr++ = frame->min_mastering_display_luminance & 0xff;
> +	*ptr++ = frame->min_mastering_display_luminance >> 8;
> +
> +	*ptr++ = frame->max_cll & 0xff;
> +	*ptr++ = frame->max_cll >> 8;
> +
> +	*ptr++ = frame->max_fall & 0xff;
> +	*ptr++ = frame->max_fall >> 8;
> +
> +	hdmi_infoframe_set_checksum(buffer, length);
> +
> +	return length;
> +}
> +
>   /*
>    * hdmi_vendor_any_infoframe_check() - check a vendor infoframe
>    */
> @@ -806,6 +893,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 +928,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 +1376,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,
> +                                  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_mastering_display_luminance: %d\n",
> +			frame->max_mastering_display_luminance);
> +	hdmi_log("min_mastering_display_luminance: %d\n",
> +			frame->min_mastering_display_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 +1498,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 e3c4048..0511607 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -365,6 +365,10 @@ int drm_av_sync_delay(struct drm_connector *connector,
>   				   bool rgb_quant_range_selectable,
>   				   bool is_hdmi2_sink);
>   
> +int
> +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
> +				    void *hdr_source_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 bed3e08..ce00e1e 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -32,6 +32,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
> @@ -170,12 +171,32 @@ struct hdmi_avi_infoframe {
>   	unsigned short right_bar;
>   };
>   
> +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 {
> +		uint16_t x, y;
> +	} display_primaries[3];
> +	struct {
> +		uint16_t x, y;
> +	} white_point;
> +	uint16_t max_mastering_display_luminance;
> +	uint16_t min_mastering_display_luminance;
> +	uint16_t max_fall;
> +	uint16_t max_cll;
> +	uint16_t min_cll;
> +};
> +
>   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);
>   
>   enum hdmi_spd_sdi {
>   	HDMI_SPD_SDI_UNKNOWN,
> @@ -350,6 +371,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,

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

Re: [v2 09/14] drm/i915: Write HDR infoframe and send to panel

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> 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
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_hdmi.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8a1e5cb..6286c4a 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -475,6 +475,29 @@ static void intel_write_infoframe(struct intel_encoder *encoder,
>   					frame->any.type, buffer, len);
>   }
>   
> +/* Set Dynamic Range and Mastering Infoframe */
> +static void intel_hdmi_set_drm_infoframe(struct drm_encoder *encoder,
> +					 const struct intel_crtc_state
> +					 *crtc_state,
> +					 const struct drm_connector_state
> +
We will need this function for lspcon too, if you want to add in a 
different series, please add a TODO: here.
> 					 *conn_state)
> +{
> +	union hdmi_infoframe frame;
> +	struct hdr_static_metadata *hdr_metadata;
> +	int ret;
> +
> +	hdr_metadata = (struct hdr_static_metadata *)
> +		conn_state->hdr_source_metadata_blob_ptr->data;
> +
> +	ret = drm_hdmi_infoframe_set_hdr_metadata(&frame.drm, hdr_metadata);
> +	if (ret < 0) {
> +		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
> +		return;
> +	}
> +
> +	intel_write_infoframe(encoder, crtc_state, &frame);
> +}
> +
>   static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
>   					 const struct intel_crtc_state *crtc_state,
>   					 const struct drm_connector_state *conn_state)
> @@ -883,6 +906,10 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
>   	intel_hdmi_set_avi_infoframe(encoder, crtc_state, conn_state);
>   	intel_hdmi_set_spd_infoframe(encoder, crtc_state);
>   	intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state);
> +
> +	/* Set Dynamic Range and Mastering Infoframe if supported and changed */
> +	if (conn_state->hdr_metadata_changed)
> +		intel_hdmi_set_drm_infoframe(encoder, crtc_state, conn_state);
>   }
>   
>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable)
- Shashank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 11/14] drm/i915: Add HLG EOTF

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> ADD HLG EOTF to the list of EOTF transfer functions
> supported.
Would it be possible to add some details about HLG ?
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c | 4 ++--
>   include/linux/hdmi.h       | 1 +
>   2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 5a7fc9b..fa86494 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3857,8 +3857,8 @@ static uint16_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 uint16_t hdr_metadata_type(const u8 *edid_ext)
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index ce00e1e..b5346c3 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -146,6 +146,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 {

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

Re: [v2 12/14] drm/i915: Enable infoframes on GLK+ for HDR

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> 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.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h   |  4 ++++
>   drivers/gpu/drm/i915/intel_hdmi.c | 12 +++++++++---
>   2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0a7d605..6f44d02 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4591,6 +4591,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
> @@ -8015,6 +8016,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
> @@ -8028,6 +8030,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
> @@ -8052,6 +8055,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_GCP(trans)		_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_GCP_A)
>   #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 6286c4a..5c2e3c9 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -123,6 +123,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;
> @@ -146,6 +148,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;
> @@ -432,7 +436,8 @@ static bool hsw_infoframe_enabled(struct intel_encoder *encoder,
>   
>   	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);
> +		      VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
> +		      VIDEO_DIP_ENABLE_DRM_GLK);
>   }
>   
>   /*
> @@ -889,7 +894,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);
> @@ -908,7 +914,7 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
>   	intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state);
>   
>   	/* Set Dynamic Range and Mastering Infoframe if supported and changed */
> -	if (conn_state->hdr_metadata_changed)
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
Shouldn't this be

if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) && conn_state->hdr_metadata_changed) ?

>   		intel_hdmi_set_drm_infoframe(encoder, crtc_state, conn_state);
>   }
>   

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

Re: [v2 14/14] drivers/video: Constantify function argument for HDMI infoframe log

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Function argument for hdmi_drm_infoframe_log is made constant.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/video/hdmi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 0937c8c..7ab8086 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -1383,8 +1383,8 @@ static void hdmi_audio_infoframe_log(const char *level,
>    * @frame: HDMI DRM infoframe
>    */
>   static void hdmi_drm_infoframe_log(const char *level,
> -                                  struct device *dev,
> -                                  struct hdmi_drm_infoframe *frame)
> +				   struct device *dev,
> +				   const struct hdmi_drm_infoframe *frame)
Why not merge this patch with 8/14 drm: Enable HDR infoframe support ?
- Shashank
>   {
>   	int i;
>   

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

RE: [v2 01/14] drm: Add HDR source metadata property

[Shankar, Uma]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Thursday, December 20, 2018 11:21 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 01/14] drm: Add HDR source metadata property
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, 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.
>>
>> v2: Rebase and modified the metadata structure elements as per Ville's
>> POC changes.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c |  6 ++++++
>>   include/drm/drm_connector.h     | 10 ++++++++++
>>   include/drm/drm_mode_config.h   |  6 ++++++
>>   include/linux/hdmi.h            | 10 ++++++++++
>>   include/uapi/drm/drm_mode.h     | 16 ++++++++++++++++
>>   5 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c
>> b/drivers/gpu/drm/drm_connector.c index da8ae80..361fcda 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1027,6 +1027,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_SOURCE_METADATA", 0);
>I see that the compositor would be using this blob to setup the output HDR curve
>post blending, which would be in most of cases, sink HDR. So we should ideally
>call it HDR_OUTPUT_METADATA or output_hdr_metadata.

Ok Sure, will update the property name.

>> +	if (!prop)
>> +		return -ENOMEM;
>> +	dev->mode_config.hdr_source_metadata_property = prop;
>> +
>>   	return 0;
>>   }
>>
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 9be2181..2ee45dc 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -520,6 +520,13 @@ struct drm_connector_state {
>>   	 * and the connector bpc limitations obtained from edid.
>>   	 */
>>   	u8 max_bpc;
>> +
>> +	/**
>> +	 * @metadata_blob_ptr:
>> +	 * DRM blob property for HDR metadata
>> +	 */
>> +	struct drm_property_blob *hdr_source_metadata_blob_ptr;
>Same goes again here, output_hdr_metadata_blob ?

Ok.

>> +	u8 hdr_metadata_changed : 1;
>>   };
>>
>>   /**
>> @@ -1154,6 +1161,9 @@ struct drm_connector {
>>   	 * &drm_mode_config.connector_free_work.
>>   	 */
>>   	struct llist_node free_node;
>> +
>> +	/* HDR metdata */
>> +	struct hdr_static_metadata hdr_metadata;
>I think while designing this framework, we should leave the scope for dynamic
>metadata, which would be following up soon. So we should have a union inside
>struct hdr_metedata, which will have option for both static and dynamic type of
>metadata. I will add details to the place where you are adding definition of
>hdr_static_metadata().

I feel since we are not yet targeting beyond HDR10, lets not add dynamic metadata
definitions as of now. We will add when the same gets enabled. I hope this is ok.

>>   };
>>
>>   #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 69ccd27..4b3211b 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_source_metadata_property;
>Again, source->output

Sure, will change it.

>> +
>>   	/* dumb ioctl parameters */
>>   	uint32_t preferred_depth, prefer_shadow;
>>
>> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index
>> d2bacf5..bed3e08 100644
>> --- a/include/linux/hdmi.h
>> +++ b/include/linux/hdmi.h
>> @@ -137,6 +137,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,
>I guess we are not bothering about HLG now, which is fine actually, less
>complicated for now.

Yeah.

>> +};
>> +
>>   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 a439c2e..5012af2 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -630,6 +630,22 @@ struct drm_color_lut {
>>   	__u16 reserved;
>>   };
>>
>> +/* HDR Metadata */
>> +struct hdr_static_metadata {
>> +	uint8_t eotf;
>> +	uint8_t metadata_type;
>> +	struct {
>> +		uint16_t x, y;
>> +		} display_primaries[3];
>> +	struct {
>> +		uint16_t x, y;
>> +		} white_point;
>> +	uint16_t max_mastering_display_luminance;
>> +	uint16_t min_mastering_display_luminance;
>> +	uint16_t max_fall;
>> +	uint16_t max_cll;
>> +};
>How about defining struct hdr_metadata {
>     uint8_t size;
>     union {
>         struct hdr_static_metadata *static;
>         struct hdr_metadata_dynamic *dynamic;
>     } metadata;
>};
>So that its easier when we are extending support for dynamic HDR ?

As mentioned above, lets add them when it gets enabled.

Regards,
Uma Shankar

>- Shashank
>> +
>>   #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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 02/14] drm: Add CEA extended tag blocks and HDR bitfield macros

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Shashank
>Sent: Thursday, December 20, 2018 11:38 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
><ville.syrjala@intel.com>; Brian.Starkey@arm.com
>Subject: Re: [v2 02/14] drm: Add CEA extended tag blocks and HDR bitfield
>macros
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> Add bit field and macro for extended tag in CEA block. Also, declare
>> macros for HDR metadata block.
>This should have been a part of patch, where these macros are being used, so
>that we can see it being used properly. While re-basing can you please merge ?

Idea was to define all new block additions and bit fields in 1 patch. But will add to
respective patches where they are getting used as you mentioned.	

>> v2: Rebase
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index b506e36..106fd38 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2843,6 +2843,22 @@ static int drm_cvt_modes(struct drm_connector
>*connector,
>>   #define EDID_CEA_YCRCB422	(1 << 4)
>>   #define EDID_CEA_VCDB_QS	(1 << 6)
>>
>> +#define DATA_BLOCK_EXTENDED_TAG		0x07
>Alignment should be one tab back, also I think we already have added
>extended tag macro (for parsing YCBCR420 blocks)

Ok, will drop this.

>> +#define VIDEO_CAPABILITY_DATA_BLOCK	0x0
>> +#define VSVD_DATA_BLOCK			0x1
>Alignment one tab back

This is some editor issue, change looks ok on applying. Will recheck
while rebasing.

>> +#define COLORIMETRY_DATA_BLOCK		0x5
>> +#define HDR_STATIC_METADATA_BLOCK	0x6
>> +
>> +/* HDR Metadata Block: Bit fields */
>> +#define SUPPORTED_EOTF_MASK            0x3f
>> +#define TRADITIONAL_GAMMA_SDR          (0x1 << 0)
>> +#define TRADITIONAL_GAMMA_HDR          (0x1 << 1)
>> +#define SMPTE_ST2084                   (0x1 << 2)
>> +#define FUTURE_EOTF                    (0x1 << 3)
>why not properly call it HLG if we are adding a bit for it already ?

Ok. Will update.

Regards,
Uma Shankar

>> +#define RESERVED_EOTF                  (0x3 << 4)
>> +
>> +#define STATIC_METADATA_TYPE1          (0x1 << 0)
>> +
>>   /*
>>    * Search EDID for CEA extension block.
>>    */
>- Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

RE: [v2 03/14] drm: Parse HDR metadata info from EDID

[Shankar, Uma]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Thursday, December 20, 2018 11:47 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 03/14] drm: Parse HDR metadata info from EDID
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, 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.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c | 45
>+++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 106fd38..d12b74e 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3818,6 +3818,49 @@ static void fixup_detailed_cea_mode_clock(struct
>drm_display_mode *mode)
>>   	mode->clock = clock;
>>   }
>I guess the previous patch (or a art of previous patch) should be merged in this
>patch.

Sure, will update this.

>> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) {
>> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>> +static uint16_t eotf_supported(const u8 *edid_ext)
>Why uint16 ? why not uint8_t ? this is clearly one byte.

Ok, will change this.

>> +{
>> +
>> +	return edid_ext[2] &
>> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
>Should we bother about SDR bit at all ? Why not just check HDR bits ?

As per spec, all of these are EOTF types. So lets update whatever is supported.
User should put correct EOTF for HDR from this list.

>> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
>> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
>> +
>> +}
>> +
>> +static uint16_t hdr_metadata_type(const u8 *edid_ext) {
>Same uint8_t stuff

Ok. Will update.

>> +
>> +	return edid_ext[3] &
>> +		BIT(HDMI_STATIC_METADATA_TYPE1);
>> +}
>> +
>> +static void
>> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const
>> +u8 *db) {
>> +	uint16_t len;
>> +
>> +	len = cea_db_payload_len(db);
>Length is  incorrect here for extended tag blocks, as this function doesn't account
>the extended tag byte's size. So the payload length is looking for is len - 1. May be
>a good time to add
>cea_extended_db_payload_len() ?

As per spec, the definition says length after this byte so it factors the extended tag byte
well, IIUC. Please correct me if my understanding is wrong.

Regards,
Uma Shankar

>> +	connector->hdr_metadata.eotf = eotf_supported(db);
>> +	connector->hdr_metadata.metadata_type = hdr_metadata_type(db);
>> +
>> +	if (len >= 5)
>> +		connector->hdr_metadata.max_fall = db[5];
>> +	if (len >= 4)
>> +		connector->hdr_metadata.max_cll = db[4]; }
>> +
>>   static void
>>   drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
>>   {
>> @@ -4468,6 +4511,8 @@ static void drm_parse_cea_ext(struct drm_connector
>*connector,
>>   			drm_parse_hdmi_forum_vsdb(connector, db);
>>   		if (cea_db_is_y420cmdb(db))
>>   			drm_parse_y420cmdb_bitmap(connector, db);
>> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
>> +			drm_parse_hdr_metadata_block(connector, db);
>>   	}
>>   }
>>
>- Shashank
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 03/14] drm: Parse HDR metadata info from EDID

[Sharma, Shashank]

Regards

Shashank


On 1/8/2019 11:10 AM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>> Sharma, Shashank
>> Sent: Thursday, December 20, 2018 11:47 PM
>> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>> dri-devel@lists.freedesktop.org
>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>> <maarten.lankhorst@intel.com>
>> Subject: Re: [v2 03/14] drm: Parse HDR metadata info from EDID
>>
>> Regards
>>
>> Shashank
>>
>>
>> On 12/12/2018 2:08 AM, 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.
>>>
>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>> ---
>>>    drivers/gpu/drm/drm_edid.c | 45
>> +++++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 45 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>> index 106fd38..d12b74e 100644
>>> --- a/drivers/gpu/drm/drm_edid.c
>>> +++ b/drivers/gpu/drm/drm_edid.c
>>> @@ -3818,6 +3818,49 @@ static void fixup_detailed_cea_mode_clock(struct
>> drm_display_mode *mode)
>>>    	mode->clock = clock;
>>>    }
>> I guess the previous patch (or a art of previous patch) should be merged in this
>> patch.
> Sure, will update this.
>
>>> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) {
>>> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
>>> +		return false;
>>> +
>>> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
>>> +		return false;
>>> +
>>> +	return true;
>>> +}
>>> +
>>> +static uint16_t eotf_supported(const u8 *edid_ext)
>> Why uint16 ? why not uint8_t ? this is clearly one byte.
> Ok, will change this.
>
>>> +{
>>> +
>>> +	return edid_ext[2] &
>>> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
>> Should we bother about SDR bit at all ? Why not just check HDR bits ?
> As per spec, all of these are EOTF types. So lets update whatever is supported.
> User should put correct EOTF for HDR from this list.
>
>>> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
>>> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
>>> +
>>> +}
>>> +
>>> +static uint16_t hdr_metadata_type(const u8 *edid_ext) {
>> Same uint8_t stuff
> Ok. Will update.
>
>>> +
>>> +	return edid_ext[3] &
>>> +		BIT(HDMI_STATIC_METADATA_TYPE1);
>>> +}
>>> +
>>> +static void
>>> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const
>>> +u8 *db) {
>>> +	uint16_t len;
>>> +
>>> +	len = cea_db_payload_len(db);
>> Length is  incorrect here for extended tag blocks, as this function doesn't account
>> the extended tag byte's size. So the payload length is looking for is len - 1. May be
>> a good time to add
>> cea_extended_db_payload_len() ?
> As per spec, the definition says length after this byte so it factors the extended tag byte
> well, IIUC. Please correct me if my understanding is wrong.
This is how the data is arranged in a normal CEA DB and Extended CEA DB
+--------------------+   +-----------------+
|Tag|Length= 3     |   |Tag|Length=3     |
+--------------------+   +-----------------+
| Data               |   |Extended tag     |
+--------------------+   +-----------------+
| Data               |   |Data             |
+--------------------+   +-----------------+
| Data               |   |Data             |
+--------------------+   +-----------------+

This function cea_db_payload_len() was written before the CEA extended 
blocks were introduced, so it is unaware of Extended tag code, and 
assumes its a part of data. But in case of extended tag block the actual 
data length is 3 -1 = 2. You can have a look at how we are parsing the 
YCBCR 4:2:0 blocks. That's why I made this comment "may be a good time 
to add cea_extended_db_payload_len() function" or enhance the existing 
function to reflect the extended tag.
- Shashank
>
> Regards,
> Uma Shankar
>
>>> +	connector->hdr_metadata.eotf = eotf_supported(db);
>>> +	connector->hdr_metadata.metadata_type = hdr_metadata_type(db);
>>> +
>>> +	if (len >= 5)
>>> +		connector->hdr_metadata.max_fall = db[5];
>>> +	if (len >= 4)
>>> +		connector->hdr_metadata.max_cll = db[4]; }
>>> +
>>>    static void
>>>    drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
>>>    {
>>> @@ -4468,6 +4511,8 @@ static void drm_parse_cea_ext(struct drm_connector
>> *connector,
>>>    			drm_parse_hdmi_forum_vsdb(connector, db);
>>>    		if (cea_db_is_y420cmdb(db))
>>>    			drm_parse_y420cmdb_bitmap(connector, db);
>>> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
>>> +			drm_parse_hdr_metadata_block(connector, db);
>>>    	}
>>>    }
>>>
>> - Shashank
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [v2 03/14] drm: Parse HDR metadata info from EDID

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Shashank
>Sent: Tuesday, January 8, 2019 11:26 AM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 03/14] drm: Parse HDR metadata info from EDID
>
>Regards
>
>Shashank
>
>
>On 1/8/2019 11:10 AM, Shankar, Uma wrote:
>>
>>> -----Original Message-----
>>> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On
>>> Behalf Of Sharma, Shashank
>>> Sent: Thursday, December 20, 2018 11:47 PM
>>> To: Shankar, Uma <uma.shankar@intel.com>;
>>> intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>>> <maarten.lankhorst@intel.com>
>>> Subject: Re: [v2 03/14] drm: Parse HDR metadata info from EDID
>>>
>>> Regards
>>>
>>> Shashank
>>>
>>>
>>> On 12/12/2018 2:08 AM, 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.
>>>>
>>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/drm_edid.c | 45
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 45 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>> index 106fd38..d12b74e 100644
>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>> @@ -3818,6 +3818,49 @@ static void
>>>> fixup_detailed_cea_mode_clock(struct
>>> drm_display_mode *mode)
>>>>    	mode->clock = clock;
>>>>    }
>>> I guess the previous patch (or a art of previous patch) should be
>>> merged in this patch.
>> Sure, will update this.
>>
>>>> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) {
>>>> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
>>>> +		return false;
>>>> +
>>>> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
>>>> +		return false;
>>>> +
>>>> +	return true;
>>>> +}
>>>> +
>>>> +static uint16_t eotf_supported(const u8 *edid_ext)
>>> Why uint16 ? why not uint8_t ? this is clearly one byte.
>> Ok, will change this.
>>
>>>> +{
>>>> +
>>>> +	return edid_ext[2] &
>>>> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
>>> Should we bother about SDR bit at all ? Why not just check HDR bits ?
>> As per spec, all of these are EOTF types. So lets update whatever is supported.
>> User should put correct EOTF for HDR from this list.
>>
>>>> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
>>>> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
>>>> +
>>>> +}
>>>> +
>>>> +static uint16_t hdr_metadata_type(const u8 *edid_ext) {
>>> Same uint8_t stuff
>> Ok. Will update.
>>
>>>> +
>>>> +	return edid_ext[3] &
>>>> +		BIT(HDMI_STATIC_METADATA_TYPE1);
>>>> +}
>>>> +
>>>> +static void
>>>> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const
>>>> +u8 *db) {
>>>> +	uint16_t len;
>>>> +
>>>> +	len = cea_db_payload_len(db);
>>> Length is  incorrect here for extended tag blocks, as this function
>>> doesn't account the extended tag byte's size. So the payload length
>>> is looking for is len - 1. May be a good time to add
>>> cea_extended_db_payload_len() ?
>> As per spec, the definition says length after this byte so it factors
>> the extended tag byte well, IIUC. Please correct me if my understanding is
>wrong.
>This is how the data is arranged in a normal CEA DB and Extended CEA DB
>+--------------------+   +-----------------+
>|Tag|Length= 3     |   |Tag|Length=3     |
>+--------------------+   +-----------------+
>| Data               |   |Extended tag     |
>+--------------------+   +-----------------+
>| Data               |   |Data             |
>+--------------------+   +-----------------+
>| Data               |   |Data             |
>+--------------------+   +-----------------+
>
>This function cea_db_payload_len() was written before the CEA extended blocks
>were introduced, so it is unaware of Extended tag code, and assumes its a part of
>data. But in case of extended tag block the actual data length is 3 -1 = 2. You can
>have a look at how we are parsing the YCBCR 4:2:0 blocks. That's why I made this
>comment "may be a good time to add cea_extended_db_payload_len() function"
>or enhance the existing function to reflect the extended tag.

Oh ok got it, thanks for the explanation. Will update the patch accordingly.

Regards,
Uma Shankar

>- Shashank
>>
>> Regards,
>> Uma Shankar
>>
>>>> +	connector->hdr_metadata.eotf = eotf_supported(db);
>>>> +	connector->hdr_metadata.metadata_type = hdr_metadata_type(db);
>>>> +
>>>> +	if (len >= 5)
>>>> +		connector->hdr_metadata.max_fall = db[5];
>>>> +	if (len >= 4)
>>>> +		connector->hdr_metadata.max_cll = db[4]; }
>>>> +
>>>>    static void
>>>>    drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8
>*db)
>>>>    {
>>>> @@ -4468,6 +4511,8 @@ static void drm_parse_cea_ext(struct
>>>> drm_connector
>>> *connector,
>>>>    			drm_parse_hdmi_forum_vsdb(connector, db);
>>>>    		if (cea_db_is_y420cmdb(db))
>>>>    			drm_parse_y420cmdb_bitmap(connector, db);
>>>> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
>>>> +			drm_parse_hdr_metadata_block(connector, db);
>>>>    	}
>>>>    }
>>>>
>>> - Shashank
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

RE: [v2 04/14] drm: Parse Colorimetry data block from EDID

[Shankar, Uma]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Thursday, December 20, 2018 11:54 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> CEA 861.3 spec adds colorimetry data block for HDMI.
>> Parsing the block to get the colorimetry data from panel.
>>
>> v2: Rebase
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c  | 24 ++++++++++++++++++++++++
>>   include/drm/drm_connector.h |  2 ++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index d12b74e..344d8c1 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3818,6 +3818,28 @@ static void fixup_detailed_cea_mode_clock(struct
>drm_display_mode *mode)
>>   	mode->clock = clock;
>>   }
>>
>> +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) {
>> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
>> +
>Again, the COLORIMETRY_DATA_BLOCK definition should be added into this
>patch.

Ok, will do that.

>> 	return false;
>> +
>> +	return true;
>> +}
>> +
>> +static void
>> +drm_parse_colorimetry_data_block(struct drm_connector *connector,
>> +const u8 *db) {
>> +	struct drm_hdmi_info *info = &connector->display_info.hdmi;
>> +	uint16_t len;
>> +
>> +	len = cea_db_payload_len(db);
>Again, the return value of this function doesn't account for extended db tag byte,
>so what we are looking for is len -1

Will update this.

>> +	info->colorimetry = db[2];
>colorimetry block is actually db[2] and db[3].bit7 (which represents
>DCI-P3 space), so we probably want to make info->colorimetryuint16_t

3 BT2020RGB BT2020YCC BT2020cYCC AdobeRGB AdobeYCC601 sYCC601 xvYCC709 xvYCC601
4 F47=0 F46=0 F45=0 F44=0 MD3 MD2 MD1 MD0

This is how db[2] and db[3] is described in spec. There is not much clarity on F47. Not sure if they
are for DCI-P3, can you clarify once.

Regards,
Uma Shankar

>> +}
>> +
>> +
>>   static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
>>   {
>>   	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) @@ -4513,6
>+4535,8
>> @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>   			drm_parse_y420cmdb_bitmap(connector, db);
>>   		if (cea_db_is_hdmi_hdr_metadata_block(db))
>>   			drm_parse_hdr_metadata_block(connector, db);
>> +		if (cea_db_is_hdmi_colorimetry_data_block(db))
>> +			drm_parse_colorimetry_data_block(connector, db);
>>   	}
>>   }
>>
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 2ee45dc..90ce364 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -206,6 +206,8 @@ struct drm_hdmi_info {
>>
>>   	/** @y420_dc_modes: bitmap of deep color support index */
>>   	u8 y420_dc_modes;
>> +
>Please add a one line description about this variable.

Sure will update this.

>> +	u8 colorimetry;
>>   };
>>
>>   /**
>Shashank
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 05/14] drm/i915: Attach HDR metadata property to connector

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Shashank
>Sent: Thursday, December 20, 2018 11:56 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
><ville.syrjala@intel.com>; Brian.Starkey@arm.com
>Subject: Re: [v2 05/14] drm/i915: Attach HDR metadata property to connector
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> Attach HDR metadata property to connector object.
>>
>> v2: Rebase
>>
>> 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 07e803a..8a1e5cb 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -2154,6 +2154,8 @@ static void intel_hdmi_destroy(struct drm_connector
>*connector)
>>   	intel_attach_aspect_ratio_property(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_source_metadata_property,
>0);
>Alignment with line above missing.

It's just to keep it within 80 characters. Hope this is ok. Not sure whether this
alignment or 80 character limit takes more priority. 

Regards,
Uma Shankar

>- Shashank
>>
>>   	if (!HAS_GMCH_DISPLAY(dev_priv))
>>   		drm_connector_attach_max_bpc_property(connector, 8, 12);

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

RE: [v2 07/14] drm: Implement HDR source metadata set and get property handling

[Shankar, Uma]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Friday, December 21, 2018 12:04 AM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 07/14] drm: Implement HDR source metadata set and get
>property handling
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> HDR source metadata set and get property implemented in this patch.
>Again, HDR output metadata ? How about re-arranging the line like "This patch
>implements get() and set() functions for HDR output metadata property" just to
>make it more clear ?

Sure, will update this.

>> 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 added Ville's POC changes
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_atomic.c      |  2 ++
>>   drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c
>> b/drivers/gpu/drm/drm_atomic.c index 5eb4013..4e71c6b 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);
>Alignment

Ok, will update.

>>
>>   	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 c408898..b721b12 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -686,6 +686,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); @@ -
>734,6
>> +736,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_source_metadata_property) {
>> +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> +				&state->hdr_source_metadata_blob_ptr,
>> +				val,
>> +				-1, sizeof(struct hdr_static_metadata),
>> +				&replaced);
>Alignment, but I know it might be difficult to contain the params within
>80 chars.

Yeah, not sure what is higher priority - 80 character limit or the
alignment here. Currently went with 80 character limit.

Regards,
Uma Shankar

>> +		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) { @@ -816,6
>> +826,9 @@ static int drm_atomic_connector_set_property(struct
>drm_connector *connector,
>>   		*val = state->content_type;
>>   	} else if (property == connector->scaling_mode_property) {
>>   		*val = state->scaling_mode;
>> +	} else if (property == config->hdr_source_metadata_property) {
>> +		*val = (state->hdr_source_metadata_blob_ptr) ?
>> +			state->hdr_source_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) {
>- Shashank
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 08/14] drm: Enable HDR infoframe support

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, December 21, 2018 2:11 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
><ville.syrjala@intel.com>; Brian.Starkey@arm.com
>Subject: Re: [v2 08/14] drm: Enable HDR infoframe support
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> 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.
>>
>> v2: Rebase and added Ville's POC changes.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c |  58 ++++++++++++++++++++
>>   drivers/video/hdmi.c       | 129
>+++++++++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_edid.h     |   4 ++
>>   include/linux/hdmi.h       |  22 ++++++++
>>   4 files changed, 213 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 344d8c1..5a7fc9b 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4916,6 +4916,64 @@ void drm_set_preferred_mode(struct
>drm_connector *connector,
>>   EXPORT_SYMBOL(drm_set_preferred_mode);
>>
>>   /**
>> + * 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,
>> +				    void *hdr_metadata)
>> +{
>> +	struct hdr_static_metadata *hdr_source_metadata;
>> +	int err, i;
>> +
>> +	if (!frame || !hdr_metadata)
>> +		return -EINVAL;
>> +
>> +	err = hdmi_drm_infoframe_init(frame);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	DRM_DEBUG_KMS("type = %x\n", frame->type);
>> +
>> +	hdr_source_metadata = (struct hdr_static_metadata *)hdr_metadata;
>> +
>> +	frame->length = sizeof(struct hdr_static_metadata);
>> +
>> +
>> +	frame->eotf = hdr_source_metadata->eotf;
>> +	frame->metadata_type = hdr_source_metadata->metadata_type;
>> +
>> +	for (i = 0; i < 3; i++) {
>> +		frame->display_primaries[i].x =
>> +			hdr_source_metadata->display_primaries[i].x;
>> +		frame->display_primaries[i].y =
>> +			hdr_source_metadata->display_primaries[i].y;
>> +	}
>> +
>> +	frame->white_point.x = hdr_source_metadata->white_point.x;
>> +	frame->white_point.y = hdr_source_metadata->white_point.y;
>> +
>> +	frame->max_mastering_display_luminance =
>> +		hdr_source_metadata->max_mastering_display_luminance;
>> +	frame->min_mastering_display_luminance =
>> +		hdr_source_metadata->min_mastering_display_luminance;
>> +
>> +	frame->max_cll = hdr_source_metadata->max_cll;
>> +	frame->max_fall = hdr_source_metadata->max_fall;
>> +
>> +	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..0937c8c 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -650,6 +650,93 @@ 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);
>> +
>> +/**
>> + * 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(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 & 0xff;
>As ptr is a u8 ptr, & 0xff is not required, it will take lower 8 bits only.

I feel keeping the mask will make it clearer and avoid any ambiguity.
I can update and drop this if you suggest that way.

Regards,
Uma Shankar

>> +		*ptr++ = frame->display_primaries[i].x >> 8;
>> +		*ptr++ = frame->display_primaries[i].y & 0xff;
>Same here and below all the places.
>> +		*ptr++ = frame->display_primaries[i].y >> 8;
>> +	}
>> +
>> +	*ptr++ = frame->white_point.x & 0xff;
>> +	*ptr++ = frame->white_point.x >> 8;
>> +
>> +	*ptr++ = frame->white_point.y & 0xff;
>> +	*ptr++ = frame->white_point.y >> 8;
>> +
>> +	*ptr++ = frame->max_mastering_display_luminance & 0xff;
>> +	*ptr++ = frame->max_mastering_display_luminance >> 8;
>> +
>> +	*ptr++ = frame->min_mastering_display_luminance & 0xff;
>> +	*ptr++ = frame->min_mastering_display_luminance >> 8;
>> +
>> +	*ptr++ = frame->max_cll & 0xff;
>> +	*ptr++ = frame->max_cll >> 8;
>> +
>> +	*ptr++ = frame->max_fall & 0xff;
>> +	*ptr++ = frame->max_fall >> 8;
>> +
>> +	hdmi_infoframe_set_checksum(buffer, length);
>> +
>> +	return length;
>> +}
>> +
>>   /*
>>    * hdmi_vendor_any_infoframe_check() - check a vendor infoframe
>>    */
>> @@ -806,6 +893,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 +928,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 +1376,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,
>> +                                  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_mastering_display_luminance: %d\n",
>> +			frame->max_mastering_display_luminance);
>> +	hdmi_log("min_mastering_display_luminance: %d\n",
>> +			frame->min_mastering_display_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 +1498,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
>> e3c4048..0511607 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -365,6 +365,10 @@ int drm_av_sync_delay(struct drm_connector
>*connector,
>>   				   bool rgb_quant_range_selectable,
>>   				   bool is_hdmi2_sink);
>>
>> +int
>> +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
>> +				    void *hdr_source_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 bed3e08..ce00e1e
>> 100644
>> --- a/include/linux/hdmi.h
>> +++ b/include/linux/hdmi.h
>> @@ -32,6 +32,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
>> @@ -170,12 +171,32 @@ struct hdmi_avi_infoframe {
>>   	unsigned short right_bar;
>>   };
>>
>> +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 {
>> +		uint16_t x, y;
>> +	} display_primaries[3];
>> +	struct {
>> +		uint16_t x, y;
>> +	} white_point;
>> +	uint16_t max_mastering_display_luminance;
>> +	uint16_t min_mastering_display_luminance;
>> +	uint16_t max_fall;
>> +	uint16_t max_cll;
>> +	uint16_t min_cll;
>> +};
>> +
>>   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);
>>
>>   enum hdmi_spd_sdi {
>>   	HDMI_SPD_SDI_UNKNOWN,
>> @@ -350,6 +371,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,

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

Re: [v2 09/14] drm/i915: Write HDR infoframe and send to panel

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, December 21, 2018 2:14 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
><ville.syrjala@intel.com>; Brian.Starkey@arm.com
>Subject: Re: [v2 09/14] drm/i915: Write HDR infoframe and send to panel
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> 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
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_hdmi.c | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 8a1e5cb..6286c4a 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -475,6 +475,29 @@ static void intel_write_infoframe(struct intel_encoder
>*encoder,
>>   					frame->any.type, buffer, len);
>>   }
>>
>> +/* Set Dynamic Range and Mastering Infoframe */ static void
>> +intel_hdmi_set_drm_infoframe(struct drm_encoder *encoder,
>> +					 const struct intel_crtc_state
>> +					 *crtc_state,
>> +					 const struct drm_connector_state
>> +
>We will need this function for lspcon too, if you want to add in a different series,
>please add a TODO: here.

Sure, will add the same.

Regards,
Uma Shankar

>> 					 *conn_state)
>> +{
>> +	union hdmi_infoframe frame;
>> +	struct hdr_static_metadata *hdr_metadata;
>> +	int ret;
>> +
>> +	hdr_metadata = (struct hdr_static_metadata *)
>> +		conn_state->hdr_source_metadata_blob_ptr->data;
>> +
>> +	ret = drm_hdmi_infoframe_set_hdr_metadata(&frame.drm,
>hdr_metadata);
>> +	if (ret < 0) {
>> +		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
>> +		return;
>> +	}
>> +
>> +	intel_write_infoframe(encoder, crtc_state, &frame); }
>> +
>>   static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
>>   					 const struct intel_crtc_state
>*crtc_state,
>>   					 const struct drm_connector_state
>*conn_state) @@ -883,6
>> +906,10 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
>>   	intel_hdmi_set_avi_infoframe(encoder, crtc_state, conn_state);
>>   	intel_hdmi_set_spd_infoframe(encoder, crtc_state);
>>   	intel_hdmi_set_hdmi_infoframe(encoder, crtc_state, conn_state);
>> +
>> +	/* Set Dynamic Range and Mastering Infoframe if supported and changed
>*/
>> +	if (conn_state->hdr_metadata_changed)
>> +		intel_hdmi_set_drm_infoframe(encoder, crtc_state,
>conn_state);
>>   }
>>
>>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi,
>> bool enable)
>- Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v2 04/14] drm: Parse Colorimetry data block from EDID

[Sharma, Shashank]

On 1/8/2019 12:10 PM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>> Sharma, Shashank
>> Sent: Thursday, December 20, 2018 11:54 PM
>> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>> dri-devel@lists.freedesktop.org
>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>> <maarten.lankhorst@intel.com>
>> Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID
>>
>> Regards
>>
>> Shashank
>>
>>
>> On 12/12/2018 2:08 AM, Uma Shankar wrote:
>>> CEA 861.3 spec adds colorimetry data block for HDMI.
>>> Parsing the block to get the colorimetry data from panel.
>>>
>>> v2: Rebase
>>>
>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>> ---
>>>    drivers/gpu/drm/drm_edid.c  | 24 ++++++++++++++++++++++++
>>>    include/drm/drm_connector.h |  2 ++
>>>    2 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>> index d12b74e..344d8c1 100644
>>> --- a/drivers/gpu/drm/drm_edid.c
>>> +++ b/drivers/gpu/drm/drm_edid.c
>>> @@ -3818,6 +3818,28 @@ static void fixup_detailed_cea_mode_clock(struct
>> drm_display_mode *mode)
>>>    	mode->clock = clock;
>>>    }
>>>
>>> +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) {
>>> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
>>> +		return false;
>>> +
>>> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
>>> +
>> Again, the COLORIMETRY_DATA_BLOCK definition should be added into this
>> patch.
> Ok, will do that.
>
>>> 	return false;
>>> +
>>> +	return true;
>>> +}
>>> +
>>> +static void
>>> +drm_parse_colorimetry_data_block(struct drm_connector *connector,
>>> +const u8 *db) {
>>> +	struct drm_hdmi_info *info = &connector->display_info.hdmi;
>>> +	uint16_t len;
>>> +
>>> +	len = cea_db_payload_len(db);
>> Again, the return value of this function doesn't account for extended db tag byte,
>> so what we are looking for is len -1
> Will update this.
>
>>> +	info->colorimetry = db[2];
>> colorimetry block is actually db[2] and db[3].bit7 (which represents
>> DCI-P3 space), so we probably want to make info->colorimetryuint16_t
> 3 BT2020RGB BT2020YCC BT2020cYCC AdobeRGB AdobeYCC601 sYCC601 xvYCC709 xvYCC601
> 4 F47=0 F46=0 F45=0 F44=0 MD3 MD2 MD1 MD0
>
> This is how db[2] and db[3] is described in spec. There is not much clarity on F47. Not sure if they
> are for DCI-P3, can you clarify once.
Ah, check CEA-861-G table 70, there there is no F47, but I could see 
DCI-P3 in that place, which spec is that you are following right now ?
- Shashank
> Regards,
> Uma Shankar
>
>>> +}
>>> +
>>> +
>>>    static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
>>>    {
>>>    	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) @@ -4513,6
>> +4535,8
>>> @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>>    			drm_parse_y420cmdb_bitmap(connector, db);
>>>    		if (cea_db_is_hdmi_hdr_metadata_block(db))
>>>    			drm_parse_hdr_metadata_block(connector, db);
>>> +		if (cea_db_is_hdmi_colorimetry_data_block(db))
>>> +			drm_parse_colorimetry_data_block(connector, db);
>>>    	}
>>>    }
>>>
>>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>>> index 2ee45dc..90ce364 100644
>>> --- a/include/drm/drm_connector.h
>>> +++ b/include/drm/drm_connector.h
>>> @@ -206,6 +206,8 @@ struct drm_hdmi_info {
>>>
>>>    	/** @y420_dc_modes: bitmap of deep color support index */
>>>    	u8 y420_dc_modes;
>>> +
>> Please add a one line description about this variable.
> Sure will update this.
>
>>> +	u8 colorimetry;
>>>    };
>>>
>>>    /**
>> Shashank
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [v2 11/14] drm/i915: Add HLG EOTF

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, December 21, 2018 2:17 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
><ville.syrjala@intel.com>; Brian.Starkey@arm.com; Ville Syrjälä
><ville.syrjala@linux.intel.com>
>Subject: Re: [v2 11/14] drm/i915: Add HLG EOTF
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> ADD HLG EOTF to the list of EOTF transfer functions supported.
>Would it be possible to add some details about HLG ?

Sure, will add that.

Regards,
Uma Shankar

>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c | 4 ++--
>>   include/linux/hdmi.h       | 1 +
>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 5a7fc9b..fa86494 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3857,8 +3857,8 @@ static uint16_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 uint16_t hdr_metadata_type(const u8 *edid_ext)
>> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
>> index ce00e1e..b5346c3 100644
>> --- a/include/linux/hdmi.h
>> +++ b/include/linux/hdmi.h
>> @@ -146,6 +146,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 {

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

RE: [v2 04/14] drm: Parse Colorimetry data block from EDID

[Shankar, Uma]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Tuesday, January 8, 2019 12:27 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID
>
>
>
>On 1/8/2019 12:10 PM, Shankar, Uma wrote:
>>
>>> -----Original Message-----
>>> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On
>>> Behalf Of Sharma, Shashank
>>> Sent: Thursday, December 20, 2018 11:54 PM
>>> To: Shankar, Uma <uma.shankar@intel.com>;
>>> intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>>> <maarten.lankhorst@intel.com>
>>> Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID
>>>
>>> Regards
>>>
>>> Shashank
>>>
>>>
>>> On 12/12/2018 2:08 AM, Uma Shankar wrote:
>>>> CEA 861.3 spec adds colorimetry data block for HDMI.
>>>> Parsing the block to get the colorimetry data from panel.
>>>>
>>>> v2: Rebase
>>>>
>>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/drm_edid.c  | 24 ++++++++++++++++++++++++
>>>>    include/drm/drm_connector.h |  2 ++
>>>>    2 files changed, 26 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>> index d12b74e..344d8c1 100644
>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>> @@ -3818,6 +3818,28 @@ static void
>>>> fixup_detailed_cea_mode_clock(struct
>>> drm_display_mode *mode)
>>>>    	mode->clock = clock;
>>>>    }
>>>>
>>>> +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) {
>>>> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
>>>> +		return false;
>>>> +
>>>> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
>>>> +
>>> Again, the COLORIMETRY_DATA_BLOCK definition should be added into
>>> this patch.
>> Ok, will do that.
>>
>>>> 	return false;
>>>> +
>>>> +	return true;
>>>> +}
>>>> +
>>>> +static void
>>>> +drm_parse_colorimetry_data_block(struct drm_connector *connector,
>>>> +const u8 *db) {
>>>> +	struct drm_hdmi_info *info = &connector->display_info.hdmi;
>>>> +	uint16_t len;
>>>> +
>>>> +	len = cea_db_payload_len(db);
>>> Again, the return value of this function doesn't account for extended
>>> db tag byte, so what we are looking for is len -1
>> Will update this.
>>
>>>> +	info->colorimetry = db[2];
>>> colorimetry block is actually db[2] and db[3].bit7 (which represents
>>> DCI-P3 space), so we probably want to make info->colorimetryuint16_t
>> 3 BT2020RGB BT2020YCC BT2020cYCC AdobeRGB AdobeYCC601 sYCC601
>xvYCC709
>> xvYCC601
>> 4 F47=0 F46=0 F45=0 F44=0 MD3 MD2 MD1 MD0
>>
>> This is how db[2] and db[3] is described in spec. There is not much
>> clarity on F47. Not sure if they are for DCI-P3, can you clarify once.
>Ah, check CEA-861-G table 70, there there is no F47, but I could see
>DCI-P3 in that place, which spec is that you are following right now ?

I am looking to 861-F. So seems like its updated in 861-G ?

Regards,
Uma Shankar

>- Shashank
>> Regards,
>> Uma Shankar
>>
>>>> +}
>>>> +
>>>> +
>>>>    static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
>>>>    {
>>>>    	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) @@ -4513,6
>>> +4535,8
>>>> @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>>>    			drm_parse_y420cmdb_bitmap(connector, db);
>>>>    		if (cea_db_is_hdmi_hdr_metadata_block(db))
>>>>    			drm_parse_hdr_metadata_block(connector, db);
>>>> +		if (cea_db_is_hdmi_colorimetry_data_block(db))
>>>> +			drm_parse_colorimetry_data_block(connector, db);
>>>>    	}
>>>>    }
>>>>
>>>> diff --git a/include/drm/drm_connector.h
>>>> b/include/drm/drm_connector.h index 2ee45dc..90ce364 100644
>>>> --- a/include/drm/drm_connector.h
>>>> +++ b/include/drm/drm_connector.h
>>>> @@ -206,6 +206,8 @@ struct drm_hdmi_info {
>>>>
>>>>    	/** @y420_dc_modes: bitmap of deep color support index */
>>>>    	u8 y420_dc_modes;
>>>> +
>>> Please add a one line description about this variable.
>> Sure will update this.
>>
>>>> +	u8 colorimetry;
>>>>    };
>>>>
>>>>    /**
>>> Shashank
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 14/14] drivers/video: Constantify function argument for HDMI infoframe log

[Shankar, Uma]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Friday, December 21, 2018 2:28 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 14/14] drivers/video: Constantify function argument for HDMI
>infoframe log
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Function argument for hdmi_drm_infoframe_log is made constant.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/video/hdmi.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index
>> 0937c8c..7ab8086 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -1383,8 +1383,8 @@ static void hdmi_audio_infoframe_log(const char
>*level,
>>    * @frame: HDMI DRM infoframe
>>    */
>>   static void hdmi_drm_infoframe_log(const char *level,
>> -                                  struct device *dev,
>> -                                  struct hdmi_drm_infoframe *frame)
>> +				   struct device *dev,
>> +				   const struct hdmi_drm_infoframe *frame)
>Why not merge this patch with 8/14 drm: Enable HDR infoframe support ?

Will update this.

Thanks Shashank for the review and valuable comments.

Regards,
Uma Shankar
>- Shashank
>>   {
>>   	int i;
>>
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx