>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, March 15, 2019 12:56 PM
>To: Shankar, Uma <uma.shan...@intel.com>; intel-gfx@lists.freedesktop.org; dri-
>de...@lists.freedesktop.org
>Cc: Lankhorst, Maarten <maarten.lankho...@intel.com>; Syrjala, Ville
><ville.syrj...@intel.com>; emil.l.veli...@gmail.com; brian.star...@arm.com;
>liviu.du...@arm.com
>Subject: RE: [v5 02/13] drm: Parse HDR metadata info from EDID
>
>
>
>> -----Original Message-----
>> From: Shankar, Uma
>> Sent: Monday, March 11, 2019 9:28 AM
>> To: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankho...@intel.com>; Syrjala, Ville
>> <ville.syrj...@intel.com>; Sharma, Shashank
>> <shashank.sha...@intel.com>; emil.l.veli...@gmail.com;
>> brian.star...@arm.com; liviu.du...@arm.com; Shankar, Uma
>> <uma.shan...@intel.com>
>> Subject: [v5 02/13] drm: Parse HDR metadata info from EDID
>>
>> 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.shan...@intel.com>
>> ---
>> drivers/gpu/drm/drm_edid.c | 52
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 52 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index
>> 5f14253..c5a81b8 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2834,6 +2834,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
>> @@ -3581,6 +3582,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;
>You might have already checked with checkpatch, but can we cross check the
>space
>before '-' ?
Yes, this seems ok. Space is there before operator.
>> +}
>> +
>> +static int
>> cea_db_extended_tag(const u8 *db)
>> {
>> return db[1];
>> @@ -3816,6 +3823,49 @@ 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)
>same here, looks like we need spaces around !=, or may be this is due to my
>mail
>client ?
This seems to be issue with mail client. Looks like it's changing alignment on
outlook.
The changes are ok and spaces are there before operators.
Executed and checked with checkpatch as well. There are some other issues
flagged with
--strict option, will be fixing those as part of v6.
Regards,
Uma Shankar
>> + 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) |
>Again, space before the '|' sign, please excuse me if this is false alarm ...
Same here.
>> + 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) {
>Shouldn't this variable go to line above ?
>> + uint16_t 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)
>> + connector->hdr_metadata.max_cll = db[4]; }
>This '}' certainly should not be here.
Same here as well.
>> +
>> static void
>> drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8
>> *db) { @@
>> -4443,6 +4493,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);
>- Shashank
>> }
>> }
>>
>> --
>> 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
