Re: [PATCH i-g-t 4/5] benchmarks/gem_exec_trace: use hars_petruska_f54_1_random() from lib/rand

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 25 Jun 2026, Zbigniew Kempczyński <zbigniew.kempczynski@intel.com> wrote:
> On Wed, Jun 24, 2026 at 11:03:51AM +0300, Jani Nikula wrote:
>> On Mon, 22 Jun 2026, Jani Nikula <jani.nikula@intel.com> wrote:
>> > On Fri, 19 Jun 2026, Zbigniew Kempczyński <zbigniew.kempczynski@intel.com> wrote:
>> >> On Thu, Jun 18, 2026 at 05:09:52PM +0300, Jani Nikula wrote:
>> >>> Remove the local duplicate of hars_petruska_f54_1_random() by switching
>> >>> to use lib/rand.
>> >>> 
>> >>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >>> ---
>> >>>  benchmarks/gem_exec_trace.c | 12 +++---------
>> >>>  1 file changed, 3 insertions(+), 9 deletions(-)
>> >>> 
>> >>> diff --git a/benchmarks/gem_exec_trace.c b/benchmarks/gem_exec_trace.c
>> >>> index 4263ccb03dbd..c27639105903 100644
>> >>> --- a/benchmarks/gem_exec_trace.c
>> >>> +++ b/benchmarks/gem_exec_trace.c
>> >>> @@ -42,6 +42,7 @@
>> >>>  #include "drm.h"
>> >>>  #include "drmtest.h"
>> >>>  #include "i915/gem_create.h"
>> >>> +#include "igt_rand.h"
>> >>>  #include "igt_stats.h"
>> >>>  #include "intel_io.h"
>> >>>  #include "ioctl_wrappers.h"
>> >>> @@ -92,14 +93,7 @@ struct trace_wait {
>> >>>  	uint32_t handle;
>> >>>  } __attribute__((packed));
>> >>>  
>> >>> -static uint32_t hars_petruska_f54_1_random(void)
>> >>> -{
>> >>> -	static uint32_t state = 0x12345678;
>> >>> -
>> >>> -#define rol(x,k) ((x << k) | (x >> (32-k)))
>> >>> -	return state = (state ^ rol (state, 5) ^ rol (state, 24)) + 0x37798849;
>> >>> -#undef rol
>> >>> -}
>> >>> +static uint32_t random_state = 0x12345678;
>> >>>  
>> >>>  static double elapsed(const struct timespec *start, const struct timespec *end)
>> >>>  {
>> >>> @@ -277,7 +271,7 @@ static double replay(const char *filename, long nop, long range)
>> >>>  				sizeof(*exec_objects)))->handle = bo[0];
>> >>>  
>> >>>  			if (nop > 0) {
>> >>> -				eb.batch_start_offset = hars_petruska_f54_1_random();
>> >>> +				eb.batch_start_offset = hars_petruska_f54_1_random(&random_state);
>> >>
>> >> With random_state as above this offset is not random. I think adding nop
>> >> to random_state would do this.
>> >
>> > I'm only aiming to do a non-functional conversion to existing library
>> > functions. Please explain how this patch changes behaviour; I don't see
>> > it.
>> 
>> Zbigniew, ping?
>
> I'm sorry for the delay.
>
> I wondered to change offsets for each run, with same random_state on the
> beginning we use same offsets sequence.
>
> But it's fine to me, I mean your change doesn't change functionality
> but code placement.
>
> Pick my r-b for this:
>
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Thanks for the reviews, pushed to igt.

BR,
Jani.


>
> --
> Zbigniew
>
>> 
>> >
>> >
>> > BR,
>> > Jani.
>> >
>> >>
>> >> --
>> >> Zbigniew
>> >>
>> >>>  				eb.batch_start_offset =
>> >>>  					((uint64_t)eb.batch_start_offset * range) >> 32;
>> >>>  				eb.batch_start_offset = ALIGN(eb.batch_start_offset, 64);
>> >>> -- 
>> >>> 2.47.3
>> >>> 
>> 
>> -- 
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel