[Intel-gfx] [v10 03/12] drm: Parse HDR metadata info from EDID

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>Sent: Thursday, May 16, 2019 1:06 AM
>To: Shankar, Uma <uma.shankar at intel.com>
>Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
>maarten.lankhorst at linux.intel.com; Sharma, Shashank
><shashank.sharma at intel.com>; emil.l.velikov at gmail.com; brian.starkey at arm.com;
>dcastagna at chromium.org; seanpaul at chromium.org; Roper, Matthew D
><matthew.d.roper at intel.com>; jonas at kwiboo.se
>Subject: Re: [v10 03/12] drm: Parse HDR metadata info from EDID
>
>On Tue, May 14, 2019 at 11:06:25PM +0530, Uma Shankar wrote:
>> HDR metadata block is introduced in CEA-861.3 spec.
>> Parsing the same to get the panel's HDR metadata.
>>
>> v2: Rebase and added Ville's POC changes to the patch.
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments
>>
>> v5: Addressed Shashank's comment and added his RB.
>>
>> v6: Addressed Jonas Karlman review comments.
>>
>> v7: Adressed Ville's review comments and fixed the issue with length
>> handling.
>>
>> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
>> Reviewed-by: Shashank Sharma <shashank.sharma at intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c | 49
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 852bdd8..2e0b5be 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2852,6 +2852,7 @@ static int drm_cvt_modes(struct drm_connector
>*connector,
>>  #define VIDEO_BLOCK     0x02
>>  #define VENDOR_BLOCK    0x03
>>  #define SPEAKER_BLOCK	0x04
>> +#define HDR_STATIC_METADATA_BLOCK	0x6
>>  #define USE_EXTENDED_TAG 0x07
>>  #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
>>  #define EXT_VIDEO_DATA_BLOCK_420	0x0E
>> @@ -3834,6 +3835,52 @@ static void fixup_detailed_cea_mode_clock(struct
>drm_display_mode *mode)
>>  	mode->clock = clock;
>>  }
>>
>> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) {
>> +	if (cea_db_tag(db) != USE_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>> +static uint8_t eotf_supported(const u8 *edid_ext) {
>> +	return edid_ext[2] &
>> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
>> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
>> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
>> +}
>> +
>> +static uint8_t hdr_metadata_type(const u8 *edid_ext) {
>> +	return edid_ext[3] &
>> +		BIT(HDMI_STATIC_METADATA_TYPE1);
>> +}
>> +
>> +static void
>> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const
>> +u8 *db) {
>> +	u16 len;
>> +
>> +	len = cea_db_payload_len(db);
>> +	if (len >= 3) {
>
>I believe in other cases we've put the length check for the mandatory bytes into the
>cea_db_is_foo() function. Would be good to follow the path laid out by existing code
>here too.

Ok got it. Will update this.

>> +		connector->hdr_sink_metadata.hdmi_type1.eotf =
>> +						eotf_supported(db);
>> +		connector->hdr_sink_metadata.hdmi_type1.metadata_type =
>> +						hdr_metadata_type(db);
>> +	}
>> +
>> +	if (len >= 4)
>> +		connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4];
>> +	if (len >= 5)
>> +		connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5];
>> +	if (len >= 6)
>> +		connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; }
>> +
>>  static void
>>  drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8
>> *db)  { @@ -4461,6 +4508,8 @@ static void drm_parse_cea_ext(struct
>> drm_connector *connector,
>>  			drm_parse_y420cmdb_bitmap(connector, db);
>>  		if (cea_db_is_vcdb(db))
>>  			drm_parse_vcdb(connector, db);
>> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
>> +			drm_parse_hdr_metadata_block(connector, db);
>>  	}
>>  }
>>
>> --
>> 1.9.1
>
>--
>Ville Syrjälä
>Intel