[Intel-gfx] [PATCH 02/18] drm/i915: Allow i915_sw_fence_await_sw_fence() to allocate

[Joonas Lahtinen]

On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> @@ -678,7 +678,7 @@ void __i915_add_request(struct drm_i915_gem_request *request, bool flush_caches)
>  				   &request->i915->drm.struct_mutex);
>  	if (prev)
>  		i915_sw_fence_await_sw_fence(&request->submit, &prev->submit,
> -					     &request->submitq);
> +					     &request->submitq, GFP_NOWAIT);

Wrt commit message, why do we pass both here? If one was to run
statistic analysis, !wq is never true if you pass &foo here.

> @@ -135,6 +135,8 @@ static int i915_sw_fence_wake(wait_queue_t *wq, unsigned mode, int flags, void *
>  	list_del(&wq->task_list);
>  	__i915_sw_fence_complete(wq->private, key);
>  	i915_sw_fence_put(wq->private);
> +	if (wq->flags)
> +		kfree(wq);

This is confusing without a comment or proper flag #define.

>  	INIT_LIST_HEAD(&wq->task_list);
> -	wq->flags = 0;
> +	wq->flags = pending;

Why not make this look proper by using I915_SW_FENCE_FLAG_FOO name.

> +static inline void i915_sw_fence_wait(struct i915_sw_fence *fence)
> +{
> +	wait_event(fence->wait, i915_sw_fence_done(fence));
> +}
> +

This seems to be a lost-in-rebasing hunk.

Above addressed;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation