| Message ID | 20180405184757.15907-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/04/2018 19:47, Chris Wilson wrote: > As we reopen the fd for each subtest, and we need a DRM_MASTER, we need > to drop master on the original before we are allowed to claim DRM_MASTER > on the second. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > tests/gem_eio.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tests/gem_eio.c b/tests/gem_eio.c > index 9599e73d..66d8c18a 100644 > --- a/tests/gem_eio.c > +++ b/tests/gem_eio.c > @@ -41,6 +41,7 @@ > #include <drm.h> > > #include "igt.h" > +#include "igt_device.h" > #include "igt_sysfs.h" > #include "sw_sync.h" > > @@ -674,6 +675,7 @@ igt_main > > igt_fixture { > fd = drm_open_driver(DRIVER_INTEL); > + igt_device_drop_master(fd); Is it master by default? I can't spot a place which would set master either in igt/lib/ or in drm. But in any case, should we have gem_reopen_driver attempt to drop master before re-opening? And if it was successful, drop it again after re-opening to end up in the same state? Regards, Tvrtko > > igt_require(i915_reset_control(true)); > igt_force_gpu_reset(fd); >
Quoting Tvrtko Ursulin (2018-04-06 09:35:41) > > On 05/04/2018 19:47, Chris Wilson wrote: > > As we reopen the fd for each subtest, and we need a DRM_MASTER, we need > > to drop master on the original before we are allowed to claim DRM_MASTER > > on the second. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > tests/gem_eio.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/tests/gem_eio.c b/tests/gem_eio.c > > index 9599e73d..66d8c18a 100644 > > --- a/tests/gem_eio.c > > +++ b/tests/gem_eio.c > > @@ -41,6 +41,7 @@ > > #include <drm.h> > > > > #include "igt.h" > > +#include "igt_device.h" > > #include "igt_sysfs.h" > > #include "sw_sync.h" > > > > @@ -674,6 +675,7 @@ igt_main > > > > igt_fixture { > > fd = drm_open_driver(DRIVER_INTEL); > > + igt_device_drop_master(fd); > > > Is it master by default? I can't spot a place which would set master > either in igt/lib/ or in drm. First person to open /dev/dri/cardN gets DRM_MASTER. > But in any case, should we have gem_reopen_driver attempt to drop master > before re-opening? And if it was successful, drop it again after > re-opening to end up in the same state? Not sure. I can argue the alternative as well, what if we were testing authentication and wanted to keep DRM_MASTER and spawn a !MASTER fd. My first thought was the same, gem_reopen_fd() should try to ensure the new fd has the same caps as the original, but API they be tricky to rule out surprises. For me it's easier to say, gem_reopen_fd() follows the DRM convention in that the second fd will not be DRM_MASTER unless you play the DRM games. Then you can choose what games to play? -Chris
On 06/04/2018 09:51, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-04-06 09:35:41) >> >> On 05/04/2018 19:47, Chris Wilson wrote: >>> As we reopen the fd for each subtest, and we need a DRM_MASTER, we need >>> to drop master on the original before we are allowed to claim DRM_MASTER >>> on the second. >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> --- >>> tests/gem_eio.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/tests/gem_eio.c b/tests/gem_eio.c >>> index 9599e73d..66d8c18a 100644 >>> --- a/tests/gem_eio.c >>> +++ b/tests/gem_eio.c >>> @@ -41,6 +41,7 @@ >>> #include <drm.h> >>> >>> #include "igt.h" >>> +#include "igt_device.h" >>> #include "igt_sysfs.h" >>> #include "sw_sync.h" >>> >>> @@ -674,6 +675,7 @@ igt_main >>> >>> igt_fixture { >>> fd = drm_open_driver(DRIVER_INTEL); >>> + igt_device_drop_master(fd); >> >> >> Is it master by default? I can't spot a place which would set master >> either in igt/lib/ or in drm. > > First person to open /dev/dri/cardN gets DRM_MASTER. Weird. And I missed to spot that magic while browsing around. >> But in any case, should we have gem_reopen_driver attempt to drop master >> before re-opening? And if it was successful, drop it again after >> re-opening to end up in the same state? > > Not sure. I can argue the alternative as well, what if we were testing > authentication and wanted to keep DRM_MASTER and spawn a !MASTER fd. > My first thought was the same, gem_reopen_fd() should try to ensure the > new fd has the same caps as the original, but API they be tricky to rule > out surprises. For me it's easier to say, gem_reopen_fd() follows the > DRM convention in that the second fd will not be DRM_MASTER unless you > play the DRM games. Then you can choose what games to play? Okay, pragmatic response says: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Apologies for causing breakage. Regards, Tvrtko
diff --git a/tests/gem_eio.c b/tests/gem_eio.c index 9599e73d..66d8c18a 100644 --- a/tests/gem_eio.c +++ b/tests/gem_eio.c @@ -41,6 +41,7 @@ #include <drm.h> #include "igt.h" +#include "igt_device.h" #include "igt_sysfs.h" #include "sw_sync.h" @@ -674,6 +675,7 @@ igt_main igt_fixture { fd = drm_open_driver(DRIVER_INTEL); + igt_device_drop_master(fd); igt_require(i915_reset_control(true)); igt_force_gpu_reset(fd);
As we reopen the fd for each subtest, and we need a DRM_MASTER, we need to drop master on the original before we are allowed to claim DRM_MASTER on the second. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- tests/gem_eio.c | 2 ++ 1 file changed, 2 insertions(+)