Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jan 20, 2026 at 11:37:59AM -0500, jpeisach@ubuntu.com wrote:
> 
> 
> > On Jan 20, 2026, at 10:41 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > 
> > On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
> >> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> wrote:
> >>> drm_parse_hdmi_vsdb_video is one of the parsers that still do not use the
> >>> cea_db struct, and currently passes a u8 pointer.
> >>> 
> >>> Set the correct struct type and update references to the data accordingly.
> >>> This also makes the same change to drm_parse_hdmi_deep_color_info as
> >>> necessary.
> >>> 
> >>> Signed-off-by: Joshua Peisach <jpeisach@ubuntu.com>
> >>> ---
> >>> drivers/gpu/drm/drm_edid.c | 26 +++++++++++++-------------
> >>> 1 file changed, 13 insertions(+), 13 deletions(-)
> >>> 
> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>> index 26bb7710a..15bd99e65 100644
> >>> --- a/drivers/gpu/drm/drm_edid.c
> >>> +++ b/drivers/gpu/drm/drm_edid.c
> >>> @@ -6290,7 +6290,7 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector,
> >>> }
> >>> 
> >>> static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
> >>> -					   const u8 *hdmi)
> >>> +					   const struct cea_db *db)
> >>> {
> >>> 	struct drm_display_info *info = &connector->display_info;
> >>> 	unsigned int dc_bpc = 0;
> >>> @@ -6298,24 +6298,24 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
> >>> 	/* HDMI supports at least 8 bpc */
> >>> 	info->bpc = 8;
> >>> 
> >>> -	if (cea_db_payload_len(hdmi) < 6)
> >>> +	if (cea_db_payload_len(db) < 6)
> >>> 		return;
> >>> 
> >>> -	if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
> >>> +	if (db->data[6] & DRM_EDID_HDMI_DC_30) {
> >> 
> >> That's not the same thing, but off-by-one now. Ditto everywhere that
> >> changes from u8* to db->data[].
> >> 
> >> The main problem with the change (even with fixed offsets) is that the
> >> *specs* typically use indexing from the beginning of the data block, not
> >> from the beginning of payload data.
> >> 
> >> We've discussed this before with Ville (Cc'd) but I'm not sure if we
> >> reached a conclusion.
> > 
> > I guess we could give up on the index matching the spec byte#.
> > Looks like the HDMI VSDB parsing is the only place where we
> > actually have the two match, and everwhere else it's
> > already inconsistent.
> > 
> > Also maybe we should add something to also exclude the
> > extended tag from the payload, for the blocks that use
> > the extended tag...
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> I personally believe it is important to match the spec for consistency
> and reference. Unless I am looking at the wrong thing, bit 6 should have
> the correct index.
> 
> Also I think cea_db in the context of the function calls here are
> just the data blocks. Unless you mean that by having the struct's first
> member being the tag_length if offsetting everything, but I don't think
> it is? Let me know if I'm wrong :)

The tag+length byte is:
byte# 1 in the CTA spec
byte# 0 in the HDMI spec

There's no super nice way to use the byte# as the index for both.
Also the length checks end up looking somewhat confusing when
comparing them with the corresponding index.

-- 
Ville Syrjälä
Intel