| Message ID | 1498041253-16426-5-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 21, 2017 at 04:04:02PM +0530, Shashank Sharma wrote: > HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. > CEA-861-F adds two new blocks in EDID's CEA extension blocks, > to provide information about sink's YCBCR420 output capabilities. > > These blocks are: > > - YCBCR420vdb(YCBCR 420 video data block): > This block contains VICs of video modes, which can be sopported only > in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal > SVD block, valid for YCBCR420 modes only. > > - YCBCR420cmdb(YCBCR 420 capability map data block): > This block gives information about video modes which can support > YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This > block contains a bitmap index of normal svd videomodes, which can > support YCBCR420 output too. > So if bit 0 from first vcb byte is set, first video mode in the svd > list can support YCBCR420 output too. Bit 1 means second video mode > from svd list can support YCBCR420 output too, and so on. > > This patch adds two bitmaps in display's hdmi_info structure, one each > for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch > adds: > - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes > an entry in the vdb_bitmap per vic. > - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Jose Abreu <joabreu@synopsys.com> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > > V2: Addressed > Review comments from Emil: > - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. > - Use the suggested method for updating dbmap. > - Add documentation for YCBCR420_vcb_map to fix kbuild warning. > > Review comments from Ville: > - Do not expose the YCBCR420 flags in uabi layer, keep it internal. > - Save a map of YCBCR420 modes for future reference. > - Check db length before trying to parse extended tag. > - Add a warning if there are > 64 modes in capability map block. > - Use y420cmdb in function names and macros while dealing with vcb > to be aligned with spec. > - Move the display information parsing block ahead of mode parsing > blocks. > > V3: Addressed design/review comments from Ville > - Do not add flags in video modes, else we have to expose them to user > - There should not be a UABI change, and kernel should detect the > choice of the output based on type of mode, and the bitmaps. > - Use standard bitops from kernel bitmap header, instead of calculating > bit positions manually. > > V4: Addressed review comments from Ville: > - s/ycbcr_420_vdb/y420vdb > - s/ycbcr_420_vcb/y420cmdb > - Be less verbose on description of do_y420vdb_modes > - Move newmode variable in the loop scope. > - Use svd_to_vic() to get a VIC, instead of 0x7f > - Remove bitmap description for CMDB modes & VDB modes > - Dont add connector->ycbcr_420_allowed check for cmdb modes > - Remove 'len' variable, in is_y420cmdb function, which is used > only once > - Add length check in is_y420vdb function > - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap > - Do not add print about YCBCR 420 modes > - Fix indentation in few places > - Move ycbcr420_dc_modes in next patch, where its used > - Add a separate patch for movement of drm_add_display_info() > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/drm_edid.c | 157 +++++++++++++++++++++++++++++++++++++++++++- > include/drm/drm_connector.h | 20 ++++++ > 2 files changed, 174 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index e2d1b30..8c7e73b 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2781,7 +2781,9 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, > #define VIDEO_BLOCK 0x02 > #define VENDOR_BLOCK 0x03 > #define SPEAKER_BLOCK 0x04 > -#define VIDEO_CAPABILITY_BLOCK 0x07 > +#define VIDEO_CAPABILITY_BLOCK 0x07 > +#define VIDEO_DATA_BLOCK_420 0x0E > +#define VIDEO_CAP_BLOCK_Y420CMDB 0x0F > #define EDID_BASIC_AUDIO (1 << 6) > #define EDID_CEA_YCRCB444 (1 << 5) > #define EDID_CEA_YCRCB422 (1 << 4) > @@ -3153,15 +3155,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, > return newmode; > } > > +/* > + * do_y420vdb_modes - Parse YCBCR 420 only modes > + * @connector: connector corresponding to the HDMI sink > + * @svds: start of the data block of CEA YCBCR 420 VDB > + * @len: length of the CEA YCBCR 420 VDB > + * > + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) > + * which contains modes which can be supported in YCBCR 420 > + * output format only. > + */ > +static int do_y420vdb_modes(struct drm_connector *connector, > + const u8 *svds, u8 svds_len) > +{ > + int modes = 0, i; > + struct drm_device *dev = connector->dev; > + struct drm_display_info *info = &connector->display_info; > + struct drm_hdmi_info *hdmi = &info->hdmi; > + > + for (i = 0; i < svds_len; i++) { > + u8 vic = svd_to_vic(svds[i]); > + struct drm_display_mode *newmode; > + > + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); > + if (!newmode) > + break; > + bitmap_set(hdmi->y420_vdb_modes, vic, 1); > + drm_mode_probed_add(connector, newmode); > + modes++; > + } > + > + if (modes > 0) > + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > + return modes; > +} > + > +/* > + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap > + * @connector: connector corresponding to the HDMI sink > + * @vic: CEA vic for the video mode to be added in the map > + * > + * Makes an entry for a videomode in the YCBCR 420 bitmap > + */ > +static void > +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) > +{ > + u8 vic = svd_to_vic(svd); > + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; > + > + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); > +} > + > static int > do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) > { > int i, modes = 0; > + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; > > for (i = 0; i < len; i++) { > struct drm_display_mode *mode; > mode = drm_display_mode_from_vic_index(connector, db, len, i); > if (mode) { > + /* > + * YCBCR420 capability block contains a bitmap which > + * gives the index of CEA modes from CEA VDB, which > + * can support YCBCR 420 sampling output also (apart > + * from RGB/YCBCR444 etc). > + * For example, if the bit 0 in bitmap is set, > + * first mode in VDB can support YCBCR420 output too. > + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. > + */ > + if (test_bit(i, &hdmi->y420_cmdb_map)) > + drm_add_cmdb_modes(connector, db[i]); > + > drm_mode_probed_add(connector, mode); > modes++; > } > @@ -3437,6 +3503,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, > } > > static int > +cea_db_extended_tag(const u8 *db) > +{ > + return db[1]; > +} > + > +static int > cea_db_payload_len(const u8 *db) > { > return db[0] & 0x1f; > @@ -3497,9 +3569,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) > return oui == HDMI_FORUM_IEEE_OUI; > } > > +static bool cea_db_is_y420cmdb(const u8 *db) > +{ > + > + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) Still would like to see that patch to clean up the current extended tag usage... > + return false; > + > + if (!cea_db_payload_len(db)) > + return false; > + > + if (cea_db_extended_tag(db) != VIDEO_CAP_BLOCK_Y420CMDB) > + return false; > + > + return true; > +} > + > +static bool cea_db_is_y420vdb(const u8 *db) > +{ > + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) > + return false; > + > + if (!cea_db_payload_len(db)) > + return false; > + > + if (cea_db_extended_tag(db) != VIDEO_DATA_BLOCK_420) > + return false; > + > + return true; > +} > + > #define for_each_cea_db(cea, i, start, end) \ > for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) > > +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, > + const u8 *db) > +{ > + struct drm_display_info *info = &connector->display_info; > + struct drm_hdmi_info *hdmi = &info->hdmi; > + u8 map_len = cea_db_payload_len(db) - 1; > + u8 count; > + u64 map = 0; > + > + if (map_len == 0) { > + /* All CEA modes support ycbcr420 sampling also.*/ > + hdmi->y420_cmdb_map = U64_MAX; > + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > + return; > + } > + > + /* > + * This map indicates which of the existing CEA block modes > + * from VDB can support YCBCR420 output too. So if bit=0 is > + * set, first mode from VDB can support YCBCR420 output too. > + * We will parse and keep this map, before parsing VDB itself > + * to avoid going through the same block again and again. > + * > + * Spec is not clear about max possible size of this block. > + * Clamping max bitmap block size at 8 bytes. Every byte can > + * address 8 CEA modes, in this way this map can address > + * 8*8 = first 64 SVDs. > + */ > + if (WARN_ON_ONCE(map_len > 8)) > + map_len = 8; > + > + for (count = 0; count < map_len; count++) > + map |= (u64)db[2 + count] << (8 * count); > + > + if (map) > + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > + > + hdmi->y420_cmdb_map = map; > +} > + > static int > add_cea_modes(struct drm_connector *connector, struct edid *edid) > { > @@ -3522,10 +3663,17 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) > video = db + 1; > video_len = dbl; > modes += do_cea_modes(connector, video, dbl); > - } > - else if (cea_db_is_hdmi_vsdb(db)) { > + } else if (cea_db_is_hdmi_vsdb(db)) { > hdmi = db; > hdmi_len = dbl; > + } else if (cea_db_is_y420vdb(db) && > + connector->ycbcr_420_allowed) { > + const u8 *vdb420 = &db[2]; > + > + /* Add 4:2:0(only) modes present in EDID */ > + modes += do_y420vdb_modes(connector, > + vdb420, > + dbl - 1); > } > } > } > @@ -4206,6 +4354,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > drm_parse_hdmi_vsdb_video(connector, db); > if (cea_db_is_hdmi_forum_vsdb(db)) > drm_parse_hdmi_forum_vsdb(connector, db); > + if (cea_db_is_y420cmdb(db)) > + drm_parse_y420cmdb_bitmap(connector, db); > } > } > > @@ -4460,6 +4610,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) > num_modes += add_cea_modes(connector, edid); > num_modes += add_alternate_cea_modes(connector, edid); > num_modes += add_displayid_detailed_modes(connector, edid); > + > if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) > num_modes += add_inferred_modes(connector, edid); > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 7493fd3..ce2212d 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -137,6 +137,25 @@ struct drm_scdc { > struct drm_hdmi_info { > /** @scdc: sink's scdc support and capabilities */ > struct drm_scdc scdc; > + > + /** > + * @y420_vdb_modes: bitmap of modes which can support ycbcr420 > + * output only (not normal RGB/YCBCR444/422 outputs). There are total > + * 107 VICs defined by CEA-861-F spec, so the size is 128 bits to map > + * upto 128 VICs; > + */ > + unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; > + > + /** > + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 > + * output also, along with normal HDMI outputs. There are total 107 > + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto > + * 128 VICs; > + */ > + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; > + > + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ > + unsigned long y420_cmdb_map; u64, otherwise it's busted on 32bit machines. Mostl this looks pretty good. I think the main problem is the lack of mode validation that would reject the 4:2:0 only modes if not supported by the driver/device. I guess that's in a later patch? If so we need to reorder the series a bit to avoid regeressions. > }; > > /** > @@ -200,6 +219,7 @@ struct drm_display_info { > #define DRM_COLOR_FORMAT_RGB444 (1<<0) > #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) > #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) > +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) > > /** > * @color_formats: HDMI Color formats, selects between RGB and YCrCb > -- > 2.7.4
Regards Shashank On 6/27/2017 5:22 PM, Ville Syrjälä wrote: > On Wed, Jun 21, 2017 at 04:04:02PM +0530, Shashank Sharma wrote: >> HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. >> CEA-861-F adds two new blocks in EDID's CEA extension blocks, >> to provide information about sink's YCBCR420 output capabilities. >> >> These blocks are: >> >> - YCBCR420vdb(YCBCR 420 video data block): >> This block contains VICs of video modes, which can be sopported only >> in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal >> SVD block, valid for YCBCR420 modes only. >> >> - YCBCR420cmdb(YCBCR 420 capability map data block): >> This block gives information about video modes which can support >> YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This >> block contains a bitmap index of normal svd videomodes, which can >> support YCBCR420 output too. >> So if bit 0 from first vcb byte is set, first video mode in the svd >> list can support YCBCR420 output too. Bit 1 means second video mode >> from svd list can support YCBCR420 output too, and so on. >> >> This patch adds two bitmaps in display's hdmi_info structure, one each >> for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch >> adds: >> - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes >> an entry in the vdb_bitmap per vic. >> - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Jose Abreu <joabreu@synopsys.com> >> Cc: Emil Velikov <emil.l.velikov@gmail.com> >> >> V2: Addressed >> Review comments from Emil: >> - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. >> - Use the suggested method for updating dbmap. >> - Add documentation for YCBCR420_vcb_map to fix kbuild warning. >> >> Review comments from Ville: >> - Do not expose the YCBCR420 flags in uabi layer, keep it internal. >> - Save a map of YCBCR420 modes for future reference. >> - Check db length before trying to parse extended tag. >> - Add a warning if there are > 64 modes in capability map block. >> - Use y420cmdb in function names and macros while dealing with vcb >> to be aligned with spec. >> - Move the display information parsing block ahead of mode parsing >> blocks. >> >> V3: Addressed design/review comments from Ville >> - Do not add flags in video modes, else we have to expose them to user >> - There should not be a UABI change, and kernel should detect the >> choice of the output based on type of mode, and the bitmaps. >> - Use standard bitops from kernel bitmap header, instead of calculating >> bit positions manually. >> >> V4: Addressed review comments from Ville: >> - s/ycbcr_420_vdb/y420vdb >> - s/ycbcr_420_vcb/y420cmdb >> - Be less verbose on description of do_y420vdb_modes >> - Move newmode variable in the loop scope. >> - Use svd_to_vic() to get a VIC, instead of 0x7f >> - Remove bitmap description for CMDB modes & VDB modes >> - Dont add connector->ycbcr_420_allowed check for cmdb modes >> - Remove 'len' variable, in is_y420cmdb function, which is used >> only once >> - Add length check in is_y420vdb function >> - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap >> - Do not add print about YCBCR 420 modes >> - Fix indentation in few places >> - Move ycbcr420_dc_modes in next patch, where its used >> - Add a separate patch for movement of drm_add_display_info() >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/drm_edid.c | 157 +++++++++++++++++++++++++++++++++++++++++++- >> include/drm/drm_connector.h | 20 ++++++ >> 2 files changed, 174 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index e2d1b30..8c7e73b 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -2781,7 +2781,9 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, >> #define VIDEO_BLOCK 0x02 >> #define VENDOR_BLOCK 0x03 >> #define SPEAKER_BLOCK 0x04 >> -#define VIDEO_CAPABILITY_BLOCK 0x07 >> +#define VIDEO_CAPABILITY_BLOCK 0x07 >> +#define VIDEO_DATA_BLOCK_420 0x0E >> +#define VIDEO_CAP_BLOCK_Y420CMDB 0x0F >> #define EDID_BASIC_AUDIO (1 << 6) >> #define EDID_CEA_YCRCB444 (1 << 5) >> #define EDID_CEA_YCRCB422 (1 << 4) >> @@ -3153,15 +3155,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, >> return newmode; >> } >> >> +/* >> + * do_y420vdb_modes - Parse YCBCR 420 only modes >> + * @connector: connector corresponding to the HDMI sink >> + * @svds: start of the data block of CEA YCBCR 420 VDB >> + * @len: length of the CEA YCBCR 420 VDB >> + * >> + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) >> + * which contains modes which can be supported in YCBCR 420 >> + * output format only. >> + */ >> +static int do_y420vdb_modes(struct drm_connector *connector, >> + const u8 *svds, u8 svds_len) >> +{ >> + int modes = 0, i; >> + struct drm_device *dev = connector->dev; >> + struct drm_display_info *info = &connector->display_info; >> + struct drm_hdmi_info *hdmi = &info->hdmi; >> + >> + for (i = 0; i < svds_len; i++) { >> + u8 vic = svd_to_vic(svds[i]); >> + struct drm_display_mode *newmode; >> + >> + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); >> + if (!newmode) >> + break; >> + bitmap_set(hdmi->y420_vdb_modes, vic, 1); >> + drm_mode_probed_add(connector, newmode); >> + modes++; >> + } >> + >> + if (modes > 0) >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >> + return modes; >> +} >> + >> +/* >> + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap >> + * @connector: connector corresponding to the HDMI sink >> + * @vic: CEA vic for the video mode to be added in the map >> + * >> + * Makes an entry for a videomode in the YCBCR 420 bitmap >> + */ >> +static void >> +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) >> +{ >> + u8 vic = svd_to_vic(svd); >> + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; >> + >> + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); >> +} >> + >> static int >> do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) >> { >> int i, modes = 0; >> + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; >> >> for (i = 0; i < len; i++) { >> struct drm_display_mode *mode; >> mode = drm_display_mode_from_vic_index(connector, db, len, i); >> if (mode) { >> + /* >> + * YCBCR420 capability block contains a bitmap which >> + * gives the index of CEA modes from CEA VDB, which >> + * can support YCBCR 420 sampling output also (apart >> + * from RGB/YCBCR444 etc). >> + * For example, if the bit 0 in bitmap is set, >> + * first mode in VDB can support YCBCR420 output too. >> + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. >> + */ >> + if (test_bit(i, &hdmi->y420_cmdb_map)) >> + drm_add_cmdb_modes(connector, db[i]); >> + >> drm_mode_probed_add(connector, mode); >> modes++; >> } >> @@ -3437,6 +3503,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, >> } >> >> static int >> +cea_db_extended_tag(const u8 *db) >> +{ >> + return db[1]; >> +} >> + >> +static int >> cea_db_payload_len(const u8 *db) >> { >> return db[0] & 0x1f; >> @@ -3497,9 +3569,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) >> return oui == HDMI_FORUM_IEEE_OUI; >> } >> >> +static bool cea_db_is_y420cmdb(const u8 *db) >> +{ >> + >> + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) > Still would like to see that patch to clean up the current extended tag > usage... Yep, its on the way, running a bit loaded. >> + return false; >> + >> + if (!cea_db_payload_len(db)) >> + return false; >> + >> + if (cea_db_extended_tag(db) != VIDEO_CAP_BLOCK_Y420CMDB) >> + return false; >> + >> + return true; >> +} >> + >> +static bool cea_db_is_y420vdb(const u8 *db) >> +{ >> + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) >> + return false; >> + >> + if (!cea_db_payload_len(db)) >> + return false; >> + >> + if (cea_db_extended_tag(db) != VIDEO_DATA_BLOCK_420) >> + return false; >> + >> + return true; >> +} >> + >> #define for_each_cea_db(cea, i, start, end) \ >> for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) >> >> +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, >> + const u8 *db) >> +{ >> + struct drm_display_info *info = &connector->display_info; >> + struct drm_hdmi_info *hdmi = &info->hdmi; >> + u8 map_len = cea_db_payload_len(db) - 1; >> + u8 count; >> + u64 map = 0; >> + >> + if (map_len == 0) { >> + /* All CEA modes support ycbcr420 sampling also.*/ >> + hdmi->y420_cmdb_map = U64_MAX; >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >> + return; >> + } >> + >> + /* >> + * This map indicates which of the existing CEA block modes >> + * from VDB can support YCBCR420 output too. So if bit=0 is >> + * set, first mode from VDB can support YCBCR420 output too. >> + * We will parse and keep this map, before parsing VDB itself >> + * to avoid going through the same block again and again. >> + * >> + * Spec is not clear about max possible size of this block. >> + * Clamping max bitmap block size at 8 bytes. Every byte can >> + * address 8 CEA modes, in this way this map can address >> + * 8*8 = first 64 SVDs. >> + */ >> + if (WARN_ON_ONCE(map_len > 8)) >> + map_len = 8; >> + >> + for (count = 0; count < map_len; count++) >> + map |= (u64)db[2 + count] << (8 * count); >> + >> + if (map) >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >> + >> + hdmi->y420_cmdb_map = map; >> +} >> + >> static int >> add_cea_modes(struct drm_connector *connector, struct edid *edid) >> { >> @@ -3522,10 +3663,17 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) >> video = db + 1; >> video_len = dbl; >> modes += do_cea_modes(connector, video, dbl); >> - } >> - else if (cea_db_is_hdmi_vsdb(db)) { >> + } else if (cea_db_is_hdmi_vsdb(db)) { >> hdmi = db; >> hdmi_len = dbl; >> + } else if (cea_db_is_y420vdb(db) && >> + connector->ycbcr_420_allowed) { >> + const u8 *vdb420 = &db[2]; >> + >> + /* Add 4:2:0(only) modes present in EDID */ >> + modes += do_y420vdb_modes(connector, >> + vdb420, >> + dbl - 1); >> } >> } >> } >> @@ -4206,6 +4354,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, >> drm_parse_hdmi_vsdb_video(connector, db); >> if (cea_db_is_hdmi_forum_vsdb(db)) >> drm_parse_hdmi_forum_vsdb(connector, db); >> + if (cea_db_is_y420cmdb(db)) >> + drm_parse_y420cmdb_bitmap(connector, db); >> } >> } >> >> @@ -4460,6 +4610,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) >> num_modes += add_cea_modes(connector, edid); >> num_modes += add_alternate_cea_modes(connector, edid); >> num_modes += add_displayid_detailed_modes(connector, edid); >> + >> if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) >> num_modes += add_inferred_modes(connector, edid); >> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >> index 7493fd3..ce2212d 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -137,6 +137,25 @@ struct drm_scdc { >> struct drm_hdmi_info { >> /** @scdc: sink's scdc support and capabilities */ >> struct drm_scdc scdc; >> + >> + /** >> + * @y420_vdb_modes: bitmap of modes which can support ycbcr420 >> + * output only (not normal RGB/YCBCR444/422 outputs). There are total >> + * 107 VICs defined by CEA-861-F spec, so the size is 128 bits to map >> + * upto 128 VICs; >> + */ >> + unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; >> + >> + /** >> + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 >> + * output also, along with normal HDMI outputs. There are total 107 >> + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto >> + * 128 VICs; >> + */ >> + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; >> + >> + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ >> + unsigned long y420_cmdb_map; > u64, otherwise it's busted on 32bit machines. Got it. > > Mostl this looks pretty good. I think the main problem is the lack of > mode validation that would reject the 4:2:0 only modes if not supported > by the driver/device. I guess that's in a later patch? Yep, its the next patch in the series. > If so we need to > reorder the series a bit to avoid regeressions. That patch uses parameters added in this patch, to detect the 4:2:0 only modes. So cant make it before this patch. Let me see what best can I do on the reordering. - Shashank >> }; >> >> /** >> @@ -200,6 +219,7 @@ struct drm_display_info { >> #define DRM_COLOR_FORMAT_RGB444 (1<<0) >> #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) >> #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) >> +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) >> >> /** >> * @color_formats: HDMI Color formats, selects between RGB and YCrCb >> -- >> 2.7.4
On Fri, Jun 30, 2017 at 10:47:48AM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 6/27/2017 5:22 PM, Ville Syrjälä wrote: > > On Wed, Jun 21, 2017 at 04:04:02PM +0530, Shashank Sharma wrote: > >> HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. > >> CEA-861-F adds two new blocks in EDID's CEA extension blocks, > >> to provide information about sink's YCBCR420 output capabilities. > >> > >> These blocks are: > >> > >> - YCBCR420vdb(YCBCR 420 video data block): > >> This block contains VICs of video modes, which can be sopported only > >> in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal > >> SVD block, valid for YCBCR420 modes only. > >> > >> - YCBCR420cmdb(YCBCR 420 capability map data block): > >> This block gives information about video modes which can support > >> YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This > >> block contains a bitmap index of normal svd videomodes, which can > >> support YCBCR420 output too. > >> So if bit 0 from first vcb byte is set, first video mode in the svd > >> list can support YCBCR420 output too. Bit 1 means second video mode > >> from svd list can support YCBCR420 output too, and so on. > >> > >> This patch adds two bitmaps in display's hdmi_info structure, one each > >> for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch > >> adds: > >> - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes > >> an entry in the vdb_bitmap per vic. > >> - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. > >> > >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > >> Cc: Jose Abreu <joabreu@synopsys.com> > >> Cc: Emil Velikov <emil.l.velikov@gmail.com> > >> > >> V2: Addressed > >> Review comments from Emil: > >> - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. > >> - Use the suggested method for updating dbmap. > >> - Add documentation for YCBCR420_vcb_map to fix kbuild warning. > >> > >> Review comments from Ville: > >> - Do not expose the YCBCR420 flags in uabi layer, keep it internal. > >> - Save a map of YCBCR420 modes for future reference. > >> - Check db length before trying to parse extended tag. > >> - Add a warning if there are > 64 modes in capability map block. > >> - Use y420cmdb in function names and macros while dealing with vcb > >> to be aligned with spec. > >> - Move the display information parsing block ahead of mode parsing > >> blocks. > >> > >> V3: Addressed design/review comments from Ville > >> - Do not add flags in video modes, else we have to expose them to user > >> - There should not be a UABI change, and kernel should detect the > >> choice of the output based on type of mode, and the bitmaps. > >> - Use standard bitops from kernel bitmap header, instead of calculating > >> bit positions manually. > >> > >> V4: Addressed review comments from Ville: > >> - s/ycbcr_420_vdb/y420vdb > >> - s/ycbcr_420_vcb/y420cmdb > >> - Be less verbose on description of do_y420vdb_modes > >> - Move newmode variable in the loop scope. > >> - Use svd_to_vic() to get a VIC, instead of 0x7f > >> - Remove bitmap description for CMDB modes & VDB modes > >> - Dont add connector->ycbcr_420_allowed check for cmdb modes > >> - Remove 'len' variable, in is_y420cmdb function, which is used > >> only once > >> - Add length check in is_y420vdb function > >> - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap > >> - Do not add print about YCBCR 420 modes > >> - Fix indentation in few places > >> - Move ycbcr420_dc_modes in next patch, where its used > >> - Add a separate patch for movement of drm_add_display_info() > >> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > >> --- > >> drivers/gpu/drm/drm_edid.c | 157 +++++++++++++++++++++++++++++++++++++++++++- > >> include/drm/drm_connector.h | 20 ++++++ > >> 2 files changed, 174 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >> index e2d1b30..8c7e73b 100644 > >> --- a/drivers/gpu/drm/drm_edid.c > >> +++ b/drivers/gpu/drm/drm_edid.c > >> @@ -2781,7 +2781,9 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, > >> #define VIDEO_BLOCK 0x02 > >> #define VENDOR_BLOCK 0x03 > >> #define SPEAKER_BLOCK 0x04 > >> -#define VIDEO_CAPABILITY_BLOCK 0x07 > >> +#define VIDEO_CAPABILITY_BLOCK 0x07 > >> +#define VIDEO_DATA_BLOCK_420 0x0E > >> +#define VIDEO_CAP_BLOCK_Y420CMDB 0x0F > >> #define EDID_BASIC_AUDIO (1 << 6) > >> #define EDID_CEA_YCRCB444 (1 << 5) > >> #define EDID_CEA_YCRCB422 (1 << 4) > >> @@ -3153,15 +3155,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, > >> return newmode; > >> } > >> > >> +/* > >> + * do_y420vdb_modes - Parse YCBCR 420 only modes > >> + * @connector: connector corresponding to the HDMI sink > >> + * @svds: start of the data block of CEA YCBCR 420 VDB > >> + * @len: length of the CEA YCBCR 420 VDB > >> + * > >> + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) > >> + * which contains modes which can be supported in YCBCR 420 > >> + * output format only. > >> + */ > >> +static int do_y420vdb_modes(struct drm_connector *connector, > >> + const u8 *svds, u8 svds_len) > >> +{ > >> + int modes = 0, i; > >> + struct drm_device *dev = connector->dev; > >> + struct drm_display_info *info = &connector->display_info; > >> + struct drm_hdmi_info *hdmi = &info->hdmi; > >> + > >> + for (i = 0; i < svds_len; i++) { > >> + u8 vic = svd_to_vic(svds[i]); > >> + struct drm_display_mode *newmode; > >> + > >> + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); > >> + if (!newmode) > >> + break; > >> + bitmap_set(hdmi->y420_vdb_modes, vic, 1); > >> + drm_mode_probed_add(connector, newmode); > >> + modes++; > >> + } > >> + > >> + if (modes > 0) > >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > >> + return modes; > >> +} > >> + > >> +/* > >> + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap > >> + * @connector: connector corresponding to the HDMI sink > >> + * @vic: CEA vic for the video mode to be added in the map > >> + * > >> + * Makes an entry for a videomode in the YCBCR 420 bitmap > >> + */ > >> +static void > >> +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) > >> +{ > >> + u8 vic = svd_to_vic(svd); > >> + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; > >> + > >> + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); > >> +} > >> + > >> static int > >> do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) > >> { > >> int i, modes = 0; > >> + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; > >> > >> for (i = 0; i < len; i++) { > >> struct drm_display_mode *mode; > >> mode = drm_display_mode_from_vic_index(connector, db, len, i); > >> if (mode) { > >> + /* > >> + * YCBCR420 capability block contains a bitmap which > >> + * gives the index of CEA modes from CEA VDB, which > >> + * can support YCBCR 420 sampling output also (apart > >> + * from RGB/YCBCR444 etc). > >> + * For example, if the bit 0 in bitmap is set, > >> + * first mode in VDB can support YCBCR420 output too. > >> + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. > >> + */ > >> + if (test_bit(i, &hdmi->y420_cmdb_map)) > >> + drm_add_cmdb_modes(connector, db[i]); > >> + > >> drm_mode_probed_add(connector, mode); > >> modes++; > >> } > >> @@ -3437,6 +3503,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, > >> } > >> > >> static int > >> +cea_db_extended_tag(const u8 *db) > >> +{ > >> + return db[1]; > >> +} > >> + > >> +static int > >> cea_db_payload_len(const u8 *db) > >> { > >> return db[0] & 0x1f; > >> @@ -3497,9 +3569,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) > >> return oui == HDMI_FORUM_IEEE_OUI; > >> } > >> > >> +static bool cea_db_is_y420cmdb(const u8 *db) > >> +{ > >> + > >> + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) > > Still would like to see that patch to clean up the current extended tag > > usage... > Yep, its on the way, running a bit loaded. > >> + return false; > >> + > >> + if (!cea_db_payload_len(db)) > >> + return false; > >> + > >> + if (cea_db_extended_tag(db) != VIDEO_CAP_BLOCK_Y420CMDB) > >> + return false; > >> + > >> + return true; > >> +} > >> + > >> +static bool cea_db_is_y420vdb(const u8 *db) > >> +{ > >> + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) > >> + return false; > >> + > >> + if (!cea_db_payload_len(db)) > >> + return false; > >> + > >> + if (cea_db_extended_tag(db) != VIDEO_DATA_BLOCK_420) > >> + return false; > >> + > >> + return true; > >> +} > >> + > >> #define for_each_cea_db(cea, i, start, end) \ > >> for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) > >> > >> +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, > >> + const u8 *db) > >> +{ > >> + struct drm_display_info *info = &connector->display_info; > >> + struct drm_hdmi_info *hdmi = &info->hdmi; > >> + u8 map_len = cea_db_payload_len(db) - 1; > >> + u8 count; > >> + u64 map = 0; > >> + > >> + if (map_len == 0) { > >> + /* All CEA modes support ycbcr420 sampling also.*/ > >> + hdmi->y420_cmdb_map = U64_MAX; > >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > >> + return; > >> + } > >> + > >> + /* > >> + * This map indicates which of the existing CEA block modes > >> + * from VDB can support YCBCR420 output too. So if bit=0 is > >> + * set, first mode from VDB can support YCBCR420 output too. > >> + * We will parse and keep this map, before parsing VDB itself > >> + * to avoid going through the same block again and again. > >> + * > >> + * Spec is not clear about max possible size of this block. > >> + * Clamping max bitmap block size at 8 bytes. Every byte can > >> + * address 8 CEA modes, in this way this map can address > >> + * 8*8 = first 64 SVDs. > >> + */ > >> + if (WARN_ON_ONCE(map_len > 8)) > >> + map_len = 8; > >> + > >> + for (count = 0; count < map_len; count++) > >> + map |= (u64)db[2 + count] << (8 * count); > >> + > >> + if (map) > >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > >> + > >> + hdmi->y420_cmdb_map = map; > >> +} > >> + > >> static int > >> add_cea_modes(struct drm_connector *connector, struct edid *edid) > >> { > >> @@ -3522,10 +3663,17 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) > >> video = db + 1; > >> video_len = dbl; > >> modes += do_cea_modes(connector, video, dbl); > >> - } > >> - else if (cea_db_is_hdmi_vsdb(db)) { > >> + } else if (cea_db_is_hdmi_vsdb(db)) { > >> hdmi = db; > >> hdmi_len = dbl; > >> + } else if (cea_db_is_y420vdb(db) && > >> + connector->ycbcr_420_allowed) { > >> + const u8 *vdb420 = &db[2]; > >> + > >> + /* Add 4:2:0(only) modes present in EDID */ > >> + modes += do_y420vdb_modes(connector, > >> + vdb420, > >> + dbl - 1); > >> } > >> } > >> } > >> @@ -4206,6 +4354,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > >> drm_parse_hdmi_vsdb_video(connector, db); > >> if (cea_db_is_hdmi_forum_vsdb(db)) > >> drm_parse_hdmi_forum_vsdb(connector, db); > >> + if (cea_db_is_y420cmdb(db)) > >> + drm_parse_y420cmdb_bitmap(connector, db); > >> } > >> } > >> > >> @@ -4460,6 +4610,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) > >> num_modes += add_cea_modes(connector, edid); > >> num_modes += add_alternate_cea_modes(connector, edid); > >> num_modes += add_displayid_detailed_modes(connector, edid); > >> + > >> if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) > >> num_modes += add_inferred_modes(connector, edid); > >> > >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > >> index 7493fd3..ce2212d 100644 > >> --- a/include/drm/drm_connector.h > >> +++ b/include/drm/drm_connector.h > >> @@ -137,6 +137,25 @@ struct drm_scdc { > >> struct drm_hdmi_info { > >> /** @scdc: sink's scdc support and capabilities */ > >> struct drm_scdc scdc; > >> + > >> + /** > >> + * @y420_vdb_modes: bitmap of modes which can support ycbcr420 > >> + * output only (not normal RGB/YCBCR444/422 outputs). There are total > >> + * 107 VICs defined by CEA-861-F spec, so the size is 128 bits to map > >> + * upto 128 VICs; > >> + */ > >> + unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; > >> + > >> + /** > >> + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 > >> + * output also, along with normal HDMI outputs. There are total 107 > >> + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto > >> + * 128 VICs; > >> + */ > >> + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; > >> + > >> + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ > >> + unsigned long y420_cmdb_map; > > u64, otherwise it's busted on 32bit machines. > Got it. > > > > Mostl this looks pretty good. I think the main problem is the lack of > > mode validation that would reject the 4:2:0 only modes if not supported > > by the driver/device. I guess that's in a later patch? > Yep, its the next patch in the series. The only thing the next patch does is set up the y420_dc_modes, but it doesn't use that for anything, and neither does it filter anything. I can't find any mode filtering patch actually. > > If so we need to > > reorder the series a bit to avoid regeressions. > That patch uses parameters added in this patch, to detect the 4:2:0 only > modes. So cant make it before this patch. > Let me see what best can I do on the reordering. > > - Shashank > >> }; > >> > >> /** > >> @@ -200,6 +219,7 @@ struct drm_display_info { > >> #define DRM_COLOR_FORMAT_RGB444 (1<<0) > >> #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) > >> #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) > >> +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) > >> > >> /** > >> * @color_formats: HDMI Color formats, selects between RGB and YCrCb > >> -- > >> 2.7.4
Regards Shashank On 6/30/2017 5:28 PM, Ville Syrjälä wrote: > On Fri, Jun 30, 2017 at 10:47:48AM +0530, Sharma, Shashank wrote: >> Regards >> >> Shashank >> >> >> On 6/27/2017 5:22 PM, Ville Syrjälä wrote: >>> On Wed, Jun 21, 2017 at 04:04:02PM +0530, Shashank Sharma wrote: >>>> HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. >>>> CEA-861-F adds two new blocks in EDID's CEA extension blocks, >>>> to provide information about sink's YCBCR420 output capabilities. >>>> >>>> These blocks are: >>>> >>>> - YCBCR420vdb(YCBCR 420 video data block): >>>> This block contains VICs of video modes, which can be sopported only >>>> in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal >>>> SVD block, valid for YCBCR420 modes only. >>>> >>>> - YCBCR420cmdb(YCBCR 420 capability map data block): >>>> This block gives information about video modes which can support >>>> YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This >>>> block contains a bitmap index of normal svd videomodes, which can >>>> support YCBCR420 output too. >>>> So if bit 0 from first vcb byte is set, first video mode in the svd >>>> list can support YCBCR420 output too. Bit 1 means second video mode >>>> from svd list can support YCBCR420 output too, and so on. >>>> >>>> This patch adds two bitmaps in display's hdmi_info structure, one each >>>> for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch >>>> adds: >>>> - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes >>>> an entry in the vdb_bitmap per vic. >>>> - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. >>>> >>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >>>> Cc: Jose Abreu <joabreu@synopsys.com> >>>> Cc: Emil Velikov <emil.l.velikov@gmail.com> >>>> >>>> V2: Addressed >>>> Review comments from Emil: >>>> - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. >>>> - Use the suggested method for updating dbmap. >>>> - Add documentation for YCBCR420_vcb_map to fix kbuild warning. >>>> >>>> Review comments from Ville: >>>> - Do not expose the YCBCR420 flags in uabi layer, keep it internal. >>>> - Save a map of YCBCR420 modes for future reference. >>>> - Check db length before trying to parse extended tag. >>>> - Add a warning if there are > 64 modes in capability map block. >>>> - Use y420cmdb in function names and macros while dealing with vcb >>>> to be aligned with spec. >>>> - Move the display information parsing block ahead of mode parsing >>>> blocks. >>>> >>>> V3: Addressed design/review comments from Ville >>>> - Do not add flags in video modes, else we have to expose them to user >>>> - There should not be a UABI change, and kernel should detect the >>>> choice of the output based on type of mode, and the bitmaps. >>>> - Use standard bitops from kernel bitmap header, instead of calculating >>>> bit positions manually. >>>> >>>> V4: Addressed review comments from Ville: >>>> - s/ycbcr_420_vdb/y420vdb >>>> - s/ycbcr_420_vcb/y420cmdb >>>> - Be less verbose on description of do_y420vdb_modes >>>> - Move newmode variable in the loop scope. >>>> - Use svd_to_vic() to get a VIC, instead of 0x7f >>>> - Remove bitmap description for CMDB modes & VDB modes >>>> - Dont add connector->ycbcr_420_allowed check for cmdb modes >>>> - Remove 'len' variable, in is_y420cmdb function, which is used >>>> only once >>>> - Add length check in is_y420vdb function >>>> - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap >>>> - Do not add print about YCBCR 420 modes >>>> - Fix indentation in few places >>>> - Move ycbcr420_dc_modes in next patch, where its used >>>> - Add a separate patch for movement of drm_add_display_info() >>>> >>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_edid.c | 157 +++++++++++++++++++++++++++++++++++++++++++- >>>> include/drm/drm_connector.h | 20 ++++++ >>>> 2 files changed, 174 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>>> index e2d1b30..8c7e73b 100644 >>>> --- a/drivers/gpu/drm/drm_edid.c >>>> +++ b/drivers/gpu/drm/drm_edid.c >>>> @@ -2781,7 +2781,9 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, >>>> #define VIDEO_BLOCK 0x02 >>>> #define VENDOR_BLOCK 0x03 >>>> #define SPEAKER_BLOCK 0x04 >>>> -#define VIDEO_CAPABILITY_BLOCK 0x07 >>>> +#define VIDEO_CAPABILITY_BLOCK 0x07 >>>> +#define VIDEO_DATA_BLOCK_420 0x0E >>>> +#define VIDEO_CAP_BLOCK_Y420CMDB 0x0F >>>> #define EDID_BASIC_AUDIO (1 << 6) >>>> #define EDID_CEA_YCRCB444 (1 << 5) >>>> #define EDID_CEA_YCRCB422 (1 << 4) >>>> @@ -3153,15 +3155,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, >>>> return newmode; >>>> } >>>> >>>> +/* >>>> + * do_y420vdb_modes - Parse YCBCR 420 only modes >>>> + * @connector: connector corresponding to the HDMI sink >>>> + * @svds: start of the data block of CEA YCBCR 420 VDB >>>> + * @len: length of the CEA YCBCR 420 VDB >>>> + * >>>> + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) >>>> + * which contains modes which can be supported in YCBCR 420 >>>> + * output format only. >>>> + */ >>>> +static int do_y420vdb_modes(struct drm_connector *connector, >>>> + const u8 *svds, u8 svds_len) >>>> +{ >>>> + int modes = 0, i; >>>> + struct drm_device *dev = connector->dev; >>>> + struct drm_display_info *info = &connector->display_info; >>>> + struct drm_hdmi_info *hdmi = &info->hdmi; >>>> + >>>> + for (i = 0; i < svds_len; i++) { >>>> + u8 vic = svd_to_vic(svds[i]); >>>> + struct drm_display_mode *newmode; >>>> + >>>> + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); >>>> + if (!newmode) >>>> + break; >>>> + bitmap_set(hdmi->y420_vdb_modes, vic, 1); >>>> + drm_mode_probed_add(connector, newmode); >>>> + modes++; >>>> + } >>>> + >>>> + if (modes > 0) >>>> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >>>> + return modes; >>>> +} >>>> + >>>> +/* >>>> + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap >>>> + * @connector: connector corresponding to the HDMI sink >>>> + * @vic: CEA vic for the video mode to be added in the map >>>> + * >>>> + * Makes an entry for a videomode in the YCBCR 420 bitmap >>>> + */ >>>> +static void >>>> +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) >>>> +{ >>>> + u8 vic = svd_to_vic(svd); >>>> + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; >>>> + >>>> + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); >>>> +} >>>> + >>>> static int >>>> do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) >>>> { >>>> int i, modes = 0; >>>> + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; >>>> >>>> for (i = 0; i < len; i++) { >>>> struct drm_display_mode *mode; >>>> mode = drm_display_mode_from_vic_index(connector, db, len, i); >>>> if (mode) { >>>> + /* >>>> + * YCBCR420 capability block contains a bitmap which >>>> + * gives the index of CEA modes from CEA VDB, which >>>> + * can support YCBCR 420 sampling output also (apart >>>> + * from RGB/YCBCR444 etc). >>>> + * For example, if the bit 0 in bitmap is set, >>>> + * first mode in VDB can support YCBCR420 output too. >>>> + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. >>>> + */ >>>> + if (test_bit(i, &hdmi->y420_cmdb_map)) >>>> + drm_add_cmdb_modes(connector, db[i]); >>>> + >>>> drm_mode_probed_add(connector, mode); >>>> modes++; >>>> } >>>> @@ -3437,6 +3503,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, >>>> } >>>> >>>> static int >>>> +cea_db_extended_tag(const u8 *db) >>>> +{ >>>> + return db[1]; >>>> +} >>>> + >>>> +static int >>>> cea_db_payload_len(const u8 *db) >>>> { >>>> return db[0] & 0x1f; >>>> @@ -3497,9 +3569,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) >>>> return oui == HDMI_FORUM_IEEE_OUI; >>>> } >>>> >>>> +static bool cea_db_is_y420cmdb(const u8 *db) >>>> +{ >>>> + >>>> + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) >>> Still would like to see that patch to clean up the current extended tag >>> usage... >> Yep, its on the way, running a bit loaded. >>>> + return false; >>>> + >>>> + if (!cea_db_payload_len(db)) >>>> + return false; >>>> + >>>> + if (cea_db_extended_tag(db) != VIDEO_CAP_BLOCK_Y420CMDB) >>>> + return false; >>>> + >>>> + return true; >>>> +} >>>> + >>>> +static bool cea_db_is_y420vdb(const u8 *db) >>>> +{ >>>> + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) >>>> + return false; >>>> + >>>> + if (!cea_db_payload_len(db)) >>>> + return false; >>>> + >>>> + if (cea_db_extended_tag(db) != VIDEO_DATA_BLOCK_420) >>>> + return false; >>>> + >>>> + return true; >>>> +} >>>> + >>>> #define for_each_cea_db(cea, i, start, end) \ >>>> for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) >>>> >>>> +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, >>>> + const u8 *db) >>>> +{ >>>> + struct drm_display_info *info = &connector->display_info; >>>> + struct drm_hdmi_info *hdmi = &info->hdmi; >>>> + u8 map_len = cea_db_payload_len(db) - 1; >>>> + u8 count; >>>> + u64 map = 0; >>>> + >>>> + if (map_len == 0) { >>>> + /* All CEA modes support ycbcr420 sampling also.*/ >>>> + hdmi->y420_cmdb_map = U64_MAX; >>>> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >>>> + return; >>>> + } >>>> + >>>> + /* >>>> + * This map indicates which of the existing CEA block modes >>>> + * from VDB can support YCBCR420 output too. So if bit=0 is >>>> + * set, first mode from VDB can support YCBCR420 output too. >>>> + * We will parse and keep this map, before parsing VDB itself >>>> + * to avoid going through the same block again and again. >>>> + * >>>> + * Spec is not clear about max possible size of this block. >>>> + * Clamping max bitmap block size at 8 bytes. Every byte can >>>> + * address 8 CEA modes, in this way this map can address >>>> + * 8*8 = first 64 SVDs. >>>> + */ >>>> + if (WARN_ON_ONCE(map_len > 8)) >>>> + map_len = 8; >>>> + >>>> + for (count = 0; count < map_len; count++) >>>> + map |= (u64)db[2 + count] << (8 * count); >>>> + >>>> + if (map) >>>> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >>>> + >>>> + hdmi->y420_cmdb_map = map; >>>> +} >>>> + >>>> static int >>>> add_cea_modes(struct drm_connector *connector, struct edid *edid) >>>> { >>>> @@ -3522,10 +3663,17 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) >>>> video = db + 1; >>>> video_len = dbl; >>>> modes += do_cea_modes(connector, video, dbl); >>>> - } >>>> - else if (cea_db_is_hdmi_vsdb(db)) { >>>> + } else if (cea_db_is_hdmi_vsdb(db)) { >>>> hdmi = db; >>>> hdmi_len = dbl; >>>> + } else if (cea_db_is_y420vdb(db) && >>>> + connector->ycbcr_420_allowed) { >>>> + const u8 *vdb420 = &db[2]; >>>> + >>>> + /* Add 4:2:0(only) modes present in EDID */ >>>> + modes += do_y420vdb_modes(connector, >>>> + vdb420, >>>> + dbl - 1); >>>> } >>>> } >>>> } >>>> @@ -4206,6 +4354,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, >>>> drm_parse_hdmi_vsdb_video(connector, db); >>>> if (cea_db_is_hdmi_forum_vsdb(db)) >>>> drm_parse_hdmi_forum_vsdb(connector, db); >>>> + if (cea_db_is_y420cmdb(db)) >>>> + drm_parse_y420cmdb_bitmap(connector, db); >>>> } >>>> } >>>> >>>> @@ -4460,6 +4610,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) >>>> num_modes += add_cea_modes(connector, edid); >>>> num_modes += add_alternate_cea_modes(connector, edid); >>>> num_modes += add_displayid_detailed_modes(connector, edid); >>>> + >>>> if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) >>>> num_modes += add_inferred_modes(connector, edid); >>>> >>>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >>>> index 7493fd3..ce2212d 100644 >>>> --- a/include/drm/drm_connector.h >>>> +++ b/include/drm/drm_connector.h >>>> @@ -137,6 +137,25 @@ struct drm_scdc { >>>> struct drm_hdmi_info { >>>> /** @scdc: sink's scdc support and capabilities */ >>>> struct drm_scdc scdc; >>>> + >>>> + /** >>>> + * @y420_vdb_modes: bitmap of modes which can support ycbcr420 >>>> + * output only (not normal RGB/YCBCR444/422 outputs). There are total >>>> + * 107 VICs defined by CEA-861-F spec, so the size is 128 bits to map >>>> + * upto 128 VICs; >>>> + */ >>>> + unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; >>>> + >>>> + /** >>>> + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 >>>> + * output also, along with normal HDMI outputs. There are total 107 >>>> + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto >>>> + * 128 VICs; >>>> + */ >>>> + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; >>>> + >>>> + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ >>>> + unsigned long y420_cmdb_map; >>> u64, otherwise it's busted on 32bit machines. >> Got it. >>> Mostl this looks pretty good. I think the main problem is the lack of >>> mode validation that would reject the 4:2:0 only modes if not supported >>> by the driver/device. I guess that's in a later patch? >> Yep, its the next patch in the series. > The only thing the next patch does is set up the y420_dc_modes, but it > doesn't use that for anything, and neither does it filter anything. I > can't find any mode filtering patch actually. Perhaps I misunderstood your question. If you are asking for the patch, in which you asked to remove the 420_only modes, at connector_probe level (instead of blocking the read of 420_only modes based on connector->y420_allowed), its being prepared now. It should be available with V5. - Shashank >>> If so we need to >>> reorder the series a bit to avoid regeressions. >> That patch uses parameters added in this patch, to detect the 4:2:0 only >> modes. So cant make it before this patch. >> Let me see what best can I do on the reordering. >> >> - Shashank >>>> }; >>>> >>>> /** >>>> @@ -200,6 +219,7 @@ struct drm_display_info { >>>> #define DRM_COLOR_FORMAT_RGB444 (1<<0) >>>> #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) >>>> #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) >>>> +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) >>>> >>>> /** >>>> * @color_formats: HDMI Color formats, selects between RGB and YCrCb >>>> -- >>>> 2.7.4
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e2d1b30..8c7e73b 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2781,7 +2781,9 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, #define VIDEO_BLOCK 0x02 #define VENDOR_BLOCK 0x03 #define SPEAKER_BLOCK 0x04 -#define VIDEO_CAPABILITY_BLOCK 0x07 +#define VIDEO_CAPABILITY_BLOCK 0x07 +#define VIDEO_DATA_BLOCK_420 0x0E +#define VIDEO_CAP_BLOCK_Y420CMDB 0x0F #define EDID_BASIC_AUDIO (1 << 6) #define EDID_CEA_YCRCB444 (1 << 5) #define EDID_CEA_YCRCB422 (1 << 4) @@ -3153,15 +3155,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, return newmode; } +/* + * do_y420vdb_modes - Parse YCBCR 420 only modes + * @connector: connector corresponding to the HDMI sink + * @svds: start of the data block of CEA YCBCR 420 VDB + * @len: length of the CEA YCBCR 420 VDB + * + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) + * which contains modes which can be supported in YCBCR 420 + * output format only. + */ +static int do_y420vdb_modes(struct drm_connector *connector, + const u8 *svds, u8 svds_len) +{ + int modes = 0, i; + struct drm_device *dev = connector->dev; + struct drm_display_info *info = &connector->display_info; + struct drm_hdmi_info *hdmi = &info->hdmi; + + for (i = 0; i < svds_len; i++) { + u8 vic = svd_to_vic(svds[i]); + struct drm_display_mode *newmode; + + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); + if (!newmode) + break; + bitmap_set(hdmi->y420_vdb_modes, vic, 1); + drm_mode_probed_add(connector, newmode); + modes++; + } + + if (modes > 0) + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + return modes; +} + +/* + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap + * @connector: connector corresponding to the HDMI sink + * @vic: CEA vic for the video mode to be added in the map + * + * Makes an entry for a videomode in the YCBCR 420 bitmap + */ +static void +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) +{ + u8 vic = svd_to_vic(svd); + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; + + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); +} + static int do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) { int i, modes = 0; + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; for (i = 0; i < len; i++) { struct drm_display_mode *mode; mode = drm_display_mode_from_vic_index(connector, db, len, i); if (mode) { + /* + * YCBCR420 capability block contains a bitmap which + * gives the index of CEA modes from CEA VDB, which + * can support YCBCR 420 sampling output also (apart + * from RGB/YCBCR444 etc). + * For example, if the bit 0 in bitmap is set, + * first mode in VDB can support YCBCR420 output too. + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. + */ + if (test_bit(i, &hdmi->y420_cmdb_map)) + drm_add_cmdb_modes(connector, db[i]); + drm_mode_probed_add(connector, mode); modes++; } @@ -3437,6 +3503,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, } static int +cea_db_extended_tag(const u8 *db) +{ + return db[1]; +} + +static int cea_db_payload_len(const u8 *db) { return db[0] & 0x1f; @@ -3497,9 +3569,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) return oui == HDMI_FORUM_IEEE_OUI; } +static bool cea_db_is_y420cmdb(const u8 *db) +{ + + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) + return false; + + if (!cea_db_payload_len(db)) + return false; + + if (cea_db_extended_tag(db) != VIDEO_CAP_BLOCK_Y420CMDB) + return false; + + return true; +} + +static bool cea_db_is_y420vdb(const u8 *db) +{ + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK) + return false; + + if (!cea_db_payload_len(db)) + return false; + + if (cea_db_extended_tag(db) != VIDEO_DATA_BLOCK_420) + return false; + + return true; +} + #define for_each_cea_db(cea, i, start, end) \ for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, + const u8 *db) +{ + struct drm_display_info *info = &connector->display_info; + struct drm_hdmi_info *hdmi = &info->hdmi; + u8 map_len = cea_db_payload_len(db) - 1; + u8 count; + u64 map = 0; + + if (map_len == 0) { + /* All CEA modes support ycbcr420 sampling also.*/ + hdmi->y420_cmdb_map = U64_MAX; + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + return; + } + + /* + * This map indicates which of the existing CEA block modes + * from VDB can support YCBCR420 output too. So if bit=0 is + * set, first mode from VDB can support YCBCR420 output too. + * We will parse and keep this map, before parsing VDB itself + * to avoid going through the same block again and again. + * + * Spec is not clear about max possible size of this block. + * Clamping max bitmap block size at 8 bytes. Every byte can + * address 8 CEA modes, in this way this map can address + * 8*8 = first 64 SVDs. + */ + if (WARN_ON_ONCE(map_len > 8)) + map_len = 8; + + for (count = 0; count < map_len; count++) + map |= (u64)db[2 + count] << (8 * count); + + if (map) + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + + hdmi->y420_cmdb_map = map; +} + static int add_cea_modes(struct drm_connector *connector, struct edid *edid) { @@ -3522,10 +3663,17 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) video = db + 1; video_len = dbl; modes += do_cea_modes(connector, video, dbl); - } - else if (cea_db_is_hdmi_vsdb(db)) { + } else if (cea_db_is_hdmi_vsdb(db)) { hdmi = db; hdmi_len = dbl; + } else if (cea_db_is_y420vdb(db) && + connector->ycbcr_420_allowed) { + const u8 *vdb420 = &db[2]; + + /* Add 4:2:0(only) modes present in EDID */ + modes += do_y420vdb_modes(connector, + vdb420, + dbl - 1); } } } @@ -4206,6 +4354,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, drm_parse_hdmi_vsdb_video(connector, db); if (cea_db_is_hdmi_forum_vsdb(db)) drm_parse_hdmi_forum_vsdb(connector, db); + if (cea_db_is_y420cmdb(db)) + drm_parse_y420cmdb_bitmap(connector, db); } } @@ -4460,6 +4610,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) num_modes += add_cea_modes(connector, edid); num_modes += add_alternate_cea_modes(connector, edid); num_modes += add_displayid_detailed_modes(connector, edid); + if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) num_modes += add_inferred_modes(connector, edid); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 7493fd3..ce2212d 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -137,6 +137,25 @@ struct drm_scdc { struct drm_hdmi_info { /** @scdc: sink's scdc support and capabilities */ struct drm_scdc scdc; + + /** + * @y420_vdb_modes: bitmap of modes which can support ycbcr420 + * output only (not normal RGB/YCBCR444/422 outputs). There are total + * 107 VICs defined by CEA-861-F spec, so the size is 128 bits to map + * upto 128 VICs; + */ + unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; + + /** + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 + * output also, along with normal HDMI outputs. There are total 107 + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto + * 128 VICs; + */ + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; + + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ + unsigned long y420_cmdb_map; }; /** @@ -200,6 +219,7 @@ struct drm_display_info { #define DRM_COLOR_FORMAT_RGB444 (1<<0) #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) /** * @color_formats: HDMI Color formats, selects between RGB and YCrCb
HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. CEA-861-F adds two new blocks in EDID's CEA extension blocks, to provide information about sink's YCBCR420 output capabilities. These blocks are: - YCBCR420vdb(YCBCR 420 video data block): This block contains VICs of video modes, which can be sopported only in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal SVD block, valid for YCBCR420 modes only. - YCBCR420cmdb(YCBCR 420 capability map data block): This block gives information about video modes which can support YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This block contains a bitmap index of normal svd videomodes, which can support YCBCR420 output too. So if bit 0 from first vcb byte is set, first video mode in the svd list can support YCBCR420 output too. Bit 1 means second video mode from svd list can support YCBCR420 output too, and so on. This patch adds two bitmaps in display's hdmi_info structure, one each for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch adds: - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes an entry in the vdb_bitmap per vic. - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jose Abreu <joabreu@synopsys.com> Cc: Emil Velikov <emil.l.velikov@gmail.com> V2: Addressed Review comments from Emil: - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. - Use the suggested method for updating dbmap. - Add documentation for YCBCR420_vcb_map to fix kbuild warning. Review comments from Ville: - Do not expose the YCBCR420 flags in uabi layer, keep it internal. - Save a map of YCBCR420 modes for future reference. - Check db length before trying to parse extended tag. - Add a warning if there are > 64 modes in capability map block. - Use y420cmdb in function names and macros while dealing with vcb to be aligned with spec. - Move the display information parsing block ahead of mode parsing blocks. V3: Addressed design/review comments from Ville - Do not add flags in video modes, else we have to expose them to user - There should not be a UABI change, and kernel should detect the choice of the output based on type of mode, and the bitmaps. - Use standard bitops from kernel bitmap header, instead of calculating bit positions manually. V4: Addressed review comments from Ville: - s/ycbcr_420_vdb/y420vdb - s/ycbcr_420_vcb/y420cmdb - Be less verbose on description of do_y420vdb_modes - Move newmode variable in the loop scope. - Use svd_to_vic() to get a VIC, instead of 0x7f - Remove bitmap description for CMDB modes & VDB modes - Dont add connector->ycbcr_420_allowed check for cmdb modes - Remove 'len' variable, in is_y420cmdb function, which is used only once - Add length check in is_y420vdb function - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap - Do not add print about YCBCR 420 modes - Fix indentation in few places - Move ycbcr420_dc_modes in next patch, where its used - Add a separate patch for movement of drm_add_display_info() Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 157 +++++++++++++++++++++++++++++++++++++++++++- include/drm/drm_connector.h | 20 ++++++ 2 files changed, 174 insertions(+), 3 deletions(-)