All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
@ 2017-06-30 19:25 Samuel Li
       [not found] ` <1498850702-13331-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
  2017-07-05 11:02 ` Emil Velikov
  0 siblings, 2 replies; 8+ messages in thread
From: Samuel Li @ 2017-06-30 19:25 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Samuel Li, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Change-Id: I24b6624789d1a9dc0fd3a446b0e6f21ed5183ff2
Signed-off-by: Samuel Li <Samuel.Li@amd.com>
---
 radeon/Makefile.am      |   6 +++
 radeon/Makefile.sources |   6 ++-
 radeon/radeon_asic_id.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++
 radeon/radeon_asic_id.h |  37 +++++++++++++++++
 4 files changed, 153 insertions(+), 2 deletions(-)
 create mode 100644 radeon/radeon_asic_id.c
 create mode 100644 radeon/radeon_asic_id.h

diff --git a/radeon/Makefile.am b/radeon/Makefile.am
index e241531..69407bc 100644
--- a/radeon/Makefile.am
+++ b/radeon/Makefile.am
@@ -30,6 +30,12 @@ AM_CFLAGS = \
 	$(PTHREADSTUBS_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 = -DRADEON_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\" \
+	-DRADEON_ASIC_ID_TABLE_NUM_ENTRIES=$(ASIC_ID_TABLE_NUM_ENTRIES)
+
 libdrm_radeon_la_LTLIBRARIES = libdrm_radeon.la
 libdrm_radeon_ladir = $(libdir)
 libdrm_radeon_la_LDFLAGS = -version-number 1:0:1 -no-undefined
diff --git a/radeon/Makefile.sources b/radeon/Makefile.sources
index 1cf482a..8eaf1c6 100644
--- a/radeon/Makefile.sources
+++ b/radeon/Makefile.sources
@@ -4,7 +4,8 @@ LIBDRM_RADEON_FILES := \
 	radeon_cs_space.c \
 	radeon_bo.c \
 	radeon_cs.c \
-	radeon_surface.c
+	radeon_surface.c \
+	radeon_asic_id.c
 
 LIBDRM_RADEON_H_FILES := \
 	radeon_bo.h \
@@ -14,7 +15,8 @@ LIBDRM_RADEON_H_FILES := \
 	radeon_cs_gem.h \
 	radeon_bo_int.h \
 	radeon_cs_int.h \
-	r600_pci_ids.h
+	r600_pci_ids.h \
+	radeon_asic_id.h
 
 LIBDRM_RADEON_BOF_FILES := \
 	bof.c \
diff --git a/radeon/radeon_asic_id.c b/radeon/radeon_asic_id.c
new file mode 100644
index 0000000..b03502b
--- /dev/null
+++ b/radeon/radeon_asic_id.c
@@ -0,0 +1,106 @@
+/*
+ * Copyright 2017 Advanced Micro Devices, Inc.
+ *
+ * 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.
+ *
+ */
+
+/**
+ * \file radeon_asic_id.c
+ *
+ *  Implementation of chipset name lookup functions for radeon device
+ *
+ */
+
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+//#include <errno.h>
+//#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <stdint.h>
+#include <assert.h>
+
+#include "xf86atomic.h"
+#include "util/util_asic_id.h"
+#include "radeon_asic_id.h"
+
+
+static pthread_mutex_t asic_id_mutex = PTHREAD_MUTEX_INITIALIZER;
+static struct util_asic_id *radeon_asic_ids;
+static atomic_t refcount;
+
+int radeon_asic_id_initialize(void)
+{
+	int r = 0;
+	pthread_mutex_lock(&asic_id_mutex);
+	if (radeon_asic_ids) {
+		atomic_inc(&refcount);
+		pthread_mutex_unlock(&asic_id_mutex);
+		return r;
+	}
+
+	r = util_parse_asic_ids(&radeon_asic_ids, RADEON_ASIC_ID_TABLE,
+				RADEON_ASIC_ID_TABLE_NUM_ENTRIES);
+	if (r) {
+		fprintf(stderr, "%s: Cannot parse ASIC IDs, 0x%x.",
+			__func__, r);
+	} else
+		atomic_inc(&refcount);
+
+	pthread_mutex_unlock(&asic_id_mutex);
+	return r;
+}
+
+void radeon_asic_id_deinitialize(void)
+{
+	const struct util_asic_id *id;
+
+	assert(atomic_read(&refcount) > 0);
+	pthread_mutex_lock(&asic_id_mutex);
+	if (atomic_dec_and_test(&refcount)) {
+		if (radeon_asic_ids) {
+			for (id = radeon_asic_ids; id->did; id++)
+				free(id->marketing_name);
+			free(radeon_asic_ids);
+			radeon_asic_ids = NULL;
+		}
+	}
+	pthread_mutex_unlock(&asic_id_mutex);
+}
+
+const char *radeon_get_marketing_name(uint32_t device_id, uint32_t pci_rev_id)
+{
+	const struct util_asic_id *id;
+
+	if (!radeon_asic_ids)
+		return NULL;
+
+	for (id = radeon_asic_ids; id->did; id++) {
+		if ((id->did == device_id) &&
+		    (id->rid == pci_rev_id))
+			return id->marketing_name;
+	}
+
+	return NULL;
+}
diff --git a/radeon/radeon_asic_id.h b/radeon/radeon_asic_id.h
new file mode 100644
index 0000000..00bc110
--- /dev/null
+++ b/radeon/radeon_asic_id.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2017 Advanced Micro Devices, Inc.
+ *
+ * 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.
+ *
+ */
+
+/**
+ * \file radeon_asic_id.h
+ *
+ * Declare public API for chipset name lookup
+ *
+ */
+#ifndef _RADEON_ASIC_ID_H_
+#define _RADEON_ASIC_ID_H_
+
+int radeon_asic_id_initialize(void);
+void radeon_asic_id_deinitialize(void);
+const char *radeon_get_marketing_name(uint32_t device_id, uint32_t pci_rev_id);
+
+#endif
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
       [not found] ` <1498850702-13331-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
@ 2017-07-04  9:43   ` Michel Dänzer
       [not found]     ` <9adcfdd9-9a6c-9077-a7ca-947af8b2a286-otUistvHUpPR7s880joybQ@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Michel Dänzer @ 2017-07-04  9:43 UTC (permalink / raw)
  To: Samuel Li
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


Hi Sam,


do you also have a Mesa patch showing how the new APIs will be used?
Without seeing that, some minor comments below.


On 01/07/17 04:25 AM, Samuel Li wrote:
> 
> +//#include <errno.h>
> +//#include <string.h>

Remove these lines.


> +#include "util/util_asic_id.h"

Patch 1 adds the util directory to include paths, which would allow just

#include "util_asic_id.h"

Please consistently either use this, or don't add the util directory to
the include path anywhere.


> +int radeon_asic_id_initialize(void)
> +{
> +	int r = 0;
> +	pthread_mutex_lock(&asic_id_mutex);

Put an empty line between declarations and statements.


-- 
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
       [not found]     ` <9adcfdd9-9a6c-9077-a7ca-947af8b2a286-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2017-07-04 22:06       ` Li, Samuel
  0 siblings, 0 replies; 8+ messages in thread
From: Li, Samuel @ 2017-07-04 22:06 UTC (permalink / raw)
  To: Michel Dänzer
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

> do you also have a Mesa patch showing how the new APIs will be used?
Sent out to mesa dev just now.

> Remove these lines.
Right.

> Please consistently either use this, or don't add the util directory to the
> include path anywhere.
OK.

> Put an empty line between declarations and statements.
OK.

Sam


> -----Original Message-----
> From: Michel Dänzer [mailto:michel@daenzer.net]
> Sent: Tuesday, July 04, 2017 5:43 AM
> To: Li, Samuel <Samuel.Li@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
> 
> 
> Hi Sam,
> 
> 
> do you also have a Mesa patch showing how the new APIs will be used?
> Without seeing that, some minor comments below.
> 
> 
> On 01/07/17 04:25 AM, Samuel Li wrote:
> >
> > +//#include <errno.h>
> > +//#include <string.h>
> 
> Remove these lines.
> 
> 
> > +#include "util/util_asic_id.h"
> 
> Patch 1 adds the util directory to include paths, which would allow just
> 
> #include "util_asic_id.h"
> 
> Please consistently either use this, or don't add the util directory to the
> include path anywhere.
> 
> 
> > +int radeon_asic_id_initialize(void)
> > +{
> > +	int r = 0;
> > +	pthread_mutex_lock(&asic_id_mutex);
> 
> Put an empty line between declarations and statements.
> 
> 
> --
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
  2017-06-30 19:25 [PATCH libdrm 2/2] radeon: use asic id table to get chipset name Samuel Li
       [not found] ` <1498850702-13331-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
@ 2017-07-05 11:02 ` Emil Velikov
  2017-07-05 21:31   ` Li, Samuel
  1 sibling, 1 reply; 8+ messages in thread
From: Emil Velikov @ 2017-07-05 11:02 UTC (permalink / raw)
  To: Samuel Li; +Cc: ML dri-devel, amd-gfx mailing list

Hi Samuel,

On 30 June 2017 at 20:25, Samuel Li <Samuel.Li@amd.com> wrote:
> Change-Id: I24b6624789d1a9dc0fd3a446b0e6f21ed5183ff2
> Signed-off-by: Samuel Li <Samuel.Li@amd.com>
> ---
>  radeon/Makefile.am      |   6 +++
>  radeon/Makefile.sources |   6 ++-
>  radeon/radeon_asic_id.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++
>  radeon/radeon_asic_id.h |  37 +++++++++++++++++
>  4 files changed, 153 insertions(+), 2 deletions(-)
>  create mode 100644 radeon/radeon_asic_id.c
>  create mode 100644 radeon/radeon_asic_id.h
>
> diff --git a/radeon/Makefile.am b/radeon/Makefile.am
> index e241531..69407bc 100644
> --- a/radeon/Makefile.am
> +++ b/radeon/Makefile.am
> @@ -30,6 +30,12 @@ AM_CFLAGS = \
>         $(PTHREADSTUBS_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 = -DRADEON_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\" \
> +       -DRADEON_ASIC_ID_TABLE_NUM_ENTRIES=$(ASIC_ID_TABLE_NUM_ENTRIES)
> +
Reusing amdgpu.ids by libdrm_radeon is not so obvious. I'm wondering
if adding a comment in the file [amdgpu.ids] may be a good idea.
"File is used by $LIST" or "File has multiple users" or anything that
hints/makes people look up the details.

Couple of other ideas/suggestions:
 - above all, as-is make check will fail
 - keeping the radeon API symmetrical to the amdgpu one would a good idea
 - is adding yet another header really justified?

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
  2017-07-05 11:02 ` Emil Velikov
@ 2017-07-05 21:31   ` Li, Samuel
       [not found]     ` <BLUPR12MB0628623A3972B7F2BB1F69A7F5D40-7LeqcoF/hwrL9O90+oShTwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Li, Samuel @ 2017-07-05 21:31 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel, amd-gfx mailing list

>  - above all, as-is make check will fail	
Right, I did not check that.

>  - keeping the radeon API symmetrical to the amdgpu one would a good idea
The issue is Radeon does not have a struct similar to amdgpu_device_handle. 
I think the current radeon API is simpler. Maybe a follow up change can change amdgpu's API similar to radeon.

>  - is adding yet another header really justified?
radeon_asic_id.h? That is going to be used by ddx/mesa.

Sam

> -----Original Message-----
> From: Emil Velikov [mailto:emil.l.velikov@gmail.com]
> Sent: Wednesday, July 05, 2017 7:03 AM
> To: Li, Samuel <Samuel.Li@amd.com>
> Cc: amd-gfx mailing list <amd-gfx@lists.freedesktop.org>; ML dri-devel <dri-
> devel@lists.freedesktop.org>
> Subject: Re: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
> 
> Hi Samuel,
> 
> On 30 June 2017 at 20:25, Samuel Li <Samuel.Li@amd.com> wrote:
> > Change-Id: I24b6624789d1a9dc0fd3a446b0e6f21ed5183ff2
> > Signed-off-by: Samuel Li <Samuel.Li@amd.com>
> > ---
> >  radeon/Makefile.am      |   6 +++
> >  radeon/Makefile.sources |   6 ++-
> >  radeon/radeon_asic_id.c | 106
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> >  radeon/radeon_asic_id.h |  37 +++++++++++++++++
> >  4 files changed, 153 insertions(+), 2 deletions(-)  create mode
> > 100644 radeon/radeon_asic_id.c  create mode 100644
> > radeon/radeon_asic_id.h
> >
> > diff --git a/radeon/Makefile.am b/radeon/Makefile.am index
> > e241531..69407bc 100644
> > --- a/radeon/Makefile.am
> > +++ b/radeon/Makefile.am
> > @@ -30,6 +30,12 @@ AM_CFLAGS = \
> >         $(PTHREADSTUBS_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 =
> > +-DRADEON_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\" \
> > +
> > +-
> DRADEON_ASIC_ID_TABLE_NUM_ENTRIES=$(ASIC_ID_TABLE_NUM_ENTRIE
> S)
> > +
> Reusing amdgpu.ids by libdrm_radeon is not so obvious. I'm wondering if
> adding a comment in the file [amdgpu.ids] may be a good idea.
> "File is used by $LIST" or "File has multiple users" or anything that
> hints/makes people look up the details.
> 
> Couple of other ideas/suggestions:
>  - above all, as-is make check will fail	
>  - keeping the radeon API symmetrical to the amdgpu one would a good idea
>  - is adding yet another header really justified?
> 
> -Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
       [not found]     ` <BLUPR12MB0628623A3972B7F2BB1F69A7F5D40-7LeqcoF/hwrL9O90+oShTwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-07-06  9:20       ` Emil Velikov
       [not found]         ` <CACvgo514x_TAaaESizA10eUQFhUQr6DuWOB_B84i363Pqvk5Xg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Emil Velikov @ 2017-07-06  9:20 UTC (permalink / raw)
  To: Li, Samuel; +Cc: ML dri-devel, amd-gfx mailing list

On 5 July 2017 at 22:31, Li, Samuel <Samuel.Li@amd.com> wrote:
>>  - above all, as-is make check will fail
> Right, I did not check that.
>
>>  - keeping the radeon API symmetrical to the amdgpu one would a good idea
> The issue is Radeon does not have a struct similar to amdgpu_device_handle.
Attach it to analogous primitive?

> I think the current radeon API is simpler. Maybe a follow up change can change amdgpu's API similar to radeon.
>
Exposing 3 entry points instead of 1 is _not_simpler. Also you cannot
change the existing API, since it also breaks the ABI.
Leading to crash/cause memory corruption when using existing binaries.

>>  - is adding yet another header really justified?
> radeon_asic_id.h? That is going to be used by ddx/mesa.
>
Where it's used is orthogonal. You don't need a separate _public_
header for nearly every entry point ;-)

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
       [not found]         ` <CACvgo514x_TAaaESizA10eUQFhUQr6DuWOB_B84i363Pqvk5Xg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-07-06 12:46           ` Deucher, Alexander
       [not found]             ` <BN6PR12MB165270A0D5A5176AED04C472F7D50-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Deucher, Alexander @ 2017-07-06 12:46 UTC (permalink / raw)
  To: 'Emil Velikov', Li, Samuel; +Cc: amd-gfx mailing list, ML dri-devel

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Emil Velikov
> Sent: Thursday, July 06, 2017 5:21 AM
> To: Li, Samuel
> Cc: ML dri-devel; amd-gfx mailing list
> Subject: Re: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
> 
> On 5 July 2017 at 22:31, Li, Samuel <Samuel.Li@amd.com> wrote:
> >>  - above all, as-is make check will fail
> > Right, I did not check that.
> >
> >>  - keeping the radeon API symmetrical to the amdgpu one would a good
> idea
> > The issue is Radeon does not have a struct similar to
> amdgpu_device_handle.
> Attach it to analogous primitive?

Radeon libdrm is much different than amdgpu.  There is no analog.

> 
> > I think the current radeon API is simpler. Maybe a follow up change can
> change amdgpu's API similar to radeon.
> >
> Exposing 3 entry points instead of 1 is _not_simpler. Also you cannot
> change the existing API, since it also breaks the ABI.
> Leading to crash/cause memory corruption when using existing binaries.
> 
> >>  - is adding yet another header really justified?
> > radeon_asic_id.h? That is going to be used by ddx/mesa.
> >
> Where it's used is orthogonal. You don't need a separate _public_
> header for nearly every entry point ;-)

Actually having a separate header makes sense for radeon.  We currently expose a separate header for each set of functionality (one for buffer management, one for command submission, one for surface management).  Adding the asic names to any of the existing ones doesn’t really make sense from a functional standpoint.

Alex

> 
> Thanks
> Emil
> _______________________________________________
> 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] 8+ messages in thread

* Re: [PATCH libdrm 2/2] radeon: use asic id table to get chipset name
       [not found]             ` <BN6PR12MB165270A0D5A5176AED04C472F7D50-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-07-11  9:41               ` Emil Velikov
  0 siblings, 0 replies; 8+ messages in thread
From: Emil Velikov @ 2017-07-11  9:41 UTC (permalink / raw)
  To: Deucher, Alexander; +Cc: amd-gfx mailing list, ML dri-devel, Li, Samuel

On 6 July 2017 at 13:46, Deucher, Alexander <Alexander.Deucher@amd.com> wrote:

>> Attach it to analogous primitive?
>
> Radeon libdrm is much different than amdgpu.  There is no analog.
>
Upon a closer look, indeed there isn't. Must have gotten confused earlier.

>>
>> > I think the current radeon API is simpler. Maybe a follow up change can
>> change amdgpu's API similar to radeon.
>> >
>> Exposing 3 entry points instead of 1 is _not_simpler. Also you cannot
>> change the existing API, since it also breaks the ABI.
>> Leading to crash/cause memory corruption when using existing binaries.
>>
>> >>  - is adding yet another header really justified?
>> > radeon_asic_id.h? That is going to be used by ddx/mesa.
>> >
>> Where it's used is orthogonal. You don't need a separate _public_
>> header for nearly every entry point ;-)
>
> Actually having a separate header makes sense for radeon.  We currently expose a separate header for each set of functionality (one for buffer management, one for command submission, one for surface management).  Adding the asic names to any of the existing ones doesn’t really make sense from a functional standpoint.
>
Seems a bit unfortunate, but you have a point.

Having three extra entry points seems a bit much, but it seems like
the best one can do atm.

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-07-11  9:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-30 19:25 [PATCH libdrm 2/2] radeon: use asic id table to get chipset name Samuel Li
     [not found] ` <1498850702-13331-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
2017-07-04  9:43   ` Michel Dänzer
     [not found]     ` <9adcfdd9-9a6c-9077-a7ca-947af8b2a286-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-07-04 22:06       ` Li, Samuel
2017-07-05 11:02 ` Emil Velikov
2017-07-05 21:31   ` Li, Samuel
     [not found]     ` <BLUPR12MB0628623A3972B7F2BB1F69A7F5D40-7LeqcoF/hwrL9O90+oShTwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-07-06  9:20       ` Emil Velikov
     [not found]         ` <CACvgo514x_TAaaESizA10eUQFhUQr6DuWOB_B84i363Pqvk5Xg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-06 12:46           ` Deucher, Alexander
     [not found]             ` <BN6PR12MB165270A0D5A5176AED04C472F7D50-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-07-11  9:41               ` Emil Velikov

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.