[PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[Joshua Peisach]

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 <jpeisach@ubuntu.com>
---
 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) {
 		dc_bpc = 10;
 		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_30;
 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 30.\n",
 			    connector->base.id, connector->name);
 	}
 
-	if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
+	if (db->data[6] & DRM_EDID_HDMI_DC_36) {
 		dc_bpc = 12;
 		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_36;
 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 36.\n",
 			    connector->base.id, connector->name);
 	}
 
-	if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
+	if (db->data[6] & DRM_EDID_HDMI_DC_48) {
 		dc_bpc = 16;
 		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_48;
 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 48.\n",
@@ -6333,7 +6333,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 	info->bpc = dc_bpc;
 
 	/* YCRCB444 is optional according to spec. */
-	if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) {
+	if (db->data[6] & DRM_EDID_HDMI_DC_Y444) {
 		info->edid_hdmi_ycbcr444_dc_modes = info->edid_hdmi_rgb444_dc_modes;
 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does YCRCB444 in deep color.\n",
 			    connector->base.id, connector->name);
@@ -6343,7 +6343,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 	 * Spec says that if any deep color mode is supported at all,
 	 * then deep color 36 bit must be supported.
 	 */
-	if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
+	if (!(db->data[6] & DRM_EDID_HDMI_DC_36)) {
 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink should do DC_36, but does not!\n",
 			    connector->base.id, connector->name);
 	}
@@ -6351,19 +6351,19 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 
 /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */
 static void
-drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db)
+drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const struct cea_db *db)
 {
 	struct drm_display_info *info = &connector->display_info;
 	u8 len = cea_db_payload_len(db);
 
 	info->is_hdmi = true;
 
-	info->source_physical_address = (db[4] << 8) | db[5];
+	info->source_physical_address = (db->data[4] << 8) | db->data[5];
 
 	if (len >= 6)
-		info->dvi_dual = db[6] & 1;
+		info->dvi_dual = db->data[6] & 1;
 	if (len >= 7)
-		info->max_tmds_clock = db[7] * 5000;
+		info->max_tmds_clock = db->data[7] * 5000;
 
 	/*
 	 * Try to infer whether the sink supports HDMI infoframes.
@@ -6371,7 +6371,7 @@ drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db)
 	 * HDMI infoframe support was first added in HDMI 1.4. Assume the sink
 	 * supports infoframes if HDMI_Video_present is set.
 	 */
-	if (len >= 8 && db[8] & BIT(5))
+	if (len >= 8 && db->data[8] & BIT(5))
 		info->has_hdmi_infoframe = true;
 
 	drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI: DVI dual %d, max TMDS clock %d kHz\n",
@@ -6443,7 +6443,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 		const u8 *data = (const u8 *)db;
 
 		if (cea_db_is_hdmi_vsdb(db))
-			drm_parse_hdmi_vsdb_video(connector, data);
+			drm_parse_hdmi_vsdb_video(connector, db);
 		else if (cea_db_is_hdmi_forum_vsdb(db) ||
 			 cea_db_is_hdmi_forum_scdb(db))
 			drm_parse_hdmi_forum_scds(connector, data);
-- 
2.51.0

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[Jani Nikula]

On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
> ---
>  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.

BR,
Jani.


>  		dc_bpc = 10;
>  		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_30;
>  		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 30.\n",
>  			    connector->base.id, connector->name);
>  	}
>  
> -	if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
> +	if (db->data[6] & DRM_EDID_HDMI_DC_36) {
>  		dc_bpc = 12;
>  		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_36;
>  		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 36.\n",
>  			    connector->base.id, connector->name);
>  	}
>  
> -	if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
> +	if (db->data[6] & DRM_EDID_HDMI_DC_48) {
>  		dc_bpc = 16;
>  		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_48;
>  		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 48.\n",
> @@ -6333,7 +6333,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  	info->bpc = dc_bpc;
>  
>  	/* YCRCB444 is optional according to spec. */
> -	if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) {
> +	if (db->data[6] & DRM_EDID_HDMI_DC_Y444) {
>  		info->edid_hdmi_ycbcr444_dc_modes = info->edid_hdmi_rgb444_dc_modes;
>  		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does YCRCB444 in deep color.\n",
>  			    connector->base.id, connector->name);
> @@ -6343,7 +6343,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  	 * Spec says that if any deep color mode is supported at all,
>  	 * then deep color 36 bit must be supported.
>  	 */
> -	if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
> +	if (!(db->data[6] & DRM_EDID_HDMI_DC_36)) {
>  		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink should do DC_36, but does not!\n",
>  			    connector->base.id, connector->name);
>  	}
> @@ -6351,19 +6351,19 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  
>  /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */
>  static void
> -drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db)
> +drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const struct cea_db *db)
>  {
>  	struct drm_display_info *info = &connector->display_info;
>  	u8 len = cea_db_payload_len(db);
>  
>  	info->is_hdmi = true;
>  
> -	info->source_physical_address = (db[4] << 8) | db[5];
> +	info->source_physical_address = (db->data[4] << 8) | db->data[5];
>  
>  	if (len >= 6)
> -		info->dvi_dual = db[6] & 1;
> +		info->dvi_dual = db->data[6] & 1;
>  	if (len >= 7)
> -		info->max_tmds_clock = db[7] * 5000;
> +		info->max_tmds_clock = db->data[7] * 5000;
>  
>  	/*
>  	 * Try to infer whether the sink supports HDMI infoframes.
> @@ -6371,7 +6371,7 @@ drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db)
>  	 * HDMI infoframe support was first added in HDMI 1.4. Assume the sink
>  	 * supports infoframes if HDMI_Video_present is set.
>  	 */
> -	if (len >= 8 && db[8] & BIT(5))
> +	if (len >= 8 && db->data[8] & BIT(5))
>  		info->has_hdmi_infoframe = true;
>  
>  	drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI: DVI dual %d, max TMDS clock %d kHz\n",
> @@ -6443,7 +6443,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>  		const u8 *data = (const u8 *)db;
>  
>  		if (cea_db_is_hdmi_vsdb(db))
> -			drm_parse_hdmi_vsdb_video(connector, data);
> +			drm_parse_hdmi_vsdb_video(connector, db);
>  		else if (cea_db_is_hdmi_forum_vsdb(db) ||
>  			 cea_db_is_hdmi_forum_scdb(db))
>  			drm_parse_hdmi_forum_scds(connector, data);

-- 
Jani Nikula, Intel

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[jpeisach]

[-- Attachment #1: Type: text/plain, Size: 5794 bytes --]



> On Jan 19, 2026, at 8:39 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> 
> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com>> 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 <jpeisach@ubuntu.com>
>> ---
>> 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.
> 

Okay, I'll thought the pointer was to the literal same information.

Keep me posted about the indexing of the specs, I will probably submit a v2.

Thanks,
-Josh

> BR,
> Jani.
> 
> 
>> 		dc_bpc = 10;
>> 		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_30;
>> 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 30.\n",
>> 			    connector->base.id, connector->name);
>> 	}
>> 
>> -	if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
>> +	if (db->data[6] & DRM_EDID_HDMI_DC_36) {
>> 		dc_bpc = 12;
>> 		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_36;
>> 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 36.\n",
>> 			    connector->base.id, connector->name);
>> 	}
>> 
>> -	if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
>> +	if (db->data[6] & DRM_EDID_HDMI_DC_48) {
>> 		dc_bpc = 16;
>> 		info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_48;
>> 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does deep color 48.\n",
>> @@ -6333,7 +6333,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>> 	info->bpc = dc_bpc;
>> 
>> 	/* YCRCB444 is optional according to spec. */
>> -	if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) {
>> +	if (db->data[6] & DRM_EDID_HDMI_DC_Y444) {
>> 		info->edid_hdmi_ycbcr444_dc_modes = info->edid_hdmi_rgb444_dc_modes;
>> 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink does YCRCB444 in deep color.\n",
>> 			    connector->base.id, connector->name);
>> @@ -6343,7 +6343,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>> 	 * Spec says that if any deep color mode is supported at all,
>> 	 * then deep color 36 bit must be supported.
>> 	 */
>> -	if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
>> +	if (!(db->data[6] & DRM_EDID_HDMI_DC_36)) {
>> 		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI sink should do DC_36, but does not!\n",
>> 			    connector->base.id, connector->name);
>> 	}
>> @@ -6351,19 +6351,19 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>> 
>> /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */
>> static void
>> -drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db)
>> +drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const struct cea_db *db)
>> {
>> 	struct drm_display_info *info = &connector->display_info;
>> 	u8 len = cea_db_payload_len(db);
>> 
>> 	info->is_hdmi = true;
>> 
>> -	info->source_physical_address = (db[4] << 8) | db[5];
>> +	info->source_physical_address = (db->data[4] << 8) | db->data[5];
>> 
>> 	if (len >= 6)
>> -		info->dvi_dual = db[6] & 1;
>> +		info->dvi_dual = db->data[6] & 1;
>> 	if (len >= 7)
>> -		info->max_tmds_clock = db[7] * 5000;
>> +		info->max_tmds_clock = db->data[7] * 5000;
>> 
>> 	/*
>> 	 * Try to infer whether the sink supports HDMI infoframes.
>> @@ -6371,7 +6371,7 @@ drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db)
>> 	 * HDMI infoframe support was first added in HDMI 1.4. Assume the sink
>> 	 * supports infoframes if HDMI_Video_present is set.
>> 	 */
>> -	if (len >= 8 && db[8] & BIT(5))
>> +	if (len >= 8 && db->data[8] & BIT(5))
>> 		info->has_hdmi_infoframe = true;
>> 
>> 	drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI: DVI dual %d, max TMDS clock %d kHz\n",
>> @@ -6443,7 +6443,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>> 		const u8 *data = (const u8 *)db;
>> 
>> 		if (cea_db_is_hdmi_vsdb(db))
>> -			drm_parse_hdmi_vsdb_video(connector, data);
>> +			drm_parse_hdmi_vsdb_video(connector, db);
>> 		else if (cea_db_is_hdmi_forum_vsdb(db) ||
>> 			 cea_db_is_hdmi_forum_scdb(db))
>> 			drm_parse_hdmi_forum_scds(connector, data);
> 
> -- 
> Jani Nikula, Intel


[-- Attachment #2: Type: text/html, Size: 27682 bytes --]

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[Ville Syrjälä]

On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
> > ---
> >  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

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[jpeisach]

> On Jan 20, 2026, at 10:41 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
>> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
>>> ---
>>> 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 :)

-Josh

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[Ville Syrjälä]

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ä <ville.syrjala@linux.intel.com> wrote:
> > 
> > On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
> >> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
> >>> ---
> >>> 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

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[jpeisach]

[-- Attachment #1: Type: text/plain, Size: 3901 bytes --]



> On Jan 20, 2026, at 12:03 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> On Tue, Jan 20, 2026 at 11:37:59AM -0500, jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com> wrote:
>> 
>> 
>>> On Jan 20, 2026, at 10:41 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>> 
>>> On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
>>>> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
>>>>> ---
>>>>> 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

[-- Attachment #2: Type: text/html, Size: 15868 bytes --]

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[Ville Syrjälä]

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ä <ville.syrjala@linux.intel.com> wrote:
> > 
> > On Tue, Jan 20, 2026 at 11:37:59AM -0500, jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com> wrote:
> >> 
> >> 
> >>> On Jan 20, 2026, at 10:41 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >>> 
> >>> On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
> >>>> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
> >>>>> ---
> >>>>> 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

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[jpeisach]

[-- Attachment #1: Type: text/plain, Size: 5414 bytes --]



> On Jan 20, 2026, at 1:09 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> On Tue, Jan 20, 2026 at 12:46:59PM -0500, jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com> wrote:
>> 
>> 
>>> On Jan 20, 2026, at 12:03 PM, Ville Syrjälä <ville.syrjala@linux.intel.com <mailto:ville.syrjala@linux.intel.com>> wrote:
>>> 
>>> On Tue, Jan 20, 2026 at 11:37:59AM -0500, jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com> <mailto:jpeisach@ubuntu.com> wrote:
>>>> 
>>>> 
>>>>> On Jan 20, 2026, at 10:41 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>>>> 
>>>>> On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
>>>>>> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
>>>>>>> ---
>>>>>>> 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


[-- Attachment #2: Type: text/html, Size: 21992 bytes --]

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[Ville Syrjälä]

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ä <ville.syrjala@linux.intel.com> wrote:
> > 
> > On Tue, Jan 20, 2026 at 12:46:59PM -0500, jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com> wrote:
> >> 
> >> 
> >>> On Jan 20, 2026, at 12:03 PM, Ville Syrjälä <ville.syrjala@linux.intel.com <mailto:ville.syrjala@linux.intel.com>> wrote:
> >>> 
> >>> On Tue, Jan 20, 2026 at 11:37:59AM -0500, jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com> <mailto:jpeisach@ubuntu.com> wrote:
> >>>> 
> >>>> 
> >>>>> On Jan 20, 2026, at 10:41 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >>>>> 
> >>>>> On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
> >>>>>> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
> >>>>>>> ---
> >>>>>>> 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

Re: [PATCH] drm/edid: Use struct cea_db when parsing HDMI VSDB

[Joshua Peisach]

On Tue Jan 20, 2026 at 3:39 PM EST, Ville Syrjälä wrote:
> 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ä <ville.syrjala@linux.intel.com> wrote:
>> > 
>> > On Tue, Jan 20, 2026 at 12:46:59PM -0500, jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com> wrote:
>> >> 
>> >> 
>> >>> On Jan 20, 2026, at 12:03 PM, Ville Syrjälä <ville.syrjala@linux.intel.com <mailto:ville.syrjala@linux.intel.com>> wrote:
>> >>> 
>> >>> On Tue, Jan 20, 2026 at 11:37:59AM -0500, jpeisach@ubuntu.com <mailto:jpeisach@ubuntu.com> <mailto:jpeisach@ubuntu.com> wrote:
>> >>>> 
>> >>>> 
>> >>>>> On Jan 20, 2026, at 10:41 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> >>>>> 
>> >>>>> On Mon, Jan 19, 2026 at 03:39:12PM +0200, Jani Nikula wrote:
>> >>>>>> On Sat, 17 Jan 2026, Joshua Peisach <jpeisach@ubuntu.com> 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 <jpeisach@ubuntu.com>
>> >>>>>>> ---
>> >>>>>>> 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.

Makes sense to me.

However, as for "drm_parse_hdmi_vsdb_video", that is not specific to
CTA. So I guess keep the function names and just correct the indexes?

--
Josh