dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer
@ 2019-03-11  3:57 Uma Shankar
  2019-03-11  3:57 ` [v5 01/13] drm: Add HDR source metadata property Uma Shankar
                   ` (12 more replies)
  0 siblings, 13 replies; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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.

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

v4: Addressed Shashank's review comments.

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

Note: Media driver and VAAPI changes for HDR are already out, with compositors
changes also expected to land soon. Some discussions have already started with
initial design on weston in community and is in active development along
with GL efforts.

Uma Shankar (10):
  drm: Add HDR source metadata property
  drm: Parse HDR metadata info from EDID
  drm: Parse Colorimetry data block from EDID
  drm/i915: Attach HDR metadata property to connector
  drm: Implement HDR output 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
  drm/i915: Set Infoframe for non modeset case for HDR
  video/hdmi: Add const variants for drm infoframe

Ville Syrjälä (3):
  drm/i915: [DO NOT MERGE] hack for glk board outputs
  drm/i915: Add HLG EOTF
  drm/i915: Enable infoframes on GLK+ for HDR

 drivers/gpu/drm/drm_atomic.c        |   2 +
 drivers/gpu/drm/drm_atomic_uapi.c   |  13 +++
 drivers/gpu/drm/drm_connector.c     |   6 ++
 drivers/gpu/drm/drm_edid.c          | 136 ++++++++++++++++++++++++++
 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_ddi.c    |   9 ++
 drivers/gpu/drm/i915/intel_drv.h    |   1 +
 drivers/gpu/drm/i915/intel_hdmi.c   |  51 +++++++++-
 drivers/video/hdmi.c                | 188 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h         |  13 +++
 include/drm/drm_edid.h              |   4 +
 include/drm/drm_mode_config.h       |   6 ++
 include/linux/hdmi.h                |  38 ++++++++
 include/uapi/drm/drm_mode.h         |  16 +++
 16 files changed, 506 insertions(+), 3 deletions(-)

-- 
1.9.1

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

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

* [v5 01/13] drm: Add HDR source metadata property
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
@ 2019-03-11  3:57 ` Uma Shankar
  2019-03-15  7:08   ` Sharma, Shashank
  2019-03-11  3:57 ` [v5 02/13] drm: Parse HDR metadata info from EDID Uma Shankar
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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.

v3: No Change

v4: Addressed Shashank's review comments

v5: Rebase.

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 07d65a1..b031079 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1054,6 +1054,12 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.non_desktop_property = prop;
 
+	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
+				   "HDR_OUTPUT_METADATA", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.hdr_output_metadata_property = prop;
+
 	return 0;
 }
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index c806199..29388bd 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -561,6 +561,13 @@ struct drm_connector_state {
 	 * and the connector bpc limitations obtained from edid.
 	 */
 	u8 max_bpc;
+
+	/**
+	 * @metadata_blob_ptr:
+	 * DRM blob property for HDR output metadata
+	 */
+	struct drm_property_blob *hdr_output_metadata_blob_ptr;
+	u8 hdr_metadata_changed : 1;
 };
 
 /**
@@ -1201,6 +1208,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 7f60e8e..ef2656b 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -836,6 +836,12 @@ struct drm_mode_config {
 	 */
 	struct drm_property *writeback_out_fence_ptr_property;
 
+	/*
+	 * hdr_metadata_property: Connector property containing hdr metatda
+	 * This will be provided by userspace compositors based on HDR content
+	 */
+	struct drm_property *hdr_output_metadata_property;
+
 	/* dumb ioctl parameters */
 	uint32_t preferred_depth, prefer_shadow;
 
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 927ad64..a065194 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -152,6 +152,16 @@ enum hdmi_content_type {
 	HDMI_CONTENT_TYPE_GAME,
 };
 
+enum hdmi_metadata_type {
+	HDMI_STATIC_METADATA_TYPE1 = 1,
+};
+
+enum hdmi_eotf {
+	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
+	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
+	HDMI_EOTF_SMPTE_ST2084,
+};
+
 struct hdmi_avi_infoframe {
 	enum hdmi_infoframe_type type;
 	unsigned char version;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 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

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

* [v5 02/13] drm: Parse HDR metadata info from EDID
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
  2019-03-11  3:57 ` [v5 01/13] drm: Add HDR source metadata property Uma Shankar
@ 2019-03-11  3:57 ` Uma Shankar
  2019-03-15  7:26   ` Sharma, Shashank
  2019-03-11  3:57 ` [v5 03/13] drm: Parse Colorimetry data block " Uma Shankar
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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

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

v3: No Change

v4: Addressed Shashank's review comments

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5f14253..c5a81b8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2834,6 +2834,7 @@ static int drm_cvt_modes(struct drm_connector *connector,
 #define VIDEO_BLOCK     0x02
 #define VENDOR_BLOCK    0x03
 #define SPEAKER_BLOCK	0x04
+#define HDR_STATIC_METADATA_BLOCK	0x6
 #define USE_EXTENDED_TAG 0x07
 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
 #define EXT_VIDEO_DATA_BLOCK_420	0x0E
@@ -3581,6 +3582,12 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
 }
 
 static int
+cea_db_payload_len_ext(const u8 *db)
+{
+	return (db[0] & 0x1f) - 1;
+}
+
+static int
 cea_db_extended_tag(const u8 *db)
 {
 	return db[1];
@@ -3816,6 +3823,49 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
 	mode->clock = clock;
 }
 
+static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
+{
+	if (cea_db_tag(db) != USE_EXTENDED_TAG)
+		return false;
+
+	if (db[1] != HDR_STATIC_METADATA_BLOCK)
+		return false;
+
+	return true;
+}
+
+static uint8_t eotf_supported(const u8 *edid_ext)
+{
+
+	return edid_ext[2] &
+		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
+		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
+		 BIT(HDMI_EOTF_SMPTE_ST2084));
+
+}
+
+static uint8_t hdr_metadata_type(const u8 *edid_ext)
+{
+
+	return edid_ext[3] &
+		BIT(HDMI_STATIC_METADATA_TYPE1);
+}
+
+static void
+drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db)
+{
+	uint16_t len;
+
+	len = cea_db_payload_len_ext(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)
 {
@@ -4443,6 +4493,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 			drm_parse_y420cmdb_bitmap(connector, db);
 		if (cea_db_is_vcdb(db))
 			drm_parse_vcdb(connector, db);
+		if (cea_db_is_hdmi_hdr_metadata_block(db))
+			drm_parse_hdr_metadata_block(connector, db);
 	}
 }
 
-- 
1.9.1

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

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

* [v5 03/13] drm: Parse Colorimetry data block from EDID
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
  2019-03-11  3:57 ` [v5 01/13] drm: Add HDR source metadata property Uma Shankar
  2019-03-11  3:57 ` [v5 02/13] drm: Parse HDR metadata info from EDID Uma Shankar
@ 2019-03-11  3:57 ` Uma Shankar
  2019-03-15  7:30   ` Sharma, Shashank
  2019-03-11  3:57 ` [v5 04/13] drm/i915: Attach HDR metadata property to connector Uma Shankar
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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

v2: Rebase

v3: No Change

v4: Addressed Shashank's review comments. Updated
colorimetry field to 16 bit as DCI-P3 got added
in CEA 861-G spec, as pointed out by Shashank.

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index c5a81b8..0470845 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2834,6 +2834,7 @@ static int drm_cvt_modes(struct drm_connector *connector,
 #define VIDEO_BLOCK     0x02
 #define VENDOR_BLOCK    0x03
 #define SPEAKER_BLOCK	0x04
+#define COLORIMETRY_DATA_BLOCK		0x5
 #define HDR_STATIC_METADATA_BLOCK	0x6
 #define USE_EXTENDED_TAG 0x07
 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
@@ -3823,6 +3824,29 @@ 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) != USE_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_ext(db);
+	/* As per CEA 861-G spec */
+	info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2];
+}
+
+
 static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
 {
 	if (cea_db_tag(db) != USE_EXTENDED_TAG)
@@ -4495,6 +4519,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 			drm_parse_vcdb(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 29388bd..94f926e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -206,6 +206,9 @@ struct drm_hdmi_info {
 
 	/** @y420_dc_modes: bitmap of deep color support index */
 	u8 y420_dc_modes;
+
+	/* @colorimetry: bitmap of supported colorimetry modes */
+	u16 colorimetry;
 };
 
 /**
-- 
1.9.1

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

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

* [v5 04/13] drm/i915: Attach HDR metadata property to connector
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (2 preceding siblings ...)
  2019-03-11  3:57 ` [v5 03/13] drm: Parse Colorimetry data block " Uma Shankar
@ 2019-03-11  3:57 ` Uma Shankar
  2019-03-15  7:36   ` Sharma, Shashank
  2019-03-11  3:57 ` [v5 05/13] drm: Implement HDR output metadata set and get property handling Uma Shankar
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

Attach HDR metadata property to connector object.

v2: Rebase

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

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 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 cd422a7..0952475 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2719,6 +2719,8 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
 
 	drm_connector_attach_content_type_property(connector);
 	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+	drm_object_attach_property(&connector->base,
+		connector->dev->mode_config.hdr_output_metadata_property, 0);
 
 	if (!HAS_GMCH(dev_priv))
 		drm_connector_attach_max_bpc_property(connector, 8, 12);
-- 
1.9.1

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

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

* [v5 05/13] drm: Implement HDR output metadata set and get property handling
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (3 preceding siblings ...)
  2019-03-11  3:57 ` [v5 04/13] drm/i915: Attach HDR metadata property to connector Uma Shankar
@ 2019-03-11  3:57 ` Uma Shankar
  2019-03-11  3:57 ` [v5 06/13] drm: Enable HDR infoframe support Uma Shankar
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel
  Cc: ville.syrjala, Liviu.Dudau, emil.l.velikov, Uma Shankar,
	maarten.lankhorst

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

v2: Rebase and added Ville's POC changes

v3: No Change

v4: Addressed Shashank's review comments

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..8b9c126 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -881,6 +881,8 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
 
 	drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name);
 	drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)");
+	drm_printf(p, "\thdr_metadata_changed=%d\n",
+		   state->hdr_metadata_changed);
 
 	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
 		if (state->writeback_job && state->writeback_job->fb)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 4eb81f1..18c8b81f 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_output_metadata_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+				&state->hdr_output_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) {
@@ -820,6 +830,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 		*val = state->colorspace;
 	} else if (property == connector->scaling_mode_property) {
 		*val = state->scaling_mode;
+	} else if (property == config->hdr_output_metadata_property) {
+		*val = (state->hdr_output_metadata_blob_ptr) ?
+			state->hdr_output_metadata_blob_ptr->base.id : 0;
 	} else if (property == connector->content_protection_property) {
 		*val = state->content_protection;
 	} else if (property == config->writeback_fb_id_property) {
-- 
1.9.1

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

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

* [v5 06/13] drm: Enable HDR infoframe support
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (4 preceding siblings ...)
  2019-03-11  3:57 ` [v5 05/13] drm: Implement HDR output metadata set and get property handling Uma Shankar
@ 2019-03-11  3:57 ` Uma Shankar
  2019-03-15  8:23   ` Sharma, Shashank
  2019-03-11  3:57 ` [v5 07/13] drm/i915: Write HDR infoframe and send to panel Uma Shankar
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel
  Cc: ville.syrjala, Liviu.Dudau, emil.l.velikov, Uma Shankar,
	maarten.lankhorst

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.

v3: No Change

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

v5: Rebase

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.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 0470845..49f8340 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4914,6 +4914,64 @@ static bool is_hdmi2_sink(struct drm_connector *connector)
 }
 
 /**
+ * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI AVI infoframe with
+ *                                         HDR metadata from userspace
+ * @frame: HDMI AVI infoframe
+ * @hdr_source_metadata: hdr_source_metadata info from userspace
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
+				    void *hdr_metadata)
+{
+	struct hdr_static_metadata *hdr_source_metadata;
+	int err, i;
+
+	if (!frame || !hdr_metadata)
+		return true;
+
+	err = hdmi_drm_infoframe_init(frame);
+	if (err < 0)
+		return err;
+
+	DRM_DEBUG_KMS("type = %x\n", frame->type);
+
+	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..7ab8086 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,
+				   const struct hdmi_drm_infoframe *frame)
+{
+	int i;
+
+	hdmi_infoframe_log_header(level, dev,
+			(struct hdmi_any_infoframe *)frame);
+	hdmi_log("length: %d\n", frame->length);
+	hdmi_log("metadata type: %d\n", frame->metadata_type);
+	hdmi_log("eotf: %d\n", frame->eotf);
+	for (i = 0; i < 3; i++) {
+		hdmi_log("x[%d]: %d\n", i, frame->display_primaries[i].x);
+		hdmi_log("y[%d]: %d\n", i, frame->display_primaries[i].y);
+	}
+
+	hdmi_log("white point x: %d\n", frame->white_point.x);
+	hdmi_log("white point y: %d\n", frame->white_point.y);
+
+	hdmi_log("max_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 9d3b5b9..973e43e 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -370,6 +370,10 @@ int drm_av_sync_delay(struct drm_connector *connector,
 				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range);
 
+int
+drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
+				    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 a065194..33243b2 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -47,6 +47,7 @@ enum hdmi_infoframe_type {
 	HDMI_INFOFRAME_TYPE_AVI = 0x82,
 	HDMI_INFOFRAME_TYPE_SPD = 0x83,
 	HDMI_INFOFRAME_TYPE_AUDIO = 0x84,
+	HDMI_INFOFRAME_TYPE_DRM = 0x87,
 };
 
 #define HDMI_IEEE_OUI 0x000c03
@@ -185,12 +186,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,
@@ -365,6 +386,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

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

* [v5 07/13] drm/i915: Write HDR infoframe and send to panel
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (5 preceding siblings ...)
  2019-03-11  3:57 ` [v5 06/13] drm: Enable HDR infoframe support Uma Shankar
@ 2019-03-11  3:57 ` Uma Shankar
  2019-03-15 11:07   ` Sharma, Shashank
  2019-03-11  3:57 ` [v5 08/13] drm/i915: [DO NOT MERGE] hack for glk board outputs Uma Shankar
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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

v2: Rebase

v3: Fixed a warning message

v4: Addressed Shashank's review comments

v5: Rebase. Added infoframe calculation in compute config.

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

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 58483f8..a5ee124 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1037,6 +1037,7 @@ struct intel_crtc_state {
 		union hdmi_infoframe avi;
 		union hdmi_infoframe spd;
 		union hdmi_infoframe hdmi;
+		union hdmi_infoframe drm;
 	} infoframes;
 
 	/* HDMI scrambling status */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 0952475..ea7afa0 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -555,6 +555,7 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
 	HDMI_INFOFRAME_TYPE_AVI,
 	HDMI_INFOFRAME_TYPE_SPD,
 	HDMI_INFOFRAME_TYPE_VENDOR,
+	HDMI_INFOFRAME_TYPE_DRM,
 };
 
 u32 intel_hdmi_infoframe_enable(unsigned int type)
@@ -777,6 +778,30 @@ void intel_read_infoframe(struct intel_encoder *encoder,
 	return true;
 }
 
+static bool
+intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
+				  struct intel_crtc_state *crtc_state,
+				  struct drm_connector_state *conn_state)
+{
+	struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm;
+	struct hdr_static_metadata *hdr_metadata;
+	int ret;
+
+	hdr_metadata = (struct hdr_static_metadata *)
+			conn_state->hdr_output_metadata_blob_ptr->data;
+
+	ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
+	if (ret < 0) {
+		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
+		return false;
+	}
+
+	crtc_state->infoframes.enable |=
+		intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);
+
+	return true;
+}
+
 static void g4x_set_infoframes(struct intel_encoder *encoder,
 			       bool enable,
 			       const struct intel_crtc_state *crtc_state,
@@ -1175,6 +1200,9 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_VENDOR,
 			      &crtc_state->infoframes.hdmi);
+	intel_write_infoframe(encoder, crtc_state,
+			      HDMI_INFOFRAME_TYPE_DRM,
+			      &crtc_state->infoframes.drm);
 }
 
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable)
@@ -2381,6 +2409,11 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 		return -EINVAL;
 	}
 
+	if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config, conn_state)) {
+		DRM_DEBUG_KMS("bad DRM infoframe\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
1.9.1

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

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

* [v5 08/13] drm/i915: [DO NOT MERGE] hack for glk board outputs
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (6 preceding siblings ...)
  2019-03-11  3:57 ` [v5 07/13] drm/i915: Write HDR infoframe and send to panel Uma Shankar
@ 2019-03-11  3:57 ` Uma Shankar
  2019-03-11  3:58 ` [v5 09/13] drm/i915: Add HLG EOTF Uma Shankar
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:57 UTC (permalink / raw)
  To: intel-gfx, dri-devel
  Cc: ville.syrjala, Liviu.Dudau, emil.l.velikov, Uma Shankar,
	maarten.lankhorst

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 b508d8a..a2fd93ab 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1380,6 +1380,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

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

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

* [v5 09/13] drm/i915: Add HLG EOTF
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (7 preceding siblings ...)
  2019-03-11  3:57 ` [v5 08/13] drm/i915: [DO NOT MERGE] hack for glk board outputs Uma Shankar
@ 2019-03-11  3:58 ` Uma Shankar
  2019-03-15 11:09   ` Sharma, Shashank
  2019-03-11  3:58 ` [v5 10/13] drm/i915: Enable infoframes on GLK+ for HDR Uma Shankar
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:58 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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

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

v2: Rebase

v3: Fixed a warning message

v4: Addressed Shashank's review comments

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 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 49f8340..4cd22e8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3864,8 +3864,8 @@ static uint8_t eotf_supported(const u8 *edid_ext)
 	return edid_ext[2] &
 		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
 		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
-		 BIT(HDMI_EOTF_SMPTE_ST2084));
-
+		 BIT(HDMI_EOTF_SMPTE_ST2084) |
+		 BIT(HDMI_EOTF_BT_2100_HLG));
 }
 
 static uint8_t hdr_metadata_type(const u8 *edid_ext)
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 33243b2..ad652e6 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -161,6 +161,7 @@ enum hdmi_eotf {
 	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
 	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
 	HDMI_EOTF_SMPTE_ST2084,
+	HDMI_EOTF_BT_2100_HLG,
 };
 
 struct hdmi_avi_infoframe {
-- 
1.9.1

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

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

* [v5 10/13] drm/i915: Enable infoframes on GLK+ for HDR
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (8 preceding siblings ...)
  2019-03-11  3:58 ` [v5 09/13] drm/i915: Add HLG EOTF Uma Shankar
@ 2019-03-11  3:58 ` Uma Shankar
  2019-03-11  3:58 ` [v5 11/13] drm/i915:Enabled Modeset when HDR Infoframe changes Uma Shankar
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:58 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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

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

v2: Addressed Shashank's review comment.

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 | 18 +++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6943e08..cb3cdd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4665,6 +4665,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
@@ -8111,6 +8112,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
@@ -8124,6 +8126,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
@@ -8149,6 +8152,7 @@ enum {
 #define HSW_TVIDEO_DIP_SPD_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_SPD_DATA_A + (i) * 4)
 #define HSW_TVIDEO_DIP_GMP_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_GMP_DATA_A + (i) * 4)
 #define HSW_TVIDEO_DIP_VSC_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_VSC_DATA_A + (i) * 4)
+#define GLK_TVIDEO_DIP_DRM_DATA(trans, i)	_MMIO_TRANS2(trans, _GLK_VIDEO_DIP_DRM_DATA_A + (i) * 4)
 #define ICL_VIDEO_DIP_PPS_DATA(trans, i)	_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_DATA_A + (i) * 4)
 #define ICL_VIDEO_DIP_PPS_ECC(trans, i)		_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_ECC_A + (i) * 4)
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ea7afa0..0a6d4b7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -134,6 +134,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;
@@ -159,6 +161,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;
@@ -545,7 +549,8 @@ static u32 hsw_infoframes_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);
 }
 
 static const u8 infoframe_type_to_idx[] = {
@@ -1177,7 +1182,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);
@@ -1200,9 +1206,11 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 	intel_write_infoframe(encoder, crtc_state,
 			      HDMI_INFOFRAME_TYPE_VENDOR,
 			      &crtc_state->infoframes.hdmi);
-	intel_write_infoframe(encoder, crtc_state,
-			      HDMI_INFOFRAME_TYPE_DRM,
-			      &crtc_state->infoframes.drm);
+	if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+	    conn_state->hdr_metadata_changed)
+		intel_write_infoframe(encoder, crtc_state,
+				      HDMI_INFOFRAME_TYPE_DRM,
+				      &crtc_state->infoframes.drm);
 }
 
 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

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

* [v5 11/13] drm/i915:Enabled Modeset when HDR Infoframe changes
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (9 preceding siblings ...)
  2019-03-11  3:58 ` [v5 10/13] drm/i915: Enable infoframes on GLK+ for HDR Uma Shankar
@ 2019-03-11  3:58 ` Uma Shankar
  2019-03-11  3:58 ` [v5 12/13] drm/i915: Set Infoframe for non modeset case for HDR Uma Shankar
  2019-03-11  3:58 ` [v5 13/13] video/hdmi: Add const variants for drm infoframe Uma Shankar
  12 siblings, 0 replies; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:58 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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 da419e1..8d5dd41 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -102,6 +102,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)
 {
@@ -129,7 +139,10 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
 	    new_conn_state->base.colorspace != old_conn_state->base.colorspace ||
 	    new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio ||
 	    new_conn_state->base.content_type != old_conn_state->base.content_type ||
-	    new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode)
+	    new_conn_state->base.scaling_mode !=
+				old_conn_state->base.scaling_mode ||
+	    !blob_equal(new_conn_state->base.hdr_output_metadata_blob_ptr,
+			old_conn_state->base.hdr_output_metadata_blob_ptr))
 		crtc_state->mode_changed = true;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 0a6d4b7..03a7761 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -792,6 +792,10 @@ void intel_read_infoframe(struct intel_encoder *encoder,
 	struct hdr_static_metadata *hdr_metadata;
 	int ret;
 
+	if (!conn_state->hdr_output_metadata_blob_ptr ||
+		conn_state->hdr_output_metadata_blob_ptr->length == 0)
+		return true;
+
 	hdr_metadata = (struct hdr_static_metadata *)
 			conn_state->hdr_output_metadata_blob_ptr->data;
 
-- 
1.9.1

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

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

* [v5 12/13] drm/i915: Set Infoframe for non modeset case for HDR
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (10 preceding siblings ...)
  2019-03-11  3:58 ` [v5 11/13] drm/i915:Enabled Modeset when HDR Infoframe changes Uma Shankar
@ 2019-03-11  3:58 ` Uma Shankar
  2019-03-11  3:58 ` [v5 13/13] video/hdmi: Add const variants for drm infoframe Uma Shankar
  12 siblings, 0 replies; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:58 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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

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

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7e3b4e8..2f2d9ae 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3566,6 +3566,10 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *intel_dig_port =
+			enc_to_dig_port(&encoder->base);
+
 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
 
@@ -3575,6 +3579,15 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 	else if (conn_state->content_protection ==
 		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
 		intel_hdcp_disable(to_intel_connector(conn_state->connector));
+
+	/* Set the infoframe for NON modeset cases as well */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
+		if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+			conn_state->hdr_metadata_changed)
+			intel_dig_port->set_infoframes(encoder,
+						       crtc_state->has_infoframe,
+						       crtc_state, conn_state);
+	}
 }
 
 static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
-- 
1.9.1

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

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

* [v5 13/13] video/hdmi: Add const variants for drm infoframe
  2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
                   ` (11 preceding siblings ...)
  2019-03-11  3:58 ` [v5 12/13] drm/i915: Set Infoframe for non modeset case for HDR Uma Shankar
@ 2019-03-11  3:58 ` Uma Shankar
  12 siblings, 0 replies; 29+ messages in thread
From: Uma Shankar @ 2019-03-11  3:58 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst

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

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/video/hdmi.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/hdmi.h |  5 +++++
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 7ab8086..b1b21ae 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -668,6 +668,30 @@ int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame)
 }
 EXPORT_SYMBOL(hdmi_drm_infoframe_init);
 
+static int hdmi_drm_infoframe_check_only(const struct hdmi_drm_infoframe *frame)
+{
+	if (frame->type != HDMI_INFOFRAME_TYPE_DRM ||
+	    frame->version != 1)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * hdmi_drm_infoframe_check() - check a HDMI DRM infoframe
+ * @frame: HDMI DRM infoframe
+ *
+ * Validates that the infoframe is consistent and updates derived fields
+ * (eg. length) based on other fields.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_drm_infoframe_check(struct hdmi_drm_infoframe *frame)
+{
+	return hdmi_drm_infoframe_check_only(frame);
+}
+EXPORT_SYMBOL(hdmi_drm_infoframe_check);
+
 /**
  * hdmi_drm_infoframe_pack() - write HDMI DRM infoframe to binary buffer
  * @frame: HDMI DRM infoframe
@@ -682,8 +706,8 @@ int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame)
  * 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)
+ssize_t hdmi_drm_infoframe_pack_only(const struct hdmi_drm_infoframe *frame,
+				     void *buffer, size_t size)
 {
 	u8 *ptr = buffer;
 	size_t length;
@@ -736,6 +760,37 @@ ssize_t hdmi_drm_infoframe_pack(struct hdmi_drm_infoframe *frame, void *buffer,
 
 	return length;
 }
+EXPORT_SYMBOL(hdmi_drm_infoframe_pack_only);
+
+/**
+ * hdmi_drm_infoframe_pack() - check a HDMI DRM infoframe,
+ *                             and write it to binary buffer
+ * @frame: HDMI DRM infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Validates that the infoframe is consistent and updates derived fields
+ * (eg. length) based on other fields, after which it packs the information
+ * contained in the @frame structure into a binary representation that
+ * can be written into the corresponding controller registers. This function
+ * also computes the checksum as required by section 5.3.5 of the HDMI 1.4
+ * specification.
+ *
+ * Returns the number of bytes packed into the binary buffer or a negative
+ * error code on failure.
+ */
+ssize_t hdmi_drm_infoframe_pack(struct hdmi_drm_infoframe *frame,
+				void *buffer, size_t size)
+{
+	int ret;
+
+	ret = hdmi_drm_infoframe_check(frame);
+	if (ret)
+		return ret;
+
+	return hdmi_drm_infoframe_pack_only(frame, buffer, size);
+}
+EXPORT_SYMBOL(hdmi_drm_infoframe_pack);
 
 /*
  * hdmi_vendor_any_infoframe_check() - check a vendor infoframe
@@ -845,6 +900,10 @@ ssize_t hdmi_drm_infoframe_pack(struct hdmi_drm_infoframe *frame, void *buffer,
 		length = hdmi_avi_infoframe_pack_only(&frame->avi,
 						      buffer, size);
 		break;
+	case HDMI_INFOFRAME_TYPE_DRM:
+		length = hdmi_drm_infoframe_pack_only(&frame->drm,
+						      buffer, size);
+		break;
 	case HDMI_INFOFRAME_TYPE_SPD:
 		length = hdmi_spd_infoframe_pack_only(&frame->spd,
 						      buffer, size);
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index ad652e6..fbeb06e3 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -213,6 +213,11 @@ ssize_t hdmi_avi_infoframe_pack_only(const struct hdmi_avi_infoframe *frame,
 				     void *buffer, size_t size);
 int hdmi_avi_infoframe_check(struct hdmi_avi_infoframe *frame);
 int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame);
+ssize_t hdmi_drm_infoframe_pack(struct hdmi_drm_infoframe *frame, void *buffer,
+				size_t size);
+ssize_t hdmi_drm_infoframe_pack_only(const struct hdmi_drm_infoframe *frame,
+				     void *buffer, size_t size);
+int hdmi_drm_infoframe_check(struct hdmi_drm_infoframe *frame);
 
 enum hdmi_spd_sdi {
 	HDMI_SPD_SDI_UNKNOWN,
-- 
1.9.1

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

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

* Re: [v5 01/13] drm: Add HDR source metadata property
  2019-03-11  3:57 ` [v5 01/13] drm: Add HDR source metadata property Uma Shankar
@ 2019-03-15  7:08   ` Sharma, Shashank
  2019-03-20  6:21     ` Shankar, Uma
  0 siblings, 1 reply; 29+ messages in thread
From: Sharma, Shashank @ 2019-03-15  7:08 UTC (permalink / raw)
  To: Shankar, Uma, intel-gfx, dri-devel; +Cc: Syrjala, Ville, Lankhorst, Maarten

Hello Uma, 

> -----Original Message-----
> From: Shankar, Uma
> Sent: Monday, March 11, 2019 9:28 AM
> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
> <ville.syrjala@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>;
> emil.l.velikov@gmail.com; brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar,
> Uma <uma.shankar@intel.com>
> Subject: [v5 01/13] drm: Add HDR source metadata property
> 
> 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.
> 
> v3: No Change
> 
> v4: Addressed Shashank's review comments
> 
> v5: Rebase.
> 
> 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 07d65a1..b031079 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1054,6 +1054,12 @@ int drm_connector_create_standard_properties(struct
> drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.non_desktop_property = prop;
> 
> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
> +				   "HDR_OUTPUT_METADATA", 0);
Needs alignment with the line above. 
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.hdr_output_metadata_property = prop;
> +
>  	return 0;
>  }
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index
> c806199..29388bd 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -561,6 +561,13 @@ struct drm_connector_state {
>  	 * and the connector bpc limitations obtained from edid.
>  	 */
>  	u8 max_bpc;
> +
> +	/**
> +	 * @metadata_blob_ptr:
> +	 * DRM blob property for HDR output metadata
> +	 */
> +	struct drm_property_blob *hdr_output_metadata_blob_ptr;
> +	u8 hdr_metadata_changed : 1;
>  };
> 
>  /**
> @@ -1201,6 +1208,9 @@ struct drm_connector {
>  	 * &drm_mode_config.connector_free_work.
>  	 */
>  	struct llist_node free_node;
> +
> +	/* HDR metdata */
> +	struct hdr_static_metadata hdr_metadata;
I was under the assumption that we are not parsing the hdr_metadata from the EDID, and the userspace needs to parse it from the EDID. If that's the case, we might not even need this pointer. Are we even using this or am I missing something here ? 
>  };
> 
>  #define obj_to_connector(x) container_of(x, struct drm_connector, base) diff --git
> a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index
> 7f60e8e..ef2656b 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -836,6 +836,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *writeback_out_fence_ptr_property;
> 
> +	/*
> +	 * hdr_metadata_property: Connector property containing hdr metatda
> +	 * This will be provided by userspace compositors based on HDR content
> +	 */
> +	struct drm_property *hdr_output_metadata_property;
> +
>  	/* dumb ioctl parameters */
>  	uint32_t preferred_depth, prefer_shadow;
> 
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 927ad64..a065194
> 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -152,6 +152,16 @@ enum hdmi_content_type {
>  	HDMI_CONTENT_TYPE_GAME,
>  };
> 
> +enum hdmi_metadata_type {
> +	HDMI_STATIC_METADATA_TYPE1 = 1,
> +};
> +
> +enum hdmi_eotf {
> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
> +	HDMI_EOTF_SMPTE_ST2084,
> +};
> +
>  struct hdmi_avi_infoframe {
>  	enum hdmi_infoframe_type type;
>  	unsigned char version;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index
> 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;
> +};
> +
- Shashank
>  #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

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

