[v2 03/14] drm: Parse HDR metadata info from EDID

[Sharma, Shashank]

Regards

Shashank


On 12/12/2018 2:08 AM, 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.
>
> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 106fd38..d12b74e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3818,6 +3818,49 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
>   	mode->clock = clock;
>   }
I guess the previous patch (or a art of previous patch) should be merged 
in this patch.
>   
> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
> +{
> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
> +		return false;
> +
> +	if (db[1] != HDR_STATIC_METADATA_BLOCK)
> +		return false;
> +
> +	return true;
> +}
> +
> +static uint16_t eotf_supported(const u8 *edid_ext)
Why uint16 ? why not uint8_t ? this is clearly one byte.
> +{
> +
> +	return edid_ext[2] &
> +		(BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
Should we bother about SDR bit at all ? Why not just check HDR bits ?
> +		 BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
> +		 BIT(HDMI_EOTF_SMPTE_ST2084));
> +
> +}
> +
> +static uint16_t hdr_metadata_type(const u8 *edid_ext)
> +{
Same uint8_t stuff
> +
> +	return edid_ext[3] &
> +		BIT(HDMI_STATIC_METADATA_TYPE1);
> +}
> +
> +static void
> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db)
> +{
> +	uint16_t len;
> +
> +	len = cea_db_payload_len(db);
Length is  incorrect here for extended tag blocks, as this function 
doesn't account the extended tag byte's size. So the payload length is 
looking for is len - 1. May be a good time to add 
cea_extended_db_payload_len() ?
> +	connector->hdr_metadata.eotf = eotf_supported(db);
> +	connector->hdr_metadata.metadata_type = hdr_metadata_type(db);
> +
> +	if (len >= 5)
> +		connector->hdr_metadata.max_fall = db[5];
> +	if (len >= 4)
> +		connector->hdr_metadata.max_cll = db[4];
> +}
> +
>   static void
>   drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
>   {
> @@ -4468,6 +4511,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>   			drm_parse_hdmi_forum_vsdb(connector, db);
>   		if (cea_db_is_y420cmdb(db))
>   			drm_parse_y420cmdb_bitmap(connector, db);
> +		if (cea_db_is_hdmi_hdr_metadata_block(db))
> +			drm_parse_hdr_metadata_block(connector, db);
>   	}
>   }
>   
- Shashank