[PATCH] Revert "drm/i915: GFX_MODE Flush TLB Invalidate Mode must be '1' for scanline waits"

[Luis Henriques]

On Tue, Apr 23, 2013 at 06:50:56AM -0700, Brad Figg wrote:
> On 04/23/2013 05:14 AM, Luis Henriques wrote:
> > On Tue, Apr 23, 2013 at 05:59:49AM -0600, Tim Gardner wrote:
> >> On 04/23/2013 02:53 AM, Luis Henriques wrote:
> >>> On Mon, Apr 22, 2013 at 04:23:17PM -0500, Steve Conklin wrote:
> >>>> This reverts commit 30ae292ec68402c773ddc8c80f83f7cd84289a39.
> >>>>
> >>>> This has been shown to cause GPU hangs
> >>>> ---
> >>>>  drivers/gpu/drm/i915/intel_ringbuffer.c |    5 -----
> >>>>  1 file changed, 5 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>>> index c3654ff..fbaa785 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>>> @@ -422,11 +422,6 @@ static int init_render_ring(struct intel_ring_buffer *ring)
> >>>>  	if (INTEL_INFO(dev)->gen >= 6)
> >>>>  		I915_WRITE(MI_MODE, GFX_MODE_ENABLE(ASYNC_FLIP_PERF_DISABLE));
> >>>>
> >>>> -	/* Required for the hardware to program scanline values for waiting */
> >>>> -	if (INTEL_INFO(dev)->gen == 6)
> >>>> -		I915_WRITE(GFX_MODE,
> >>>> -			   GFX_MODE_ENABLE(GFX_TLB_INVALIDATE_ALWAYS));
> >>>> -
> >>>>  	if (IS_GEN7(dev))
> >>>>  		I915_WRITE(GFX_MODE_GEN7,
> >>>>  			   GFX_MODE_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
> >>>> --
> >>>> 1.7.9.5
> >>>>
> >>>>
> >>>> --
> >>>> kernel-team mailing list
> >>>> kernel-team at lists.ubuntu.com
> >>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> >>>
> >>> Comment 99 in bug #1167114 refers to a debian bug that seems to be a
> >>> duplicate.  And the solution was commit
> >>> 054430e773c9a1e26f38e30156eff02dedfffc17 (fbcon: fix locking harder).
> >>> Maybe its worth trying a kernel with this fix.
> >>>
> >>> (This commit is already in 3.5.y, so I should have already identified
> >>> it as a possible fix for these bugs... My bad, I've queued it without
> >>> taking a look at the full context.)
> >>>
> >>> Cheers,
> >>> --
> >>> Luis
> >>>
> >>
> >> Is that really related ? It seems like it might be a different bug
> >> altogether since most of the reporters don't have issues until well
> >> after boot.
> > 
> > Well, hard to say... the bugs in LP are now really messy and, as Steve
> > already said, we're almost for sure dealing here with more than one
> > issue.
> > 
> > So, maybe our best option is to go ahead and revert this commit.  And
> > probably also the other (5?) we've added last cycle.  Next SRU cycle
> > we'll have the 054430e773c9a1e26f38e30156eff02dedfffc17 from stable
> > updates.
> > 
> > Cheers,
> > --
> > Luis
> > 
> 
> I'm only talking about Quantal here.
> 
> The relevant revert is for 4c443ec9afe7f6fab41106c617136aca44d69a7b
> for which Steve will submit a SRU request.
> 
> This upstream commit supposedly fixes an issue but seems to be causing
> other issues. We'll just have to see if reverting it helps or hurts
> us overall.

Makes perfect sense.

> 
> I've seen other people suggest reverting the other 5 drm/i915
> patches but I've not seen any testing that shows these are bad and
> that reverting them would improve things for anyone.

True.  I just referred these extra patches because they were added to
fix another i915 regression (bug #1140716).  And this bug, if I
remember correctly, seemed to be triggered by the commit we're now
trying to revert.

Instead of reverting it, we have decided to follow an upstream
suggestion to add these 5 drm/i915 patches.

Since we are now definitely reverting it, we *may* or *may not* want
to revert these extra 5 patches.  And this is why I referred them in
my previous email.  But I'm Ok one way or the other :)

> 
> Do we know if 054430e773c9a1e26f38e30156eff02dedfffc17 does in fact
> fix any of this or are we just hoping?

No test has been done, so just hoping :)

Cheers,
--
Luis