[Intel-gfx] [PATCH] drm/i915: Broadwell expands ACTHD to 64bit

[Chris Wilson]

On Mon, Mar 24, 2014 at 07:43:48PM -0700, Ben Widawsky wrote:
> On Mon, Mar 24, 2014 at 07:41:17PM -0700, Ben Widawsky wrote:
> > On Fri, Mar 21, 2014 at 12:41:53PM +0000, Chris Wilson wrote:
> > > As Broadwell has an increased virtual address size, it requires more
> > > than 32 bits to store offsets into its address space. This includes the
> > > debug registers to track the current HEAD of the individual rings, which
> > > may be anywhere within the per-process address spaces. In order to find
> > > the full location, we need to read the high bits from a second register.
> > > We then also need to expand our storage to keep track of the larger
> > > address.
> > > 
> > > v2: Carefully read the two registers to catch wraparound between
> > >     the reads.
> > > v3: Use a WARN_ON rather than loop indefinitely on an unstable
> > >     register read.
> > > 
> > 
> > I truly feel bad for saying this at v3, but we can probably simplify
> > this.  Unless I am missing something, we only actually care about the
> > value of acthd in:
> > 
> > if (ring->hangcheck.acthd != acthd)
> > 	return HANGCHECK_ACTIVE;
> > 
> > I think therefore it would be safe to make hangcheck.acthd a u64.
> > Compare the lower dword. If they're not equal, then we're done. If they
> > are equal, compare the UDW. If UDW doesn't match, we're done. If UDW
> > does match, do another read of the LDW and call it good:
> > 
> > ring_stuck(u32 acthd)
> > {
> > 	if (lower_32_bits(ring->hangcheck.acthd) != acthd)
> > 		return HANGCHECK_ACTIVE;
> > 	else if (upper_32_bits(ring->hangcheck.acthd) !=
> > 			I915_READ(ACTHD_UDW))
> > 		return HANGCHECK_ACTIVE
> > 	else if (acthd != I915_READ(RING_ACTHD))
> > 		return HANGCHECK_ACTIVE;
> > }
> > 
> > After writing that, I'm not really sure how much less complex it is, but I
> > think it gets the same job done. Potentially there is some MMIO savings,
> > but I'd guess it to be negligible.
> > 
> > Jesse, please request ACTHD is expanded to a proper 64b register for
> > gen100000000.
> 
> Right after I sent that, I realized that doesn't provide for potentially
> corrupting ring->hangcheck.acthd. So I am back to thinking this method
> is better.

Plus having read64_2x32 should make it easier to dtrt next time.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre