[Intel-gfx] [PATCH 1/2] drm/i915: fix up semaphore_waits_for

[Chris Wilson]

On Sat, Mar 15, 2014 at 12:08:55AM +0100, Daniel Vetter wrote:
> There's an entire pile of issues in here:
> 
> - Use the main RING_HEAD register, not ACTHD. ACTHD points at the gtt
>   offset of the batch buffer when a batch is executed. Semaphores are
>   always emitted to the main ring, so we always want to look at that.

True, nice catch that would explain a hard hang quite neatly if we tried
to read from beyond the aperture.
 
> - Mask the obtained HEAD pointer with the actual ring size, which is
>   much smaller. Together with the above issue this resulted us in
>   trying to dereference a pointer way outside of the ring mmio
>   mapping. The resulting invalid access in interrupt context
>   (hangcheck is executed from timers) lead to a full blown kernel
>   panic. The fbcon panic handler then tried to frob our driver harder,
>   resulting in a full machine hang at least on my snb here where I've
>   stumbled over this.
> 
> - Handle ring wrapping correctly and be a bit more explicit about how
>   many dwords we're scanning. We probably should also scan more than
>   just 4 ...

You can ignore ring wrapping as valid commands cannot wrap, hence the
formulation of acthd_min.

 /*
  * Be paranoid and presume the hw has gone off into the wild -
  * our ring is smaller than what the hardware (and hence
  * HEAD_ADDR) allows.
  */
 head = I915_READ_HEAD(ring) & (ring->size - 1);
 head_min = max((int)head - 3 * 4, 0);
 while (head >= head_min) {
	cmd = ioread32(ring->virtual_start + head);
	if (cmd == ipehr)
		break;
	head -= 4;
 }

> +		/*
> +		 * Be paranoid and presume the hw has gone off into the wild -
> +		 * our ring is smaller than what the hardware (and hence
> +		 * HEAD_ADDR) allows. Also handles wrap-around.
> +		 */
> +		head &= ring->size - 1;
> +
> +		/* This here seems to blow up */
> +		cmd = ioread32(ring->virtual_start + head);
So what does this mean?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre