Re: [Intel-gfx] [v10 04/12] drm: Enable HDR infoframe support
>-Original Message-
>From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
>Sent: Thursday, May 16, 2019 12:45 AM
>To: Shankar, Uma
>Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
>maarten.lankho...@linux.intel.com; Sharma, Shashank
>; emil.l.veli...@gmail.com; brian.star...@arm.com;
>dcasta...@chromium.org; seanp...@chromium.org; Roper, Matthew D
>; jo...@kwiboo.se
>Subject: Re: [v10 04/12] drm: Enable HDR infoframe support
>
>On Tue, May 14, 2019 at 11:06:26PM +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.
>>
>> Added the const version of infoframe for DRM metadata for 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.
>>
>> v7: Addressed Brian Starkey's review comments. Merged 2 patches into
>> one.
>>
>> v8: Addressed Jonas Karlman review comments.
>>
>> v9: Addressed Jonas Karlman review comments.
>>
>> Signed-off-by: Uma Shankar
>> Signed-off-by: Ville Syrjälä
>> Reviewed-by: Shashank Sharma
>> ---
>> drivers/gpu/drm/drm_edid.c | 43 +++
>> drivers/video/hdmi.c | 187
>+
>> include/drm/drm_edid.h | 5 ++
>> include/linux/hdmi.h | 27 +++
>> 4 files changed, 262 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 2e0b5be..73b7905 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4903,6 +4903,49 @@ static bool is_hdmi2_sink(struct drm_connector
>> *connector) }
>>
>> /**
>> + * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI DRM infoframe with
>> + * HDR metadata from userspace
>> + * @frame: HDMI DRM infoframe
>> + * @hdr_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,
>> +const struct hdr_output_metadata
>*hdr_metadata) {
>> +int err;
>> +
>> +if (!frame || !hdr_metadata)
>> +return -EINVAL;
>> +
>> +err = hdmi_drm_infoframe_init(frame);
>> +if (err < 0)
>> +return err;
>> +
>> +DRM_DEBUG_KMS("type = %x\n", frame->type);
>
>This debug seems pointless.
Ok, will drop this.
>> +
>> +frame->eotf = hdr_metadata->hdmi_metadata_type1.eotf;
>> +frame->metadata_type =
>> +hdr_metadata->hdmi_metadata_type1.metadata_type;
>> +
>> +memcpy(&frame->display_primaries,
>> + &hdr_metadata->hdmi_metadata_type1.display_primaries, 12);
>
>sizeof() can be used here.
Sure.
>Could also slap on a BUILD_BUG_ON() to make sure both are the same size.
Ok, will addd it.
>
>> +
>> +memcpy(&frame->white_point,
>> + &hdr_metadata->hdmi_metadata_type1.white_point, 4);
>
>ditto.
Sure.
>> +
>> +frame->max_display_mastering_luminance =
>> +hdr_metadata-
>>hdmi_metadata_type1.max_display_mastering_luminance;
>> +frame->min_display_mastering_luminance =
>> +hdr_metadata-
>>hdmi_metadata_type1.min_display_mastering_luminance;
>> +frame->max_fall = hdr_metadata->hdmi_metadata_type1.max_fall;
>> +frame->max_cll = hdr_metadata->hdmi_metadata_type1.max_cll;
>> +
>> +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..c5ecd16 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -650,6 +650,147 @@ 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;
>> +frame->length = HDMI_DRM_INFOFRAME_SIZE;
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL(hdmi_drm_infoframe_init);
>> +
>> +static int hdmi_drm_infoframe_check_only(const struct
>> +hdmi_drm_i
Re: [Intel-gfx] [v10 04/12] drm: Enable HDR infoframe support
On Tue, May 14, 2019 at 11:06:26PM +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.
>
> Added the const version of infoframe for DRM metadata
> for 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.
>
> v7: Addressed Brian Starkey's review comments. Merged 2 patches
> into one.
>
> v8: Addressed Jonas Karlman review comments.
>
> v9: Addressed Jonas Karlman review comments.
>
> Signed-off-by: Uma Shankar
> Signed-off-by: Ville Syrjälä
> Reviewed-by: Shashank Sharma
> ---
> drivers/gpu/drm/drm_edid.c | 43 +++
> drivers/video/hdmi.c | 187
> +
> include/drm/drm_edid.h | 5 ++
> include/linux/hdmi.h | 27 +++
> 4 files changed, 262 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 2e0b5be..73b7905 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4903,6 +4903,49 @@ static bool is_hdmi2_sink(struct drm_connector
> *connector)
> }
>
> /**
> + * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI DRM infoframe with
> + * HDR metadata from userspace
> + * @frame: HDMI DRM infoframe
> + * @hdr_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,
> + const struct hdr_output_metadata
> *hdr_metadata)
> +{
> + int err;
> +
> + if (!frame || !hdr_metadata)
> + return -EINVAL;
> +
> + err = hdmi_drm_infoframe_init(frame);
> + if (err < 0)
> + return err;
> +
> + DRM_DEBUG_KMS("type = %x\n", frame->type);
This debug seems pointless.
> +
> + frame->eotf = hdr_metadata->hdmi_metadata_type1.eotf;
> + frame->metadata_type = hdr_metadata->hdmi_metadata_type1.metadata_type;
> +
> + memcpy(&frame->display_primaries,
> +&hdr_metadata->hdmi_metadata_type1.display_primaries, 12);
sizeof() can be used here.
Could also slap on a BUILD_BUG_ON() to make sure both are the same size.
> +
> + memcpy(&frame->white_point,
> +&hdr_metadata->hdmi_metadata_type1.white_point, 4);
ditto.
> +
> + frame->max_display_mastering_luminance =
> +
> hdr_metadata->hdmi_metadata_type1.max_display_mastering_luminance;
> + frame->min_display_mastering_luminance =
> +
> hdr_metadata->hdmi_metadata_type1.min_display_mastering_luminance;
> + frame->max_fall = hdr_metadata->hdmi_metadata_type1.max_fall;
> + frame->max_cll = hdr_metadata->hdmi_metadata_type1.max_cll;
> +
> + 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..c5ecd16 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -650,6 +650,147 @@ 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;
> + frame->length = HDMI_DRM_INFOFRAME_SIZE;
> +
> + return 0;
> +}
> +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)
Missing the length check.
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> + * hdmi_drm_infoframe_check() - check a HDMI DRM infoframe
> + * @frame: HDMI DRM infoframe
> + *
> + * Validates that the infoframe is consistent.
> + * 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_
[Intel-gfx] [v10 04/12] drm: Enable HDR infoframe support
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.
Added the const version of infoframe for DRM metadata
for 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.
v7: Addressed Brian Starkey's review comments. Merged 2 patches
into one.
v8: Addressed Jonas Karlman review comments.
v9: Addressed Jonas Karlman review comments.
Signed-off-by: Uma Shankar
Signed-off-by: Ville Syrjälä
Reviewed-by: Shashank Sharma
---
drivers/gpu/drm/drm_edid.c | 43 +++
drivers/video/hdmi.c | 187 +
include/drm/drm_edid.h | 5 ++
include/linux/hdmi.h | 27 +++
4 files changed, 262 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 2e0b5be..73b7905 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4903,6 +4903,49 @@ static bool is_hdmi2_sink(struct drm_connector
*connector)
}
/**
+ * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI DRM infoframe with
+ * HDR metadata from userspace
+ * @frame: HDMI DRM infoframe
+ * @hdr_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,
+ const struct hdr_output_metadata
*hdr_metadata)
+{
+ int err;
+
+ if (!frame || !hdr_metadata)
+ return -EINVAL;
+
+ err = hdmi_drm_infoframe_init(frame);
+ if (err < 0)
+ return err;
+
+ DRM_DEBUG_KMS("type = %x\n", frame->type);
+
+ frame->eotf = hdr_metadata->hdmi_metadata_type1.eotf;
+ frame->metadata_type = hdr_metadata->hdmi_metadata_type1.metadata_type;
+
+ memcpy(&frame->display_primaries,
+ &hdr_metadata->hdmi_metadata_type1.display_primaries, 12);
+
+ memcpy(&frame->white_point,
+ &hdr_metadata->hdmi_metadata_type1.white_point, 4);
+
+ frame->max_display_mastering_luminance =
+
hdr_metadata->hdmi_metadata_type1.max_display_mastering_luminance;
+ frame->min_display_mastering_luminance =
+
hdr_metadata->hdmi_metadata_type1.min_display_mastering_luminance;
+ frame->max_fall = hdr_metadata->hdmi_metadata_type1.max_fall;
+ frame->max_cll = hdr_metadata->hdmi_metadata_type1.max_cll;
+
+ 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..c5ecd16 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -650,6 +650,147 @@ 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;
+ frame->length = HDMI_DRM_INFOFRAME_SIZE;
+
+ return 0;
+}
+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.
+ * 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_only() - 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
