[v6 01/13] drm: Add HDR source metadata property

Shankar, Uma uma.shankar at intel.com
Fri Mar 29 06:13:59 UTC 2019



>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf Of Brian
>Starkey
>Sent: Thursday, March 21, 2019 5:01 PM
>To: Shankar, Uma <uma.shankar at intel.com>
>Cc: Syrjala, Ville <ville.syrjala at intel.com>; Liviu Dudau <Liviu.Dudau at arm.com>;
>intel-gfx at lists.freedesktop.org; emil.l.velikov at gmail.com; dri-
>devel at lists.freedesktop.org; nd <nd at arm.com>; Lankhorst, Maarten
><maarten.lankhorst at intel.com>
>Subject: Re: [v6 01/13] drm: Add HDR source metadata property
>
>Hi Uma,
>
>On Wed, Mar 20, 2019 at 04:18:14PM +0530, 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.
>>
>> 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.
>>
>> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
>> ---
>>  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     | 16 ++++++++++++++++
>>  5 files changed, 48 insertions(+)
>>
>> 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 c806199..29388bd 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -561,6 +561,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;
>>  };
>>
>>  /**
>> @@ -1201,6 +1208,9 @@ struct drm_connector {
>>  	 * &drm_mode_config.connector_free_work.
>>  	 */
>>  	struct llist_node free_node;
>> +
>> +	/* HDR metdata */
>> +	struct hdr_static_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 a439c2e..5012af2 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -630,6 +630,22 @@ struct drm_color_lut {
>>  	__u16 reserved;
>>  };
>>
>> +/* HDR Metadata */
>
>If this is meant to exactly match the layout in the CTA spec, maybe it's worth a note
>here saying that, to make it clear it isn't arbitrary.
>
>> +struct hdr_static_metadata {
>> +	uint8_t eotf;
>> +	uint8_t 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;
>
>In my copy of CTA-861, fall and cll are the other way around. cll in bytes 23/24, fall in
>25/26.

Sure Brian, will re-order these.

>> +};
>
>The types should be __u8 etc in this kernel header.

Ok will update them.

>It occurred to me that we're likely to want to support dynamic metadata before too
>long, and I'm sure there'll be other new HDR metadata types too. So, perhaps we
>should add a "header" of sorts to the HDR_OUTPUT_METADATA blob, to let it be re-
>used for stuff which isn't "STATIC_METADATA_TYPE1" in the future.
>
>Something like:
>
>struct hdr_output_metadata {
>	__u32 metadata_type;
>	union {
>		struct hdr_static_metadata hdmi_type1;
>	};
>};
>
>Not sure how DRM feels about unions in ioctl arguments.


Sounds good, I will update like this and will try to test this out.

Thanks & Regards,
Uma Shankar

>Thanks,
>-Brian
>
>> +
>>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01  #define
>> DRM_MODE_PAGE_FLIP_ASYNC 0x02  #define
>> DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
>> --
>> 1.9.1
>>
>_______________________________________________
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list