| Message ID | 1552276684-25358-4-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 03/13] drm: Parse Colorimetry data block from EDID > > CEA 861.3 spec adds colorimetry data block for HDMI. > Parsing the block to get the colorimetry data from panel. > > v2: Rebase > > v3: No Change > > v4: Addressed Shashank's review comments. Updated colorimetry field to 16 bit as > DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/drm_edid.c | 26 ++++++++++++++++++++++++++ > include/drm/drm_connector.h | 3 +++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index > c5a81b8..0470845 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 COLORIMETRY_DATA_BLOCK 0x5 > #define HDR_STATIC_METADATA_BLOCK 0x6 > #define USE_EXTENDED_TAG 0x07 > #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@ static > void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) > mode->clock = clock; > } > > +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) { Same comment for the opening brace above ? > + if (cea_db_tag(db) != USE_EXTENDED_TAG) > + return false; > + > + if (db[1] != COLORIMETRY_DATA_BLOCK) Space before '!' ? > + return false; > + > + return true; > +} > + > +static void > +drm_parse_colorimetry_data_block(struct drm_connector *connector, const > +u8 *db) { > + struct drm_hdmi_info *info = &connector->display_info.hdmi; > + uint16_t len; > + > + len = cea_db_payload_len_ext(db); > + /* As per CEA 861-G spec */ > + info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; } This means we are setting the 15th bit for DCI-P3 then there is a gap of 7 bits, and then there are other bits. Do we want to make DCI-P3 as bit 8 ? Anyways this can be differed for a discussion later. > + > + > static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) { > if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@ static > void drm_parse_cea_ext(struct drm_connector *connector, > drm_parse_vcdb(connector, db); > if (cea_db_is_hdmi_hdr_metadata_block(db)) > drm_parse_hdr_metadata_block(connector, db); > + if (cea_db_is_hdmi_colorimetry_data_block(db)) > + drm_parse_colorimetry_data_block(connector, db); > } > } > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index > 29388bd..94f926e 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -206,6 +206,9 @@ struct drm_hdmi_info { > > /** @y420_dc_modes: bitmap of deep color support index */ > u8 y420_dc_modes; > + > + /* @colorimetry: bitmap of supported colorimetry modes */ > + u16 colorimetry; - Shashank > }; > > /** > -- > 1.9.1
>-----Original Message----- >From: Sharma, Shashank >Sent: Friday, March 15, 2019 1:01 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 03/13] drm: Parse Colorimetry data block 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 03/13] drm: Parse Colorimetry data block from EDID >> >> CEA 861.3 spec adds colorimetry data block for HDMI. >> Parsing the block to get the colorimetry data from panel. >> >> v2: Rebase >> >> v3: No Change >> >> v4: Addressed Shashank's review comments. Updated colorimetry field to >> 16 bit as >> DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/drm_edid.c | 26 ++++++++++++++++++++++++++ >> include/drm/drm_connector.h | 3 +++ >> 2 files changed, 29 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index >> c5a81b8..0470845 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 COLORIMETRY_DATA_BLOCK 0x5 >> #define HDR_STATIC_METADATA_BLOCK 0x6 >> #define USE_EXTENDED_TAG 0x07 >> #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@ static >> void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) >> mode->clock = clock; >> } >> >> +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) { >Same comment for the opening brace above ? Again this looks like mail client issue. >> + if (cea_db_tag(db) != USE_EXTENDED_TAG) >> + return false; >> + >> + if (db[1] != COLORIMETRY_DATA_BLOCK) >Space before '!' ? Same here. >> + return false; >> + >> + return true; >> +} >> + >> +static void >> +drm_parse_colorimetry_data_block(struct drm_connector *connector, >> +const >> +u8 *db) { >> + struct drm_hdmi_info *info = &connector->display_info.hdmi; >> + uint16_t len; >> + >> + len = cea_db_payload_len_ext(db); >> + /* As per CEA 861-G spec */ >> + info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; } >This means we are setting the 15th bit for DCI-P3 then there is a gap of 7 bits, and >then there are other bits. Do we want to make DCI-P3 as bit 8 ? Anyways this can be >differed for a discussion later. This is at 8th position only. We are shifting the db[3] which is 8 bit by 1 and keeping the 7th bit (DCI-P3) at the 8th position (if we count from 0). Regards, Uma Shankar >> + >> + >> static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) { >> if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@ static >> void drm_parse_cea_ext(struct drm_connector *connector, >> drm_parse_vcdb(connector, db); >> if (cea_db_is_hdmi_hdr_metadata_block(db)) >> drm_parse_hdr_metadata_block(connector, db); >> + if (cea_db_is_hdmi_colorimetry_data_block(db)) >> + drm_parse_colorimetry_data_block(connector, db); >> } >> } >> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >> index 29388bd..94f926e 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -206,6 +206,9 @@ struct drm_hdmi_info { >> >> /** @y420_dc_modes: bitmap of deep color support index */ >> u8 y420_dc_modes; >> + >> + /* @colorimetry: bitmap of supported colorimetry modes */ >> + u16 colorimetry; >- Shashank >> }; >> >> /** >> -- >> 1.9.1
On 3/20/2019 12:47 PM, Shankar, Uma wrote: > >> -----Original Message----- >> From: Sharma, Shashank >> Sent: Friday, March 15, 2019 1:01 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 03/13] drm: Parse Colorimetry data block 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 03/13] drm: Parse Colorimetry data block from EDID >>> >>> CEA 861.3 spec adds colorimetry data block for HDMI. >>> Parsing the block to get the colorimetry data from panel. >>> >>> v2: Rebase >>> >>> v3: No Change >>> >>> v4: Addressed Shashank's review comments. Updated colorimetry field to >>> 16 bit as >>> DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank. >>> >>> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >>> --- >>> drivers/gpu/drm/drm_edid.c | 26 ++++++++++++++++++++++++++ >>> include/drm/drm_connector.h | 3 +++ >>> 2 files changed, 29 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>> index >>> c5a81b8..0470845 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 COLORIMETRY_DATA_BLOCK 0x5 >>> #define HDR_STATIC_METADATA_BLOCK 0x6 >>> #define USE_EXTENDED_TAG 0x07 >>> #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@ static >>> void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) >>> mode->clock = clock; >>> } >>> >>> +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) { >> Same comment for the opening brace above ? > Again this looks like mail client issue. > >>> + if (cea_db_tag(db) != USE_EXTENDED_TAG) >>> + return false; >>> + >>> + if (db[1] != COLORIMETRY_DATA_BLOCK) >> Space before '!' ? > Same here. > >>> + return false; >>> + >>> + return true; >>> +} >>> + >>> +static void >>> +drm_parse_colorimetry_data_block(struct drm_connector *connector, >>> +const >>> +u8 *db) { >>> + struct drm_hdmi_info *info = &connector->display_info.hdmi; >>> + uint16_t len; >>> + >>> + len = cea_db_payload_len_ext(db); >>> + /* As per CEA 861-G spec */ >>> + info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; } >> This means we are setting the 15th bit for DCI-P3 then there is a gap of 7 bits, and >> then there are other bits. Do we want to make DCI-P3 as bit 8 ? Anyways this can be >> differed for a discussion later. > This is at 8th position only. We are shifting the db[3] which is 8 bit by 1 and keeping the 7th bit (DCI-P3) > at the 8th position (if we count from 0). > > Regards, > Uma Shankar My bad, its 8th bit only. If those above alignment problems are checked already, feel free to use Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> - Shashank >>> + >>> + >>> static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) { >>> if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@ static >>> void drm_parse_cea_ext(struct drm_connector *connector, >>> drm_parse_vcdb(connector, db); >>> if (cea_db_is_hdmi_hdr_metadata_block(db)) >>> drm_parse_hdr_metadata_block(connector, db); >>> + if (cea_db_is_hdmi_colorimetry_data_block(db)) >>> + drm_parse_colorimetry_data_block(connector, db); >>> } >>> } >>> >>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >>> index 29388bd..94f926e 100644 >>> --- a/include/drm/drm_connector.h >>> +++ b/include/drm/drm_connector.h >>> @@ -206,6 +206,9 @@ struct drm_hdmi_info { >>> >>> /** @y420_dc_modes: bitmap of deep color support index */ >>> u8 y420_dc_modes; >>> + >>> + /* @colorimetry: bitmap of supported colorimetry modes */ >>> + u16 colorimetry; >> - Shashank >>> }; >>> >>> /** >>> -- >>> 1.9.1
>On 3/20/2019 12:47 PM, Shankar, Uma wrote: >> >>> -----Original Message----- >>> From: Sharma, Shashank >>> Sent: Friday, March 15, 2019 1:01 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 03/13] drm: Parse Colorimetry data block 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 03/13] drm: Parse Colorimetry data block from EDID >>>> >>>> CEA 861.3 spec adds colorimetry data block for HDMI. >>>> Parsing the block to get the colorimetry data from panel. >>>> >>>> v2: Rebase >>>> >>>> v3: No Change >>>> >>>> v4: Addressed Shashank's review comments. Updated colorimetry field >>>> to >>>> 16 bit as >>>> DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank. >>>> >>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_edid.c | 26 ++++++++++++++++++++++++++ >>>> include/drm/drm_connector.h | 3 +++ >>>> 2 files changed, 29 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>>> index >>>> c5a81b8..0470845 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 COLORIMETRY_DATA_BLOCK 0x5 >>>> #define HDR_STATIC_METADATA_BLOCK 0x6 >>>> #define USE_EXTENDED_TAG 0x07 >>>> #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@ >>>> static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) >>>> mode->clock = clock; >>>> } >>>> >>>> +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) { >>> Same comment for the opening brace above ? >> Again this looks like mail client issue. >> >>>> + if (cea_db_tag(db) != USE_EXTENDED_TAG) >>>> + return false; >>>> + >>>> + if (db[1] != COLORIMETRY_DATA_BLOCK) >>> Space before '!' ? >> Same here. >> >>>> + return false; >>>> + >>>> + return true; >>>> +} >>>> + >>>> +static void >>>> +drm_parse_colorimetry_data_block(struct drm_connector *connector, >>>> +const >>>> +u8 *db) { >>>> + struct drm_hdmi_info *info = &connector->display_info.hdmi; >>>> + uint16_t len; >>>> + >>>> + len = cea_db_payload_len_ext(db); >>>> + /* As per CEA 861-G spec */ >>>> + info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; } >>> This means we are setting the 15th bit for DCI-P3 then there is a gap >>> of 7 bits, and then there are other bits. Do we want to make DCI-P3 >>> as bit 8 ? Anyways this can be differed for a discussion later. >> This is at 8th position only. We are shifting the db[3] which is 8 >> bit by 1 and keeping the 7th bit (DCI-P3) at the 8th position (if we count from 0). >> >> Regards, >> Uma Shankar > >My bad, its 8th bit only. > >If those above alignment problems are checked already, feel free to use > >Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> No Issues, Thanks Shashank. Regards, Uma Shankar >- Shashank > >>>> + >>>> + >>>> static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) { >>>> if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@ >>>> static void drm_parse_cea_ext(struct drm_connector *connector, >>>> drm_parse_vcdb(connector, db); >>>> if (cea_db_is_hdmi_hdr_metadata_block(db)) >>>> drm_parse_hdr_metadata_block(connector, db); >>>> + if (cea_db_is_hdmi_colorimetry_data_block(db)) >>>> + drm_parse_colorimetry_data_block(connector, db); >>>> } >>>> } >>>> >>>> diff --git a/include/drm/drm_connector.h >>>> b/include/drm/drm_connector.h index 29388bd..94f926e 100644 >>>> --- a/include/drm/drm_connector.h >>>> +++ b/include/drm/drm_connector.h >>>> @@ -206,6 +206,9 @@ struct drm_hdmi_info { >>>> >>>> /** @y420_dc_modes: bitmap of deep color support index */ >>>> u8 y420_dc_modes; >>>> + >>>> + /* @colorimetry: bitmap of supported colorimetry modes */ >>>> + u16 colorimetry; >>> - Shashank >>>> }; >>>> >>>> /** >>>> -- >>>> 1.9.1
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c5a81b8..0470845 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 COLORIMETRY_DATA_BLOCK 0x5 #define HDR_STATIC_METADATA_BLOCK 0x6 #define USE_EXTENDED_TAG 0x07 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 @@ -3823,6 +3824,29 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) mode->clock = clock; } +static bool cea_db_is_hdmi_colorimetry_data_block(const u8 *db) +{ + if (cea_db_tag(db) != USE_EXTENDED_TAG) + return false; + + if (db[1] != COLORIMETRY_DATA_BLOCK) + return false; + + return true; +} + +static void +drm_parse_colorimetry_data_block(struct drm_connector *connector, const u8 *db) +{ + struct drm_hdmi_info *info = &connector->display_info.hdmi; + uint16_t len; + + len = cea_db_payload_len_ext(db); + /* As per CEA 861-G spec */ + info->colorimetry = ((db[3] & (0x1 << 7)) << 1) | db[2]; +} + + static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) { if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4495,6 +4519,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, drm_parse_vcdb(connector, db); if (cea_db_is_hdmi_hdr_metadata_block(db)) drm_parse_hdr_metadata_block(connector, db); + if (cea_db_is_hdmi_colorimetry_data_block(db)) + drm_parse_colorimetry_data_block(connector, db); } } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 29388bd..94f926e 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -206,6 +206,9 @@ struct drm_hdmi_info { /** @y420_dc_modes: bitmap of deep color support index */ u8 y420_dc_modes; + + /* @colorimetry: bitmap of supported colorimetry modes */ + u16 colorimetry; }; /**
CEA 861.3 spec adds colorimetry data block for HDMI. Parsing the block to get the colorimetry data from panel. v2: Rebase v3: No Change v4: Addressed Shashank's review comments. Updated colorimetry field to 16 bit as DCI-P3 got added in CEA 861-G spec, as pointed out by Shashank. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/drm_edid.c | 26 ++++++++++++++++++++++++++ include/drm/drm_connector.h | 3 +++ 2 files changed, 29 insertions(+)