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

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

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",
> +				   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?

>  	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