* RE: [v5 02/13] drm: Parse HDR metadata info from EDID
  2019-03-11  3:57 ` [v5 02/13] drm: Parse HDR metadata info from EDID Uma Shankar
@ 2019-03-15  7:26   ` Sharma, Shashank
  2019-03-20  6:37     ` Shankar, Uma
  0 siblings, 1 reply; 29+ messages in thread
From: Sharma, Shashank @ 2019-03-15  7:26 UTC (permalink / raw)
  To: Shankar, Uma, intel-gfx, dri-devel
  Cc: emil.l.velikov, Liviu.Dudau, Syrjala, Ville, Lankhorst, Maarten



> -----Original Message-----
> From: Shankar, Uma
> Sent: Monday, March 11, 2019 9:28 AM
> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
> <ville.syrjala@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>;
> emil.l.velikov@gmail.com; brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar,
> Uma <uma.shankar@intel.com>
> Subject: [v5 02/13] drm: Parse HDR metadata info from EDID
> 
> HDR metadata block is introduced in CEA-861.3 spec.
> Parsing the same to get the panel's HDR metadata.
> 
> v2: Rebase and added Ville's POC changes to the patch.
> 
> v3: No Change
> 
> v4: Addressed Shashank's review comments
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 52
> ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index
> 5f14253..c5a81b8 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2834,6 +2834,7 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
>  #define VIDEO_BLOCK     0x02
>  #define VENDOR_BLOCK    0x03
>  #define SPEAKER_BLOCK	0x04
> +#define HDR_STATIC_METADATA_BLOCK	0x6
>  #define USE_EXTENDED_TAG 0x07
>  #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
>  #define EXT_VIDEO_DATA_BLOCK_420	0x0E
> @@ -3581,6 +3582,12 @@ static int add_3d_struct_modes(struct drm_connector
> *connector, u16 structure,  }
> 
>  static int
> +cea_db_payload_len_ext(const u8 *db)
> +{
> +	return (db[0] & 0x1f) - 1;
You might have already checked with checkpatch, but can we cross check the space before '-' ? 
> +}
> +
> +static int
>  cea_db_extended_tag(const u8 *db)
>  {
>  	return db[1];
> @@ -3816,6 +3823,49 @@ static void fixup_detailed_cea_mode_clock(struct
> drm_display_mode *mode)
>  	mode->clock = clock;
>  }
> 
> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) {
> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
> +		return false;
> +
> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
same here,  looks like we need spaces around !=, or may be this is due to my mail client  ? 
> +		return false;
> +
> +	return true;
> +}
> +
> +static uint8_t eotf_supported(const u8 *edid_ext) {
> +
> +	return edid_ext[2] &
> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
Again, space before the '|' sign, please excuse me if this is false alarm ... 
> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
> +
> +}
> +
> +static uint8_t hdr_metadata_type(const u8 *edid_ext) {
> +
> +	return edid_ext[3] &
> +		BIT(HDMI_STATIC_METADATA_TYPE1);
> +}
> +
> +static void
> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8
> +*db) {
Shouldn't this variable go to line above ? 
> +	uint16_t len;
> +
> +	len = cea_db_payload_len_ext(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]; }
This '}' certainly should not be here.  
> +
>  static void
>  drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)  { @@
> -4443,6 +4493,8 @@ static void drm_parse_cea_ext(struct drm_connector
> *connector,
>  			drm_parse_y420cmdb_bitmap(connector, db);
>  		if (cea_db_is_vcdb(db))
>  			drm_parse_vcdb(connector, db);
> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
> +			drm_parse_hdr_metadata_block(connector, db);
- Shashank
>  	}
>  }
> 
> --
> 1.9.1

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

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

* Re: [v5 03/13] drm: Parse Colorimetry data block from EDID
  2019-03-11  3:57 ` [v5 03/13] drm: Parse Colorimetry data block " Uma Shankar
@ 2019-03-15  7:30   ` Sharma, Shashank
  2019-03-20  7:17     ` Shankar, Uma
  0 siblings, 1 reply; 29+ messages in thread
From: Sharma, Shashank @ 2019-03-15  7:30 UTC (permalink / raw)
  To: Shankar, Uma, intel-gfx, dri-devel; +Cc: Syrjala, Ville, Lankhorst, Maarten



> -----Original Message-----
> From: Shankar, Uma
> Sent: Monday, March 11, 2019 9:28 AM
> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
> <ville.syrjala@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>;
> emil.l.velikov@gmail.com; brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar,
> Uma <uma.shankar@intel.com>
> Subject: [v5 03/13] drm: Parse Colorimetry data block from EDID
> 
> CEA 861.3 spec adds colorimetry data block for HDMI.
> Parsing the block to get the colorimetry data from panel.
> 
> v2: Rebase
> 
> v3: No Change
> 
> v4: Addressed Shashank's review comments. Updated colorimetry field to 16 bit as
> DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c  | 26 ++++++++++++++++++++++++++
> include/drm/drm_connector.h |  3 +++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index
> c5a81b8..0470845 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2834,6 +2834,7 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
>  #define VIDEO_BLOCK     0x02
>  #define VENDOR_BLOCK    0x03
>  #define SPEAKER_BLOCK	0x04
> +#define COLORIMETRY_DATA_BLOCK		0x5
>  #define HDR_STATIC_METADATA_BLOCK	0x6
>  #define USE_EXTENDED_TAG 0x07
>  #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@ 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) {
Same comment for the opening brace above ? 
> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
> +		return false;
> +
> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
Space before '!' ? 
> +		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_ext(db);
> +	/* As per CEA 861-G spec */
> +	info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; }
This means we are setting the 15th bit for DCI-P3 then there is a gap of 7 bits, and then there are other bits. Do we want to make DCI-P3 as bit 8 ? Anyways this can be differed for a discussion later. 
> +
> +
>  static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)  {
>  	if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@ static
> void drm_parse_cea_ext(struct drm_connector *connector,
>  			drm_parse_vcdb(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
> 29388bd..94f926e 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -206,6 +206,9 @@ struct drm_hdmi_info {
> 
>  	/** @y420_dc_modes: bitmap of deep color support index */
>  	u8 y420_dc_modes;
> +
> +	/* @colorimetry: bitmap of supported colorimetry modes */
> +	u16 colorimetry;
- Shashank
>  };
> 
>  /**
> --
> 1.9.1

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

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

* Re: [v5 04/13] drm/i915: Attach HDR metadata property to connector
  2019-03-11  3:57 ` [v5 04/13] drm/i915: Attach HDR metadata property to connector Uma Shankar
@ 2019-03-15  7:36   ` Sharma, Shashank
  2019-03-20  7:58     ` Shankar, Uma
  0 siblings, 1 reply; 29+ messages in thread
From: Sharma, Shashank @ 2019-03-15  7:36 UTC (permalink / raw)
  To: Shankar, Uma, intel-gfx, dri-devel; +Cc: Syrjala, Ville, Lankhorst, Maarten



> -----Original Message-----
> From: Shankar, Uma
> Sent: Monday, March 11, 2019 9:28 AM
> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
> <ville.syrjala@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>;
> emil.l.velikov@gmail.com; brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar,
> Uma <uma.shankar@intel.com>
> Subject: [v5 04/13] drm/i915: Attach HDR metadata property to connector
> 
> Attach HDR metadata property to connector object.
> 
> v2: Rebase
> 
> v3: Updated the property name as per updated name while creating hdr metadata
> property
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  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 cd422a7..0952475 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2719,6 +2719,8 @@ static void intel_hdmi_destroy(struct drm_connector
> *connector)
> 
>  	drm_connector_attach_content_type_property(connector);
>  	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> +	drm_object_attach_property(&connector->base,
> +		connector->dev->mode_config.hdr_output_metadata_property, 0);
'connector->dev' should be aligned to the line above, but seems like you have done this to meet the 80 char limit. 
May be a local ptr variable will help with 80 char. 

Apart from that, looks good to me, and feel free to use:
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> 
>  	if (!HAS_GMCH(dev_priv))
>  		drm_connector_attach_max_bpc_property(connector, 8, 12);
> --
> 1.9.1

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

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

* Re: [v5 06/13] drm: Enable HDR infoframe support
  2019-03-11  3:57 ` [v5 06/13] drm: Enable HDR infoframe support Uma Shankar
@ 2019-03-15  8:23   ` Sharma, Shashank
  2019-03-20  9:24     ` Shankar, Uma
  0 siblings, 1 reply; 29+ messages in thread
From: Sharma, Shashank @ 2019-03-15  8:23 UTC (permalink / raw)
  To: Uma Shankar, intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst


On 3/11/2019 9:27 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.
>
> v3: No Change
>
> v4: Addressed Shashank's review comments and merged the
> patch making drm infoframe function arguments as constant.
>
> v5: Rebase
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.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 0470845..49f8340 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4914,6 +4914,64 @@ static bool is_hdmi2_sink(struct drm_connector *connector)
>   }
>   
>   /**
> + * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI AVI infoframe with
> + *                                         HDR metadata from userspace
> + * @frame: HDMI AVI infoframe
> + * @hdr_source_metadata: hdr_source_metadata info from userspace
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int
> +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
> +				    void *hdr_metadata)
Alignment with bracket above
> +{
> +	struct hdr_static_metadata *hdr_source_metadata;
> +	int err, i;
> +
> +	if (!frame || !hdr_metadata)
> +		return true;
> +
> +	err = hdmi_drm_infoframe_init(frame);
> +	if (err < 0)
> +		return err;
> +
> +	DRM_DEBUG_KMS("type = %x\n", frame->type);
> +
> +	hdr_source_metadata = (struct hdr_static_metadata *)hdr_metadata;
> +
> +	frame->length = sizeof(struct hdr_static_metadata);
> +
extra blank line
> +
> +	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;
> +	}
memcpy here to avoid loop ?
> +
> +	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;
> +
In fact the structures look so analogues, can we do a complete memcpy of 
one structure to other ? or the sequence of the elements is different ?
> +	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..7ab8086 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;
&0xff is not required
> +		*ptr++ = frame->display_primaries[i].x >> 8;
> +		*ptr++ = frame->display_primaries[i].y & 0xff;
&0xff is not required, applicable for all below
> +		*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,
> +				   const struct hdmi_drm_infoframe *frame)
> +{
> +	int i;
> +
> +	hdmi_infoframe_log_header(level, dev,
> +			(struct hdmi_any_infoframe *)frame);
Alignment with above '('
> +	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);
Alignment for these 2 lines above
> +
> +	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 9d3b5b9..973e43e 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -370,6 +370,10 @@ int drm_av_sync_delay(struct drm_connector *connector,
>   				   const struct drm_display_mode *mode,
>   				   enum hdmi_quantization_range rgb_quant_range);
>   
> +int
> +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
> +				    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 a065194..33243b2 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -47,6 +47,7 @@ enum hdmi_infoframe_type {
>   	HDMI_INFOFRAME_TYPE_AVI = 0x82,
>   	HDMI_INFOFRAME_TYPE_SPD = 0x83,
>   	HDMI_INFOFRAME_TYPE_AUDIO = 0x84,
> +	HDMI_INFOFRAME_TYPE_DRM = 0x87,
>   };
>   
>   #define HDMI_IEEE_OUI 0x000c03
> @@ -185,12 +186,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,
> @@ -365,6 +386,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;
>   };

With the minor issues pointed above fixed, feel free to use:

Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

- Shashank

>   
>   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

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

* Re: [v5 07/13] drm/i915: Write HDR infoframe and send to panel
  2019-03-11  3:57 ` [v5 07/13] drm/i915: Write HDR infoframe and send to panel Uma Shankar
@ 2019-03-15 11:07   ` Sharma, Shashank
  2019-03-20  9:46     ` Shankar, Uma
  0 siblings, 1 reply; 29+ messages in thread
From: Sharma, Shashank @ 2019-03-15 11:07 UTC (permalink / raw)
  To: Uma Shankar, intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst


On 3/11/2019 9:27 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
>
> v3: Fixed a warning message
>
> v4: Addressed Shashank's review comments
>
> v5: Rebase. Added infoframe calculation in compute config.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_drv.h  |  1 +
>   drivers/gpu/drm/i915/intel_hdmi.c | 33 +++++++++++++++++++++++++++++++++
>   2 files changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 58483f8..a5ee124 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1037,6 +1037,7 @@ struct intel_crtc_state {
>   		union hdmi_infoframe avi;
>   		union hdmi_infoframe spd;
>   		union hdmi_infoframe hdmi;
> +		union hdmi_infoframe drm;
>   	} infoframes;
>   
>   	/* HDMI scrambling status */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 0952475..ea7afa0 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -555,6 +555,7 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
>   	HDMI_INFOFRAME_TYPE_AVI,
>   	HDMI_INFOFRAME_TYPE_SPD,
>   	HDMI_INFOFRAME_TYPE_VENDOR,
> +	HDMI_INFOFRAME_TYPE_DRM,
>   };
>   
>   u32 intel_hdmi_infoframe_enable(unsigned int type)
> @@ -777,6 +778,30 @@ void intel_read_infoframe(struct intel_encoder *encoder,
>   	return true;
>   }
>   
> +static bool
> +intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
> +				  struct intel_crtc_state *crtc_state,
> +				  struct drm_connector_state *conn_state)
> +{
> +	struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm;
> +	struct hdr_static_metadata *hdr_metadata;
> +	int ret;
> +

Don't we need a GEN/monitor check before going ahead with HDR metadata ?

- Shashank

> +	hdr_metadata = (struct hdr_static_metadata *)
> +			conn_state->hdr_output_metadata_blob_ptr->data;
> +
> +	ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
> +	if (ret < 0) {
> +		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
> +		return false;
> +	}
> +
> +	crtc_state->infoframes.enable |=
> +		intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);
> +
> +	return true;
> +}
> +
>   static void g4x_set_infoframes(struct intel_encoder *encoder,
>   			       bool enable,
>   			       const struct intel_crtc_state *crtc_state,
> @@ -1175,6 +1200,9 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
>   	intel_write_infoframe(encoder, crtc_state,
>   			      HDMI_INFOFRAME_TYPE_VENDOR,
>   			      &crtc_state->infoframes.hdmi);
> +	intel_write_infoframe(encoder, crtc_state,
> +			      HDMI_INFOFRAME_TYPE_DRM,
> +			      &crtc_state->infoframes.drm);
>   }
>   
>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable)
> @@ -2381,6 +2409,11 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>   		return -EINVAL;
>   	}
>   
> +	if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config, conn_state)) {
> +		DRM_DEBUG_KMS("bad DRM infoframe\n");
> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [v5 09/13] drm/i915: Add HLG EOTF
  2019-03-11  3:58 ` [v5 09/13] drm/i915: Add HLG EOTF Uma Shankar
@ 2019-03-15 11:09   ` Sharma, Shashank
  0 siblings, 0 replies; 29+ messages in thread
From: Sharma, Shashank @ 2019-03-15 11:09 UTC (permalink / raw)
  To: Uma Shankar, intel-gfx, dri-devel; +Cc: ville.syrjala, maarten.lankhorst


On 3/11/2019 9:28 AM, Uma Shankar wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> ADD HLG EOTF to the list of EOTF transfer functions supported.
> Hybrid Log-Gamma (HLG) is a high dynamic range (HDR) standard.
> HLG defines a nonlinear transfer function in which the lower
> half of the signal values use a gamma curve and the upper half
> of the signal values use a logarithmic curve.
>
> v2: Rebase
>
> v3: Fixed a warning message
>
> v4: Addressed Shashank's review comments
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   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 49f8340..4cd22e8 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3864,8 +3864,8 @@ static uint8_t eotf_supported(const u8 *edid_ext)
>   	return edid_ext[2] &
>   		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
>   		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
> -		 BIT(HDMI_EOTF_SMPTE_ST2084));
> -
> +		 BIT(HDMI_EOTF_SMPTE_ST2084) |
> +		 BIT(HDMI_EOTF_BT_2100_HLG));
>   }
>   
>   static uint8_t hdr_metadata_type(const u8 *edid_ext)
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 33243b2..ad652e6 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -161,6 +161,7 @@ enum hdmi_eotf {
>   	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
>   	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
>   	HDMI_EOTF_SMPTE_ST2084,
> +	HDMI_EOTF_BT_2100_HLG,
>   };
>   

Looks good to me,

Feel free to use: Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

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

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

* RE: [v5 01/13] drm: Add HDR source metadata property
  2019-03-15  7:08   ` Sharma, Shashank
@ 2019-03-20  6:21     ` Shankar, Uma
  0 siblings, 0 replies; 29+ messages in thread
From: Shankar, Uma @ 2019-03-20  6:21 UTC (permalink / raw)
  To: Sharma, Shashank, intel-gfx, dri-devel
  Cc: emil.l.velikov, Liviu.Dudau, Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, March 15, 2019 12:39 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>; emil.l.velikov@gmail.com; brian.starkey@arm.com;
>Liviu.Dudau@arm.com
>Subject: RE: [v5 01/13] drm: Add HDR source metadata property
>
>Hello Uma,
>
>> -----Original Message-----
>> From: Shankar, Uma
>> Sent: Monday, March 11, 2019 9:28 AM
>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
>> <ville.syrjala@intel.com>; Sharma, Shashank
>> <shashank.sharma@intel.com>; emil.l.velikov@gmail.com;
>> brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar, Uma
>> <uma.shankar@intel.com>
>> Subject: [v5 01/13] drm: Add HDR source metadata property
>>
>> 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.
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments
>>
>> v5: Rebase.
>>
>> 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 07d65a1..b031079 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1054,6 +1054,12 @@ int
>> drm_connector_create_standard_properties(struct
>> drm_device *dev)
>>  		return -ENOMEM;
>>  	dev->mode_config.non_desktop_property = prop;
>>
>> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
>> +				   "HDR_OUTPUT_METADATA", 0);
>Needs alignment with the line above.

Look like this is an issue with outlook. The changes are aligned actually.

>> +	if (!prop)
>> +		return -ENOMEM;
>> +	dev->mode_config.hdr_output_metadata_property = prop;
>> +
>>  	return 0;
>>  }
>>
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index c806199..29388bd 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -561,6 +561,13 @@ struct drm_connector_state {
>>  	 * and the connector bpc limitations obtained from edid.
>>  	 */
>>  	u8 max_bpc;
>> +
>> +	/**
>> +	 * @metadata_blob_ptr:
>> +	 * DRM blob property for HDR output metadata
>> +	 */
>> +	struct drm_property_blob *hdr_output_metadata_blob_ptr;
>> +	u8 hdr_metadata_changed : 1;
>>  };
>>
>>  /**
>> @@ -1201,6 +1208,9 @@ struct drm_connector {
>>  	 * &drm_mode_config.connector_free_work.
>>  	 */
>>  	struct llist_node free_node;
>> +
>> +	/* HDR metdata */
>> +	struct hdr_static_metadata hdr_metadata;
>I was under the assumption that we are not parsing the hdr_metadata from the EDID,
>and the userspace needs to parse it from the EDID. If that's the case, we might not
>even need this pointer. Are we even using this or am I missing something here ?

