[v9,12/13] video/hdmi: Add Unpack function for DRM infoframe

Submitted by Shankar, Uma on May 8, 2019, 6:38 p.m.

Details

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

Commit Message

Shankar, Uma May 8, 2019, 6:38 p.m.
Added unpack function for DRM infoframe for dynamic
range and mastering infoframe readout.

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

Patch hide | download patch | download mbox

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 717ed7fb..110d405 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -1801,6 +1801,57 @@  static int hdmi_audio_infoframe_unpack(struct hdmi_audio_infoframe *frame,
 }
 
 /**
+ * hdmi_drm_infoframe_unpack() - unpack binary buffer to a HDMI DRM infoframe
+ * @frame: HDMI DRM infoframe
+ * @buffer: source buffer
+ * @size: size of buffer
+ *
+ * Unpacks the information contained in binary @buffer into a structured
+ * @frame of the HDMI Dynamic Range and Mastering (DRM) information frame.
+ * Also verifies the checksum as required by section 5.3.5 of the HDMI 1.4
+ * specification.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+static int hdmi_drm_infoframe_unpack(struct hdmi_drm_infoframe *frame,
+				     const void *buffer, size_t size)
+{
+	const u8 *ptr = buffer;
+	int ret;
+
+	if (size < HDMI_INFOFRAME_SIZE(DRM))
+		return -EINVAL;
+
+	if (ptr[0] != HDMI_INFOFRAME_TYPE_DRM ||
+	    ptr[1] != 1 ||
+	    ptr[2] != HDMI_DRM_INFOFRAME_SIZE)
+		return -EINVAL;
+
+	if (hdmi_infoframe_checksum(buffer, HDMI_INFOFRAME_SIZE(DRM)) != 0)
+		return -EINVAL;
+
+	ret = hdmi_drm_infoframe_init(frame);
+	if (ret)
+		return ret;
+
+	frame->length = ptr[2];
+	ptr += HDMI_INFOFRAME_HEADER_SIZE;
+
+	frame->eotf = ptr[0] & 0x7;
+	frame->metadata_type = ptr[1] & 0x7;
+
+	memcpy(&frame->display_primaries, &ptr[2], 12);
+	memcpy(&frame->white_point, &ptr[14], 4);
+
+	frame->max_display_mastering_luminance = (ptr[19] << 8) | ptr[18];
+	frame->min_display_mastering_luminance = (ptr[21] << 8) | ptr[20];
+	frame->max_cll = (ptr[23] << 8) | ptr[22];
+	frame->max_fall = (ptr[25] << 8) | ptr[24];
+
+	return 0;
+}
+
+/**
  * hdmi_infoframe_unpack() - unpack binary buffer to a HDMI infoframe
  * @frame: HDMI infoframe
  * @buffer: source buffer
@@ -1826,6 +1877,9 @@  int hdmi_infoframe_unpack(union hdmi_infoframe *frame,
 	case HDMI_INFOFRAME_TYPE_AVI:
 		ret = hdmi_avi_infoframe_unpack(&frame->avi, buffer, size);
 		break;
+	case HDMI_INFOFRAME_TYPE_DRM:
+		ret = hdmi_drm_infoframe_unpack(&frame->drm, buffer, size);
+		break;
 	case HDMI_INFOFRAME_TYPE_SPD:
 		ret = hdmi_spd_infoframe_unpack(&frame->spd, buffer, size);
 		break;
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 3d7f10f..ee55ba5 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -56,6 +56,7 @@  enum hdmi_infoframe_type {
 #define HDMI_AVI_INFOFRAME_SIZE    13
 #define HDMI_SPD_INFOFRAME_SIZE    25
 #define HDMI_AUDIO_INFOFRAME_SIZE  10
+#define HDMI_DRM_INFOFRAME_SIZE    26
 
 #define HDMI_INFOFRAME_SIZE(type)	\
 	(HDMI_INFOFRAME_HEADER_SIZE + HDMI_ ## type ## _INFOFRAME_SIZE)

Comments

On Thu, May 09, 2019 at 12:08:52AM +0530, Uma Shankar wrote:
> Added unpack function for DRM infoframe for dynamic
> range and mastering infoframe readout.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/video/hdmi.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/hdmi.h |  1 +
>  2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 717ed7fb..110d405 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -1801,6 +1801,57 @@ static int hdmi_audio_infoframe_unpack(struct hdmi_audio_infoframe *frame,
>  }
>  
>  /**
> + * hdmi_drm_infoframe_unpack() - unpack binary buffer to a HDMI DRM infoframe
> + * @frame: HDMI DRM infoframe
> + * @buffer: source buffer
> + * @size: size of buffer
> + *
> + * Unpacks the information contained in binary @buffer into a structured
> + * @frame of the HDMI Dynamic Range and Mastering (DRM) information frame.
> + * Also verifies the checksum as required by section 5.3.5 of the HDMI 1.4
> + * specification.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +static int hdmi_drm_infoframe_unpack(struct hdmi_drm_infoframe *frame,
> +				     const void *buffer, size_t size)
> +{
> +	const u8 *ptr = buffer;
> +	int ret;
> +
> +	if (size < HDMI_INFOFRAME_SIZE(DRM))
> +		return -EINVAL;
> +
> +	if (ptr[0] != HDMI_INFOFRAME_TYPE_DRM ||
> +	    ptr[1] != 1 ||
> +	    ptr[2] != HDMI_DRM_INFOFRAME_SIZE)
> +		return -EINVAL;
> +
> +	if (hdmi_infoframe_checksum(buffer, HDMI_INFOFRAME_SIZE(DRM)) != 0)
> +		return -EINVAL;
> +
> +	ret = hdmi_drm_infoframe_init(frame);
> +	if (ret)
> +		return ret;
> +
> +	frame->length = ptr[2];

The length handling is inconsistnet between the compute and readout
sides. I would suggest for now just put the length initialization
into hdmi_drm_infoframe_init() (and remove it from the drm code),
and check it in the check_only(). We can revisit it if/when other
metadata formats become relevant.

> +	ptr += HDMI_INFOFRAME_HEADER_SIZE;
> +
> +	frame->eotf = ptr[0] & 0x7;
> +	frame->metadata_type = ptr[1] & 0x7;
> +
> +	memcpy(&frame->display_primaries, &ptr[2], 12);
> +	memcpy(&frame->white_point, &ptr[14], 4);

Endianness fail.

> +
> +	frame->max_display_mastering_luminance = (ptr[19] << 8) | ptr[18];
> +	frame->min_display_mastering_luminance = (ptr[21] << 8) | ptr[20];
> +	frame->max_cll = (ptr[23] << 8) | ptr[22];
> +	frame->max_fall = (ptr[25] << 8) | ptr[24];
> +
> +	return 0;
> +}
> +
> +/**
>   * hdmi_infoframe_unpack() - unpack binary buffer to a HDMI infoframe
>   * @frame: HDMI infoframe
>   * @buffer: source buffer
> @@ -1826,6 +1877,9 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame,
>  	case HDMI_INFOFRAME_TYPE_AVI:
>  		ret = hdmi_avi_infoframe_unpack(&frame->avi, buffer, size);
>  		break;
> +	case HDMI_INFOFRAME_TYPE_DRM:
> +		ret = hdmi_drm_infoframe_unpack(&frame->drm, buffer, size);
> +		break;
>  	case HDMI_INFOFRAME_TYPE_SPD:
>  		ret = hdmi_spd_infoframe_unpack(&frame->spd, buffer, size);
>  		break;
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 3d7f10f..ee55ba5 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -56,6 +56,7 @@ enum hdmi_infoframe_type {
>  #define HDMI_AVI_INFOFRAME_SIZE    13
>  #define HDMI_SPD_INFOFRAME_SIZE    25
>  #define HDMI_AUDIO_INFOFRAME_SIZE  10
> +#define HDMI_DRM_INFOFRAME_SIZE    26
>  
>  #define HDMI_INFOFRAME_SIZE(type)	\
>  	(HDMI_INFOFRAME_HEADER_SIZE + HDMI_ ## type ## _INFOFRAME_SIZE)
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>-----Original Message-----

>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Ville

>Syrjälä

>Sent: Tuesday, May 14, 2019 1:20 AM

>To: Shankar, Uma <uma.shankar@intel.com>

>Cc: dcastagna@chromium.org; jonas@kwiboo.se; intel-gfx@lists.freedesktop.org;

>emil.l.velikov@gmail.com; dri-devel@lists.freedesktop.org;

>seanpaul@chromium.org; Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten

><maarten.lankhorst@intel.com>

>Subject: Re: [v9 12/13] video/hdmi: Add Unpack function for DRM infoframe

>

>On Thu, May 09, 2019 at 12:08:52AM +0530, Uma Shankar wrote:

>> Added unpack function for DRM infoframe for dynamic range and

>> mastering infoframe readout.

>>

>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>

>> ---

>>  drivers/video/hdmi.c | 54

>> ++++++++++++++++++++++++++++++++++++++++++++++++++++

>>  include/linux/hdmi.h |  1 +

>>  2 files changed, 55 insertions(+)

>>

>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index

>> 717ed7fb..110d405 100644

>> --- a/drivers/video/hdmi.c

>> +++ b/drivers/video/hdmi.c

>> @@ -1801,6 +1801,57 @@ static int hdmi_audio_infoframe_unpack(struct

>> hdmi_audio_infoframe *frame,  }

>>

>>  /**

>> + * hdmi_drm_infoframe_unpack() - unpack binary buffer to a HDMI DRM

>> +infoframe

>> + * @frame: HDMI DRM infoframe

>> + * @buffer: source buffer

>> + * @size: size of buffer

>> + *

>> + * Unpacks the information contained in binary @buffer into a

>> +structured

>> + * @frame of the HDMI Dynamic Range and Mastering (DRM) information frame.

>> + * Also verifies the checksum as required by section 5.3.5 of the

>> +HDMI 1.4

>> + * specification.

>> + *

>> + * Returns 0 on success or a negative error code on failure.

>> + */

