[v6,06/13] drm: Enable HDR infoframe support

Submitted by Shankar, Uma on March 20, 2019, 10:48 a.m.

Details

Message ID 1553078906-5991-7-git-send-email-uma.shankar@intel.com
State New
Headers show
Series "Add HDR Metadata Parsing and handling in DRM layer" ( rev: 6 ) in DRI devel

Commit Message

Shankar, Uma March 20, 2019, 10:48 a.m.
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

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

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

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 676569b..78c0b97 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4916,6 +4916,62 @@  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..80bb0ee 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;
+		*ptr++ = frame->display_primaries[i].x >> 8;
+		*ptr++ = frame->display_primaries[i].y;
+		*ptr++ = frame->display_primaries[i].y >> 8;
+	}
+
+	*ptr++ = frame->white_point.x;
+	*ptr++ = frame->white_point.x >> 8;
+
+	*ptr++ = frame->white_point.y;
+	*ptr++ = frame->white_point.y >> 8;
+
+	*ptr++ = frame->max_mastering_display_luminance;
+	*ptr++ = frame->max_mastering_display_luminance >> 8;
+
+	*ptr++ = frame->min_mastering_display_luminance;
+	*ptr++ = frame->min_mastering_display_luminance >> 8;
+
+	*ptr++ = frame->max_cll;
+	*ptr++ = frame->max_cll >> 8;
+
+	*ptr++ = frame->max_fall;
+	*ptr++ = frame->max_fall >> 8;
+
+	hdmi_infoframe_set_checksum(buffer, length);
+
+	return length;
+}
+
 /*
  * 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..b925b24 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 {
+		u16 x, y;
+	} display_primaries[3];
+	struct {
+		u16 x, y;
+	} white_point;
+	u16 max_mastering_display_luminance;
+	u16 min_mastering_display_luminance;
+	u16 max_fall;
+	u16 max_cll;
+	u16 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,

Comments

On Wed, Mar 20, 2019 at 04:18:19PM +0530, 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
> 
> v6: Fixed checkpatch warnings with --strict option. Addressed
> Shashank's review comments and added his RB.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c |  56 ++++++++++++++++++++
>  drivers/video/hdmi.c       | 129 +++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     |   4 ++
>  include/linux/hdmi.h       |  22 ++++++++
>  4 files changed, 211 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 676569b..78c0b97 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4916,6 +4916,62 @@ 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)

I don't think this needs to be/should be a void *, you might as well
us (struct hdr_static_metadata *). Looks like you do the cast in i915
before calling this function anyway.

> +{
> +	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;

Couldn't the infoframe definition embed the UAPI structure, making
this a straight memcpy() ?

> +
> +	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..80bb0ee 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;
> +		*ptr++ = frame->display_primaries[i].x >> 8;
> +		*ptr++ = frame->display_primaries[i].y;
> +		*ptr++ = frame->display_primaries[i].y >> 8;
> +	}
> +
> +	*ptr++ = frame->white_point.x;
> +	*ptr++ = frame->white_point.x >> 8;
> +
> +	*ptr++ = frame->white_point.y;
> +	*ptr++ = frame->white_point.y >> 8;
> +
> +	*ptr++ = frame->max_mastering_display_luminance;
> +	*ptr++ = frame->max_mastering_display_luminance >> 8;
> +
> +	*ptr++ = frame->min_mastering_display_luminance;
> +	*ptr++ = frame->min_mastering_display_luminance >> 8;
> +
> +	*ptr++ = frame->max_cll;
> +	*ptr++ = frame->max_cll >> 8;
> +
> +	*ptr++ = frame->max_fall;
> +	*ptr++ = frame->max_fall >> 8;
> +
> +	hdmi_infoframe_set_checksum(buffer, length);
> +
> +	return length;
> +}
> +
>  /*
>   * 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..b925b24 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 {
> +		u16 x, y;
> +	} display_primaries[3];
> +	struct {
> +		u16 x, y;
> +	} white_point;
> +	u16 max_mastering_display_luminance;
> +	u16 min_mastering_display_luminance;
> +	u16 max_fall;
> +	u16 max_cll;

Less confusing IMO if these are the same way around as in the spec.
max_cll first, then max_fall.

> +	u16 min_cll;

This guy isn't in the copy of I have (CTA-861-G). I'm probably out of
date, but you also don't print or encode this value, so should it be
dropped?

Cheers,
-Brian

> +};
> +
>  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
>
>-----Original Message-----
>From: Brian Starkey [mailto:Brian.Starkey@arm.com]
>Sent: Thursday, March 21, 2019 5:12 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Lankhorst,
>Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>; emil.l.velikov@gmail.com; Liviu
>Dudau <Liviu.Dudau@arm.com>; Ville Syrjälä <ville.syrjala@linux.intel.com>; nd
><nd@arm.com>
>Subject: Re: [v6 06/13] drm: Enable HDR infoframe support
>
>On Wed, Mar 20, 2019 at 04:18:19PM +0530, 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
>>
>> v6: Fixed checkpatch warnings with --strict option. Addressed
>> Shashank's review comments and added his RB.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c |  56 ++++++++++++++++++++
>>  drivers/video/hdmi.c       | 129
>+++++++++++++++++++++++++++++++++++++++++++++
>>  include/drm/drm_edid.h     |   4 ++
>>  include/linux/hdmi.h       |  22 ++++++++
>>  4 files changed, 211 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 676569b..78c0b97 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4916,6 +4916,62 @@ 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)
>
>I don't think this needs to be/should be a void *, you might as well us (struct
>hdr_static_metadata *). Looks like you do the cast in i915 before calling this function
>anyway.

Ok, will update this.

>> +{
>> +	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;
>
>Couldn't the infoframe definition embed the UAPI structure, making this a straight
>memcpy() ?

Yeah it's doable, currently the structures were not exactly same. Will try to make them
Identical and perform a memcpy as even suggested by Shashank.

>> +
>> +	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..80bb0ee 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;
>> +		*ptr++ = frame->display_primaries[i].x >> 8;
>> +		*ptr++ = frame->display_primaries[i].y;
>> +		*ptr++ = frame->display_primaries[i].y >> 8;
>> +	}
>> +
>> +	*ptr++ = frame->white_point.x;
>> +	*ptr++ = frame->white_point.x >> 8;
>> +
>> +	*ptr++ = frame->white_point.y;
>> +	*ptr++ = frame->white_point.y >> 8;
>> +
>> +	*ptr++ = frame->max_mastering_display_luminance;
>> +	*ptr++ = frame->max_mastering_display_luminance >> 8;
>> +
>> +	*ptr++ = frame->min_mastering_display_luminance;
>> +	*ptr++ = frame->min_mastering_display_luminance >> 8;
>> +
>> +	*ptr++ = frame->max_cll;
>> +	*ptr++ = frame->max_cll >> 8;
>> +
>> +	*ptr++ = frame->max_fall;
>> +	*ptr++ = frame->max_fall >> 8;
>> +
>> +	hdmi_infoframe_set_checksum(buffer, length);
>> +
>> +	return length;
>> +}
>> +
>>  /*
>>   * 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..b925b24
>> 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 {
>> +		u16 x, y;
>> +	} display_primaries[3];
>> +	struct {
>> +		u16 x, y;
>> +	} white_point;
>> +	u16 max_mastering_display_luminance;
>> +	u16 min_mastering_display_luminance;
>> +	u16 max_fall;
>> +	u16 max_cll;
>
>Less confusing IMO if these are the same way around as in the spec.
>max_cll first, then max_fall.
>
>> +	u16 min_cll;
>
>This guy isn't in the copy of I have (CTA-861-G). I'm probably out of date, but you also
>don't print or encode this value, so should it be dropped?

Sure, will match exactly to 861.G. There were certain deltas here b/w 861.F and G. Will update this.

Thanks & Regards,
Uma Shankar

>Cheers,
>-Brian
>
>> +};
>> +
>>  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
>>