On Wed, 04 May 2016, Marius Vlad <marius.c.vlad@xxxxxxxxx> wrote:
> On Tue, May 03, 2016 at 05:18:54PM +0300, Jani Nikula wrote:
>> Not sure it's a great idea to do platform specific parsing of the BIOS,
>> but at least make it possible to pass in the devid on the command line
>> and not just the environment.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> tools/intel_bios_reader.c | 35 ++++++++++++++++++++++++-----------
>> 1 file changed, 24 insertions(+), 11 deletions(-)
>>
>> diff --git a/tools/intel_bios_reader.c b/tools/intel_bios_reader.c
>> index b424b17e4852..d74e766250df 100644
>> --- a/tools/intel_bios_reader.c
>> +++ b/tools/intel_bios_reader.c
>> @@ -41,7 +41,7 @@
>> #include "intel_chipset.h"
>> #include "drmtest.h"
>>
>> -static uint32_t devid = -1;
>> +static uint32_t devid;
>>
>> /* no bother to include "edid.h" */
>> #define _H_ACTIVE(x) (x[2] + ((x[4] & 0xF0) << 4))
>> @@ -149,8 +149,10 @@ static void dump_general_features(const struct bdb_header *bdb,
>> printf("\tExternal VBT: %s\n", YESNO(features->download_ext_vbt));
>> printf("\tEnable SSC: %s\n", YESNO(features->enable_ssc));
>> if (features->enable_ssc) {
>> - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid) ||
>> - IS_BROXTON(devid))
>> + if (!devid)
>> + printf("\tSSC frequency: <unknown platform>\n");
>> + else if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid) ||
>> + IS_BROXTON(devid))
>> printf("\tSSC frequency: 100 MHz\n");
>> else if (HAS_PCH_SPLIT(devid))
>> printf("\tSSC frequency: %s\n", features->ssc_freq ?
>> @@ -1375,6 +1377,7 @@ enum opt {
>> OPT_UNKNOWN = '?',
>> OPT_END = -1,
>> OPT_FILE,
>> + OPT_DEVID,
>> };
>>
>> int main(int argc, char **argv)
>> @@ -1392,10 +1395,11 @@ int main(int argc, char **argv)
>> struct bdb_block *block;
>> struct bdb_header *bdb;
>> char signature[17];
>> - char *devid_string;
>> + char *endp;
>>
>> static struct option options[] = {
>> { "file", required_argument, NULL, OPT_FILE },
>> + { "devid", required_argument, NULL, OPT_DEVID },
>> { 0 }
>> };
>>
>> @@ -1406,6 +1410,13 @@ int main(int argc, char **argv)
>> case OPT_FILE:
>> filename = optarg;
>> break;
>> + case OPT_DEVID:
>> + devid = strtoul(optarg, &endp, 16);
>> + if (!devid || *endp) {
>> + fprintf(stderr, "invalid devid '%s'\n", optarg);
>> + return EXIT_FAILURE;
>> + }
>> + break;
>> case OPT_END:
>> break;
>> case OPT_UNKNOWN:
>> @@ -1426,9 +1437,6 @@ int main(int argc, char **argv)
>> }
>> }
>>
>> - if ((devid_string = getenv("DEVICE")))
>> - devid = strtoul(devid_string, NULL, 0);
>> -
>> fd = open(filename, O_RDONLY);
>> if (fd == -1) {
>> printf("Couldn't open \"%s\": %s\n", filename, strerror(errno));
>> @@ -1506,10 +1514,15 @@ int main(int argc, char **argv)
>> }
>> printf("\n");
>>
>> - if (devid == -1)
>> - devid = get_device_id(VBIOS, size);
>> - if (devid == -1)
>> - printf("Warning: could not find PCI device ID!\n");
>> + if (!devid) {
>> + const char *devid_string = getenv("DEVICE");
>> + if (devid_string)
>> + devid = strtoul(devid_string, NULL, 0);
> Wouldn't this allow to pass either base 10, 16 or 8? (as an argument
> devid seems to be always specified in base 16).
That's a change in how the DEVICE environment variable was handled
previously, but indeed should be changed.
Thanks,
Jani.
>> + }
>> + if (!devid)
>> + devid = get_device_id(VBIOS, size);
>> + if (!devid)
>> + fprintf(stderr, "Warning: could not find PCI device ID!\n");
>>
>> dump_section(bdb, BDB_GENERAL_FEATURES, size);
>> dump_section(bdb, BDB_GENERAL_DEFINITIONS, size);
>> --
>> 2.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
