Re: [PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
On 07/06/17 08:12 PM, Emil Velikov wrote:
> On 7 June 2017 at 09:40, Michel Dänzer wrote:
>> On 06/06/17 10:43 PM, Emil Velikov wrote:
>>> On 31 May 2017 at 21:22, Samuel Li wrote:
>>>
--- /dev/null
+++ b/amdgpu/amdgpu_asic_id.c
>>>
+static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
+{
+ char *buf, *saveptr;
+ char *s_did;
+ char *s_rid;
+ char *s_name;
+ char *endptr;
+ int r = 0;
+
+ buf = strdup(line);
>>> You don't need the extra strdup here if you use strchr over strtok.
>>
>> Beware that without strdup here, amdgpu_parse_asic_ids must set line =
>> NULL after table_line++, so that getline allocates a new buffer for the
>> next line.
>>
> A simple "line = NULL" will lead to a memory leak, AFAICT.
>
> In either case, I'm a bit baffled how that is affected by the
> presence/lack of strdup?
> We don't alter or reuse the backing storage only
> strcmp/isblank/strtol/strdup it.
Oh, I missed that id->marketing_name is strdup'd again.
Anyway, it's probably better not to change the logic too much at this
point, other than anything needed to fix immediate bugs. It can always
be improved with follow-up patches.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
On 7 June 2017 at 09:40, Michel Dänzer wrote:
> On 06/06/17 10:43 PM, Emil Velikov wrote:
>> On 31 May 2017 at 21:22, Samuel Li wrote:
>>
>>> --- /dev/null
>>> +++ b/amdgpu/amdgpu_asic_id.c
>>
>>> +static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
>>> +{
>>> + char *buf, *saveptr;
>>> + char *s_did;
>>> + char *s_rid;
>>> + char *s_name;
>>> + char *endptr;
>>> + int r = 0;
>>> +
>>> + buf = strdup(line);
>> You don't need the extra strdup here if you use strchr over strtok.
>
> Beware that without strdup here, amdgpu_parse_asic_ids must set line =
> NULL after table_line++, so that getline allocates a new buffer for the
> next line.
>
A simple "line = NULL" will lead to a memory leak, AFAICT.
In either case, I'm a bit baffled how that is affected by the
presence/lack of strdup?
We don't alter or reuse the backing storage only
strcmp/isblank/strtol/strdup it.
>
>>> +int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table)
>>> +{
>>
>>> + /* 1st valid line is file version */
>>> + while ((n = getline(&line, &len, fp)) != -1) {
>>> + /* trim trailing newline */
>>> + if (line[n - 1] == '\n')
>>> + line[n - 1] = '\0';
>> Why do we need this - afaict none of the parsing code cares if we have
>> \n or not?
>
> The \n has to be removed somehow, otherwise it ends up as part of the
> marketing name returned to the application.
>
Wouldn't be better to do that in parse_one_line() around the
marketing_name = strdup(...) call?
It's a matter of taste, so feel free to ignore me.
>
>>> + /* end of table */
>>> + id = asic_id_table + table_size;
>>> + memset(id, 0, sizeof(struct amdgpu_asic_id));
>> Here one clears the sentinel, which is needed as we hit realloc above,
>> correct?
>>
>>> + asic_id_table = realloc(asic_id_table, (table_size+1) *
>>> + sizeof(struct amdgpu_asic_id));
>> But why do we realloc again?
>
> I asked for that, in order not to waste memory for unused table entries.
>
D'oh, indeed. Thank you. Worth adding a note?
>
>>> +free:
>>> + free(line);
>>> +
>>> + if (r && asic_id_table) {
>>> + while (table_size--) {
>>> + id = asic_id_table + table_size;
>>> + free(id->marketing_name);
>>> + }
>>> + free(asic_id_table);
>>> + asic_id_table = NULL;
>>> + }
>>> +close:
>>> + fclose(fp);
>>> +
>>> + *p_asic_id_table = asic_id_table;
>>> +
>> Please don't entwine the error path with the normal one.
>>
>> Setting *p_asic_id_table (or any user provided pointer) when the
>> function fails is bad design.
>
> I don't really see the issue with that; it's fine for the only caller of
> this function.
>
it's not obvious and might come to bite. Since *p_asic_id_table is
already NULL (we're using calloc) I'd opt for dropping it.
Not trying to force my opinion, just stating concerns.
Another crazy idea that just came to mind:
Since getline() can do multiple implicit realloc's one can allocate "a
sane" default and feed that instead of the current NULL.
Regards,
Emil
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
On 06/06/17 10:43 PM, Emil Velikov wrote:
> On 31 May 2017 at 21:22, Samuel Li wrote:
>
>> --- /dev/null
>> +++ b/amdgpu/amdgpu_asic_id.c
>
>> +static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
>> +{
>> + char *buf, *saveptr;
>> + char *s_did;
>> + char *s_rid;
>> + char *s_name;
>> + char *endptr;
>> + int r = 0;
>> +
>> + buf = strdup(line);
> You don't need the extra strdup here if you use strchr over strtok.
Beware that without strdup here, amdgpu_parse_asic_ids must set line =
NULL after table_line++, so that getline allocates a new buffer for the
next line.
>> +int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table)
>> +{
>
>> + /* 1st valid line is file version */
>> + while ((n = getline(&line, &len, fp)) != -1) {
>> + /* trim trailing newline */
>> + if (line[n - 1] == '\n')
>> + line[n - 1] = '\0';
> Why do we need this - afaict none of the parsing code cares if we have
> \n or not?
The \n has to be removed somehow, otherwise it ends up as part of the
marketing name returned to the application.
>> + /* end of table */
>> + id = asic_id_table + table_size;
>> + memset(id, 0, sizeof(struct amdgpu_asic_id));
> Here one clears the sentinel, which is needed as we hit realloc above,
> correct?
>
>> + asic_id_table = realloc(asic_id_table, (table_size+1) *
>> + sizeof(struct amdgpu_asic_id));
> But why do we realloc again?
I asked for that, in order not to waste memory for unused table entries.
>> +free:
>> + free(line);
>> +
>> + if (r && asic_id_table) {
>> + while (table_size--) {
>> + id = asic_id_table + table_size;
>> + free(id->marketing_name);
>> + }
>> + free(asic_id_table);
>> + asic_id_table = NULL;
>> + }
>> +close:
>> + fclose(fp);
>> +
>> + *p_asic_id_table = asic_id_table;
>> +
> Please don't entwine the error path with the normal one.
>
> Setting *p_asic_id_table (or any user provided pointer) when the
> function fails is bad design.
I don't really see the issue with that; it's fine for the only caller of
this function.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
Hi Samuel,
With all the other discussion aside here is some code specific input
which I'd hope you agree with.
On 31 May 2017 at 21:22, Samuel Li wrote:
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -45,6 +45,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
>
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm.pc
> +libdrmdatadir = $(datadir)/libdrm
> +dist_libdrmdata_DATA = include/drm/amdgpu.ids
> +export libdrmdatadir
Don't place exports in the makefiles. See how pkgconfigdir is managed
in configure.ac and do do with libdrmdatadir.
> --- /dev/null
> +++ b/amdgpu/amdgpu_asic_id.c
> +static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
> +{
> + char *buf, *saveptr;
> + char *s_did;
> + char *s_rid;
> + char *s_name;
> + char *endptr;
> + int r = 0;
> +
> + buf = strdup(line);
You don't need the extra strdup here if you use strchr over strtok.
> + if (!buf)
> + return -ENOMEM;
> +
> + /* ignore empty line and commented line */
> + if (strlen(line) == 0 || line[0] == '#') {
You might want to check/trim whitespace before #? Same question applies below.
> + r = -EAGAIN;
> + goto out;
With the strdup, hence free() gone, all the errors will be "return -EFOO;"
> + /* trim leading whitespaces or tabs */
> + while (*s_name == ' ' || *s_name == '\t')
> + s_name++;
Use isblank/isspace - they should handle \r et al, which will creep as
someone on the team uses Windows ;-)
> +int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table)
> +{
> + /* 1st valid line is file version */
> + while ((n = getline(&line, &len, fp)) != -1) {
> + /* trim trailing newline */
> + if (line[n - 1] == '\n')
> + line[n - 1] = '\0';
Why do we need this - afaict none of the parsing code cares if we have
\n or not?
Same question applies for the second loop, below.
> +
> + /* ignore empty line and commented line */
> + if (strlen(line) == 0 || line[0] == '#') {
> + line_num++;
> + continue;
> + }
> +
> + drmMsg("%s version: %s\n", AMDGPU_ASIC_ID_TABLE, line);
> + break;
> + }
> +
Should the lack of a version line/tag be considered fatal or not? An
inline comment is welcome.
> + while ((n = getline(&line, &len, fp)) != -1) {
> +
> + if (table_size >= table_max_size) {
Move it at the top of the loop, since as-is it will end up reallocate
even when it doesn't need to.
> + /* double table size */
> + table_max_size *= 2;
> + asic_id_table = realloc(asic_id_table, table_max_size
> *
> + sizeof(struct
> amdgpu_asic_id));
> + if (!asic_id_table) {
Memory originally pointed by asic_id_table is leaked.
> + r = -ENOMEM;
> + goto free;
> + }
> + }
> + }
> +
> + /* end of table */
> + id = asic_id_table + table_size;
> + memset(id, 0, sizeof(struct amdgpu_asic_id));
Here one clears the sentinel, which is needed as we hit realloc above, correct?
> + asic_id_table = realloc(asic_id_table, (table_size+1) *
> + sizeof(struct amdgpu_asic_id));
But why do we realloc again?
> + if (!asic_id_table)
As above - we have a leak original asic_id_table.
> + r = -ENOMEM;
> +
> +free:
> + free(line);
> +
> + if (r && asic_id_table) {
> + while (table_size--) {
> + id = asic_id_table + table_size;
> + free(id->marketing_name);
> + }
> + free(asic_id_table);
> + asic_id_table = NULL;
> + }
> +close:
> + fclose(fp);
> +
> + *p_asic_id_table = asic_id_table;
> +
Please don't entwine the error path with the normal one.
Setting *p_asic_id_table (or any user provided pointer) when the
function fails is bad design.
Regards,
Emil
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
> what is the purpose of the file version
The initial purpose is to act like a version tag, not necessarily format
related.
Sam
-Original Message-
From: Michel Dänzer [mailto:mic...@daenzer.net]
Sent: Sunday, June 04, 2017 10:10 PM
To: Li, Samuel
Cc: amd-gfx@lists.freedesktop.org; Yuan, Xiaojie
Subject: Re: [PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
On 01/06/17 05:22 AM, Samuel Li wrote:
> From: Xiaojie Yuan
>
> v2: fix an off by one error and leading white spaces
> v3: use thread safe strtok_r(); initialize len before calling getline();
> change printf() to drmMsg(); add initial amdgpu.ids
> v4: integrate some recent internal changes, including format changes
> v5: fix line number for empty/commented lines; realloc to save memory;
> indentation changes
> v6: remove a line error
>
> Change-Id: I12216da14910f5e2b0970bc1fafc2a20b0ef1ba9
> Reviewed-by: Junwei Zhang
> Signed-off-by: Samuel Li
[...]
> + /* 1st valid line is file version */
> + while ((n = getline(&line, &len, fp)) != -1) {
> + /* trim trailing newline */
> + if (line[n - 1] == '\n')
> + line[n - 1] = '\0';
> +
> + /* ignore empty line and commented line */
> + if (strlen(line) == 0 || line[0] == '#') {
> + line_num++;
> + continue;
> + }
> +
> + drmMsg("%s version: %s\n", AMDGPU_ASIC_ID_TABLE, line);
> + break;
> + }
BTW, what is the purpose of the file version? If it's about the format of the
file, we need to check here that the file has a format we can parse, something
like
if ( != 1)
return -EINVAL;
Note that making backwards incompatible changes to the file format would pretty
much kill the idea of updating the file with a script like update-pciids.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
On 01/06/17 05:22 AM, Samuel Li wrote:
> From: Xiaojie Yuan
>
> v2: fix an off by one error and leading white spaces
> v3: use thread safe strtok_r(); initialize len before calling getline();
> change printf() to drmMsg(); add initial amdgpu.ids
> v4: integrate some recent internal changes, including format changes
> v5: fix line number for empty/commented lines; realloc to save memory;
> indentation changes
> v6: remove a line error
>
> Change-Id: I12216da14910f5e2b0970bc1fafc2a20b0ef1ba9
> Reviewed-by: Junwei Zhang
> Signed-off-by: Samuel Li
[...]
> + /* 1st valid line is file version */
> + while ((n = getline(&line, &len, fp)) != -1) {
> + /* trim trailing newline */
> + if (line[n - 1] == '\n')
> + line[n - 1] = '\0';
> +
> + /* ignore empty line and commented line */
> + if (strlen(line) == 0 || line[0] == '#') {
> + line_num++;
> + continue;
> + }
> +
> + drmMsg("%s version: %s\n", AMDGPU_ASIC_ID_TABLE, line);
> + break;
> + }
BTW, what is the purpose of the file version? If it's about the format
of the file, we need to check here that the file has a format we can
parse, something like
if ( != 1)
return -EINVAL;
Note that making backwards incompatible changes to the file format would
pretty much kill the idea of updating the file with a script like
update-pciids.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
On Wed, May 31, 2017 at 4:22 PM, Samuel Li wrote:
> From: Xiaojie Yuan
>
> v2: fix an off by one error and leading white spaces
> v3: use thread safe strtok_r(); initialize len before calling getline();
> change printf() to drmMsg(); add initial amdgpu.ids
> v4: integrate some recent internal changes, including format changes
> v5: fix line number for empty/commented lines; realloc to save memory;
> indentation changes
> v6: remove a line error
>
> Change-Id: I12216da14910f5e2b0970bc1fafc2a20b0ef1ba9
> Reviewed-by: Junwei Zhang
> Signed-off-by: Samuel Li
Reviewed-by: Alex Deucher
> ---
> Makefile.am | 3 +
> amdgpu/Makefile.am | 2 +
> amdgpu/Makefile.sources | 2 +-
> amdgpu/amdgpu_asic_id.c | 211
> +++
> amdgpu/amdgpu_asic_id.h | 165
> amdgpu/amdgpu_device.c | 28 +--
> amdgpu/amdgpu_internal.h | 10 +++
> include/drm/amdgpu.ids | 170 ++
> 8 files changed, 418 insertions(+), 173 deletions(-)
> create mode 100644 amdgpu/amdgpu_asic_id.c
> delete mode 100644 amdgpu/amdgpu_asic_id.h
> create mode 100644 include/drm/amdgpu.ids
>
> diff --git a/Makefile.am b/Makefile.am
> index dfb8fcd..8de8f6c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -45,6 +45,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
>
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm.pc
> +libdrmdatadir = $(datadir)/libdrm
> +dist_libdrmdata_DATA = include/drm/amdgpu.ids
> +export libdrmdatadir
>
> if HAVE_LIBKMS
> LIBKMS_SUBDIR = libkms
> diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
> index cf7bc1b..da71c1c 100644
> --- a/amdgpu/Makefile.am
> +++ b/amdgpu/Makefile.am
> @@ -30,6 +30,8 @@ AM_CFLAGS = \
> $(PTHREADSTUBS_CFLAGS) \
> -I$(top_srcdir)/include/drm
>
> +AM_CPPFLAGS = -DAMDGPU_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\"
> +
> libdrm_amdgpu_la_LTLIBRARIES = libdrm_amdgpu.la
> libdrm_amdgpu_ladir = $(libdir)
> libdrm_amdgpu_la_LDFLAGS = -version-number 1:0:0 -no-undefined
> diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
> index 487b9e0..bc3abaa 100644
> --- a/amdgpu/Makefile.sources
> +++ b/amdgpu/Makefile.sources
> @@ -1,5 +1,5 @@
> LIBDRM_AMDGPU_FILES := \
> - amdgpu_asic_id.h \
> + amdgpu_asic_id.c \
> amdgpu_bo.c \
> amdgpu_cs.c \
> amdgpu_device.c \
> diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
> new file mode 100644
> index 000..be39f4e
> --- /dev/null
> +++ b/amdgpu/amdgpu_asic_id.c
> @@ -0,0 +1,211 @@
> +/*
> + * Copyright © 2017 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "xf86drm.h"
> +#include "amdgpu_drm.h"
> +#include "amdgpu_internal.h"
> +
> +static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
> +{
> + char *buf, *saveptr;
> + char *s_did;
> + char *s_rid;
> + char *s_name;
> + char *endptr;
> + int r = 0;
> +
> + buf = strdup(line);
> + if (!buf)
> + return -ENOMEM;
> +
> + /* ignore empty line and commented line */
> + if (strlen(line) == 0 || line[0] == '#') {
> + r = -EAGAIN;
> + goto out;
> + }
> +
> + /* device id */
> + s_did = strtok_r(buf, ",", &saveptr);
> + if (!s_did) {
> + r = -EINVAL;
> + goto out;
> + }
> +
> + id->did = strtol(s_did, &endptr, 16);
> + if (*endptr) {
> + r = -EINVAL;
> + goto out;
> + }
> +
> + /* revision id */
> + s_rid = strtok_r(NULL, ",", &saveptr);
> + if (!s_rid) {
> +
[PATCH libdrm v6 1/1] amdgpu: move asic id table to a separate file
From: Xiaojie Yuan
v2: fix an off by one error and leading white spaces
v3: use thread safe strtok_r(); initialize len before calling getline();
change printf() to drmMsg(); add initial amdgpu.ids
v4: integrate some recent internal changes, including format changes
v5: fix line number for empty/commented lines; realloc to save memory;
indentation changes
v6: remove a line error
Change-Id: I12216da14910f5e2b0970bc1fafc2a20b0ef1ba9
Reviewed-by: Junwei Zhang
Signed-off-by: Samuel Li
---
Makefile.am | 3 +
amdgpu/Makefile.am | 2 +
amdgpu/Makefile.sources | 2 +-
amdgpu/amdgpu_asic_id.c | 211 +++
amdgpu/amdgpu_asic_id.h | 165
amdgpu/amdgpu_device.c | 28 +--
amdgpu/amdgpu_internal.h | 10 +++
include/drm/amdgpu.ids | 170 ++
8 files changed, 418 insertions(+), 173 deletions(-)
create mode 100644 amdgpu/amdgpu_asic_id.c
delete mode 100644 amdgpu/amdgpu_asic_id.h
create mode 100644 include/drm/amdgpu.ids
diff --git a/Makefile.am b/Makefile.am
index dfb8fcd..8de8f6c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -45,6 +45,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm.pc
+libdrmdatadir = $(datadir)/libdrm
+dist_libdrmdata_DATA = include/drm/amdgpu.ids
+export libdrmdatadir
if HAVE_LIBKMS
LIBKMS_SUBDIR = libkms
diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
index cf7bc1b..da71c1c 100644
--- a/amdgpu/Makefile.am
+++ b/amdgpu/Makefile.am
@@ -30,6 +30,8 @@ AM_CFLAGS = \
$(PTHREADSTUBS_CFLAGS) \
-I$(top_srcdir)/include/drm
+AM_CPPFLAGS = -DAMDGPU_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\"
+
libdrm_amdgpu_la_LTLIBRARIES = libdrm_amdgpu.la
libdrm_amdgpu_ladir = $(libdir)
libdrm_amdgpu_la_LDFLAGS = -version-number 1:0:0 -no-undefined
diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
index 487b9e0..bc3abaa 100644
--- a/amdgpu/Makefile.sources
+++ b/amdgpu/Makefile.sources
@@ -1,5 +1,5 @@
LIBDRM_AMDGPU_FILES := \
- amdgpu_asic_id.h \
+ amdgpu_asic_id.c \
amdgpu_bo.c \
amdgpu_cs.c \
amdgpu_device.c \
diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
new file mode 100644
index 000..be39f4e
--- /dev/null
+++ b/amdgpu/amdgpu_asic_id.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright © 2017 Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "xf86drm.h"
+#include "amdgpu_drm.h"
+#include "amdgpu_internal.h"
+
+static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
+{
+ char *buf, *saveptr;
+ char *s_did;
+ char *s_rid;
+ char *s_name;
+ char *endptr;
+ int r = 0;
+
+ buf = strdup(line);
+ if (!buf)
+ return -ENOMEM;
+
+ /* ignore empty line and commented line */
+ if (strlen(line) == 0 || line[0] == '#') {
+ r = -EAGAIN;
+ goto out;
+ }
+
+ /* device id */
+ s_did = strtok_r(buf, ",", &saveptr);
+ if (!s_did) {
+ r = -EINVAL;
+ goto out;
+ }
+
+ id->did = strtol(s_did, &endptr, 16);
+ if (*endptr) {
+ r = -EINVAL;
+ goto out;
+ }
+
+ /* revision id */
+ s_rid = strtok_r(NULL, ",", &saveptr);
+ if (!s_rid) {
+ r = -EINVAL;
+ goto out;
+ }
+
+ id->rid = strtol(s_rid, &endptr, 16);
+ if (*endptr) {
+ r = -EINVAL;
+ goto out;
+ }
+
+ /* marketing name */
+ s_name = strtok_r(NULL, ",", &saveptr);
+ if (!s_name) {
+ r = -EINVAL;
+ goto out;
+ }
+
