[v5 06/13] drm: Enable HDR infoframe support

Sharma, Shashank shashank.sharma at intel.com
Fri Mar 15 08:23:32 UTC 2019


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 at intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at 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 at intel.com>

- Shashank

>   
>   ssize_t hdmi_infoframe_pack(union hdmi_infoframe *frame, void *buffer,


More information about the dri-devel mailing list