> On Jan 20, 2026, at 1:09 PM, Ville Syrjälä <[email protected]> > wrote: > > On Tue, Jan 20, 2026 at 12:46:59PM -0500, [email protected] > <mailto:[email protected]> wrote: >> >> >>> On Jan 20, 2026, at 12:03 PM, Ville Syrjälä <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> On Tue, Jan 20, 2026 at 11:37:59AM -0500, [email protected] >>> <mailto:[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. > > I'm saying that there is no really sane way to deal with the CTA byte# > convention. So I think it's probably best to just go for a single > consistent approach for both CTA and HDMI. That way people at least > won't get confused by the different convetion between the functions. > And the length checks wouldn't look incorrect. >
I agree. I can't think of anything though, other than to assume CTA. The other parsers that already use struct cea_db refer to the data using the CTA spec (starting at 1). Maybe just go with that? But yes, as Jani said, the db->data indexing is off by one. Is fixing that fine for a v2 patch or should I put this aside? -Josh > Another option might be to add some kind of hdmi_db_byte() and > cta_db_byte() helpers and use those instead of the bare index. > But the length checks would still look off, unless we also hide > those in some kind of helpers. Not sure what those would look like > though. Also some blocks can eg. contain multiple descriptors of some > size, and for those the spec defines the byte# relative to the > individual descriptor rather than the whole block. > > -- > Ville Syrjälä > Intel
