Re: [PATCH i-g-t v2 5/8] tools/vbt_decode: guess devid from VBT signature

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jun 10, 2026 at 05:42:58PM +0300, Jani Nikula wrote:
> On Mon, 08 Jun 2026, Michał Grzelak <michal.grzelak@intel.com> wrote:
> > Nowadays Device ID needs to be passed to decode VBT precisely. VBT lacks
> > it since VBT has been moved from PCI ROM into opregion.
> >
> > However we can narrow from which platform VBT originated basing on VBT
> > signature.
> >
> > Add a command-line option to guess devid using VBT signature. Create an
> > array of signatures and check if any of those is a substring of
> > signature extracted from given VBT. Map each signature into first Device
> > ID of a platform picked from include/drm/intel/pciids.h. Log that devid
> > has not been matched if signature is not found.
> >
> > Note that the platform-specific name should appear after "$VBT ", hence
> > the match.
> >
> > Suggested-by: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
> > ---
> >  tools/intel_vbt_decode.c | 36 +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
> > index 5681643f4b..ec82b00939 100644
> > --- a/tools/intel_vbt_decode.c
> > +++ b/tools/intel_vbt_decode.c
> > @@ -77,6 +77,7 @@ struct context {
> >  	uint32_t devid;
> >  	int panel_type, panel_type2;
> >  	int sdvo_panel_type;
> > +	bool devid_from_signature;
> >  	bool dump_all_panel_types;
> >  	bool hexdump;
> >  };
> > @@ -3693,6 +3694,31 @@ static int get_sdvo_panel_type(struct context *context)
> >  	return panel_type;
> >  }
> >  
> > +static int
> > +get_devid_from_signature(const char *vbt_signature)
> > +{
> > +	int i;
> > +
> > +	const char *signature[] = {"PANTHERLAKE",
> > +				   "LUNARLAKE",
> > +				   "METEORLAKE",
> > +				   "JASPERLAKE",

That's a somewhat weird selection of platforms.

> > +				   NULL};
> > +
> > +	int devid[] = {0xB080, /* PTL */
> > +		       0x6420, /* LNL */
> > +		       0x7D40, /* MTL */
> > +		       0x4E51, /* JSL */
> > +		       0x0};
> 
> Please use a struct, something along the lines of:
> 
> 	static const struct {
> 		const char *signature;
> 		uint16_t devid;
> 	} devids[] = {
> 		{ .signature = "PANTHERLAKE", .devid = 0xb080, },
> 		...
> 	};
> 
> Then you can also ditch the /* PTL */ etc. comments.
> 
> > +
> > +	for (i = 0; signature[i]; i++)
> > +		if (strstr(vbt_signature, signature[i]))
> > +			return devid[i];
> > +
> > +	fprintf(stderr, "Warning: cannot find devid for signature %s\n", vbt_signature);
> 
> This should be a fallback, please don't warn anything here. Only one
> warning for the whole devid thing.
> 
> > +	return 0;
> > +}
> > +
> >  static int
> >  get_device_id(unsigned char *bios, int size)
> >  {
> > @@ -4108,6 +4134,7 @@ enum opt {
> >  	OPT_PANEL_TYPE2,
> >  	OPT_PANEL_EDID,
> >  	OPT_PANEL_EDID2,
> > +	OPT_VBT_DEVID,
> >  	OPT_ALL_PANELS,
> >  	OPT_HEXDUMP,
> >  	OPT_BLOCK,
> > @@ -4162,6 +4189,8 @@ int main(int argc, char **argv)
> >  		{ "panel-edid",	required_argument,	NULL,	OPT_PANEL_EDID },
> >  		{ "panel-type2",	required_argument,	NULL,	OPT_PANEL_TYPE2 },
> >  		{ "panel-edid2",	required_argument,	NULL,	OPT_PANEL_EDID2 },
> > +		{ "devid-from-signature",
> > +				no_argument,		NULL,	OPT_VBT_DEVID },
> >  		{ "all-panels",	no_argument,		NULL,	OPT_ALL_PANELS },
> >  		{ "hexdump",	no_argument,		NULL,	OPT_HEXDUMP },
> >  		{ "block",	required_argument,	NULL,	OPT_BLOCK },
> > @@ -4207,6 +4236,9 @@ int main(int argc, char **argv)
> >  		case OPT_PANEL_EDID2:
> >  			panel_edid2 = optarg;
> >  			break;
> > +		case OPT_VBT_DEVID:
> > +			context.devid_from_signature = true;
> > +			break;
> >  		case OPT_ALL_PANELS:
> >  			context.dump_all_panel_types = true;
> >  			break;
> > @@ -4313,6 +4345,8 @@ int main(int argc, char **argv)
> >  	context.bdb = (const struct bdb_header *)(VBIOS + bdb_off);
> >  	context.size = size;
> >  
> > +	if (!context.devid && context.devid_from_signature)
> > +		context.devid = get_devid_from_signature((char *) vbt->signature);
> 
> This should be last. The priority should be:
> 
> 	1. command-line, if set
> 	2. environment variable, if set
> 	3. VBIOS, if available
> 	4. deduce from signature, if requested
> 
> Come to think of it, not sure about the "if requested". Maybe just use
> it if not otherwise specified. This is userspace decoding, not kernel,
> after all.
> 
> Ville, thoughts?

Yeah, could probably just take the fallback when we don't know
any better.

But I'm thinking we should just add the devid to the VBT dump produced
by the kernel. Would avoid having to guess, especially since vendors
have been known to sometime reuse older VBTs across platforms. Perhaps
just prepend the VBT image with a PCIR structure which can then be
parsed by the code in get_device_id().

> 
> >  	if (!context.devid) {
> >  		const char *devid_string = getenv("DEVICE");
> >  		if (devid_string) {
> > @@ -4326,7 +4360,7 @@ int main(int argc, char **argv)
> >  	if (!context.devid)
> >  		context.devid = get_device_id(VBIOS, size);
> >  	if (!context.devid)
> > -		fprintf(stderr, "Warning: could not find PCI device ID!\n");
> > +		fprintf(stderr, "Warning: could not find PCI device ID! Use --devid or --devid-from-signature\n");
> >  
> >  	if (context.panel_type == -1)
> >  		context.panel_type = get_panel_type(&context, false);
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel