Hi Christian,
Thanks for your suggestion.
Regarding the configuration file moving to test/amdgpu, I am fine with this.
But reg putting all the tests in C code, though it looks like the unit test can stay self containing, but without one independent configuration file, user will find it's inconvenient when performing the tests.
For example, if we move all RAS inject tests into C code, however, if the user only cares the RAS unit test in certain module/submodule test, he/she would not like to see inject tests in all modules run after launching amdgpu_test, so it's not friendly to user.
Anyway, your suggestion still makes sense, as amdgpu_test should get rid of other external packages except CUnit framework.
So we still the balance between above two cases:
1. Make amdgpu_test self containing without dependency on other external packages.
2. Allow user can configure the tests when running amdgpu_test.
Then the possible solutions I can illustrate are:
1. Still keep one configuration file, but not in json format. It’s a normal configuration file, we will add code in amdgpu_test to parse the configuration file, and remove all json APIs.
2. Put all tests in c code, and extend the args when running amdgpu_test, which will allow user can run special RAS test.
I am opened to above two solutions or other missed ones.
Regards,
Guchun
-----Original Message-----
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: Monday, August 19, 2019 3:14 PM
To: Chen, Guchun <Guchun.Chen@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Li, Dennis <Dennis.Li@xxxxxxx>; Cui, Flora <Flora.Cui@xxxxxxx>; Zhou1, Tao <Tao.Zhou1@xxxxxxx>
Cc: Li, Candice <Candice.Li@xxxxxxx>
Subject: Re: [PATCH libdrm] amdgpu: add mmhub ras inject unit test
Hi Guchun,
in this case this is a bit awkward implemented.
See the files in the data directory are for installation together with the libdrm library and NOT for the unit tests. Please move the file to tests/amdgpu instead.
I would also re-consider this approach since we intentionally use the CUnit framework to avoid dependencies on external libraries like json and external files.
We should probably better configure the tests directly in the C code so that the unit test stays self containing.
Regards,
Christian.
Am 19.08.19 um 05:16 schrieb Chen, Guchun:
> Hi Christian,
>
> Yes, we added one configuration file named "amdgpu_ras.json" for RAS inject unit test to drm master branch.
> This unit test will be maintained to illustrate all the RAS tests we absolutely support in IP modules/submodules.
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
> Sent: Friday, August 16, 2019 7:12 PM
> To: Chen, Guchun <Guchun.Chen@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx;
> Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Li, Dennis
> <Dennis.Li@xxxxxxx>; Cui, Flora <Flora.Cui@xxxxxxx>; Zhou1, Tao
> <Tao.Zhou1@xxxxxxx>
> Cc: Li, Candice <Candice.Li@xxxxxxx>
> Subject: Re: [PATCH libdrm] amdgpu: add mmhub ras inject unit test
>
> Well this doesn't look like C to me. Did we added a configuration file for the ras unit tests or something like that?
>
> Christian.
>
> Am 16.08.19 um 13:04 schrieb Guchun Chen:
>> Change-Id: Ia76b95162f5f6f419f70b53ef443bceaf2e092e0
>> Signed-off-by: Guchun Chen <guchun.chen@xxxxxxx>
>> ---
>> data/amdgpu_ras.json | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/data/amdgpu_ras.json b/data/amdgpu_ras.json index
>> 26fd9465..484f12f2 100644
>> --- a/data/amdgpu_ras.json
>> +++ b/data/amdgpu_ras.json
>> @@ -121,6 +121,9 @@
>> "utc_atcl2_cache_4k_bank": 111
>> }
>> },
>> + "mmhub": {
>> + "index": 3
>> + },
>> },
>> "type": {
>> "parity": 1,
>> @@ -263,5 +266,12 @@
>> "address": 0,
>> "value": 0
>> },
>> + {
>> + "name": "ras_mmhub.1.0",
>> + "block": "mmhub",
>> + "type": "single_correctable",
>> + "address": 0,
>> + "value": 0
>> + },
>> ]
>> }
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
