[v7,1/9] drm: Add HDR source metadata property

Submitted by Shankar, Uma on April 2, 2019, 8:20 p.m.

Details

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

Browsing this patch as part of:
"Add HDR Metadata Parsing and handling in DRM layer" rev 7 in DRI devel
<< prev patch [1/9] next patch >>

Commit Message

Shankar, Uma April 2, 2019, 8:20 p.m.
This patch adds a blob property to get HDR metadata
information from userspace. This will be send as part
of AVI Infoframe to panel.

It also 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 modified the metadata structure elements
as per Ville's POC changes.

v3: No Change

v4: Addressed Shashank's review comments

v5: Rebase.

v6: Addressed Brian Starkey's review comments, defined
new structure with header for dynamic metadata scalability.
Merge get/set property functions for metadata in this patch.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_atomic.c      |  2 ++
 drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
 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       | 22 ++++++++++++++++++++++
 7 files changed, 69 insertions(+)

Patch hide | download patch | download mbox

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 ea797d4..6d0d161 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -673,6 +673,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);
@@ -721,6 +723,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_output_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) {
@@ -807,6 +817,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) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2355124..0bdae90 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1058,6 +1058,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 02a1312..9e3a39c 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -599,6 +599,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;
 };
 
 /**
@@ -1239,6 +1246,9 @@  struct drm_connector {
 	 * &drm_mode_config.connector_free_work.
 	 */
 	struct llist_node free_node;
+
+	/* HDR metdata */
+	struct hdr_output_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 09d7296..9d6bdf5 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -629,6 +629,28 @@  struct drm_color_lut {
 	__u16 reserved;
 };
 
+/* HDR Metadata as per 861.G spec */
+struct hdr_static_metadata {
+	__u8 eotf;
+	__u8 metadata_type;
+	struct {
+		__u16 x, y;
+		} display_primaries[3];
+	struct {
+		__u16 x, y;
+		} white_point;
+	__u16 max_cll;
+	__u16 max_fall;
+	__u16 min_cll;
+};
+
+struct hdr_output_metadata {
+	__u32 metadata_type;
+	union {
+		struct hdr_static_metadata hdmi_type1;
+	};
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4

Comments

Hi Uma.

Some kerneldoc nits below.

	Sam

> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -599,6 +599,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;

The kerneldoc comment uses the wrong name.

> +	u8 hdr_metadata_changed : 1;

kerneldoc is missing.

>  };
>  
>  /**
> @@ -1239,6 +1246,9 @@ struct drm_connector {
>  	 * &drm_mode_config.connector_free_work.
>  	 */
>  	struct llist_node free_node;
> +
> +	/* HDR metdata */
> +	struct hdr_output_metadata hdr_metadata;
Please format as kerneldoc.
>  };
>  

>  #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;
> +
kerneldoc comment uses wrong name

>  	/* dumb ioctl parameters */
>  	uint32_t preferred_depth, prefer_shadow;
Please format as kerneldoc.
On 2019-04-02 22:20, Uma Shankar wrote:
> This patch adds a blob property to get HDR metadata

> information from userspace. This will be send as part

> of AVI Infoframe to panel.

>

> It also 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 modified the metadata structure elements

> as per Ville's POC changes.

>

> v3: No Change

>

> v4: Addressed Shashank's review comments

>

> v5: Rebase.

>

> v6: Addressed Brian Starkey's review comments, defined

> new structure with header for dynamic metadata scalability.

> Merge get/set property functions for metadata in this patch.

>

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

> ---

>  drivers/gpu/drm/drm_atomic.c      |  2 ++

>  drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++

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

>  7 files changed, 69 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 ea797d4..6d0d161 100644

> --- a/drivers/gpu/drm/drm_atomic_uapi.c

> +++ b/drivers/gpu/drm/drm_atomic_uapi.c

> @@ -673,6 +673,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);

> @@ -721,6 +723,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_output_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) {

> @@ -807,6 +817,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) {

> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c

> index 2355124..0bdae90 100644

> --- a/drivers/gpu/drm/drm_connector.c

> +++ b/drivers/gpu/drm/drm_connector.c

> @@ -1058,6 +1058,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 02a1312..9e3a39c 100644

> --- a/include/drm/drm_connector.h

> +++ b/include/drm/drm_connector.h

> @@ -599,6 +599,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;

>  };

>  

>  /**

> @@ -1239,6 +1246,9 @@ struct drm_connector {

>  	 * &drm_mode_config.connector_free_work.

>  	 */

>  	struct llist_node free_node;

> +

> +	/* HDR metdata */

> +	struct hdr_output_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 09d7296..9d6bdf5 100644

> --- a/include/uapi/drm/drm_mode.h

> +++ b/include/uapi/drm/drm_mode.h

> @@ -629,6 +629,28 @@ struct drm_color_lut {

>  	__u16 reserved;

>  };

>  

> +/* HDR Metadata as per 861.G spec */

> +struct hdr_static_metadata {

> +	__u8 eotf;

> +	__u8 metadata_type;

> +	struct {

> +		__u16 x, y;

> +		} display_primaries[3];

> +	struct {

> +		__u16 x, y;

> +		} white_point;

> +	__u16 max_cll;

> +	__u16 max_fall;

> +	__u16 min_cll;

> +};


Does this struct represent a mix of the HDR Static Metadata Data Block (Table 84) and Static Metadata Descriptor Type 1 (Table 45) in CTA-861-G [1] ?

display_primaries and white_point is only part of the Static Metadata Descriptor and min_cll is only part of the HDR Static Metadata.
min_cll should be parsed and set in drm_parse_hdr_metadata_block() in next patch or be removed from the struct.

max_display_mastering_luminance and min_display_mastering_luminance from Static Metadata Descriptor Type 1 is also missing in this struct.
Why is max_cll and min_cll used for max_display_mastering_luminance and min_display_mastering_luminance in infoframe?

"frame->length = sizeof(struct hdr_static_metadata);" also needs to be changed in another patch of the patchset.
The Static Metadata Descriptor Type 1 is 26 bytes and is set to 24 bytes using sizeof the above struct.

[1] https://standards.cta.tech/kwspub/published_docs/CTA-861-G_FINAL_revised_2017.pdf

Regards,
Jonas

> +

> +struct hdr_output_metadata {

> +	__u32 metadata_type;

> +	union {

> +		struct hdr_static_metadata hdmi_type1;

> +	};

> +};

> +

>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01

>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02

>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
>-----Original Message-----

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

>Karlman

>Sent: Monday, April 8, 2019 3:51 PM

>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-

>devel@lists.freedesktop.org

>Cc: seanpaul@chromium.org; emil.l.velikov@gmail.com; dcastagna@chromium.org;

>Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville

><ville.syrjala@intel.com>

>Subject: Re: [v7 1/9] drm: Add HDR source metadata property

>

>On 2019-04-02 22:20, Uma Shankar wrote:

>> This patch adds a blob property to get HDR metadata information from

>> userspace. This will be send as part of AVI Infoframe to panel.

>>

>> It also 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 modified the metadata structure elements as per Ville's

>> POC changes.

>>

>> v3: No Change

>>

>> v4: Addressed Shashank's review comments

>>

>> v5: Rebase.

>>

>> v6: Addressed Brian Starkey's review comments, defined new structure

>> with header for dynamic metadata scalability.

>> Merge get/set property functions for metadata in this patch.

>>

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

>> ---

>>  drivers/gpu/drm/drm_atomic.c      |  2 ++

>>  drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++

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

>>  7 files changed, 69 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 ea797d4..6d0d161 100644

>> --- a/drivers/gpu/drm/drm_atomic_uapi.c

>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c

>> @@ -673,6 +673,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); @@ -721,6

>> +723,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_output_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) { @@ -807,6

>> +817,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) { diff

>> --git a/drivers/gpu/drm/drm_connector.c

>> b/drivers/gpu/drm/drm_connector.c index 2355124..0bdae90 100644

>> --- a/drivers/gpu/drm/drm_connector.c

>> +++ b/drivers/gpu/drm/drm_connector.c

>> @@ -1058,6 +1058,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 02a1312..9e3a39c 100644

>> --- a/include/drm/drm_connector.h

>> +++ b/include/drm/drm_connector.h

>> @@ -599,6 +599,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;

>>  };

>>

>>  /**

>> @@ -1239,6 +1246,9 @@ struct drm_connector {

>>  	 * &drm_mode_config.connector_free_work.

>>  	 */

>>  	struct llist_node free_node;

>> +

>> +	/* HDR metdata */

>> +	struct hdr_output_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 09d7296..9d6bdf5 100644

>> --- a/include/uapi/drm/drm_mode.h

>> +++ b/include/uapi/drm/drm_mode.h

>> @@ -629,6 +629,28 @@ struct drm_color_lut {

>>  	__u16 reserved;

>>  };

>>

>> +/* HDR Metadata as per 861.G spec */

>> +struct hdr_static_metadata {

>> +	__u8 eotf;

>> +	__u8 metadata_type;

>> +	struct {

>> +		__u16 x, y;

>> +		} display_primaries[3];

>> +	struct {

>> +		__u16 x, y;

>> +		} white_point;

>> +	__u16 max_cll;

>> +	__u16 max_fall;

>> +	__u16 min_cll;

>> +};

>

>Does this struct represent a mix of the HDR Static Metadata Data Block (Table 84) and

>Static Metadata Descriptor Type 1 (Table 45) in CTA-861-G [1] ?

>

>display_primaries and white_point is only part of the Static Metadata Descriptor and

>min_cll is only part of the HDR Static Metadata.

>min_cll should be parsed and set in drm_parse_hdr_metadata_block() in next patch or

>be removed from the struct.

>

>max_display_mastering_luminance and min_display_mastering_luminance from

>Static Metadata Descriptor Type 1 is also missing in this struct.

>Why is max_cll and min_cll used for max_display_mastering_luminance and

>min_display_mastering_luminance in infoframe?

>

>"frame->length = sizeof(struct hdr_static_metadata);" also needs to be changed in

>another patch of the patchset.

>The Static Metadata Descriptor Type 1 is 26 bytes and is set to 24 bytes using sizeof

>the above struct.

>

>[1] https://standards.cta.tech/kwspub/published_docs/CTA-861-

>G_FINAL_revised_2017.pdf

>


Thanks Jonas for your feedback. I have created a separate structure for Sink metadata
to avoid these issues and fixed all the inputs you have given. Please review the v8.

Regards,
Uma Shankar

>Regards,

>Jonas

>

>> +

>> +struct hdr_output_metadata {

>> +	__u32 metadata_type;

>> +	union {

>> +		struct hdr_static_metadata hdmi_type1;

>> +	};

>> +};

>> +

>>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01  #define

>> DRM_MODE_PAGE_FLIP_ASYNC 0x02  #define

>> DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4

>

>_______________________________________________

>dri-devel mailing list

>dri-devel@lists.freedesktop.org

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