We are parsing the metadata and keeping the sink capability in driver as well. This can help
in some state checks and any default programming in case we need to do that.

>>  };
>>
>>  #define obj_to_connector(x) container_of(x, struct drm_connector,
>> base) diff --git a/include/drm/drm_mode_config.h
>> b/include/drm/drm_mode_config.h index 7f60e8e..ef2656b 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -836,6 +836,12 @@ struct drm_mode_config {
>>  	 */
>>  	struct drm_property *writeback_out_fence_ptr_property;
>>
>> +	/*
>> +	 * hdr_metadata_property: Connector property containing hdr metatda
>> +	 * This will be provided by userspace compositors based on HDR content
>> +	 */
>> +	struct drm_property *hdr_output_metadata_property;
>> +
>>  	/* dumb ioctl parameters */
>>  	uint32_t preferred_depth, prefer_shadow;
>>
>> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index
>> 927ad64..a065194
>> 100644
>> --- a/include/linux/hdmi.h
>> +++ b/include/linux/hdmi.h
>> @@ -152,6 +152,16 @@ enum hdmi_content_type {
>>  	HDMI_CONTENT_TYPE_GAME,
>>  };
>>
>> +enum hdmi_metadata_type {
>> +	HDMI_STATIC_METADATA_TYPE1 = 1,
>> +};
>> +
>> +enum hdmi_eotf {
>> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
>> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
>> +	HDMI_EOTF_SMPTE_ST2084,
>> +};
>> +
>>  struct hdmi_avi_infoframe {
>>  	enum hdmi_infoframe_type type;
>>  	unsigned char version;
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index
>> 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;
>> +};
>> +
>- Shashank
>>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01  #define
>> DRM_MODE_PAGE_FLIP_ASYNC 0x02  #define
>> DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
>> --
>> 1.9.1

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

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

* Re: [v5 02/13] drm: Parse HDR metadata info from EDID
  2019-03-15  7:26   ` Sharma, Shashank
@ 2019-03-20  6:37     ` Shankar, Uma
  0 siblings, 0 replies; 29+ messages in thread
From: Shankar, Uma @ 2019-03-20  6:37 UTC (permalink / raw)
  To: Sharma, Shashank, intel-gfx, dri-devel; +Cc: Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, March 15, 2019 12: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>; emil.l.velikov@gmail.com; brian.starkey@arm.com;
>Liviu.Dudau@arm.com
>Subject: RE: [v5 02/13] drm: Parse HDR metadata info from EDID
>
>
>
>> -----Original Message-----
>> From: Shankar, Uma
>> Sent: Monday, March 11, 2019 9:28 AM
>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
>> <ville.syrjala@intel.com>; Sharma, Shashank
>> <shashank.sharma@intel.com>; emil.l.velikov@gmail.com;
>> brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar, Uma
>> <uma.shankar@intel.com>
>> Subject: [v5 02/13] drm: Parse HDR metadata info from EDID
>>
>> HDR metadata block is introduced in CEA-861.3 spec.
>> Parsing the same to get the panel's HDR metadata.
>>
>> v2: Rebase and added Ville's POC changes to the patch.
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c | 52
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 52 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index
>> 5f14253..c5a81b8 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2834,6 +2834,7 @@ static int drm_cvt_modes(struct drm_connector
>> *connector,
>>  #define VIDEO_BLOCK     0x02
>>  #define VENDOR_BLOCK    0x03
>>  #define SPEAKER_BLOCK	0x04
>> +#define HDR_STATIC_METADATA_BLOCK	0x6
>>  #define USE_EXTENDED_TAG 0x07
>>  #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
>>  #define EXT_VIDEO_DATA_BLOCK_420	0x0E
>> @@ -3581,6 +3582,12 @@ static int add_3d_struct_modes(struct
>> drm_connector *connector, u16 structure,  }
>>
>>  static int
>> +cea_db_payload_len_ext(const u8 *db)
>> +{
>> +	return (db[0] & 0x1f) - 1;
>You might have already checked with checkpatch, but can we cross check the space
>before '-' ?

Yes, this seems ok. Space is there before operator.

>> +}
>> +
>> +static int
>>  cea_db_extended_tag(const u8 *db)
>>  {
>>  	return db[1];
>> @@ -3816,6 +3823,49 @@ static void
>> fixup_detailed_cea_mode_clock(struct
>> drm_display_mode *mode)
>>  	mode->clock = clock;
>>  }
>>
>> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) {
>> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
>same here,  looks like we need spaces around !=, or may be this is due to my mail
>client  ?

This seems to be issue with mail client. Looks like it's changing alignment on outlook.
The changes are ok and spaces are there before operators. 

Executed and checked with checkpatch as well. There are some other issues flagged with
--strict option, will be fixing those as part of v6.

Regards,
Uma Shankar

>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>> +static uint8_t eotf_supported(const u8 *edid_ext) {
>> +
>> +	return edid_ext[2] &
>> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
>> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
>Again, space before the '|' sign, please excuse me if this is false alarm ...

Same here.

>> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
>> +
>> +}
>> +
>> +static uint8_t hdr_metadata_type(const u8 *edid_ext) {
>> +
>> +	return edid_ext[3] &
>> +		BIT(HDMI_STATIC_METADATA_TYPE1);
>> +}
>> +
>> +static void
>> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const
>> +u8
>> +*db) {
>Shouldn't this variable go to line above ?
>> +	uint16_t len;
>> +
>> +	len = cea_db_payload_len_ext(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]; }
>This '}' certainly should not be here.

Same here as well.

>> +
>>  static void
>>  drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8
>> *db)  { @@
>> -4443,6 +4493,8 @@ static void drm_parse_cea_ext(struct drm_connector
>> *connector,
>>  			drm_parse_y420cmdb_bitmap(connector, db);
>>  		if (cea_db_is_vcdb(db))
>>  			drm_parse_vcdb(connector, db);
>> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
>> +			drm_parse_hdr_metadata_block(connector, db);
>- Shashank
>>  	}
>>  }
>>
>> --
>> 1.9.1

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

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

* RE: [v5 03/13] drm: Parse Colorimetry data block from EDID
  2019-03-15  7:30   ` Sharma, Shashank
@ 2019-03-20  7:17     ` Shankar, Uma
  2019-03-20  8:15       ` Sharma, Shashank
  0 siblings, 1 reply; 29+ messages in thread
From: Shankar, Uma @ 2019-03-20  7:17 UTC (permalink / raw)
  To: Sharma, Shashank, intel-gfx, dri-devel
  Cc: emil.l.velikov, Liviu.Dudau, Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, March 15, 2019 1:01 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>; emil.l.velikov@gmail.com; brian.starkey@arm.com;
>Liviu.Dudau@arm.com
>Subject: RE: [v5 03/13] drm: Parse Colorimetry data block from EDID
>
>
>
>> -----Original Message-----
>> From: Shankar, Uma
>> Sent: Monday, March 11, 2019 9:28 AM
>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
>> <ville.syrjala@intel.com>; Sharma, Shashank
>> <shashank.sharma@intel.com>; emil.l.velikov@gmail.com;
>> brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar, Uma
>> <uma.shankar@intel.com>
>> Subject: [v5 03/13] drm: Parse Colorimetry data block from EDID
>>
>> CEA 861.3 spec adds colorimetry data block for HDMI.
>> Parsing the block to get the colorimetry data from panel.
>>
>> v2: Rebase
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments. Updated colorimetry field to
>> 16 bit as
>> DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c  | 26 ++++++++++++++++++++++++++
>> include/drm/drm_connector.h |  3 +++
>>  2 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index
>> c5a81b8..0470845 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2834,6 +2834,7 @@ static int drm_cvt_modes(struct drm_connector
>> *connector,
>>  #define VIDEO_BLOCK     0x02
>>  #define VENDOR_BLOCK    0x03
>>  #define SPEAKER_BLOCK	0x04
>> +#define COLORIMETRY_DATA_BLOCK		0x5
>>  #define HDR_STATIC_METADATA_BLOCK	0x6
>>  #define USE_EXTENDED_TAG 0x07
>>  #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@ 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) {
>Same comment for the opening brace above ?

Again this looks like  mail client issue.

>> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
>Space before '!' ?

Same here.

>> +		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_ext(db);
>> +	/* As per CEA 861-G spec */
>> +	info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; }
>This means we are setting the 15th bit for DCI-P3 then there is a gap of 7 bits, and
>then there are other bits. Do we want to make DCI-P3 as bit 8 ? Anyways this can be
>differed for a discussion later.

This is at 8th position only.  We are shifting the db[3] which is 8 bit by 1 and keeping the 7th bit (DCI-P3)
at the 8th position (if we count from 0).

Regards,
Uma Shankar

>> +
>> +
>>  static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)  {
>>  	if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@ static
>> void drm_parse_cea_ext(struct drm_connector *connector,
>>  			drm_parse_vcdb(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 29388bd..94f926e 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -206,6 +206,9 @@ struct drm_hdmi_info {
>>
>>  	/** @y420_dc_modes: bitmap of deep color support index */
>>  	u8 y420_dc_modes;
>> +
>> +	/* @colorimetry: bitmap of supported colorimetry modes */
>> +	u16 colorimetry;
>- Shashank
>>  };
>>
>>  /**
>> --
>> 1.9.1

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

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

* Re: [v5 04/13] drm/i915: Attach HDR metadata property to connector
  2019-03-15  7:36   ` Sharma, Shashank
