[Intel-gfx] [v2 04/14] drm: Parse Colorimetry data block from EDID

[Shankar, Uma]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Thursday, December 20, 2018 11:54 PM
>To: Shankar, Uma <uma.shankar at intel.com>; intel-gfx at lists.freedesktop.org;
>dri-devel at lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala at intel.com>; Lankhorst, Maarten
><maarten.lankhorst at intel.com>
>Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> CEA 861.3 spec adds colorimetry data block for HDMI.
>> Parsing the block to get the colorimetry data from panel.
>>
>> v2: Rebase
>>
>> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c  | 24 ++++++++++++++++++++++++
>>   include/drm/drm_connector.h |  2 ++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index d12b74e..344d8c1 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3818,6 +3818,28 @@ static void fixup_detailed_cea_mode_clock(struct
>drm_display_mode *mode)
>>   	mode->clock = clock;
>>   }
>>
>> +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) {
>> +	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
>> +
>Again, the COLORIMETRY_DATA_BLOCK definition should be added into this
>patch.

Ok, will do that.

>> 	return false;
>> +
>> +	return true;
>> +}
>> +
>> +static void
>> +drm_parse_colorimetry_data_block(struct drm_connector *connector,
>> +const u8 *db) {
>> +	struct drm_hdmi_info *info = &connector->display_info.hdmi;
>> +	uint16_t len;
>> +
>> +	len = cea_db_payload_len(db);
>Again, the return value of this function doesn't account for extended db tag byte,
>so what we are looking for is len -1

Will update this.

>> +	info->colorimetry = db[2];
>colorimetry block is actually db[2] and db[3].bit7 (which represents
>DCI-P3 space), so we probably want to make info->colorimetryuint16_t

3 BT2020RGB BT2020YCC BT2020cYCC AdobeRGB AdobeYCC601 sYCC601 xvYCC709 xvYCC601
4 F47=0 F46=0 F45=0 F44=0 MD3 MD2 MD1 MD0

This is how db[2] and db[3] is described in spec. There is not much clarity on F47. Not sure if they
are for DCI-P3, can you clarify once.

Regards,
Uma Shankar

>> +}
>> +
>> +
>>   static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
>>   {
>>   	if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) @@ -4513,6
>+4535,8
>> @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>   			drm_parse_y420cmdb_bitmap(connector, db);
>>   		if (cea_db_is_hdmi_hdr_metadata_block(db))
>>   			drm_parse_hdr_metadata_block(connector, db);
>> +		if (cea_db_is_hdmi_colorimetry_data_block(db))
>> +			drm_parse_colorimetry_data_block(connector, db);
>>   	}
>>   }
>>
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 2ee45dc..90ce364 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -206,6 +206,8 @@ struct drm_hdmi_info {
>>
>>   	/** @y420_dc_modes: bitmap of deep color support index */
>>   	u8 y420_dc_modes;
>> +
>Please add a one line description about this variable.

Sure will update this.

>> +	u8 colorimetry;
>>   };
>>
>>   /**
>Shashank
>_______________________________________________
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel