[igt-dev] [V5 i-g-t 16/81] tests/i915/kms_dsc: Document each subtest for testplan

[Sharma, Swati2]

On 14-Aug-23 3:09 PM, Mauro Carvalho Chehab wrote:
> On Mon, 14 Aug 2023 10:34:52 +0530
> "Sharma, Swati2" <swati2.sharma at intel.com> wrote:
> 
>> On 10-Aug-23 10:53 PM, Modem, Bhanuprakash wrote:
>>> Hi Swati,
>>>
>>> On Wed-09-08-2023 01:20 pm, Sharma, Swati2 wrote:
>>>> On 29-Jun-23 8:18 PM, Bhanuprakash Modem wrote:
>>>>> Add documentation for all kms tests which is used by IGT testplan
>>>>> documentation.
>>>>>
>>>>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
>>>>> ---
>>>>>    tests/i915/kms_dsc.c | 27 +++++++++++++++++++++++++++
>>>>>    1 file changed, 27 insertions(+)
>>>>>
>>>>> diff --git a/tests/i915/kms_dsc.c b/tests/i915/kms_dsc.c
>>>>> index 0a3e29924..4cce7face 100644
>>>>> --- a/tests/i915/kms_dsc.c
>>>>> +++ b/tests/i915/kms_dsc.c
>>>>> @@ -30,8 +30,35 @@
>>>>>     *
>>>>>     */
>>>>> +/**
>>>>> + * TEST: kms dsc
>>>>> + * Category: Display
>>>>> + * Description: Test to validate display stream compression
>>>>> + */
>>>>>    #include "kms_dsc_helper.h"
>>>>> +/**
>>>>> + * SUBTEST: dsc-%s
>>>>> + * Description: Tests basic display stream compression functionality
>>>>> if supported
>>>>> + *              by a connector by forcing %arg[1]
>>>>
>>>> forcing DSC on all connectors that support it with %arg[1]
>>>> (remove this from below subtests description)
>>>
>>> This description is adopted from igt_describe(), Feel free to update the
>>> subtest description I can pick it from there.
>>
>> But you haven't copied it as it is..right? Its the optimized version
>> of igt_describe(). I simply meant.
>> "forcing DSC on all connectors that support it with %arg[1]" => is
>> redundant in descriptions you have written below for each subtest.
>> We don't have to change igt_describe() for this. This should be handled
>> here.
>>
>>>    
>>>>   
>>>>> + * Driver requirement: any
>>>>
>>>> This is intel specific test.
>>>
>>> Yes. As this kms testplan is designed for Intel, the allowed values are:
>>>
>>> i915: supports i915 driver only
>>> xe: supports xe driver only
>>> any: supports both i915 & xe
>>
>> This ANY seems to correspond to drm_open_driver_master(DRIVER_ANY).
>> IDK this is right. @mauro do you have any inputs here?
>> Shouldn't "any" correspond to "any" driver. Even if this documentation
>> is specific to intel but tests outside i915/ directory are used by other
>> vendors too.
> 
> IMO, the best here is to provide a list of drivers that support the test,
> e. g.:
> 
> 	Driver requirement: i915, xe
> 
> Rationale:
> 
> - It lets it clear that only Intel drivers are supported;
> - It is clear what drivers support it;
> - There's no need to document somewhere what "any" means;
> - If in the future we have a new XYZ driver that it is not supported
>    by this test, there won't be any need to change existing documentation;
> - We already use comma-separated lists for other fields, like feature
>    (used on some core tests and at i915 driver).

Agree. @modem can we make these changes in complete series?

> 
> Regards,
> Mauro