>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >Sharma, Shashank >Sent: Thursday, December 20, 2018 11:47 PM >To: Shankar, Uma <uma.shankar@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; >dri-devel@xxxxxxxxxxxxxxxxxxxxx >Cc: Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Lankhorst, Maarten ><maarten.lankhorst@xxxxxxxxx> >Subject: Re: [v2 03/14] drm: Parse HDR metadata info from EDID > >Regards > >Shashank > > >On 12/12/2018 2:08 AM, Uma Shankar wrote: >> 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. >> >> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> >> --- >> drivers/gpu/drm/drm_edid.c | 45 >+++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 45 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index 106fd38..d12b74e 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -3818,6 +3818,49 @@ static void fixup_detailed_cea_mode_clock(struct >drm_display_mode *mode) >> mode->clock = clock; >> } >I guess the previous patch (or a art of previous patch) should be merged in this >patch. Sure, will update this. >> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) { >> + if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) >> + return false; >> + >> + if (db[1] != HDR_STATIC_METADATA_BLOCK) >> + return false; >> + >> + return true; >> +} >> + >> +static uint16_t eotf_supported(const u8 *edid_ext) >Why uint16 ? why not uint8_t ? this is clearly one byte. Ok, will change this. >> +{ >> + >> + return edid_ext[2] & >> + (BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) | >Should we bother about SDR bit at all ? Why not just check HDR bits ? As per spec, all of these are EOTF types. So lets update whatever is supported. User should put correct EOTF for HDR from this list. >> + BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) | >> + BIT(HDMI_EOTF_SMPTE_ST2084)); >> + >> +} >> + >> +static uint16_t hdr_metadata_type(const u8 *edid_ext) { >Same uint8_t stuff Ok. Will update. >> + >> + return edid_ext[3] & >> + BIT(HDMI_STATIC_METADATA_TYPE1); >> +} >> + >> +static void >> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const >> +u8 *db) { >> + uint16_t len; >> + >> + len = cea_db_payload_len(db); >Length is incorrect here for extended tag blocks, as this function doesn't account >the extended tag byte's size. So the payload length is looking for is len - 1. May be >a good time to add >cea_extended_db_payload_len() ? As per spec, the definition says length after this byte so it factors the extended tag byte well, IIUC. Please correct me if my understanding is wrong. Regards, Uma Shankar >> + 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]; } >> + >> static void >> drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) >> { >> @@ -4468,6 +4511,8 @@ static void drm_parse_cea_ext(struct drm_connector >*connector, >> drm_parse_hdmi_forum_vsdb(connector, db); >> if (cea_db_is_y420cmdb(db)) >> drm_parse_y420cmdb_bitmap(connector, db); >> + if (cea_db_is_hdmi_hdr_metadata_block(db)) >> + drm_parse_hdr_metadata_block(connector, db); >> } >> } >> >- Shashank > >_______________________________________________ >dri-devel mailing list >dri-devel@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
