[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: Tuesday, January 8, 2019 12:27 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
>
>
>
>On 1/8/2019 12:10 PM, Shankar, Uma wrote:
>>
>>> -----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.
>Ah, check CEA-861-G table 70, there there is no F47, but I could see
>DCI-P3 in that place, which spec is that you are following right now ?

I am looking to 861-F. So seems like its updated in 861-G ?

Regards,
Uma Shankar

>- Shashank
>> 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
>
>_______________________________________________
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel