[libdrm,v8] amdgpu: move asic id table to a separate file

Commit Message

Emil Velikov June 15, 2017, 7:42 a.m. UTC

On 15 June 2017 at 04:16, Michel Dänzer <[e-mail]> wrote:
> On 14/06/17 08:34 PM, Emil Velikov wrote:
>> On 13 June 2017 at 10:45, Michel Dänzer <[e-mail]> wrote:
>>> From: Xiaojie Yuan <[e-mail]>
>>>
>>> v2: fix an off by one error and leading white spaces
>>> v3: use thread safe strtok_r(); initialize len before calling getline();
>>>     change printf() to drmMsg(); add initial amdgpu.ids
>>> v4: integrate some recent internal changes, including format changes
>>> v5: fix line number for empty/commented lines; realloc to save memory;
>>>     indentation changes
>>> v6: remove a line error
>>> v7: [Michel Dänzer]
>>> * Move amdgpu.ids to new data directory
>>> * Remove placeholder entries from amdgpu.ids
>>> * Set libdrmdatadir variable in configure.ac instead of Makefile.am
>>>   [Emil Velikov]
>>> * Use isblank() instead of open-coding it [Emil Velikov]
>>> * Don't leak asic_id_table memory if realloc fails [Emil Velikov]
>>> * Check and bump table_max_size at the beginning of the while loop [Emil
>>>   Velikov]
>>> * Initialize table_max_size to the number of entries in data/amdgpu.ids
>> Thank you for addressing some of my suggestions.
>> Reviewed-by: Emil Velikov <[e-mail]>
>
> Thanks! Pushed.
>
>
>> Personally I would not have bothered with the table_max_size thing
>
> It seemed silly to reallocate the memory in the default case where the
> amdgpu.ids file from this repository is used. :)
>
Agreed. Yet the single, "reduce memory consumption" realloc seems to
diminish amongst the ~150 [unneeded] strdup/free, in parse_one_line
</tease>

>> or the separate Makefile.
>
> You mean data/Makefile.am? What would you have done instead?
>
One can fold the two lines within the top makefile (see below) since
I'm lazy to complete the "use non-recursive makefiles" [1] branch.

-Emil
[1] https://github.com/evelikov/libdrm/commits/hello-world

dri-devel mailing list
[e-mail]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Michel Dänzer June 15, 2017, 8:09 a.m. UTC | #1

On 15/06/17 04:42 PM, Emil Velikov wrote:
> On 15 June 2017 at 04:16, Michel Dänzer <[e-mail]> wrote:
>> On 14/06/17 08:34 PM, Emil Velikov wrote:
>>
>>> Personally I would not have bothered with the table_max_size thing
>>
>> It seemed silly to reallocate the memory in the default case where the
>> amdgpu.ids file from this repository is used. :)
>>
> Agreed. Yet the single, "reduce memory consumption" realloc seems to
> diminish amongst the ~150 [unneeded] strdup/free, in parse_one_line
> </tease>

True.


>>> or the separate Makefile.
>>
>> You mean data/Makefile.am? What would you have done instead?
>>
> One can fold the two lines within the top makefile (see below) since
> I'm lazy to complete the "use non-recursive makefiles" [1] branch.
> 
> -Emil
> [1] https://github.com/evelikov/libdrm/commits/hello-world
> 
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -43,6 +43,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
>        --enable-manpages \
>        --enable-valgrind
> 
> +libdrmdatadir = @libdrmdatadir@
> +dist_libdrmdata_DATA = data/amdgpu.ids
> +
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm.pc

Thanks. I'm afraid I don't care enough right now, but maybe somebody
else can pick up your suggestions.

Patch

--- a/Makefile.am
+++ b/Makefile.am
@@ -43,6 +43,9 @@  AM_DISTCHECK_CONFIGURE_FLAGS = \
       --enable-manpages \
       --enable-valgrind

+libdrmdatadir = @libdrmdatadir@
+dist_libdrmdata_DATA = data/amdgpu.ids
+
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm.pc
_______________________________________________