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