Re: [PATCH i-g-t v2] tests/intel/xe_oa: Check gtidle before rc6 disable test

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Thu, 03 Oct 2024 05:25:18 -0700, Riana Tauro wrote:
>
Hi Soham

> reword the subject
>
> On 10/3/2024 5:27 PM, Soham Purkait wrote:
> > Rc6 disable test has been modified in order to check the
> > presence of gtidle node in sysfs gt0 path through the following
> > code snippets :
> > igt_require(xe_sysfs_gt_has_node(drm_fd, 0, "gtidle")).
> do not add code snippet in commit message

Commit message should state the "why" of this patch, not described contents
of the patch. "Why" meaning why is the patch needed or the reason it is
being sent.

> >
> > As of now only Render GT (gt0) is used for rc6 disable test through OA
> > activation. Standalone Media (gt1) is not supported yet for the same.
> reword?
>  OA is supported only on render gt.

This information looks irrelevant for the patch. It is also incorrect I
think.

>
> add version history
> >
> > Signed-off-by: Soham Purkait <soham.purkait@intel.com>
> > ---
> >   tests/intel/xe_oa.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> > index 9be4fd633..6538d6323 100644
> > --- a/tests/intel/xe_oa.c
> > +++ b/tests/intel/xe_oa.c
> > @@ -4644,8 +4644,10 @@ igt_main
> >			test_oa_unit_concurrent_oa_buffer_read();
> >	}
> >   -	igt_subtest("rc6-disable")
> > +	igt_subtest("rc6-disable"){
> space before {

Check tabs too (use tabs not spaces).

>
> Thanks,
> Riana
> > +		igt_require(xe_sysfs_gt_has_node(drm_fd, 0, "gtidle"));

Is this even possible? Why do we need to check this?


> >		test_rc6_disable();
> > +	}
> >		igt_subtest_with_dynamic("stress-open-close") {
> >		__for_one_hwe_in_oag(hwe)