>> +static int hdmi_drm_infoframe_unpack(struct hdmi_drm_infoframe *frame,

>> +				     const void *buffer, size_t size) {

>> +	const u8 *ptr = buffer;

>> +	int ret;

>> +

>> +	if (size < HDMI_INFOFRAME_SIZE(DRM))

>> +		return -EINVAL;

>> +

>> +	if (ptr[0] != HDMI_INFOFRAME_TYPE_DRM ||

>> +	    ptr[1] != 1 ||

>> +	    ptr[2] != HDMI_DRM_INFOFRAME_SIZE)

>> +		return -EINVAL;

>> +

>> +	if (hdmi_infoframe_checksum(buffer, HDMI_INFOFRAME_SIZE(DRM)) != 0)

>> +		return -EINVAL;

>> +

>> +	ret = hdmi_drm_infoframe_init(frame);

>> +	if (ret)

>> +		return ret;

>> +

>> +	frame->length = ptr[2];

>

>The length handling is inconsistnet between the compute and readout sides. I would

>suggest for now just put the length initialization into hdmi_drm_infoframe_init() (and

>remove it from the drm code), and check it in the check_only(). We can revisit it

>if/when other metadata formats become relevant.


Sure, will update this.

>> +	ptr += HDMI_INFOFRAME_HEADER_SIZE;

>> +

>> +	frame->eotf = ptr[0] & 0x7;

>> +	frame->metadata_type = ptr[1] & 0x7;

>> +

>> +	memcpy(&frame->display_primaries, &ptr[2], 12);

>> +	memcpy(&frame->white_point, &ptr[14], 4);

>

>Endianness fail.


Ok, will explicitly copy these.

>> +

>> +	frame->max_display_mastering_luminance = (ptr[19] << 8) | ptr[18];

>> +	frame->min_display_mastering_luminance = (ptr[21] << 8) | ptr[20];

>> +	frame->max_cll = (ptr[23] << 8) | ptr[22];

>> +	frame->max_fall = (ptr[25] << 8) | ptr[24];

>> +

>> +	return 0;

>> +}

>> +

>> +/**

>>   * hdmi_infoframe_unpack() - unpack binary buffer to a HDMI infoframe

>>   * @frame: HDMI infoframe

>>   * @buffer: source buffer

>> @@ -1826,6 +1877,9 @@ int hdmi_infoframe_unpack(union hdmi_infoframe

>*frame,

>>  	case HDMI_INFOFRAME_TYPE_AVI:

>>  		ret = hdmi_avi_infoframe_unpack(&frame->avi, buffer, size);

>>  		break;

>> +	case HDMI_INFOFRAME_TYPE_DRM:

>> +		ret = hdmi_drm_infoframe_unpack(&frame->drm, buffer, size);

>> +		break;

>>  	case HDMI_INFOFRAME_TYPE_SPD:

>>  		ret = hdmi_spd_infoframe_unpack(&frame->spd, buffer, size);

>>  		break;

>> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index

>> 3d7f10f..ee55ba5 100644

>> --- a/include/linux/hdmi.h

>> +++ b/include/linux/hdmi.h

>> @@ -56,6 +56,7 @@ enum hdmi_infoframe_type {

>>  #define HDMI_AVI_INFOFRAME_SIZE    13

>>  #define HDMI_SPD_INFOFRAME_SIZE    25

>>  #define HDMI_AUDIO_INFOFRAME_SIZE  10

>> +#define HDMI_DRM_INFOFRAME_SIZE    26

>>

>>  #define HDMI_INFOFRAME_SIZE(type)	\

>>  	(HDMI_INFOFRAME_HEADER_SIZE + HDMI_ ## type ## _INFOFRAME_SIZE)

>> --

>> 1.9.1

>>

>> _______________________________________________

>> dri-devel mailing list

>> dri-devel@lists.freedesktop.org

>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

>

>--

>Ville Syrjälä

>Intel

>_______________________________________________

>dri-devel mailing list

>dri-devel@lists.freedesktop.org

>https://lists.freedesktop.org/mailman/listinfo/dri-devel