Re: [v6 02/13] drm: Parse HDR metadata info from EDID

From: "Sharma, Shashank" <shashank.sharma@intel.com>
To: Uma Shankar <uma.shankar@intel.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: ville.syrjala@intel.com, maarten.lankhorst@intel.com
Subject: Re: [v6 02/13] drm: Parse HDR metadata info from EDID
Date: Fri, 29 Mar 2019 17:11:46 +0530	[thread overview]
Message-ID: <822e49a0-7ae5-16d2-0ab9-0f9f762c2a7d@intel.com> (raw)
In-Reply-To: <1553078906-5991-3-git-send-email-uma.shankar@intel.com>

On 3/20/2019 4:18 PM, 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
>
> Signed-off-by: Uma Shankar <uma.shankar@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 fa39592..fd8a621a 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2840,6 +2840,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
> @@ -3587,6 +3588,12 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
>   }
>   
>   static int
> +cea_db_payload_len_ext(const u8 *db)
> +{
> +	return (db[0] & 0x1f) - 1;
> +}
> +
> +static int
>   cea_db_extended_tag(const u8 *db)
>   {
>   	return db[1];
> @@ -3822,6 +3829,46 @@ 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_ext(db);
> +	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)

Little bike shed, If we pass >=5 we need not to compare >= 4, or we 
should check >=4 first and then >=5 in sequence.

With that change ( and assuming you fixed the checkpatch stuff already), 
please feel free to use:

Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

> +		connector->hdr_metadata.max_cll = db[4];
> +}
> +
>   static void
>   drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
>   {
> @@ -4449,6 +4496,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);
>   	}
>   }
>   
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx