[PATCH i-g-t v9 4/7] lib/intel_multigpu: Introduced gem_multigpu_count_class and igt_multi_fork_foreach_gpu

[Kamil Konieczny]

Hi Zbigniew,
On 2024-02-19 at 12:20:30 +0100, Zbigniew Kempczyński wrote:
> On Thu, Feb 15, 2024 at 05:10:11PM +0100, Kamil Konieczny wrote:
> > From: Dominik Karol Piątkowski <dominik.karol.piatkowski at intel.com>
> > 
> > Introduced gem_multigpu_count_class function that returns an actual
> > number of GPUs present in system, which allows for writing multi-GPU
> > test scenarios that does not require
> > --device pci:vendor=intel,device=discrete,card=all
> > to run as intended. Based on patch by Chris Wilson.
> > 
> > Introduced igt_multi_fork_foreach_gpu macro that helps with
> > writing multi-GPU test scenarios in idiomatic form:
> > 
> > igt_multi_fork_foreach_gpu(i915, DRIVER_INTEL)
> > 	test_function(i915);
> > igt_waitchildren();
> > 
> > Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski at intel.com>
> > Cc: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> > Signed-off-by: "Dominik Karol Piątkowski" <dominik.karol.piatkowski at intel.com>
> > Signed-off-by: Chris Wilson <chris.p.wilson at linux.intel.com>
> > Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > ---
> >  lib/intel_multigpu.c | 11 +++++++++++
> >  lib/intel_multigpu.h | 15 +++++++++++++++
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c
> > index 34c9f936d..0e76d8aa3 100644
> > --- a/lib/intel_multigpu.c
> > +++ b/lib/intel_multigpu.c
> > @@ -3,10 +3,21 @@
> >   * Copyright © 2023 Intel Corporation
> >   */
> >  
> > +#include "drmtest.h"
> >  #include "igt_core.h"
> >  #include "igt_device_scan.h"
> >  #include "intel_multigpu.h"
> >  
> > +int gem_multigpu_count_class(int class)
> > +{
> > +	int count = 0;
> > +
> > +	igt_foreach_gpu(fd, class)
> > +		count++;
> > +
> > +	return count;
> > +}
> > +
> >  void gem_require_multigpu(int count)
> >  {
> >  	struct igt_devices_print_format fmt = {
> > diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h
> > index 98dc5a4ce..0ebc73e4a 100644
> > --- a/lib/intel_multigpu.h
> > +++ b/lib/intel_multigpu.h
> > @@ -6,6 +6,21 @@
> >  #ifndef __INTEL_MULTIGPU_H
> >  #define __INTEL_MULTIGPU_H
> >  
> > +#include "drmtest.h"
> > +#include "igt_core.h"
> > +
> >  void gem_require_multigpu(int count);
> > +int gem_multigpu_count_class(int class);
> > +
> > +#define igt_foreach_gpu(fd__, id__) \
> > +	for (int igt_unique(i) = 0, fd__; \
> > +		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >= 0; \
> > +		close(fd__))
> > +
> > +#define igt_multi_fork_foreach_gpu(__fd, __id) \
> > +	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
> > +		for (int __fd = drm_open_driver_another(igt_unique(__i), __id); \
> > +			__fd >= 0; \
> > +			close(__fd), __fd = -1) \

One more thing I spotted recently, instead of close() it would be
better to use drm_close_driver(__fd), also using __chipset would
make clear what a second parameter should be.

Regards,
Kamil

> >  
> >  #endif /* __INTEL_MULTIGPU_H */
> > -- 
> > 2.42.0
> > 
> 
> For me those helpers looks correct, so:
> 
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> 
> --
> Zbigniew