On Fri, Dec 1, 2017 at 11:56 AM, Michel Dänzer <michel at daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> There's no point in keeping around the full table of marketing names,
> when amdgpu_get_marketing_name only ever returns the device's marketing
> name.
>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
Series is:
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> amdgpu/Android.mk | 3 +-
> amdgpu/Makefile.am | 5 +--
> amdgpu/amdgpu_asic_id.c | 88 ++++++++++++------------------------------------
> amdgpu/amdgpu_device.c | 23 ++-----------
> amdgpu/amdgpu_internal.h | 11 ++----
> 5 files changed, 28 insertions(+), 102 deletions(-)
>
> diff --git a/amdgpu/Android.mk b/amdgpu/Android.mk
> index ce273019..1f028d0b 100644
> --- a/amdgpu/Android.mk
> +++ b/amdgpu/Android.mk
> @@ -11,8 +11,7 @@ LOCAL_SHARED_LIBRARIES := libdrm
> LOCAL_SRC_FILES := $(LIBDRM_AMDGPU_FILES)
>
> LOCAL_CFLAGS := \
> - -DAMDGPU_ASIC_ID_TABLE=\"/vendor/etc/hwdata/amdgpu.ids\" \
> - -DAMDGPU_ASIC_ID_TABLE_NUM_ENTRIES=$(shell egrep -ci '^[0-9a-f]{4},.*[0-9a-f]+,' $(LIBDRM_TOP)/data/amdgpu.ids)
> + -DAMDGPU_ASIC_ID_TABLE=\"/vendor/etc/hwdata/amdgpu.ids\"
>
> LOCAL_REQUIRED_MODULES := amdgpu.ids
>
> diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
> index 66f6f676..a1b0d05c 100644
> --- a/amdgpu/Makefile.am
> +++ b/amdgpu/Makefile.am
> @@ -31,10 +31,7 @@ AM_CFLAGS = \
> -I$(top_srcdir)/include/drm
>
> libdrmdatadir = @libdrmdatadir@
> -ASIC_ID_TABLE_NUM_ENTRIES := $(shell egrep -ci '^[0-9a-f]{4},.*[0-9a-f]+,' \
> - $(top_srcdir)/data/amdgpu.ids)
> -AM_CPPFLAGS = -DAMDGPU_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\" \
> - -DAMDGPU_ASIC_ID_TABLE_NUM_ENTRIES=$(ASIC_ID_TABLE_NUM_ENTRIES)
> +AM_CPPFLAGS = -DAMDGPU_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\"
>
> libdrm_amdgpu_la_LTLIBRARIES = libdrm_amdgpu.la
> libdrm_amdgpu_ladir = $(libdir)
> diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
> index 0b5f2962..0c8925e5 100644
> --- a/amdgpu/amdgpu_asic_id.c
> +++ b/amdgpu/amdgpu_asic_id.c
> @@ -38,11 +38,13 @@
> #include "amdgpu_drm.h"
> #include "amdgpu_internal.h"
>
> -static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
> +static int parse_one_line(struct amdgpu_device *dev, const char *line)
> {
> char *buf, *saveptr;
> char *s_did;
> + uint32_t did;
> char *s_rid;
> + uint32_t rid;
> char *s_name;
> char *endptr;
> int r = -EINVAL;
> @@ -60,19 +62,29 @@ static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
> if (!s_did)
> goto out;
>
> - id->did = strtol(s_did, &endptr, 16);
> + did = strtol(s_did, &endptr, 16);
> if (*endptr)
> goto out;
>
> + if (did != dev->info.asic_id) {
> + r = -EAGAIN;
> + goto out;
> + }
> +
> /* revision id */
> s_rid = strtok_r(NULL, ",", &saveptr);
> if (!s_rid)
> goto out;
>
> - id->rid = strtol(s_rid, &endptr, 16);
> + rid = strtol(s_rid, &endptr, 16);
> if (*endptr)
> goto out;
>
> + if (rid != dev->info.pci_rev_id) {
> + r = -EAGAIN;
> + goto out;
> + }
> +
> /* marketing name */
> s_name = strtok_r(NULL, ",", &saveptr);
> if (!s_name)
> @@ -84,8 +96,8 @@ static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
> if (strlen(s_name) == 0)
> goto out;
>
> - id->marketing_name = strdup(s_name);
> - if (id->marketing_name)
> + dev->marketing_name = strdup(s_name);
> + if (dev->marketing_name)
> r = 0;
> else
> r = -ENOMEM;
> @@ -96,17 +108,13 @@ out:
> return r;
> }
>
> -void amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table)
> +void amdgpu_parse_asic_ids(struct amdgpu_device *dev)
> {
> - struct amdgpu_asic_id *asic_id_table;
> - struct amdgpu_asic_id *id;
> FILE *fp;
> char *line = NULL;
> size_t len = 0;
> ssize_t n;
> int line_num = 1;
> - size_t table_size = 0;
> - size_t table_max_size = AMDGPU_ASIC_ID_TABLE_NUM_ENTRIES;
> int r = 0;
>
> fp = fopen(AMDGPU_ASIC_ID_TABLE, "r");
> @@ -116,13 +124,6 @@ void amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table)
> return;
> }
>
> - asic_id_table = calloc(table_max_size + 1,
> - sizeof(struct amdgpu_asic_id));
> - if (!asic_id_table) {
> - r = -ENOMEM;
> - goto close;
> - }
> -
> /* 1st valid line is file version */
> while ((n = getline(&line, &len, fp)) != -1) {
> /* trim trailing newline */
> @@ -140,52 +141,17 @@ void amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table)
> }
>
> while ((n = getline(&line, &len, fp)) != -1) {
> - if (table_size > table_max_size) {
> - /* double table size */
> - table_max_size *= 2;
> - id = realloc(asic_id_table, (table_max_size + 1) *
> - sizeof(struct amdgpu_asic_id));
> - if (!id) {
> - r = -ENOMEM;
> - goto free;
> - }
> - asic_id_table = id;
> - }
> -
> - id = asic_id_table + table_size;
> -
> /* trim trailing newline */
> if (line[n - 1] == '\n')
> line[n - 1] = '\0';
>
> - r = parse_one_line(line, id);
> - if (r) {
> - if (r == -EAGAIN) {
> - line_num++;
> - continue;
> - }
> - goto free;
> - }
> + r = parse_one_line(dev, line);
> + if (r != -EAGAIN)
> + break;
>
> line_num++;
> - table_size++;
> }
>
> - if (table_size != table_max_size) {
> - id = realloc(asic_id_table, (table_size + 1) *
> - sizeof(struct amdgpu_asic_id));
> - if (!id) {
> - r = -ENOMEM;
> - goto free;
> - }
> - asic_id_table = id;
> - }
> -
> - /* end of table */
> - id = asic_id_table + table_size;
> - memset(id, 0, sizeof(struct amdgpu_asic_id));
> -
> -free:
> if (r == -EINVAL) {
> fprintf(stderr, "Invalid format: %s: line %d: %s\n",
> AMDGPU_ASIC_ID_TABLE, line_num, line);
> @@ -195,17 +161,5 @@ 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;
> }
> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
> index e7aaf4fc..eb4b2745 100644
> --- a/amdgpu/amdgpu_device.c
> +++ b/amdgpu/amdgpu_device.c
> @@ -130,7 +130,6 @@ static int amdgpu_get_auth(int fd, int *auth)
>
> static void amdgpu_device_free_internal(amdgpu_device_handle dev)
> {
> - const struct amdgpu_asic_id *id;
> amdgpu_vamgr_deinit(&dev->vamgr_32);
> amdgpu_vamgr_deinit(&dev->vamgr);
> util_hash_table_destroy(dev->bo_flink_names);
> @@ -140,12 +139,7 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
> close(dev->fd);
> if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
> close(dev->flink_fd);
> - if (dev->asic_ids) {
> - for (id = dev->asic_ids; id->did; id++)
> - free(id->marketing_name);
> -
> - free(dev->asic_ids);
> - }
> + free(dev->marketing_name);
> free(dev);
> }
>
> @@ -280,7 +274,7 @@ int amdgpu_device_initialize(int fd,
> amdgpu_vamgr_init(&dev->vamgr, start, max,
> dev->dev_info.virtual_address_alignment);
>
> - amdgpu_parse_asic_ids(&dev->asic_ids);
> + amdgpu_parse_asic_ids(dev);
>
> *major_version = dev->major_version;
> *minor_version = dev->minor_version;
> @@ -306,16 +300,5 @@ int amdgpu_device_deinitialize(amdgpu_device_handle dev)
>
> const char *amdgpu_get_marketing_name(amdgpu_device_handle dev)
> {
> - const struct amdgpu_asic_id *id;
> -
> - if (!dev->asic_ids)
> - return NULL;
> -
> - for (id = dev->asic_ids; id->did; id++) {
> - if ((id->did == dev->info.asic_id) &&
> - (id->rid == dev->info.pci_rev_id))
> - return id->marketing_name;
> - }
> -
> - return NULL;
> + return dev->marketing_name;
> }
> diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h
> index 1aff7f8e..3e044f11 100644
> --- a/amdgpu/amdgpu_internal.h
> +++ b/amdgpu/amdgpu_internal.h
> @@ -69,12 +69,6 @@ struct amdgpu_va {
> struct amdgpu_bo_va_mgr *vamgr;
> };
>
> -struct amdgpu_asic_id {
> - uint32_t did;
> - uint32_t rid;
> - char *marketing_name;
> -};
> -
> struct amdgpu_device {
> atomic_t refcount;
> int fd;
> @@ -82,8 +76,7 @@ struct amdgpu_device {
> unsigned major_version;
> unsigned minor_version;
>
> - /** Lookup table of asic device id, revision id and marketing name */
> - struct amdgpu_asic_id *asic_ids;
> + char *marketing_name;
> /** List of buffer handles. Protected by bo_table_mutex. */
> struct util_hash_table *bo_handles;
> /** List of buffer GEM flink names. Protected by bo_table_mutex. */
> @@ -148,7 +141,7 @@ drm_private void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, uint64_t start,
>
> drm_private void amdgpu_vamgr_deinit(struct amdgpu_bo_va_mgr *mgr);
>
> -drm_private void amdgpu_parse_asic_ids(struct amdgpu_asic_id **asic_ids);
> +drm_private void amdgpu_parse_asic_ids(struct amdgpu_device *dev);
>
> drm_private int amdgpu_query_gpu_info_init(amdgpu_device_handle dev);
>
> --
> 2.15.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
