Re: [PATCH RFC i-g-t 0/2] helper function

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Apr 30, 2024 at 09:34:19PM -0600, Zeng, Oak wrote:
> @Brost, Matthew This is the improved version of the igt helpers. It has some clear concepts of creating buffer and command buffer, and command submission. With those simple concepts, we are able to write an igt test in <20 LOC, see patch 2.
> 

Sure you can write an IGT with 20 LOC but these helpers ultimately put
limits on the complexity of tests you can build. To get true coverage,
you almost always are going to open code something. Or the helpers get
so complex it actually makes things more confusing...

The open coding admittedly could be a lot better in the current IGT
suite. A lot of these tests I just wrote as quickly as possible while
writing Xe in a PoC stage and then they got merged. If I could rewrite
these tests, they would a look a bit different.

Let give some examples of the limitations of the current helpers in this
series.

xe_create_buffer - Things I could conviable want it to do beyond what it does for a system allocator test...
	- I want to use mmap with no file backing, with
	  several different flag combos (MAP_SHARED, MAP_PRIVATE,
	  MAP_ANONYMOUS, etc...)
	- I want to use mmap with file backing
	- I want various possible alignments
	- I want various possible sizes
	- I already have an exec queue
	- I don't want to bind it (e.g. already have system allocator
	  bind for it)
	- I don't want to alloc a new ufence
	- I may or may not want to memset it
	- I want to mlock it

This is quickly gotten out of hand for a single helper. I could go on
with the other helpers too.

If you really want to build powerful, complex tests in a generic way
from my experience you really need to build an entire framework.
Typcially these involve building operations lists and the passing these
off these opertaions to a test runner which executes them. Building
something like that takes quite a bit of skill and time. I haven't seen
anything like that in the IGT suites and is likely out scope without a
serious commitment (like a year plus dev on just the framework).

With all this, I'm not saying don't do this but I'm very sceptical this
is going to be all that useful beyond fairly simple tests. I could be
wrong or other might have different opinions than me too.

Matt

> We might still fine tune those helpers but roughly the concept shouldn't change much. 
> 
> Do you think we can simplify existing igt tests such as xe-exec-fault-mode using those helpers? Do you suggest us to give it a try? If not, we will just use those helpers for svm test only.
> 
> Hi Krishna, 
> 
> See comments inline
> 
> > -----Original Message-----
> > From: Bommu, Krishnaiah <krishnaiah.bommu@intel.com>
> > Sent: Tuesday, April 30, 2024 2:28 PM
> > To: igt-dev@lists.freedesktop.org
> > Cc: Bommu, Krishnaiah <krishnaiah.bommu@intel.com>; Zeng, Oak
> > <oak.zeng@intel.com>; Ghimiray, Himal Prasad
> > <himal.prasad.ghimiray@intel.com>
> > Subject: [PATCH RFC i-g-t 0/2] helper function
> 
> The subject can be something like: Introduce helper functions for buffer creation and command submission etc
> 
> Oak
> 
> > 
> > Introduce helper functions for object creation, binding, submission,
> > and destruction, applicable for SVM and other tests
> > 
> > xe_svm test is validating the helper function introduced in 'lib/xe/xe_util:
> > helper function'
> > 
> > In this test I haven’t included the svm functionality, next patch I will include
> > svm functionality
> > 
> > Test results
> > root@DUT7032PVCMella:/home/gta/xe/new/igt-gpu-
> > tools# ./build/tests/xe_svm
> > IGT-Version: 1.28-g365f81646 (x86_64) (Linux: 6.8.0-rc5-xedrmtip+ x86_64)
> > Using IGT_SRANDOM=1714498247 for randomisation
> > Opened device: /dev/dri/card1
> > Starting subtest: svm-basic-malloc
> > Subtest svm-basic-malloc: SUCCESS (0.022s)
> > 
> > Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
> > Cc: Oak Zeng <oak.zeng@intel.com>
> > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > 
> > Bommu Krishnaiah (2):
> >   lib/xe/xe_util: helper function
> >   tests/intel/xe_svm: basic xe_svm test
> > 
> >  lib/xe/xe_util.c     | 113
> > +++++++++++++++++++++++++++++++++++++++++++
> >  lib/xe/xe_util.h     |  32 ++++++++++++
> >  tests/intel/xe_svm.c |  99 +++++++++++++++++++++++++++++++++++++
> >  tests/meson.build    |   1 +
> >  4 files changed, 245 insertions(+)
> >  create mode 100644 tests/intel/xe_svm.c
> > 
> > --
> > 2.25.1
>