> On Jan 20, 2026, at 12:03 PM, Ville Syrjälä <[email protected]> > wrote: > > On Tue, Jan 20, 2026 at 11:37:59AM -0500, [email protected] > <mailto:[email protected]> wrote: >> >> >>> On Jan 20, 2026, at 10:41 AM, Ville Syrjälä <[email protected]> >>> wrote: >>> >>> On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote: >>>> On Sat, 17 Jan 2026, Joshua Peisach <[email protected]> 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 <[email protected]> >>>>> --- >>>>> 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
The name of the functions specifically say HDMI, so I think that's the system to use: there are functions that say CTA in the name, like parse_cta_y420cmdb - so that is CTA, and these functions follow HDMI. Maybe in a v2 patch, drop a comment clarifying this? At the top of the drm_parse_hdmi_vsdb_video function it does say HDMI. -Josh
