On Tue, Jan 20, 2026 at 12:46:59PM -0500, [email protected] wrote: > > > > 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.
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. 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
