* [PATCH libdrm 0/3] amdgpu: amdgpu_get_marketing_name cleanups
@ 2017-12-01 16:56 Michel Dänzer
[not found] ` <20171201165618.19629-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Michel Dänzer @ 2017-12-01 16:56 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
From: Michel Dänzer <michel.daenzer@amd.com>
The first two patches are preparatory for patch 3, which is the meat of
the series.
Michel Dänzer (3):
amdgpu: Clean up amdgpu_parse_asic_ids error handling
amdgpu: Simplify error handling in parse_one_line
amdgpu: Only remember the device's marketing name
amdgpu/Android.mk | 3 +-
amdgpu/Makefile.am | 5 +-
amdgpu/amdgpu_asic_id.c | 139 ++++++++++++++---------------------------------
amdgpu/amdgpu_device.c | 27 +--------
amdgpu/amdgpu_internal.h | 11 +---
5 files changed, 49 insertions(+), 136 deletions(-)
--
2.15.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread[parent not found: <20171201165618.19629-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>]
* [PATCH libdrm 1/3] amdgpu: Clean up amdgpu_parse_asic_ids error handling [not found] ` <20171201165618.19629-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org> @ 2017-12-01 16:56 ` Michel Dänzer [not found] ` <20171201165618.19629-2-michel-otUistvHUpPR7s880joybQ@public.gmane.org> 2017-12-01 16:56 ` [PATCH libdrm 2/3] amdgpu: Simplify error handling in parse_one_line Michel Dänzer ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Michel Dänzer @ 2017-12-01 16:56 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW From: Michel Dänzer <michel.daenzer@amd.com> * Move error message printing into amdgpu_parse_asic_ids and make it return void * Print only "Invalid format" error message if parse_one_line returns -EINVAL * Use strerror instead of printing the (negative) error code in hex Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> --- amdgpu/amdgpu_asic_id.c | 16 ++++++++++------ amdgpu/amdgpu_device.c | 6 +----- amdgpu/amdgpu_internal.h | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c index e8218974..eb42bbc2 100644 --- a/amdgpu/amdgpu_asic_id.c +++ b/amdgpu/amdgpu_asic_id.c @@ -109,7 +109,7 @@ out: return r; } -int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) +void amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) { struct amdgpu_asic_id *asic_id_table; struct amdgpu_asic_id *id; @@ -126,7 +126,7 @@ int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) if (!fp) { fprintf(stderr, "%s: %s\n", AMDGPU_ASIC_ID_TABLE, strerror(errno)); - return -EINVAL; + return; } asic_id_table = calloc(table_max_size + 1, @@ -177,8 +177,6 @@ int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) line_num++; continue; } - fprintf(stderr, "Invalid format: %s: line %d: %s\n", - AMDGPU_ASIC_ID_TABLE, line_num, line); goto free; } @@ -201,6 +199,14 @@ int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) 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); + } else if (r) { + fprintf(stderr, "%s: Cannot parse ASIC IDs: %s\n", + __func__, strerror(-r)); + } + free(line); if (r && asic_id_table) { @@ -215,6 +221,4 @@ close: fclose(fp); *p_asic_id_table = asic_id_table; - - return r; } diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c index 344e87ed..e7aaf4fc 100644 --- a/amdgpu/amdgpu_device.c +++ b/amdgpu/amdgpu_device.c @@ -280,11 +280,7 @@ int amdgpu_device_initialize(int fd, amdgpu_vamgr_init(&dev->vamgr, start, max, dev->dev_info.virtual_address_alignment); - r = amdgpu_parse_asic_ids(&dev->asic_ids); - if (r) { - fprintf(stderr, "%s: Cannot parse ASIC IDs, 0x%x.", - __func__, r); - } + amdgpu_parse_asic_ids(&dev->asic_ids); *major_version = dev->major_version; *minor_version = dev->minor_version; diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h index e26e5190..1aff7f8e 100644 --- a/amdgpu/amdgpu_internal.h +++ b/amdgpu/amdgpu_internal.h @@ -148,7 +148,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 int amdgpu_parse_asic_ids(struct amdgpu_asic_id **asic_ids); +drm_private void amdgpu_parse_asic_ids(struct amdgpu_asic_id **asic_ids); drm_private int amdgpu_query_gpu_info_init(amdgpu_device_handle dev); -- 2.15.0 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20171201165618.19629-2-michel-otUistvHUpPR7s880joybQ@public.gmane.org>]
* Re: [PATCH libdrm 1/3] amdgpu: Clean up amdgpu_parse_asic_ids error handling [not found] ` <20171201165618.19629-2-michel-otUistvHUpPR7s880joybQ@public.gmane.org> @ 2018-01-05 21:12 ` Marek Olšák [not found] ` <CAAxE2A7ScPLqDsZ-hczacTeYMx8qr=XxoHi8225kB0M4TRcdeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Marek Olšák @ 2018-01-05 21:12 UTC (permalink / raw) To: Michel Dänzer; +Cc: amd-gfx mailing list On Fri, Dec 1, 2017 at 5:56 PM, Michel Dänzer <michel@daenzer.net> wrote: > From: Michel Dänzer <michel.daenzer@amd.com> > > * Move error message printing into amdgpu_parse_asic_ids and make it > return void > * Print only "Invalid format" error message if parse_one_line returns > -EINVAL > * Use strerror instead of printing the (negative) error code in hex > > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> > --- > amdgpu/amdgpu_asic_id.c | 16 ++++++++++------ > amdgpu/amdgpu_device.c | 6 +----- > amdgpu/amdgpu_internal.h | 2 +- > 3 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c > index e8218974..eb42bbc2 100644 > --- a/amdgpu/amdgpu_asic_id.c > +++ b/amdgpu/amdgpu_asic_id.c > @@ -109,7 +109,7 @@ out: > return r; > } > > -int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) > +void amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) > { > struct amdgpu_asic_id *asic_id_table; > struct amdgpu_asic_id *id; > @@ -126,7 +126,7 @@ int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) > if (!fp) { > fprintf(stderr, "%s: %s\n", AMDGPU_ASIC_ID_TABLE, > strerror(errno)); > - return -EINVAL; > + return; > } > > asic_id_table = calloc(table_max_size + 1, > @@ -177,8 +177,6 @@ int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) > line_num++; > continue; > } > - fprintf(stderr, "Invalid format: %s: line %d: %s\n", > - AMDGPU_ASIC_ID_TABLE, line_num, line); > goto free; > } > > @@ -201,6 +199,14 @@ int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table) > 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); > + } else if (r) { > + fprintf(stderr, "%s: Cannot parse ASIC IDs: %s\n", > + __func__, strerror(-r)); This is a good example how strerror should not be used in graphics drivers. It makes no sense: "amdgpu_parse_asic_ids: Cannot parse ASIC IDs: Resource temporarily unavailable" Is my graphics card temporary unavailable? Marek _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <CAAxE2A7ScPLqDsZ-hczacTeYMx8qr=XxoHi8225kB0M4TRcdeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* [PATCH libdrm] amdgpu: Don't print error message if parse_one_line returned -EAGAIN [not found] ` <CAAxE2A7ScPLqDsZ-hczacTeYMx8qr=XxoHi8225kB0M4TRcdeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-01-08 10:24 ` Michel Dänzer [not found] ` <20180108102431.15362-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Michel Dänzer @ 2018-01-08 10:24 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW From: Michel Dänzer <michel.daenzer@amd.com> It means it just didn't find an entry for the GPU in the amdgpu.ids file. Fixes spurious amdgpu_parse_asic_ids: Cannot parse ASIC IDs: Resource temporarily unavailable error messages in that case. Reported-by: Marek Olšák <marek.olsak@amd.com> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> --- amdgpu/amdgpu_asic_id.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c index 0c8925e5..62459c09 100644 --- a/amdgpu/amdgpu_asic_id.c +++ b/amdgpu/amdgpu_asic_id.c @@ -155,7 +155,7 @@ void amdgpu_parse_asic_ids(struct amdgpu_device *dev) if (r == -EINVAL) { fprintf(stderr, "Invalid format: %s: line %d: %s\n", AMDGPU_ASIC_ID_TABLE, line_num, line); - } else if (r) { + } else if (r && r != -EAGAIN) { fprintf(stderr, "%s: Cannot parse ASIC IDs: %s\n", __func__, strerror(-r)); } -- 2.15.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20180108102431.15362-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>]
* Re: [PATCH libdrm] amdgpu: Don't print error message if parse_one_line returned -EAGAIN [not found] ` <20180108102431.15362-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org> @ 2018-01-08 15:18 ` Marek Olšák 0 siblings, 0 replies; 9+ messages in thread From: Marek Olšák @ 2018-01-08 15:18 UTC (permalink / raw) To: Michel Dänzer; +Cc: amd-gfx mailing list Reviewed-by: Marek Olšák <marek.olsak@amd.com> Marek On Mon, Jan 8, 2018 at 11:24 AM, Michel Dänzer <michel@daenzer.net> wrote: > From: Michel Dänzer <michel.daenzer@amd.com> > > It means it just didn't find an entry for the GPU in the amdgpu.ids file. > > Fixes spurious > > amdgpu_parse_asic_ids: Cannot parse ASIC IDs: Resource temporarily unavailable > > error messages in that case. > > Reported-by: Marek Olšák <marek.olsak@amd.com> > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> > --- > amdgpu/amdgpu_asic_id.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c > index 0c8925e5..62459c09 100644 > --- a/amdgpu/amdgpu_asic_id.c > +++ b/amdgpu/amdgpu_asic_id.c > @@ -155,7 +155,7 @@ void amdgpu_parse_asic_ids(struct amdgpu_device *dev) > if (r == -EINVAL) { > fprintf(stderr, "Invalid format: %s: line %d: %s\n", > AMDGPU_ASIC_ID_TABLE, line_num, line); > - } else if (r) { > + } else if (r && r != -EAGAIN) { > fprintf(stderr, "%s: Cannot parse ASIC IDs: %s\n", > __func__, strerror(-r)); > } > -- > 2.15.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH libdrm 2/3] amdgpu: Simplify error handling in parse_one_line [not found] ` <20171201165618.19629-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org> 2017-12-01 16:56 ` [PATCH libdrm 1/3] amdgpu: Clean up amdgpu_parse_asic_ids error handling Michel Dänzer @ 2017-12-01 16:56 ` Michel Dänzer 2017-12-01 16:56 ` [PATCH libdrm 3/3] amdgpu: Only remember the device's marketing name Michel Dänzer 2017-12-01 17:27 ` [PATCH libdrm 0/3] amdgpu: amdgpu_get_marketing_name cleanups Slava Abramov 3 siblings, 0 replies; 9+ messages in thread From: Michel Dänzer @ 2017-12-01 16:56 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW From: Michel Dänzer <michel.daenzer@amd.com> * Move empty/commented line check before the strdup and return -EAGAIN directly * Initialize r = -EAGAIN and remove redundant assignments * Set r = -ENOMEM if last strdup fails, and remove redundant goto Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> --- amdgpu/amdgpu_asic_id.c | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c index eb42bbc2..0b5f2962 100644 --- a/amdgpu/amdgpu_asic_id.c +++ b/amdgpu/amdgpu_asic_id.c @@ -45,63 +45,50 @@ static int parse_one_line(const char *line, struct amdgpu_asic_id *id) char *s_rid; char *s_name; char *endptr; - int r = 0; + int r = -EINVAL; + + /* ignore empty line and commented line */ + if (strlen(line) == 0 || line[0] == '#') + return -EAGAIN; 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; + if (!s_did) goto out; - } id->did = strtol(s_did, &endptr, 16); - if (*endptr) { - r = -EINVAL; + if (*endptr) goto out; - } /* revision id */ s_rid = strtok_r(NULL, ",", &saveptr); - if (!s_rid) { - r = -EINVAL; + if (!s_rid) goto out; - } id->rid = strtol(s_rid, &endptr, 16); - if (*endptr) { - r = -EINVAL; + if (*endptr) goto out; - } /* marketing name */ s_name = strtok_r(NULL, ",", &saveptr); - if (!s_name) { - r = -EINVAL; + if (!s_name) goto out; - } + /* trim leading whitespaces or tabs */ while (isblank(*s_name)) s_name++; - if (strlen(s_name) == 0) { - r = -EINVAL; + if (strlen(s_name) == 0) goto out; - } id->marketing_name = strdup(s_name); - if (id->marketing_name == NULL) { - r = -EINVAL; - goto out; - } + if (id->marketing_name) + r = 0; + else + r = -ENOMEM; out: free(buf); -- 2.15.0 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH libdrm 3/3] amdgpu: Only remember the device's marketing name [not found] ` <20171201165618.19629-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org> 2017-12-01 16:56 ` [PATCH libdrm 1/3] amdgpu: Clean up amdgpu_parse_asic_ids error handling Michel Dänzer 2017-12-01 16:56 ` [PATCH libdrm 2/3] amdgpu: Simplify error handling in parse_one_line Michel Dänzer @ 2017-12-01 16:56 ` Michel Dänzer [not found] ` <20171201165618.19629-4-michel-otUistvHUpPR7s880joybQ@public.gmane.org> 2017-12-01 17:27 ` [PATCH libdrm 0/3] amdgpu: amdgpu_get_marketing_name cleanups Slava Abramov 3 siblings, 1 reply; 9+ messages in thread From: Michel Dänzer @ 2017-12-01 16:56 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW From: Michel Dänzer <michel.daenzer@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@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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20171201165618.19629-4-michel-otUistvHUpPR7s880joybQ@public.gmane.org>]
* Re: [PATCH libdrm 3/3] amdgpu: Only remember the device's marketing name [not found] ` <20171201165618.19629-4-michel-otUistvHUpPR7s880joybQ@public.gmane.org> @ 2017-12-04 19:41 ` Alex Deucher 0 siblings, 0 replies; 9+ messages in thread From: Alex Deucher @ 2017-12-04 19:41 UTC (permalink / raw) To: Michel Dänzer; +Cc: amd-gfx list On Fri, Dec 1, 2017 at 11:56 AM, Michel Dänzer <michel@daenzer.net> wrote: > From: Michel Dänzer <michel.daenzer@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@amd.com> Series is: Reviewed-by: Alex Deucher <alexander.deucher@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH libdrm 0/3] amdgpu: amdgpu_get_marketing_name cleanups [not found] ` <20171201165618.19629-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org> ` (2 preceding siblings ...) 2017-12-01 16:56 ` [PATCH libdrm 3/3] amdgpu: Only remember the device's marketing name Michel Dänzer @ 2017-12-01 17:27 ` Slava Abramov 3 siblings, 0 replies; 9+ messages in thread From: Slava Abramov @ 2017-12-01 17:27 UTC (permalink / raw) To: Michel Dänzer, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2017-12-01 11:56 AM, Michel Dänzer wrote: > From: Michel Dänzer <michel.daenzer@amd.com> > > The first two patches are preparatory for patch 3, which is the meat of > the series. > > Michel Dänzer (3): > amdgpu: Clean up amdgpu_parse_asic_ids error handling > amdgpu: Simplify error handling in parse_one_line > amdgpu: Only remember the device's marketing name > > amdgpu/Android.mk | 3 +- > amdgpu/Makefile.am | 5 +- > amdgpu/amdgpu_asic_id.c | 139 ++++++++++++++--------------------------------- > amdgpu/amdgpu_device.c | 27 +-------- > amdgpu/amdgpu_internal.h | 11 +--- > 5 files changed, 49 insertions(+), 136 deletions(-) > Series Acked-by: Slava Abramov <slava.abramov@amd.com> _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-01-08 15:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01 16:56 [PATCH libdrm 0/3] amdgpu: amdgpu_get_marketing_name cleanups Michel Dänzer
[not found] ` <20171201165618.19629-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-12-01 16:56 ` [PATCH libdrm 1/3] amdgpu: Clean up amdgpu_parse_asic_ids error handling Michel Dänzer
[not found] ` <20171201165618.19629-2-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-01-05 21:12 ` Marek Olšák
[not found] ` <CAAxE2A7ScPLqDsZ-hczacTeYMx8qr=XxoHi8225kB0M4TRcdeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-08 10:24 ` [PATCH libdrm] amdgpu: Don't print error message if parse_one_line returned -EAGAIN Michel Dänzer
[not found] ` <20180108102431.15362-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-01-08 15:18 ` Marek Olšák
2017-12-01 16:56 ` [PATCH libdrm 2/3] amdgpu: Simplify error handling in parse_one_line Michel Dänzer
2017-12-01 16:56 ` [PATCH libdrm 3/3] amdgpu: Only remember the device's marketing name Michel Dänzer
[not found] ` <20171201165618.19629-4-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-12-04 19:41 ` Alex Deucher
2017-12-01 17:27 ` [PATCH libdrm 0/3] amdgpu: amdgpu_get_marketing_name cleanups Slava Abramov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.