Quoting Daniele Ceraolo Spurio (2017-10-18 17:50:33) > > > On 18/10/17 09:04, Chris Wilson wrote: > > Quoting Daniele Ceraolo Spurio (2017-10-18 16:49:24) > >> > >> > >> On 13/10/17 09:37, Daniele Ceraolo Spurio wrote: > >>> > >>> > >>> On 13/10/17 01:31, Chris Wilson wrote: > >>>> Quoting Chris Wilson (2017-10-12 23:57:38) > >>>>> Quoting Daniele Ceraolo Spurio (2017-10-12 23:27:27) > >>>>>> +igt_cork_t *igt_cork_new(int fd); > >>>>> > >>>>> _new does not imply plugged. > >>>>> > >>>>>> +void igt_cork_signal(igt_cork_t *cork); > >>>>> > >>>>> When have you signaled a cork? > >>>>> > >>>>>> +void igt_cork_free(int fd, igt_cork_t *cork); > >>>>> > >>>>> _free does not imply unplug. > >>>> > >>>> To be clear the verbs are to plug and unplug a queue/schedule. Cork is a > >>>> reference to TCP_CORK which does the same thing, but plug/unplug are > >>>> more commonplace (at least in kernel code). > >>>> > >>>> I don't see any reason why we need a malloc here. > >>>> -Chris > >>>> > >>> > >>> I added the malloc just to use the same approach as the spin_batch, I'll > >>> get rid of it. > >>> My concern with the existing plug/unplug scheme was that the plug() > >>> function in the various tests didn't really plug anything but just > >>> created the bo and that was slightly confusing. > > > > It created a bo with an unsignaled fence, that's enough to plug anything > > attached to it. Since we can't just say plug(device) we have to say > > execbuf(device, plug()). > > > >>> What do you think of going with: > >>> > >>> struct igt_cork { > >>> int device; > >>> uint32_t handle; > >>> uint32_t fence; > >>> }; > >>> > >>> struct igt_cork igt_cork_create(int fd); > >>> void igt_cork_unplug(struct igt_cork *cork); > >>> void igt_cork_close(int fd, struct igt_cork *cork); > >>> void igt_cork_unplug_and_close(int fd, struct igt_cork *cork); > > > > close will always be unplug; there's no differentiation, in both APIs we > > ensure that any fence associated with the device or timeline fd is > > signaled upon release. We could lose the fence and still work, but for > > us it gives us the means by which we can do a test-and-set and report an > > issue where the fence was signaled too early (due to slow test setup). > > Similarly once unplugged, there is no use for the struct anymore, you > > could release the device/timeline, but we've embedded it because in > > terms of overhead, so far it has been insignificant. > > > > Leaving a fence dangling by separating unplug/close is a good way to > > leave lots of timeouts and GPU resets behind. > > -Chris > > > > What I wanted to separate is the unplugging from the closing of the BO > handle, because in some case we keep the BO around for a while after > unblocking the execution. Where? What value could it possibly have? You know the state of its fence, so presumably you want the contents. In such a situation you don't need a dummy cork to plug the queue, you have a real object with which you want interact. > In most of those cases the BO handle is not > currently closed, Every single unplug() function is closing the device; which closes the handle; gem_close() is superfluous. Early on I kept the vgem fd around and just needed to close the handle, but it looks like all of those functions have now been converted to own their device. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx