| Message ID | 1544560702-16447-5-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 |
Regards Shashank On 12/12/2018 2:08 AM, Uma Shankar wrote: > CEA 861.3 spec adds colorimetry data block for HDMI. > Parsing the block to get the colorimetry data from > panel. > > v2: Rebase > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++++++++ > include/drm/drm_connector.h | 2 ++ > 2 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index d12b74e..344d8c1 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -3818,6 +3818,28 @@ 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) != DATA_BLOCK_EXTENDED_TAG) > + return false; > + > + if (db[1] != COLORIMETRY_DATA_BLOCK) > + Again, the COLORIMETRY_DATA_BLOCK definition should be added into this patch. > 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(db); Again, the return value of this function doesn't account for extended db tag byte, so what we are looking for is len -1 > + info->colorimetry = db[2]; colorimetry block is actually db[2] and db[3].bit7 (which represents DCI-P3 space), so we probably want to make info->colorimetryuint16_t > +} > + > + > static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) > { > if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) > @@ -4513,6 +4535,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > drm_parse_y420cmdb_bitmap(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 2ee45dc..90ce364 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -206,6 +206,8 @@ struct drm_hdmi_info { > > /** @y420_dc_modes: bitmap of deep color support index */ > u8 y420_dc_modes; > + Please add a one line description about this variable. > + u8 colorimetry; > }; > > /** Shashank
>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >Sharma, Shashank >Sent: Thursday, December 20, 2018 11:54 PM >To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; >dri-devel@lists.freedesktop.org >Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten ><maarten.lankhorst@intel.com> >Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID > >Regards > >Shashank > > >On 12/12/2018 2:08 AM, Uma Shankar wrote: >> CEA 861.3 spec adds colorimetry data block for HDMI. >> Parsing the block to get the colorimetry data from panel. >> >> v2: Rebase >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++++++++ >> include/drm/drm_connector.h | 2 ++ >> 2 files changed, 26 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index d12b74e..344d8c1 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -3818,6 +3818,28 @@ 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) != DATA_BLOCK_EXTENDED_TAG) >> + return false; >> + >> + if (db[1] != COLORIMETRY_DATA_BLOCK) >> + >Again, the COLORIMETRY_DATA_BLOCK definition should be added into this >patch. Ok, will do that. >> 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(db); >Again, the return value of this function doesn't account for extended db tag byte, >so what we are looking for is len -1 Will update this. >> + info->colorimetry = db[2]; >colorimetry block is actually db[2] and db[3].bit7 (which represents >DCI-P3 space), so we probably want to make info->colorimetryuint16_t 3 BT2020RGB BT2020YCC BT2020cYCC AdobeRGB AdobeYCC601 sYCC601 xvYCC709 xvYCC601 4 F47=0 F46=0 F45=0 F44=0 MD3 MD2 MD1 MD0 This is how db[2] and db[3] is described in spec. There is not much clarity on F47. Not sure if they are for DCI-P3, can you clarify once. Regards, Uma Shankar >> +} >> + >> + >> static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) >> { >> if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) @@ -4513,6 >+4535,8 >> @@ static void drm_parse_cea_ext(struct drm_connector *connector, >> drm_parse_y420cmdb_bitmap(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 2ee45dc..90ce364 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -206,6 +206,8 @@ struct drm_hdmi_info { >> >> /** @y420_dc_modes: bitmap of deep color support index */ >> u8 y420_dc_modes; >> + >Please add a one line description about this variable. Sure will update this. >> + u8 colorimetry; >> }; >> >> /** >Shashank >_______________________________________________ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 1/8/2019 12:10 PM, Shankar, Uma wrote: > >> -----Original Message----- >> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >> Sharma, Shashank >> Sent: Thursday, December 20, 2018 11:54 PM >> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; >> dri-devel@lists.freedesktop.org >> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten >> <maarten.lankhorst@intel.com> >> Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID >> >> Regards >> >> Shashank >> >> >> On 12/12/2018 2:08 AM, Uma Shankar wrote: >>> CEA 861.3 spec adds colorimetry data block for HDMI. >>> Parsing the block to get the colorimetry data from panel. >>> >>> v2: Rebase >>> >>> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >>> --- >>> drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++++++++ >>> include/drm/drm_connector.h | 2 ++ >>> 2 files changed, 26 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>> index d12b74e..344d8c1 100644 >>> --- a/drivers/gpu/drm/drm_edid.c >>> +++ b/drivers/gpu/drm/drm_edid.c >>> @@ -3818,6 +3818,28 @@ 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) != DATA_BLOCK_EXTENDED_TAG) >>> + return false; >>> + >>> + if (db[1] != COLORIMETRY_DATA_BLOCK) >>> + >> Again, the COLORIMETRY_DATA_BLOCK definition should be added into this >> patch. > Ok, will do that. > >>> 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(db); >> Again, the return value of this function doesn't account for extended db tag byte, >> so what we are looking for is len -1 > Will update this. > >>> + info->colorimetry = db[2]; >> colorimetry block is actually db[2] and db[3].bit7 (which represents >> DCI-P3 space), so we probably want to make info->colorimetryuint16_t > 3 BT2020RGB BT2020YCC BT2020cYCC AdobeRGB AdobeYCC601 sYCC601 xvYCC709 xvYCC601 > 4 F47=0 F46=0 F45=0 F44=0 MD3 MD2 MD1 MD0 > > This is how db[2] and db[3] is described in spec. There is not much clarity on F47. Not sure if they > are for DCI-P3, can you clarify once. Ah, check CEA-861-G table 70, there there is no F47, but I could see DCI-P3 in that place, which spec is that you are following right now ? - Shashank > Regards, > Uma Shankar > >>> +} >>> + >>> + >>> static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) >>> { >>> if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) @@ -4513,6 >> +4535,8 >>> @@ static void drm_parse_cea_ext(struct drm_connector *connector, >>> drm_parse_y420cmdb_bitmap(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 2ee45dc..90ce364 100644 >>> --- a/include/drm/drm_connector.h >>> +++ b/include/drm/drm_connector.h >>> @@ -206,6 +206,8 @@ struct drm_hdmi_info { >>> >>> /** @y420_dc_modes: bitmap of deep color support index */ >>> u8 y420_dc_modes; >>> + >> Please add a one line description about this variable. > Sure will update this. > >>> + u8 colorimetry; >>> }; >>> >>> /** >> Shashank >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >Sharma, Shashank >Sent: Tuesday, January 8, 2019 12:27 PM >To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; >dri-devel@lists.freedesktop.org >Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten ><maarten.lankhorst@intel.com> >Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID > > > >On 1/8/2019 12:10 PM, Shankar, Uma wrote: >> >>> -----Original Message----- >>> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On >>> Behalf Of Sharma, Shashank >>> Sent: Thursday, December 20, 2018 11:54 PM >>> To: Shankar, Uma <uma.shankar@intel.com>; >>> intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org >>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten >>> <maarten.lankhorst@intel.com> >>> Subject: Re: [v2 04/14] drm: Parse Colorimetry data block from EDID >>> >>> Regards >>> >>> Shashank >>> >>> >>> On 12/12/2018 2:08 AM, Uma Shankar wrote: >>>> CEA 861.3 spec adds colorimetry data block for HDMI. >>>> Parsing the block to get the colorimetry data from panel. >>>> >>>> v2: Rebase >>>> >>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++++++++ >>>> include/drm/drm_connector.h | 2 ++ >>>> 2 files changed, 26 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>>> index d12b74e..344d8c1 100644 >>>> --- a/drivers/gpu/drm/drm_edid.c >>>> +++ b/drivers/gpu/drm/drm_edid.c >>>> @@ -3818,6 +3818,28 @@ 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) != DATA_BLOCK_EXTENDED_TAG) >>>> + return false; >>>> + >>>> + if (db[1] != COLORIMETRY_DATA_BLOCK) >>>> + >>> Again, the COLORIMETRY_DATA_BLOCK definition should be added into >>> this patch. >> Ok, will do that. >> >>>> 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(db); >>> Again, the return value of this function doesn't account for extended >>> db tag byte, so what we are looking for is len -1 >> Will update this. >> >>>> + info->colorimetry = db[2]; >>> colorimetry block is actually db[2] and db[3].bit7 (which represents >>> DCI-P3 space), so we probably want to make info->colorimetryuint16_t >> 3 BT2020RGB BT2020YCC BT2020cYCC AdobeRGB AdobeYCC601 sYCC601 >xvYCC709 >> xvYCC601 >> 4 F47=0 F46=0 F45=0 F44=0 MD3 MD2 MD1 MD0 >> >> This is how db[2] and db[3] is described in spec. There is not much >> clarity on F47. Not sure if they are for DCI-P3, can you clarify once. >Ah, check CEA-861-G table 70, there there is no F47, but I could see >DCI-P3 in that place, which spec is that you are following right now ? I am looking to 861-F. So seems like its updated in 861-G ? Regards, Uma Shankar >- Shashank >> Regards, >> Uma Shankar >> >>>> +} >>>> + >>>> + >>>> static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) >>>> { >>>> if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) @@ -4513,6 >>> +4535,8 >>>> @@ static void drm_parse_cea_ext(struct drm_connector *connector, >>>> drm_parse_y420cmdb_bitmap(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 2ee45dc..90ce364 100644 >>>> --- a/include/drm/drm_connector.h >>>> +++ b/include/drm/drm_connector.h >>>> @@ -206,6 +206,8 @@ struct drm_hdmi_info { >>>> >>>> /** @y420_dc_modes: bitmap of deep color support index */ >>>> u8 y420_dc_modes; >>>> + >>> Please add a one line description about this variable. >> Sure will update this. >> >>>> + u8 colorimetry; >>>> }; >>>> >>>> /** >>> Shashank >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >_______________________________________________ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d12b74e..344d8c1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3818,6 +3818,28 @@ 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) != DATA_BLOCK_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(db); + info->colorimetry = db[2]; +} + + static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) { if (cea_db_tag(db) != DATA_BLOCK_EXTENDED_TAG) @@ -4513,6 +4535,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, drm_parse_y420cmdb_bitmap(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 2ee45dc..90ce364 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -206,6 +206,8 @@ struct drm_hdmi_info { /** @y420_dc_modes: bitmap of deep color support index */ u8 y420_dc_modes; + + u8 colorimetry; }; /**
CEA 861.3 spec adds colorimetry data block for HDMI. Parsing the block to get the colorimetry data from panel. v2: Rebase Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++++++++ include/drm/drm_connector.h | 2 ++ 2 files changed, 26 insertions(+)