@ 2019-03-20  7:58     ` Shankar, Uma
  0 siblings, 0 replies; 29+ messages in thread
From: Shankar, Uma @ 2019-03-20  7:58 UTC (permalink / raw)
  To: Sharma, Shashank, intel-gfx, dri-devel; +Cc: Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, March 15, 2019 1:06 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>; emil.l.velikov@gmail.com; brian.starkey@arm.com;
>Liviu.Dudau@arm.com
>Subject: RE: [v5 04/13] drm/i915: Attach HDR metadata property to connector
>
>
>
>> -----Original Message-----
>> From: Shankar, Uma
>> Sent: Monday, March 11, 2019 9:28 AM
>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
>> <ville.syrjala@intel.com>; Sharma, Shashank
>> <shashank.sharma@intel.com>; emil.l.velikov@gmail.com;
>> brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar, Uma
>> <uma.shankar@intel.com>
>> Subject: [v5 04/13] drm/i915: Attach HDR metadata property to
>> connector
>>
>> Attach HDR metadata property to connector object.
>>
>> v2: Rebase
>>
>> v3: Updated the property name as per updated name while creating hdr
>> metadata property
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  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 cd422a7..0952475 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -2719,6 +2719,8 @@ static void intel_hdmi_destroy(struct
>> drm_connector
>> *connector)
>>
>>  	drm_connector_attach_content_type_property(connector);
>>  	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>> +	drm_object_attach_property(&connector->base,
>> +		connector->dev->mode_config.hdr_output_metadata_property, 0);
>'connector->dev' should be aligned to the line above, but seems like you have done
>this to meet the 80 char limit.
>May be a local ptr variable will help with 80 char.

Thanks Shashank, the name itself is a bit long. I will align it and can see crossing 80
characters for these kind of cases is ok. Being done in driver at several places.

Thanks for the review.

Regards,
Uma Shankar
>
>Apart from that, looks good to me, and feel free to use:
>Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>>
>>  	if (!HAS_GMCH(dev_priv))
>>  		drm_connector_attach_max_bpc_property(connector, 8, 12);
>> --
>> 1.9.1

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

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

* Re: [v5 03/13] drm: Parse Colorimetry data block from EDID
  2019-03-20  7:17     ` Shankar, Uma
@ 2019-03-20  8:15       ` Sharma, Shashank
  2019-03-20  8:58         ` Shankar, Uma
  0 siblings, 1 reply; 29+ messages in thread
From: Sharma, Shashank @ 2019-03-20  8:15 UTC (permalink / raw)
  To: Shankar, Uma, intel-gfx, dri-devel; +Cc: Syrjala, Ville, Lankhorst, Maarten


On 3/20/2019 12:47 PM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Sharma, Shashank
>> Sent: Friday, March 15, 2019 1:01 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>; emil.l.velikov@gmail.com; brian.starkey@arm.com;
>> Liviu.Dudau@arm.com
>> Subject: RE: [v5 03/13] drm: Parse Colorimetry data block from EDID
>>
>>
>>
>>> -----Original Message-----
>>> From: Shankar, Uma
>>> Sent: Monday, March 11, 2019 9:28 AM
>>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
>>> <ville.syrjala@intel.com>; Sharma, Shashank
>>> <shashank.sharma@intel.com>; emil.l.velikov@gmail.com;
>>> brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar, Uma
>>> <uma.shankar@intel.com>
>>> Subject: [v5 03/13] drm: Parse Colorimetry data block from EDID
>>>
>>> CEA 861.3 spec adds colorimetry data block for HDMI.
>>> Parsing the block to get the colorimetry data from panel.
>>>
>>> v2: Rebase
>>>
>>> v3: No Change
>>>
>>> v4: Addressed Shashank's review comments. Updated colorimetry field to
>>> 16 bit as
>>> DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank.
>>>
>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>> ---
>>>   drivers/gpu/drm/drm_edid.c  | 26 ++++++++++++++++++++++++++
>>> include/drm/drm_connector.h |  3 +++
>>>   2 files changed, 29 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>> index
>>> c5a81b8..0470845 100644
>>> --- a/drivers/gpu/drm/drm_edid.c
>>> +++ b/drivers/gpu/drm/drm_edid.c
>>> @@ -2834,6 +2834,7 @@ static int drm_cvt_modes(struct drm_connector
>>> *connector,
>>>   #define VIDEO_BLOCK     0x02
>>>   #define VENDOR_BLOCK    0x03
>>>   #define SPEAKER_BLOCK	0x04
>>> +#define COLORIMETRY_DATA_BLOCK		0x5
>>>   #define HDR_STATIC_METADATA_BLOCK	0x6
>>>   #define USE_EXTENDED_TAG 0x07
>>>   #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@ 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) {
>> Same comment for the opening brace above ?
> Again this looks like  mail client issue.
>
>>> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
>>> +		return false;
>>> +
>>> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
>> Space before '!' ?
> Same here.
>
>>> +		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_ext(db);
>>> +	/* As per CEA 861-G spec */
>>> +	info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; }
>> This means we are setting the 15th bit for DCI-P3 then there is a gap of 7 bits, and
>> then there are other bits. Do we want to make DCI-P3 as bit 8 ? Anyways this can be
>> differed for a discussion later.
> This is at 8th position only.  We are shifting the db[3] which is 8 bit by 1 and keeping the 7th bit (DCI-P3)
> at the 8th position (if we count from 0).
>
> Regards,
> Uma Shankar

My bad,  its 8th bit only.

If those above alignment problems are checked already, feel free to use

Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

- Shashank

>>> +
>>> +
>>>   static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)  {
>>>   	if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@ static
>>> void drm_parse_cea_ext(struct drm_connector *connector,
>>>   			drm_parse_vcdb(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 29388bd..94f926e 100644
>>> --- a/include/drm/drm_connector.h
>>> +++ b/include/drm/drm_connector.h
>>> @@ -206,6 +206,9 @@ struct drm_hdmi_info {
>>>
>>>   	/** @y420_dc_modes: bitmap of deep color support index */
>>>   	u8 y420_dc_modes;
>>> +
>>> +	/* @colorimetry: bitmap of supported colorimetry modes */
>>> +	u16 colorimetry;
>> - Shashank
>>>   };
>>>
>>>   /**
>>> --
>>> 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [v5 03/13] drm: Parse Colorimetry data block from EDID
  2019-03-20  8:15       ` Sharma, Shashank
@ 2019-03-20  8:58         ` Shankar, Uma
  0 siblings, 0 replies; 29+ messages in thread
From: Shankar, Uma @ 2019-03-20  8:58 UTC (permalink / raw)
  To: Sharma, Shashank, intel-gfx, dri-devel; +Cc: Syrjala, Ville, Lankhorst, Maarten

>On 3/20/2019 12:47 PM, Shankar, Uma wrote:
>>
>>> -----Original Message-----
>>> From: Sharma, Shashank
>>> Sent: Friday, March 15, 2019 1:01 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>; emil.l.velikov@gmail.com;
>>> brian.starkey@arm.com; Liviu.Dudau@arm.com
>>> Subject: RE: [v5 03/13] drm: Parse Colorimetry data block from EDID
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Shankar, Uma
>>>> Sent: Monday, March 11, 2019 9:28 AM
>>>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
>>>> <ville.syrjala@intel.com>; Sharma, Shashank
>>>> <shashank.sharma@intel.com>; emil.l.velikov@gmail.com;
>>>> brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar, Uma
>>>> <uma.shankar@intel.com>
>>>> Subject: [v5 03/13] drm: Parse Colorimetry data block from EDID
>>>>
>>>> CEA 861.3 spec adds colorimetry data block for HDMI.
>>>> Parsing the block to get the colorimetry data from panel.
>>>>
>>>> v2: Rebase
>>>>
>>>> v3: No Change
>>>>
>>>> v4: Addressed Shashank's review comments. Updated colorimetry field
>>>> to
>>>> 16 bit as
>>>> DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank.
>>>>
>>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/drm_edid.c  | 26 ++++++++++++++++++++++++++
>>>> include/drm/drm_connector.h |  3 +++
>>>>   2 files changed, 29 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>> index
>>>> c5a81b8..0470845 100644
>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>> @@ -2834,6 +2834,7 @@ static int drm_cvt_modes(struct drm_connector
>>>> *connector,
>>>>   #define VIDEO_BLOCK     0x02
>>>>   #define VENDOR_BLOCK    0x03
>>>>   #define SPEAKER_BLOCK	0x04
>>>> +#define COLORIMETRY_DATA_BLOCK		0x5
>>>>   #define HDR_STATIC_METADATA_BLOCK	0x6
>>>>   #define USE_EXTENDED_TAG 0x07
>>>>   #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@
>>>> 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) {
>>> Same comment for the opening brace above ?
>> Again this looks like  mail client issue.
>>
>>>> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
>>>> +		return false;
>>>> +
>>>> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
>>> Space before '!' ?
>> Same here.
>>
>>>> +		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_ext(db);
>>>> +	/* As per CEA 861-G spec */
>>>> +	info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; }
>>> This means we are setting the 15th bit for DCI-P3 then there is a gap
>>> of 7 bits, and then there are other bits. Do we want to make DCI-P3
>>> as bit 8 ? Anyways this can be differed for a discussion later.
>> This is at 8th position only.  We are shifting the db[3] which is 8
>> bit by 1 and keeping the 7th bit (DCI-P3) at the 8th position (if we count from 0).
>>
>> Regards,
>> Uma Shankar
>
>My bad,  its 8th bit only.
>
>If those above alignment problems are checked already, feel free to use
>
>Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

No Issues, Thanks Shashank.

Regards,
Uma Shankar

>- Shashank
>
>>>> +
>>>> +
>>>>   static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)  {
>>>>   	if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@
>>>> static void drm_parse_cea_ext(struct drm_connector *connector,
>>>>   			drm_parse_vcdb(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 29388bd..94f926e 100644
>>>> --- a/include/drm/drm_connector.h
>>>> +++ b/include/drm/drm_connector.h
>>>> @@ -206,6 +206,9 @@ struct drm_hdmi_info {
>>>>
>>>>   	/** @y420_dc_modes: bitmap of deep color support index */
>>>>   	u8 y420_dc_modes;
>>>> +
>>>> +	/* @colorimetry: bitmap of supported colorimetry modes */
>>>> +	u16 colorimetry;
>>> - Shashank
>>>>   };
>>>>
>>>>   /**
>>>> --
>>>> 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* RE: [v5 06/13] drm: Enable HDR infoframe support
  2019-03-15  8:23   ` Sharma, Shashank
@ 2019-03-20  9:24     ` Shankar, Uma
  0 siblings, 0 replies; 29+ messages in thread
From: Shankar, Uma @ 2019-03-20  9:24 UTC (permalink / raw)
  To: Sharma, Shashank, intel-gfx, dri-devel
  Cc: Syrjala, Ville, Liviu.Dudau, emil.l.velikov, Lankhorst, Maarten



>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, March 15, 2019 1:54 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>; emil.l.velikov@gmail.com; brian.starkey@arm.com;
>Liviu.Dudau@arm.com; Ville Syrjälä <ville.syrjala@linux.intel.com>
>Subject: Re: [v5 06/13] drm: Enable HDR infoframe support
>
>
>On 3/11/2019 9:27 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.
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments and merged the patch making
>> drm infoframe function arguments as constant.
>>
>> v5: Rebase
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.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 0470845..49f8340 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4914,6 +4914,64 @@ static bool is_hdmi2_sink(struct drm_connector
>*connector)
>>   }
>>
>>   /**
>> + * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI AVI infoframe with
>> + *                                         HDR metadata from userspace
>> + * @frame: HDMI AVI infoframe
>> + * @hdr_source_metadata: hdr_source_metadata info from userspace
>> + *
>> + * Return: 0 on success or a negative error code on failure.
>> + */
>> +int
>> +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
>> +				    void *hdr_metadata)
>Alignment with bracket above

This is email client issue. 

>> +{
>> +	struct hdr_static_metadata *hdr_source_metadata;
>> +	int err, i;
>> +
>> +	if (!frame || !hdr_metadata)
>> +		return true;
>> +
>> +	err = hdmi_drm_infoframe_init(frame);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	DRM_DEBUG_KMS("type = %x\n", frame->type);
>> +
>> +	hdr_source_metadata = (struct hdr_static_metadata *)hdr_metadata;
>> +
>> +	frame->length = sizeof(struct hdr_static_metadata);
>> +
>extra blank line

Sure, will fix this.

>> +
>> +	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;
>> +	}
>memcpy here to avoid loop ?

I feel it makes it more clear as what data is getting copied. If no hard objection,
let's keep it this way.

>> +
>> +	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;
>> +
>In fact the structures look so analogues, can we do a complete memcpy of one
>structure to other ? or the sequence of the elements is different ?

There are slightly different elements as well so it would be good to have member by member copy.
This will also help check in case any issues with metadata input.

>> +	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..7ab8086 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;
>&0xff is not required

Sure, will drop those.

>> +		*ptr++ = frame->display_primaries[i].x >> 8;
>> +		*ptr++ = frame->display_primaries[i].y & 0xff;
>&0xff is not required, applicable for all below

Sure. Will modify this.

>> +		*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,
>> +				   const struct hdmi_drm_infoframe *frame) {
>> +	int i;
>> +
>> +	hdmi_infoframe_log_header(level, dev,
>> +			(struct hdmi_any_infoframe *)frame);
>Alignment with above '('
>> +	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);
>Alignment for these 2 lines above

Will fix this.

>> +
>> +	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
>> 9d3b5b9..973e43e 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -370,6 +370,10 @@ int drm_av_sync_delay(struct drm_connector *connector,
>>   				   const struct drm_display_mode *mode,
>>   				   enum hdmi_quantization_range rgb_quant_range);
>>
>> +int
>> +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
>> +				    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 a065194..33243b2
>> 100644
>> --- a/include/linux/hdmi.h
>> +++ b/include/linux/hdmi.h
>> @@ -47,6 +47,7 @@ enum hdmi_infoframe_type {
>>   	HDMI_INFOFRAME_TYPE_AVI = 0x82,
>>   	HDMI_INFOFRAME_TYPE_SPD = 0x83,
>>   	HDMI_INFOFRAME_TYPE_AUDIO = 0x84,
>> +	HDMI_INFOFRAME_TYPE_DRM = 0x87,
>>   };
>>
>>   #define HDMI_IEEE_OUI 0x000c03
>> @@ -185,12 +186,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,
>> @@ -365,6 +386,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;
>>   };
>
>With the minor issues pointed above fixed, feel free to use:
>
>Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

Thanks Shashank for the review.

Regards,
Uma Shankar

>- Shashank
>
>>
>>   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

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

* RE: [v5 07/13] drm/i915: Write HDR infoframe and send to panel
  2019-03-15 11:07   ` Sharma, Shashank
@ 2019-03-20  9:46     ` Shankar, Uma
  0 siblings, 0 replies; 29+ messages in thread
From: Shankar, Uma @ 2019-03-20  9:46 UTC (permalink / raw)
  To: Sharma, Shashank, intel-gfx, dri-devel
  Cc: emil.l.velikov, Liviu.Dudau, Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, March 15, 2019 4: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>; emil.l.velikov@gmail.com; brian.starkey@arm.com;
>Liviu.Dudau@arm.com
>Subject: Re: [v5 07/13] drm/i915: Write HDR infoframe and send to panel
>
>
>On 3/11/2019 9:27 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
>>
>> v3: Fixed a warning message
>>
>> v4: Addressed Shashank's review comments
>>
>> v5: Rebase. Added infoframe calculation in compute config.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_drv.h  |  1 +
>>   drivers/gpu/drm/i915/intel_hdmi.c | 33 +++++++++++++++++++++++++++++++++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 58483f8..a5ee124 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1037,6 +1037,7 @@ struct intel_crtc_state {
>>   		union hdmi_infoframe avi;
>>   		union hdmi_infoframe spd;
>>   		union hdmi_infoframe hdmi;
>> +		union hdmi_infoframe drm;
>>   	} infoframes;
>>
>>   	/* HDMI scrambling status */
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 0952475..ea7afa0 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -555,6 +555,7 @@ static u32 hsw_infoframes_enabled(struct intel_encoder
>*encoder,
>>   	HDMI_INFOFRAME_TYPE_AVI,
>>   	HDMI_INFOFRAME_TYPE_SPD,
>>   	HDMI_INFOFRAME_TYPE_VENDOR,
>> +	HDMI_INFOFRAME_TYPE_DRM,
>>   };
>>
>>   u32 intel_hdmi_infoframe_enable(unsigned int type) @@ -777,6 +778,30
>> @@ void intel_read_infoframe(struct intel_encoder *encoder,
>>   	return true;
>>   }
>>
>> +static bool
>> +intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
>> +				  struct intel_crtc_state *crtc_state,
>> +				  struct drm_connector_state *conn_state) {
>> +	struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm;
>> +	struct hdr_static_metadata *hdr_metadata;
>> +	int ret;
>> +
>
>Don't we need a GEN/monitor check before going ahead with HDR metadata ?

Sure, will add that check.

Regards,
Uma Shankar

>- Shashank
>
>> +	hdr_metadata = (struct hdr_static_metadata *)
>> +			conn_state->hdr_output_metadata_blob_ptr->data;
>> +
>> +	ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata);
>> +	if (ret < 0) {
>> +		DRM_ERROR("couldn't set HDR metadata in infoframe\n");
>> +		return false;
>> +	}
>> +
>> +	crtc_state->infoframes.enable |=
>> +		intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);
>> +
>> +	return true;
>> +}
>> +
>>   static void g4x_set_infoframes(struct intel_encoder *encoder,
>>   			       bool enable,
>>   			       const struct intel_crtc_state *crtc_state, @@ -1175,6
>> +1200,9 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
>>   	intel_write_infoframe(encoder, crtc_state,
>>   			      HDMI_INFOFRAME_TYPE_VENDOR,
>>   			      &crtc_state->infoframes.hdmi);
>> +	intel_write_infoframe(encoder, crtc_state,
>> +			      HDMI_INFOFRAME_TYPE_DRM,
>> +			      &crtc_state->infoframes.drm);
>>   }
>>
>>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi,
>> bool enable) @@ -2381,6 +2409,11 @@ int intel_hdmi_compute_config(struct
>intel_encoder *encoder,
>>   		return -EINVAL;
>>   	}
>>
>> +	if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config, conn_state)) {
>> +		DRM_DEBUG_KMS("bad DRM infoframe\n");
>> +		return -EINVAL;
>> +	}
>> +
>>   	return 0;
>>   }
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2019-03-20  9:46 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11  3:57 [v5 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
2019-03-11  3:57 ` [v5 01/13] drm: Add HDR source metadata property Uma Shankar
2019-03-15  7:08   ` Sharma, Shashank
2019-03-20  6:21     ` Shankar, Uma
2019-03-11  3:57 ` [v5 02/13] drm: Parse HDR metadata info from EDID Uma Shankar
2019-03-15  7:26   ` Sharma, Shashank
2019-03-20  6:37     ` Shankar, Uma
2019-03-11  3:57 ` [v5 03/13] drm: Parse Colorimetry data block " Uma Shankar
2019-03-15  7:30   ` Sharma, Shashank
2019-03-20  7:17     ` Shankar, Uma
2019-03-20  8:15       ` Sharma, Shashank
2019-03-20  8:58         ` Shankar, Uma
2019-03-11  3:57 ` [v5 04/13] drm/i915: Attach HDR metadata property to connector Uma Shankar
2019-03-15  7:36   ` Sharma, Shashank
2019-03-20  7:58     ` Shankar, Uma
2019-03-11  3:57 ` [v5 05/13] drm: Implement HDR output metadata set and get property handling Uma Shankar
2019-03-11  3:57 ` [v5 06/13] drm: Enable HDR infoframe support Uma Shankar
2019-03-15  8:23   ` Sharma, Shashank
2019-03-20  9:24     ` Shankar, Uma
2019-03-11  3:57 ` [v5 07/13] drm/i915: Write HDR infoframe and send to panel Uma Shankar
2019-03-15 11:07   ` Sharma, Shashank
2019-03-20  9:46     ` Shankar, Uma
2019-03-11  3:57 ` [v5 08/13] drm/i915: [DO NOT MERGE] hack for glk board outputs Uma Shankar
2019-03-11  3:58 ` [v5 09/13] drm/i915: Add HLG EOTF Uma Shankar
2019-03-15 11:09   ` Sharma, Shashank
2019-03-11  3:58 ` [v5 10/13] drm/i915: Enable infoframes on GLK+ for HDR Uma Shankar
2019-03-11  3:58 ` [v5 11/13] drm/i915:Enabled Modeset when HDR Infoframe changes Uma Shankar
2019-03-11  3:58 ` [v5 12/13] drm/i915: Set Infoframe for non modeset case for HDR Uma Shankar
2019-03-11  3:58 ` [v5 13/13] video/hdmi: Add const variants for drm infoframe Uma Shankar

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