[Intel-gfx] [PATCH 11/15] drm/i915: Implementation of GuC client

[Chris Wilson]

On Mon, Jun 15, 2015 at 07:36:29PM +0100, Dave Gordon wrote:
> +/* Get valid workqueue item and return it back to offset */
> +static int guc_get_workqueue_space(struct i915_guc_client *gc, u32 *offset)
> +{
> +	struct guc_process_desc *desc;
> +	void *base;
> +	u32 size = sizeof(struct guc_wq_item);
> +	int ret = 0, timeout_counter = 200;
> +	unsigned long flags;
> +
> +	base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0));
> +	desc = base + gc->proc_desc_offset;
> +
> +	while (timeout_counter-- > 0) {
> +		spin_lock_irqsave(&gc->wq_lock, flags);
> +
> +		ret = wait_for_atomic(CIRC_SPACE(gc->wq_tail, desc->head,
> +				gc->wq_size) >= size, 1);

What is the point of this loop? Drop the spinlock 200 times? You already
have a timeout, the loop extends that by a factor or 200. You merely
allow gazzumping, however I haven't looked at the locking to see what
you intend to lock (since it is not described at all).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre