| Message ID | 1554828284-17776-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 |
On 4/9/19 12:44 PM, 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. > > v3: No Change > > v4: Addressed Shashank's review comments > > v5: Addressed Shashank's comment and added his RB. > > v6: Addressed Jonas Karlman review comments. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > Reviewed-by: Shashank Sharma <shashank.sharma@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 2c22ea4..1fc371b 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2830,6 +2830,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 > @@ -3577,6 +3578,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]; > @@ -3812,6 +3819,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; Shouldn't this just be cea_db_extended_tag(db) != HDR_STATIC_METADATA_BLOCK? Also, the other functions all check that we're given a valid here here with: if (!cea_db_payload_len_ext(db)) return false; Is there any reason this isn't done here? > + > + 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) > +{ > + u16 len; > + > + len = cea_db_payload_len_ext(db); > + connector->hdr_sink_metadata.hdmi_type1.eotf = eotf_supported(db); > + connector->hdr_sink_metadata.hdmi_type1.metadata_type = > + hdr_metadata_type(db); While metadata_type is assigned here like it should be, it isn't assigned to the outer metadata_type in the hdr_sink_metadata. I also can't see anything in the other patches that assigns this anywhere. Shouldn't it also be set here? > + > + if (len >= 4) > + connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; > + if (len >= 5) > + connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; > + if (len >= 6) > + connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; > +} > + > static void > drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) > { > @@ -4439,6 +4489,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); > } > } > > Nicholas Kazlauskas
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2c22ea4..1fc371b 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2830,6 +2830,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 @@ -3577,6 +3578,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]; @@ -3812,6 +3819,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) +{ + u16 len; + + len = cea_db_payload_len_ext(db); + connector->hdr_sink_metadata.hdmi_type1.eotf = eotf_supported(db); + connector->hdr_sink_metadata.hdmi_type1.metadata_type = + hdr_metadata_type(db); + + if (len >= 4) + connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; + if (len >= 5) + connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; + if (len >= 6) + connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; +} + static void drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) { @@ -4439,6 +4489,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); } }