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

[Mauro Carvalho Chehab]

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).

Regards,
Mauro