RE: [v5 03/13] drm: Parse Colorimetry data block from EDID
>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 ; >>> intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org >>> Cc: Lankhorst, Maarten ; Syrjala, Ville >>> ; emil.l.veli...@gmail.com; >>> brian.star...@arm.com; liviu.du...@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-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org >>>> Cc: Lankhorst, Maarten ; Syrjala, Ville >>>> ; Sharma, Shashank >>>> ; emil.l.veli...@gmail.com; >>>> brian.star...@arm.com; liviu.du...@arm.com; Shankar, Uma >>>> >>>> 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 >>>> --- >>>> 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_BLOCK0x03 >>>> #define SPEAKER_BLOCK0x04 >>>> +#define COLORIMETRY_DATA_BLOCK0x5 >>>> #define HDR_STATIC_METADATA_BLOCK0x6 >>>> #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 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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v5 03/13] drm: Parse Colorimetry data block from EDID
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 ; intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org Cc: Lankhorst, Maarten ; Syrjala, Ville ; emil.l.veli...@gmail.com; brian.star...@arm.com; liviu.du...@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-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Lankhorst, Maarten ; Syrjala, Ville ; Sharma, Shashank ; emil.l.veli...@gmail.com; brian.star...@arm.com; liviu.du...@arm.com; Shankar, Uma 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 --- 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_BLOCK0x03 #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 + + 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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [v5 03/13] drm: Parse Colorimetry data block from EDID
>-Original Message- >From: Sharma, Shashank >Sent: Friday, March 15, 2019 1:01 PM >To: Shankar, Uma ; intel-...@lists.freedesktop.org; dri- >de...@lists.freedesktop.org >Cc: Lankhorst, Maarten ; Syrjala, Ville >; emil.l.veli...@gmail.com; brian.star...@arm.com; >liviu.du...@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-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org >> Cc: Lankhorst, Maarten ; Syrjala, Ville >> ; Sharma, Shashank >> ; emil.l.veli...@gmail.com; >> brian.star...@arm.com; liviu.du...@arm.com; Shankar, Uma >> >> 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 >> --- >> 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_BLOCK0x03 >> #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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: Lankhorst, Maarten ; Syrjala, Ville > ; Sharma, Shashank ; > emil.l.veli...@gmail.com; brian.star...@arm.com; liviu.du...@arm.com; Shankar, > Uma > 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 > --- > 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_BLOCK0x03 > #define SPEAKER_BLOCK0x04 > +#define COLORIMETRY_DATA_BLOCK 0x5 > #define HDR_STATIC_METADATA_BLOCK0x6 > #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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel