Hi Christian,
Yes, we already have functionality to select one specific test from the command
line, but that command line is limited.
It only allows user to select one general test, like ras test, but inside ras
test, it has the enable/disable/error inject test respectively. Now what we are
talking about is exactly the inject test. We support plenty of ras inject test
in submodules like GFX, UMC, MMHUB. So without a command extension, when we run
RAS error inject test, we will see all inject tests in different sub IP blocks
are performed. My previous concern is the experience is not good if user wants
to run one special ras inject test in sub block like UMC. However, I had one
offline discussion with Hawking just now. We are now fine to maintain the test
configuration in C code for upstream. This will keep amdgpu_test not depending
on other external libraries.
Regards,
Guchun
-----Original Message-----
From: Koenig, Christian <christian.koe...@amd.com>
Sent: Monday, August 19, 2019 3:53 PM
To: Chen, Guchun <guchun.c...@amd.com>; amd-gfx@lists.freedesktop.org; Zhang,
Hawking <hawking.zh...@amd.com>; Li, Dennis <dennis...@amd.com>; Cui, Flora
<flora....@amd.com>; Zhou1, Tao <tao.zh...@amd.com>
Cc: Li, Candice <candice...@amd.com>
Subject: Re: [PATCH libdrm] amdgpu: add mmhub ras inject unit test
Hi Guchun,
> Put all tests in c code, and extend the args when running amdgpu_test, which
> will allow user can run special RAS test.
Ah! Sounds like we have just a misunderstanding here. As far as I know we
already have the functionality to select only specific tests to run from the
command line of amdgpu_test.
That's actually the background reason why we use the CUnit framework.
I haven't used that functionality in a long time, could be that it is broken or
something. But at least in theory it should be there.
Regards,
Christian.
Am 19.08.19 um 09:48 schrieb Chen, Guchun:
> 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.koe...@amd.com>
> Sent: Monday, August 19, 2019 3:14 PM
> To: Chen, Guchun <guchun.c...@amd.com>; amd-gfx@lists.freedesktop.org;
> Zhang, Hawking <hawking.zh...@amd.com>; Li, Dennis
> <dennis...@amd.com>; Cui, Flora <flora....@amd.com>; Zhou1, Tao
> <tao.zh...@amd.com>
> Cc: Li, Candice <candice...@amd.com>
> 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.leichtzumer...@gmail.com>
>> Sent: Friday, August 16, 2019 7:12 PM
>> To: Chen, Guchun <guchun.c...@amd.com>;
>> amd-gfx@lists.freedesktop.org; Zhang, Hawking
>> <hawking.zh...@amd.com>; Li, Dennis <dennis...@amd.com>; Cui, Flora
>> <flora....@amd.com>; Zhou1, Tao <tao.zh...@amd.com>
>> Cc: Li, Candice <candice...@amd.com>
>> 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.c...@amd.com>
>>> ---
>>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
