[Intel-gfx] [PATCH i-g-t v4 01/13] lib/sw_sync: Add helper functions for managing synchronization primitives

[Robert Foss]

On 2016-09-15 02:46 AM, Chris Wilson wrote:
> On Wed, Sep 14, 2016 at 11:04:30AM -0400, robert.foss at collabora.com wrote:
>> +void sw_sync_timeline_inc(int fd, uint32_t count)
>> +{
>> +	uint32_t arg = count;
>> +
>> +	if (fd == 0)
>> +		return;
>
> But fd = 0 is a valid fd, and might be a timeline somewhere.
>
> Did you mean count == 0 ?
>
> And even then (unless it is defined as an error condition in the kernel
> ABI, and it should not be...) we should pass it through to the kernel.

You're right, I'll change it in v5.

>
>> +	do_ioctl(fd, SW_SYNC_IOC_INC, &arg);
>> +}
>> +
>
>> +int sw_sync_wait(int fence, int timeout)
>> +{
>> +	struct pollfd fds;
>> +	int ret;
>> +
>> +	fds.fd = fence;
>> +	fds.events = POLLIN | POLLERR;
>
> POLLERR is always implied and doesn't need to be specified (it is
> meaningless in .events).
>
> int sw_sync_wait(int fence, int timeout)
> {
> #if BEING_FANCY
> 	return poll(&(struct pollfd){fd, POLLIN}, 1, timeout);
> #else
> 	struct pollfd pfd = { fd, POLLIN };
> 	return poll(&pfd, 1, timeout);
> #endif
> }
>
> Indentation has gone wrong, double check the whitespace.

That is definitely nicer looking. I'll drop it in for v5.

>
>
> How do fences operate after their timeline is closed? (Are they
> automatically signaled, or do they persist and are signaled normally?) Is
> there a test for using fences from a closed timeline (I was looking but
> didn't notice one).

I did some quick tests just to confirm, closing the timeline signals all 
of its fences.