[PATCH i-g-t v9 2/7] lib/drmtest: Introduced drm_open_driver_another

[Kamil Konieczny]

Hi Janusz,
On 2024-02-20 at 10:32:58 +0100, Janusz Krzysztofik wrote:
> Hi Kamil,
> 
> On Monday, 19 February 2024 18:42:32 CET Kamil Konieczny wrote:
> > Hi Janusz,
> > On 2024-02-16 at 11:23:58 +0100, Janusz Krzysztofik wrote:
> > > Hi Kamil, Dominik,
> > > 
> > > On Thursday, 15 February 2024 17:10:09 CET Kamil Konieczny wrote:
> > > > From: Dominik Karol Piątkowski <dominik.karol.piatkowski at intel.com>
> > > > 
> > > > For user convenience, introduce drm_open_driver_another as
> > > > a wrapper for __drm_open_driver_another with successful open
> > > > assert.
> > > 
> > > Out of several existing variants of *drm_open_driver* functions, those which 
> > > names start with "drm_", wrapped around their counterparts which names start 
> > > with double underscores, used to set up an exit handler that performs 
> > > necessary cleanups before exit, freeing users from taking care of that.  Don't 
> > > we want to keep that convention also for drm_open_driver_another() and set up 
> > > the exit handler from inside that wrapper on success?
> > > 
> > > Thanks,
> > > Janusz
> > > 
> > 
> > How and what should we unwind here? There is only fd, I do not see
> > any other var or state to unwind? Am I missing something?
> 
> Have you had a look at drm_open()?
> 
>         /* For i915, at least, we ensure that the driver is idle before
>          * starting a test and we install an exit handler to wait until
>          * idle before quitting.
>          */
>         if (is_i915_device(fd)) {
>                 if (__sync_fetch_and_add(&open_count, 1) == 0) {
>                         __cancel_work_at_exit(fd);
>                         at_exit_drm_fd = drm_reopen_driver(fd);
>                         igt_install_exit_handler(cancel_work_at_exit);
>                 }
>         }
> 
> From that point of view, how does multi-gpu testing differ from single-gpu?
> 
> And yes, that can be not trivial to extend the existing exit handlers solution 
> over multiple device file descriptor handling.
> 

This is another problem, I try to solve one, not all at once.
I could add a helper to stop resets from children as it can be
helpfull not only in multi-gpu scenarios.

> Also ...
> 
> > 
> > Regards,
> > Kamil
> > 
> > > > 
> > > > v2: rebased (Kamil)
> > > > v6: reword description (Janusz)
> > > > 
> > > > 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: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > > > Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > > > ---
> > > >  lib/drmtest.c | 19 +++++++++++++++++++
> > > >  lib/drmtest.h |  1 +
> > > >  2 files changed, 20 insertions(+)
> > > > 
> > > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > > index 52b5a2020..73d9159af 100644
> > > > --- a/lib/drmtest.c
> > > > +++ b/lib/drmtest.c
> > > > @@ -563,6 +563,25 @@ int __drm_open_driver_another(int idx, int chipset)
> > > >  	return fd;
> > > >  }
> > > >  
> > > > +/*
> > > > + * drm_open_driver_another
> > > > + * @idx: index of the device you are opening
> > > > + * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
> > > > + *
> > > > + * A wrapper for __drm_open_driver with successful open assert.
> > > > + *
> > > > + * Returns:
> > > > + * An open DRM fd
> > > > + */
> > > > +int drm_open_driver_another(int idx, int chipset)
> > > > +{
> > > > +	int fd = __drm_open_driver_another(idx, chipset);
> > > > +
> > > > +	igt_assert_fd(fd);
> 
> drm_open_driver() skips in that case.  Shouldn't drm_open_driver_another() 
> also skip for consistency?

Good point, I will re-read this,

Regards,
Kamil

> 
> Thanks,
> Janusz
> 
> > > > +
> > > > +	return fd;
> > > > +}
> > > > +
> > > >  /**
> > > >   * __drm_open_driver:
> > > >   * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
> > > > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > > > index 6bc819734..856208c48 100644
> > > > --- a/lib/drmtest.h
> > > > +++ b/lib/drmtest.h
> > > > @@ -101,6 +101,7 @@ void __set_forced_driver(const char *name);
> > > >  
> > > >  int __drm_open_device(const char *name, unsigned int chipset);
> > > >  void drm_load_module(unsigned int chipset);
> > > > +int drm_open_driver_another(int idx, int chipset);
> > > >  int drm_open_driver(int chipset);
> > > >  int drm_open_driver_master(int chipset);
> > > >  int drm_open_driver_render(int chipset);
> > > > 
> > > 
> > > 
> > > 
> > > 
> > 
> 
> 
> 
>