[PATCH libdrm 2/2] radeon: use asic id table to get chipset name

[Li, Samuel]

>  - 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 at gmail.com]
> Sent: Wednesday, July 05, 2017 7:03 AM
> To: Li, Samuel <Samuel.Li at amd.com>
> Cc: amd-gfx mailing list <amd-gfx at lists.freedesktop.org>; ML dri-devel <dri-
> devel at 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 at amd.com> wrote:
> > Change-Id: I24b6624789d1a9dc0fd3a446b0e6f21ed5183ff2
> > Signed-off-by: Samuel Li <Samuel.Li at 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