Re: [Intel-gfx] [PATCH v2 18/20] drm/edid: detect color formats and CTA revision in all CTA extensions

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 05 May 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, May 03, 2022 at 12:24:03PM +0300, Jani Nikula wrote:
>> Convert drm_find_cea_extension() to EDID block iterator in color format
>> and CTA revision detection. Detect them in all CTA extensions.
>> 
>> Also parse CTA Data Blocks in DisplayID even if there's no CTA EDID
>> extension.
>> 
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++--------
>>  1 file changed, 16 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 3b18a6e501df..41f24f4c2d23 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -5447,32 +5447,40 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>  			      const struct edid *edid)
>>  {
>>  	struct drm_display_info *info = &connector->display_info;
>> +	struct drm_edid_iter edid_iter;
>>  	const struct cea_db *db;
>>  	struct cea_db_iter iter;
>>  	const u8 *edid_ext;
>>  
>> -	edid_ext = drm_find_cea_extension(edid);
>> -	if (!edid_ext)
>> -		return;
>> +	drm_edid_iter_begin(edid, &edid_iter);
>> +	drm_edid_iter_for_each(edid_ext, &edid_iter) {
>> +		if (edid_ext[0] != CEA_EXT)
>> +			continue;
>>  
>> -	info->cea_rev = edid_ext[1];
>> +		if (!info->cea_rev)
>> +			info->cea_rev = edid_ext[1];
>>  
>> -	/* The existence of a CEA block should imply RGB support */
>> -	info->color_formats = DRM_COLOR_FORMAT_RGB444;
>> +		if (info->cea_rev != edid_ext[1])
>> +			DRM_DEBUG_KMS("CEA extension version mismatch %u != %u\n",
>> +				      info->cea_rev, edid_ext[1]);
>>  
>> -	/* CTA DisplayID Data Block does not have byte #3 */
>> -	if (edid_ext[0] == CEA_EXT) {
>> +		/* The existence of a CTA extension should imply RGB support */
>> +		info->color_formats = DRM_COLOR_FORMAT_RGB444;
>>  		if (edid_ext[3] & EDID_CEA_YCRCB444)
>>  			info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
>>  		if (edid_ext[3] & EDID_CEA_YCRCB422)
>>  			info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
>>  	}
>> +	drm_edid_iter_end(&edid_iter);
>>  
>>  	cea_db_iter_edid_begin(edid, &iter);
>>  	cea_db_iter_for_each(db, &iter) {
>>  		/* FIXME: convert parsers to use struct cea_db */
>>  		const u8 *data = (const u8 *)db;
>>  
>> +		/* The existence of a CTA block should imply RGB support */
>> +		info->color_formats = DRM_COLOR_FORMAT_RGB444;
>
> This part seems a bit iffy. DisplayID has its own way to specify the
> supported color depths/encodings. So I think eventually we'll just
> want to split this into three logical parts: parse CTA ext block,
> parse DispID color stuff, parse CTA data blocks.

Right. Sent a new version with the above hunk dropped.

BR,
Jani.

>
>> +
>>  		if (cea_db_is_hdmi_vsdb(db))
>>  			drm_parse_hdmi_vsdb_video(connector, data);
>>  		if (cea_db_is_hdmi_forum_vsdb(db) ||
>> -- 
>> 2.30.2

-- 
Jani Nikula, Intel Open Source Graphics Center