Re: [v6 03/13] drm: Parse Colorimetry data block from EDID

From: "Shankar, Uma" <uma.shankar@intel.com>
To: Brian Starkey <Brian.Starkey@arm.com>
Cc: "Syrjala, Ville" <ville.syrjala@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>, nd <nd@arm.com>,
	"Lankhorst, Maarten" <maarten.lankhorst@intel.com>
Subject: Re: [v6 03/13] drm: Parse Colorimetry data block from EDID
Date: Fri, 29 Mar 2019 06:16:36 +0000	[thread overview]
Message-ID: <E7C9878FBA1C6D42A1CA3F62AEB6945F81F90C60@BGSMSX104.gar.corp.intel.com> (raw)
In-Reply-To: <20190321111713.qali5wffykezlrpe@DESKTOP-E1NTVVP.localdomain>

>-----Original Message-----
>From: Brian Starkey [mailto:Brian.Starkey@arm.com]
>Sent: Thursday, March 21, 2019 4:47 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Lankhorst,
>Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
>Sharma, Shashank <shashank.sharma@intel.com>; emil.l.velikov@gmail.com; Liviu
>Dudau <Liviu.Dudau@arm.com>; nd <nd@arm.com>
>Subject: Re: [v6 03/13] drm: Parse Colorimetry data block from EDID
>
>Hi Uma,
>
>On Wed, Mar 20, 2019 at 04:18:16PM +0530, Uma Shankar wrote:
>> CEA 861.3 spec adds colorimetry data block for HDMI.
>> Parsing the block to get the colorimetry data from panel.
>>
>
>While code which parses these parts (patches 2 and 3) out of EDID could be useful - it
>doesn't seem to actually be used anywhere in the kernel (sorry if I missed it), and I'm
>not sure it will/can be unless we expose more properties.
>
>The discussion last time leant towards a more useful/generic userspace library for
>EDID parsing, so perhaps patches 2 and 3 should be dropped.
>
>Did you have an intended use for them in the kernel?

The idea was to use them for any kernel defaults or some state and error checking, when we
have more such types exposed. But yeah, may be till we have it I can drop it for now and
re-introduced them later when needed.

Thanks & Regards,
Uma Shankar

>Thanks,
>-Brian
>
>> v2: Rebase
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments. Updated colorimetry field to
>> 16 bit as DCI-P3 got added in CEA 861-G spec, as pointed out by
>> Shashank.
>>
>> v5: Fixed checkpatch warnings with --strict option.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c  | 25 +++++++++++++++++++++++++
>> include/drm/drm_connector.h |  3 +++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index fd8a621a..676569b 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 COLORIMETRY_DATA_BLOCK		0x5
>>  #define HDR_STATIC_METADATA_BLOCK	0x6
>>  #define USE_EXTENDED_TAG 0x07
>>  #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3829,6 +3830,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) != USE_EXTENDED_TAG)
>> +		return false;
>> +
>> +	if (db[1] != COLORIMETRY_DATA_BLOCK)
>> +		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;
>> +	u16 len;
>> +
>> +	len = cea_db_payload_len_ext(db);
>> +	/* As per CEA 861-G spec */
>> +	info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; }
>> +
>>  static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)  {
>>  	if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4498,6 +4521,8 @@ static
>> void drm_parse_cea_ext(struct drm_connector *connector,
>>  			drm_parse_vcdb(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 29388bd..94f926e 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -206,6 +206,9 @@ struct drm_hdmi_info {
>>
>>  	/** @y420_dc_modes: bitmap of deep color support index */
>>  	u8 y420_dc_modes;
>> +
>> +	/* @colorimetry: bitmap of supported colorimetry modes */
>> +	u16 colorimetry;
>>  };
>>
>>  /**
>> --
>> 1.9.1
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx