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

[Chris Wilson]

On Sat, Mar 15, 2014 at 05:31:04PM +0100, Daniel Vetter wrote:
> On Sat, Mar 15, 2014 at 4:46 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > 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.
> 
> Well I've thought that HEAD usually points at the next command. I'm
> not sure about the exact semantics at wrap-around time but since
> implementing it was trivially I've figured it's better to be paranoid.

Nah, it points to ring->size upon parsing a command that exactly ends
upon the wrap around, But your point stands.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre