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

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 10 Jun 2026, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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.

Come to think of it, I wonder how hard and/or useful it would be to add
some fuzzy lookup into intel_device_info.c based on the signature. It
would bypass the need to add anything here.

>
>> > +				   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().

Maybe should've thought of that when we added i915_vbt... I wonder
what's going to break if we do this?

Anyway, even if we added it, not all VBTs are going to have it. I think
the idea behind the patch at hand is sound, and it should be added as a
fallback. The kernel part can be added separately, and if we follow the
order outlined above, it'll take precedence over the guesses here.

BR,
Jani.


>
>> 
>> >  	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

-- 
Jani Nikula, Intel