Re: [PATCH i-g-t 1/8] lib/igt_dummyload: add igt_cork

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

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.
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);

The plug() function is still missing, as we do the actual plugging by 
adding the object to the execbuf and I don't think that would be clean 
to wrap in the library. I thought of adding something like 
"get_plugging_handle()" to return cork->handle and make it more explicit 
that that handle was responsible for the plugging but it seemed a bit 
overkill.

Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx