Re: [PATCH i-g-t] tests/intel/kms_psr2_sf: Fix primary_format setting

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

On Tue, 2023-12-19 at 06:43 +0000, B, Jeevan wrote:
> > -----Original Message-----
> > From: Govindapillai, Vinod <vinod.govindapillai@intel.com>
> > Sent: Monday, December 18, 2023 5:43 PM
> > To: igt-dev@lists.freedesktop.org; B, Jeevan <jeevan.b@intel.com>
> > Cc: Sharma, Swati2 <swati2.sharma@intel.com>
> > Subject: Re: [PATCH i-g-t] tests/intel/kms_psr2_sf: Fix primary_format setting
> > 
> > Hi jeevan,
> > 
> > This might not be enough.
> > 
> > For example, if you are executing all the tests, after the first iteration y = 0, the
> > data.primary_format is set to DRM_FORMAT_NV12. Then in y = 1 (FBC cases) will
> > be executed with format as DRM_FORMAT_NV12 which will fail.
> > 
> > After introducing the FBC on/off loop, I think there could be other issues as well,
> > because "data"
> > members are directly modified as required by the tests in that loop.
> > 
> > Also for pre-lnl platforms, do we need to even list and execute "fbc-" cases as it is
> > not suported and will be skipped?
> > 
> Should we restructure the test by add new subtests instead of looping it ?
> 
A quick solution could be to move the initialization some of the common "data" members to the the
beginning of the "for" loop including the data.primary_format from "igt_fixture". Or create another
function to init the data members according to the test. I am not sure if that is according to the
practices in IGT.

> NV12 should be fixed with this logic, 
I dont think so! As mentioned if you execute the full "kms_psr2_sf", y = 1 (FBC) iteration tests
will be executed with format as NV12 and will fail.

BR
Vinod

> but other parameters might be affected as you pointed out. 

> 
> > BR
> > Vinod
> > 
> > 
> > On Thu, 2023-12-14 at 21:12 +0530, Jeevan B wrote:
> > > The format is being changed globally to DRM_FORMAT_NV12. additionally,
> > > FBC doesn't support the NV12 format, so this issue is being addressed.
> > > 
> > > Signed-off-by: Jeevan B <jeevan.b@intel.com>
> > > ---
> > >  tests/intel/kms_psr2_sf.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/intel/kms_psr2_sf.c b/tests/intel/kms_psr2_sf.c
> > > index 76435f501..272260cdf 100644
> > > --- a/tests/intel/kms_psr2_sf.c
> > > +++ b/tests/intel/kms_psr2_sf.c
> > > @@ -1387,7 +1387,6 @@ igt_main
> > >                  * plane and continuous updates.
> > >                  */
> > >                 data.op = PLANE_UPDATE_CONTINUOUS;
> > > -               data.primary_format = DRM_FORMAT_NV12;
> > >                 igt_describe("Test that selective fetch works on
> > > overlay plane");
> > >                 igt_subtest_with_dynamic_f("%soverlay-%s-sf",
> > > append_fbc_subtest[y],
> > >                                            op_str(data.op)) { @@
> > > -1404,6 +1403,11 @@ igt_main
> > >                                                 data.pipe = pipes[i];
> > >                                                 data.output =
> > > outputs[i];
> > >                                                 data.damage_area_count
> > > = 1;
> > > +                                               if (data.fbc_flag ==
> > > +true &&
> > > +                                                   data.op_fbc_mode
> > > +== FBC_ENABLED)
> > > +
> > > +data.primary_format = DRM_FORMAT_XRGB8888;
> > > +                                               else
> > > +
> > > +data.primary_format = DRM_FORMAT_NV12;
> > >                                                 data.test_plane_id =
> > > DRM_PLANE_TYPE_OVERLAY;
> > >                                                 data.coexist_feature =
> > > j;
> > >                                                 prepare(&data);
>