From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B4CACD262A1 for ; Tue, 20 Jan 2026 20:39:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1926C10E64A; Tue, 20 Jan 2026 20:39:10 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="UdwPtlTw"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.17]) by gabe.freedesktop.org (Postfix) with ESMTPS id 744EC10E221 for ; Tue, 20 Jan 2026 20:39:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1768941549; x=1800477549; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=doWtJm5e0kAz+vXzkK2fTjo81o5cOg5ipGsukbsRUdg=; b=UdwPtlTwXBuJ3rqc6Dv+gCK4wXo5AUhcKVzhisxXY8AKkJemEcrSP2Bu 1M1rNJaXLVJvRv1LNjgvQXcgZf+DHlQZFVOYC1NVd6TVex0LCbkeg5Hyc YLXFBPjaDUAPEg3hya0GEY6FjtBwgKYzD+p4PfjWSXt0sZzgVOGmdTNyc BVhNB9MYgfa9YFnpQkYNP8+4bIPZFttSvFZ1gDPL5RMpOGoQ1m2hoCDMu MqBmtqLb3tZuTkvgqOGl/M6vqMwptTAj7tfoDYBAdU45JRVGsIkdOn0Dl aPOVALiwlVI3vaiJIU7i009MjRHoiHHnwjwrQVJQZiNykpeW3+Eyq3Qnu g==; X-CSE-ConnectionGUID: mO+PKOInRh+NrF3qAK3ZDg== X-CSE-MsgGUID: sLFG7gvPSh+Q70Q3amyMbA== X-IronPort-AV: E=McAfee;i="6800,10657,11677"; a="70134989" X-IronPort-AV: E=Sophos;i="6.21,241,1763452800"; d="scan'208";a="70134989" Received: from fmviesa010.fm.intel.com ([10.60.135.150]) by orvoesa109.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2026 12:39:08 -0800 X-CSE-ConnectionGUID: Upls4MQvSU6vwAuQZF9QmQ== X-CSE-MsgGUID: zb19xNcXSEqwWUeCnowFbA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.21,241,1763452800"; d="scan'208";a="206564018" Received: from smoticic-mobl1.ger.corp.intel.com (HELO localhost) ([10.245.245.13]) by fmviesa010-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2026 12:39:05 -0800 Date: Tue, 20 Jan 2026 22:39:03 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: "jpeisach@ubuntu.com" Cc: Jani Nikula , dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB Message-ID: References: <20260117205139.13991-1-jpeisach@ubuntu.com> <253467F5-8BB3-4A96-A2D8-90785999A023@ubuntu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <253467F5-8BB3-4A96-A2D8-90785999A023@ubuntu.com> X-Patchwork-Hint: comment Organization: Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs Bertel Jungin Aukio 5, 02600 Espoo, Finland X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, Jan 20, 2026 at 03:01:14PM -0500, jpeisach@ubuntu.com wrote: > > > > On Jan 20, 2026, at 1:09 PM, Ville Syrjälä wrote: > > > > On Tue, Jan 20, 2026 at 12:46:59PM -0500, jpeisach@ubuntu.com wrote: > >> > >> > >>> On Jan 20, 2026, at 12:03 PM, Ville Syrjälä > wrote: > >>> > >>> 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ä wrote: > >>>>> > >>>>> On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote: > >>>>>> On Sat, 17 Jan 2026, Joshua Peisach 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 > >>>>>>> --- > >>>>>>> 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