[Intel-gfx] [v8 01/10] drm: Add HDR source metadata property

Jonas Karlman jonas at kwiboo.se
Sat May 4 10:17:40 UTC 2019


On 2019-04-09 18:44, 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.
>
> v7: Addressed Jonas Karlman review comments and defined separate
> structure for infoframe to better align with CTA 861.G spec. Added
> Shashank's RB.
>
> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> Reviewed-by: Shashank Sharma <shashank.sharma at 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       | 11 +++++++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  include/linux/hdmi.h              | 10 ++++++++++
>  include/uapi/drm/drm_mode.h       | 39 +++++++++++++++++++++++++++++++++++++++
>  7 files changed, 87 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..5343f60 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,10 @@ struct drm_connector {
>  	 * &drm_mode_config.connector_free_work.
>  	 */
>  	struct llist_node free_node;
> +
> +	/* HDR metdata */
> +	struct hdr_output_metadata hdr_output_metadata;
> +	struct hdr_sink_metadata hdr_sink_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 83cd163..7b6a519 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -630,6 +630,45 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/* HDR Metadata Infoframe as per 861.G spec */
> +struct hdr_metadata_infoframe {
> +	__u8 eotf;
> +	__u8 metadata_type;
> +	struct {
> +		__u16 x, y;
> +		} display_primaries[3];
> +	struct {
> +		__u16 x, y;
> +		} white_point;
> +	__u16 max_display_mastering_luminance;
> +	__u16 min_display_mastering_luminance;
> +	__u16 max_cll;
> +	__u16 max_fall;
> +};
> +
> +struct hdr_output_metadata {
> +	__u32 metadata_type;
> +	union {
> +		struct hdr_metadata_infoframe hdmi_metadata_type1;
> +	};
> +};
> +
> +/* HDR Metadata as per 861.G spec */
> +struct hdr_static_metadata {
> +	__u8 eotf;
> +	__u8 metadata_type;
> +	__u16 max_cll;
> +	__u16 max_fall;
> +	__u16 min_cll;
> +};
> +
> +struct hdr_sink_metadata {
> +	__u32 metadata_type;
> +	union {
> +		struct hdr_static_metadata hdmi_type1;
> +	};
> +};

These two structs (hdr_static_metadata and hdr_sink_metadata) should probably not be part of uapi,
unless the metadata is intended to be exposed in a HDR_SINK_METADATA property.

The commit "drm/i915: Add HLG EOTF" should probably not use a i915 in prefix as it affects drm core.

The blob_ptr should probably be reference counted in duplicate_state/destroy_state helper similar to [1].
I know too little about drm core in order to know if that is correct, please feel free to pick/squash if this is correct.

[1] https://github.com/Kwiboo/linux-rockchip/commit/5f065ff0bbcca145ee46e9466bb8ca048c7a7b1e

I have tested this patchset on rockchip / dw-hdmi using dw-hdmi [2] and Kodi [3] patches with a successful result of enabling HDR mode on the sink.
There is more work needed to get a full 10-bit pipeline for dw-hdmi in order to make full use of HDR,
but for the purpose of enabling HDR on the sink this patchset seems ready.

[2] https://github.com/Kwiboo/linux-rockchip/commit/d63e38d1cb905e5d885c903286402b202be8541e
[3] https://github.com/Kwiboo/xbmc/commit/c41f85ddaa48995659786ba6d7df6b61c7276aa0

Regards,
Jonas

> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4



More information about the Intel-gfx mailing list