Re: [PATCH i-g-t 4/5] tools/intel_vbt_decode: Also dump the second panel (panel_type2)

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

On Fri, 22 Mar 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Modern VBTs can declare two panel types in order to support
> dual panel systems. Dump the panel information for the second
> panel as well. Since panel_type2 could also be declared as 255
> (== match pnpid to EDID) we also add a new command line knob
> to select panel_type2 by hand.
>
> Data for the second panel will be indicated by "(2)", as opposed
> to "(1)" for the first panel.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tools/intel_vbt_decode.c | 38 ++++++++++++++++++++++++++++++++------
>  1 file changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
> index 98f64d0822c6..f223ce2bf5a1 100644
> --- a/tools/intel_vbt_decode.c
> +++ b/tools/intel_vbt_decode.c
> @@ -79,7 +79,7 @@ struct context {
>  	int size;
>  
>  	uint32_t devid;
> -	int panel_type;
> +	int panel_type, panel_type2;
>  	bool dump_all_panel_types;
>  	bool hexdump;
>  };
> @@ -87,12 +87,16 @@ struct context {
>  static bool dump_panel(const struct context *context, int panel_type)
>  {
>  	return panel_type == context->panel_type ||
> +		panel_type == context->panel_type2 ||
>  		context->dump_all_panel_types;
>  }
>  
>  static const char *panel_str(const struct context *context, int panel_type)
>  {
> -	return panel_type == context->panel_type ? " (1)" : "";
> +	return panel_type == context->panel_type &&
> +		panel_type == context->panel_type2 ? " (1)(2)" :
> +		panel_type == context->panel_type ? " (1)" :
> +		panel_type == context->panel_type2 ? " (2)" : "";

This might be better broken down to regular if statements for clarity.

Please clue me in, when is it possible for both panel types to match?


Other than that,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>  }
>  
>  /* Get BDB block size given a pointer to Block ID. */
> @@ -2503,18 +2507,21 @@ static void dump_compression_parameters(struct context *context,
>  }
>  
>  /* get panel type from lvds options block, or -1 if block not found */
> -static int get_panel_type(struct context *context)
> +static int get_panel_type(struct context *context, bool is_panel_type2)
>  {
>  	struct bdb_block *block;
>  	const struct bdb_lvds_options *options;
> -	int panel_type;
> +	int panel_type = -1;
>  
>  	block = find_section(context, BDB_LVDS_OPTIONS);
>  	if (!block)
>  		return -1;
>  
>  	options = block_data(block);
> -	panel_type = options->panel_type;
> +	if (!is_panel_type2)
> +		panel_type = options->panel_type;
> +	else if (context->bdb->version >= 212)
> +		panel_type = options->panel_type2;
>  
>  	free(block);
>  
> @@ -2776,6 +2783,7 @@ enum opt {
>  	OPT_FILE,
>  	OPT_DEVID,
>  	OPT_PANEL_TYPE,
> +	OPT_PANEL_TYPE2,
>  	OPT_ALL_PANELS,
>  	OPT_HEXDUMP,
>  	OPT_BLOCK,
> @@ -2812,6 +2820,7 @@ int main(int argc, char **argv)
>  	int size;
>  	struct context context = {
>  		.panel_type = -1,
> +		.panel_type2 = -1,
>  	};
>  	char *endp;
>  	int block_number = -1;
> @@ -2821,6 +2830,7 @@ int main(int argc, char **argv)
>  		{ "file",	required_argument,	NULL,	OPT_FILE },
>  		{ "devid",	required_argument,	NULL,	OPT_DEVID },
>  		{ "panel-type",	required_argument,	NULL,	OPT_PANEL_TYPE },
> +		{ "panel-type2",	required_argument,	NULL,	OPT_PANEL_TYPE2 },
>  		{ "all-panels",	no_argument,		NULL,	OPT_ALL_PANELS },
>  		{ "hexdump",	no_argument,		NULL,	OPT_HEXDUMP },
>  		{ "block",	required_argument,	NULL,	OPT_BLOCK },
> @@ -2852,6 +2862,14 @@ int main(int argc, char **argv)
>  				return EXIT_FAILURE;
>  			}
>  			break;
> +		case OPT_PANEL_TYPE2:
> +			context.panel_type2 = strtoul(optarg, &endp, 0);
> +			if (*endp || context.panel_type2 > 15) {
> +				fprintf(stderr, "invalid panel type2 '%s'\n",
> +					optarg);
> +				return EXIT_FAILURE;
> +			}
> +			break;
>  		case OPT_ALL_PANELS:
>  			context.dump_all_panel_types = true;
>  			break;
> @@ -2969,12 +2987,20 @@ int main(int argc, char **argv)
>  		fprintf(stderr, "Warning: could not find PCI device ID!\n");
>  
>  	if (context.panel_type == -1)
> -		context.panel_type = get_panel_type(&context);
> +		context.panel_type = get_panel_type(&context, false);
>  	if (context.panel_type == -1) {
>  		fprintf(stderr, "Warning: panel type not set, using 0\n");
>  		context.panel_type = 0;
>  	}
>  
> +	if (context.panel_type2 == -1)
> +		context.panel_type2 = get_panel_type(&context, true);
> +	if (context.panel_type2 != -1 && context.bdb->version < 212) {
> +		fprintf(stderr, "Warning: panel type2 not valid for BDB version %d\n",
> +			context.bdb->version);
> +		context.panel_type2 = -1;
> +	}
> +
>  	if (describe) {
>  		print_description(&context);
>  	} else if (header_only) {

-- 
Jani Nikula, Intel