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

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, March 15, 2019 12:39 PM
>To: Shankar, Uma <uma.shankar at intel.com>; intel-gfx at lists.freedesktop.org; dri-
>devel at lists.freedesktop.org
>Cc: Lankhorst, Maarten <maarten.lankhorst at intel.com>; Syrjala, Ville
><ville.syrjala at intel.com>; emil.l.velikov at gmail.com; brian.starkey at arm.com;
>Liviu.Dudau at arm.com
>Subject: RE: [v5 01/13] drm: Add HDR source metadata property
>
>Hello Uma,
>
>> -----Original Message-----
>> From: Shankar, Uma
>> Sent: Monday, March 11, 2019 9:28 AM
>> To: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst at intel.com>; Syrjala, Ville
>> <ville.syrjala at intel.com>; Sharma, Shashank
>> <shashank.sharma at intel.com>; emil.l.velikov at gmail.com;
>> brian.starkey at arm.com; Liviu.Dudau at arm.com; Shankar, Uma
>> <uma.shankar at intel.com>
>> Subject: [v5 01/13] drm: Add HDR source metadata property
>>
>> 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 07d65a1..b031079 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1054,6 +1054,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);
>Needs alignment with the line above.

Look like this is an issue with outlook. The changes are aligned actually.

>> +	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;
>I was under the assumption that we are not parsing the hdr_metadata from the EDID,
>and the userspace needs to parse it from the EDID. If that's the case, we might not
>even need this pointer. Are we even using this or am I missing something here ?

We are parsing the metadata and keeping the sink capability in driver as well. This can help
in some state checks and any default programming in case we need to do that.

>>  };
>>
>>  #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 */
>> +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;
>> +};
>> +
>- Shashank
>>  #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