[igt-dev] [PATCH i-g-t] tests/intel/xe_exec_store: Test to check CONDITIONAL_BATCH_BUFFER_END

[Dandamudi, Priyanka]

> -----Original Message-----
> From: Kumar, Janga Rahul <janga.rahul.kumar at intel.com>
> Sent: Tuesday, December 5, 2023 5:13 PM
> To: Dandamudi, Priyanka <priyanka.dandamudi at intel.com>; Ch, Sai
> Gowtham <sai.gowtham.ch at intel.com>; igt-dev at lists.freedesktop.org
> Subject: RE: [PATCH i-g-t] tests/intel/xe_exec_store: Test to check
> CONDITIONAL_BATCH_BUFFER_END
> 
> 
> 
> > -----Original Message-----
> > From: Dandamudi, Priyanka <priyanka.dandamudi at intel.com>
> > Sent: Thursday, November 30, 2023 7:23 PM
> > To: Dandamudi, Priyanka <priyanka.dandamudi at intel.com>; Kumar, Janga
> > Rahul <janga.rahul.kumar at intel.com>; Ch, Sai Gowtham
> > <sai.gowtham.ch at intel.com>; igt-dev at lists.freedesktop.org
> > Subject: [PATCH i-g-t] tests/intel/xe_exec_store: Test to check
> > CONDITIONAL_BATCH_BUFFER_END
> >
> > From: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> >
> > Added a basic test to check MI_CONDITIONAL_BATCH_BUFFER_END
> > instruction.
> >
> > Cc: Janga Rahul Kumar <janga.rahul.kumar at intel.com>
> > Cc: Sai Gowtham Ch <sai.gowtham.ch at intel.com>
> > Signed-off-by: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> > ---
> >  tests/intel/xe_exec_store.c | 45
> > ++++++++++++++++++++++++++++++++++---
> >  1 file changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/intel/xe_exec_store.c b/tests/intel/xe_exec_store.c
> > index
> > 9c14bfd14..66fbacc77 100644
> > --- a/tests/intel/xe_exec_store.c
> > +++ b/tests/intel/xe_exec_store.c
> > @@ -21,6 +21,8 @@
> >   */
> >
> >  #define MAX_INSTANCE 9
> > +#define STORE 0
> > +#define COND_BATCH 1
> >
> >  struct data {
> >  	uint32_t batch[16];
> > @@ -48,11 +50,38 @@ static void store_dword_batch(struct data *data,
> > uint64_t addr, int value)
> >  	data->addr = batch_addr;
> >  }
> >
> > +static void cond_batch(struct data *data, uint64_t addr, int value) {
> > +	int b;
> > +	uint64_t batch_offset = (char *)&(data->batch) - (char *)data;
> > +	uint64_t batch_addr = addr + batch_offset;
> > +	uint64_t sdi_offset = (char *)&(data->data) - (char *)data;
> > +	uint64_t sdi_addr = addr + sdi_offset;
> > +
> > +	b = 0;
> > +	data->batch[b++] = MI_ATOMIC | MI_ATOMIC_INC;
> > +	data->batch[b++] = sdi_addr;
> > +	data->batch[b++] = sdi_addr >> 32;
> > +	data->batch[b++] = MI_COND_BATCH_BUFFER_END |
> > MI_DO_COMPARE | 5 << 12 | 2;
> MI_DO_COMPARE is setting 21st bit in the instruction, Does it make any
> difference if we don't add it ?
> 
> > +	data->batch[b++] = value;
> > +	data->batch[b++] = sdi_addr;
> > +	data->batch[b++] = sdi_addr >> 32;
> > +	data->batch[b++] = MI_BATCH_BUFFER_START | 1;
> > +	data->batch[b++] = lower_32_bits(batch_addr);
> > +	data->batch[b++] = upper_32_bits(batch_addr);
> > +	data->batch[b++] = MI_BATCH_BUFFER_END;
> MI_BATCH_BUFFER_END might not be needed here as we are trying to END
> buffer execution conditionally using MI_COND_BATCH_BUFFER_END
> Will look into it, and update it if required.
-Priyanka
> > +	igt_assert(b <= ARRAY_SIZE(data->batch));
> > +
> > +	data->addr = batch_addr;
> > +}
> > +
> >  /**
> >   * SUBTEST: basic-store
> >   * Description: Basic test to verify store dword.
> > + * SUBTEST: basic-cond-batch
> > + * Description: Basic test to verify cond batch end instruction.
> >   */
> > -static void store(int fd)
> > +static void basic_inst(int fd, int inst_type)
> >  {
> >  	struct drm_xe_sync sync = {
> >  		.flags = DRM_XE_SYNC_FLAG_SYNCOBJ |
> DRM_XE_SYNC_FLAG_SIGNAL, @@
> > -86,7 +115,14 @@ static void store(int fd)
> >
> >  	xe_vm_bind_async(fd, vm, hw_engine->gt_id, bo, 0, addr, bo_size,
> > &sync, 1);
> >  	data = xe_bo_map(fd, bo, bo_size);
> > -	store_dword_batch(data, addr, value);
> > +
> > +	if (inst_type == STORE)
> > +		store_dword_batch(data, addr, value);
> > +	else {
> Use COND_BATCH check to make this function future proof.
> 
> > +		/* A random value where it stops at the below value. */
> > +		value = 20;
> > +		cond_batch(data, addr, value);
> Validate that the buffer executed in a conditional loop by checking the
> incremented address with the value "20" here.
> 
> -Rahul

This validation is already present in the test in the next lines. So, no need to add it.
-Priyanka
> 
> > +	}
> >
> >  	exec_queue = xe_exec_queue_create(fd, vm, hw_engine, 0);
> >  	exec.exec_queue_id = exec_queue;
> > @@ -302,7 +338,10 @@ igt_main
> >  	}
> >
> >  	igt_subtest("basic-store")
> > -		store(fd);
> > +		basic_inst(fd, STORE);
> > +
> > +	igt_subtest("basic-cond-batch")
> > +		basic_inst(fd, COND_BATCH);
> >
> >  	igt_subtest("basic-all") {
> >  		xe_for_each_gt(fd, gt)
> > --
> > 2.25.1