On Tue, Jan 20, 2026 at 03:01:14PM -0500, [email protected] wrote: > > > > 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?
Nothing is indexed using the CTA byte#. To do that we'd have to do something like 'u8 *db = start_of_db - 1'. I can more or less see these variants: 1. relative to the start of the data block (eg. drm_parse_hdr_metadata_block(), drm_parse_vcdb()) 2. relative to the start of the payload (eg. parse_cta_vdb()) 3. relative to the first byte past the extended tag (y420* stuff) And I'm suggesting that perhaps everything should use either 2 or 3 depending on whether the extended tag is present. -- Ville Syrjälä Intel
