| Message ID | 1552276684-25358-3-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add HDR Metadata Parsing and handling in DRM layer | expand |
> -----Original Message----- > From: Shankar, Uma > Sent: Monday, March 11, 2019 9:28 AM > To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville > <ville.syrjala@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>; > emil.l.velikov@gmail.com; brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar, > Uma <uma.shankar@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.shankar@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
>-----Original Message----- >From: Sharma, Shashank >Sent: Friday, March 15, 2019 12:56 PM >To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri- >devel@lists.freedesktop.org >Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville ><ville.syrjala@intel.com>; emil.l.velikov@gmail.com; brian.starkey@arm.com; >Liviu.Dudau@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-devel@lists.freedesktop.org >> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville >> <ville.syrjala@intel.com>; Sharma, Shashank >> <shashank.sharma@intel.com>; emil.l.velikov@gmail.com; >> brian.starkey@arm.com; Liviu.Dudau@arm.com; Shankar, Uma >> <uma.shankar@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.shankar@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
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; +} + +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) + 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) | + 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) +{ + 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]; +} + 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); } }
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.shankar@intel.com> --- drivers/gpu/drm/drm_edid.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)