> -----Original Message-----
> From: Shankar, Uma
> Sent: Monday, March 11, 2019 9:28 AM
> To: intel-...@lists.freedesktop.org; dri-devel@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 '-' ? 
> +}
> +
> +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  ? 
> +             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 ... 
> +              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.  
> +
>  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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to