[PATCH libdrm] amdgpu: add mmhub ras inject unit test

[Koenig, Christian]

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.Koenig at amd.com>
> Sent: Monday, August 19, 2019 3:14 PM
> To: Chen, Guchun <Guchun.Chen at amd.com>; amd-gfx at lists.freedesktop.org; Zhang, Hawking <Hawking.Zhang at amd.com>; Li, Dennis <Dennis.Li at amd.com>; Cui, Flora <Flora.Cui at amd.com>; Zhou1, Tao <Tao.Zhou1 at amd.com>
> Cc: Li, Candice <Candice.Li at 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.leichtzumerken at gmail.com>
>> Sent: Friday, August 16, 2019 7:12 PM
>> To: Chen, Guchun <Guchun.Chen at amd.com>; amd-gfx at lists.freedesktop.org;
>> Zhang, Hawking <Hawking.Zhang at amd.com>; Li, Dennis
>> <Dennis.Li at amd.com>; Cui, Flora <Flora.Cui at amd.com>; Zhou1, Tao
>> <Tao.Zhou1 at amd.com>
>> Cc: Li, Candice <Candice.Li at 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.chen at 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
>>> +        },
>>>         ]
>>>     }