diff mbox

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

Message ID 1366665797-29551-2-git-send-email-sconklin@canonical.com
State New
Headers show

Commit Message

Steve Conklin April 22, 2013, 9:23 p.m. UTC 
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(-)

Comments

Stefan Bader April 23, 2013, 8:12 a.m. UTC | #1 
Tentatively ACK since testing seems good. No way of knowing it as even they
don't seem to... :-P
Luis Henriques April 23, 2013, 8:53 a.m. UTC | #2 
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@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
Tim Gardner April 23, 2013, 11:59 a.m. UTC | #3 
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@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.

rtg
Luis Henriques April 23, 2013, 12:14 p.m. UTC | #4 
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@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
Tim Gardner April 23, 2013, 12:17 p.m. UTC | #5 
This patch only applies to Precise. Given the difficulty that this 
regression has caused, perhaps you should recheck your work and make 
sure you're sending the right patches for each release. I'm also a bit 
concerned that the frame buffer console patch mentioned by Luis is 
getting conflated into this bug.

rtg
Brad Figg April 23, 2013, 1:50 p.m. UTC | #6 
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@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.

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.

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

Brad
Luis Henriques April 23, 2013, 2:08 p.m. UTC | #7 
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@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
Steve Conklin April 23, 2013, 2:14 p.m. UTC | #8 
On 04/23/2013 08:50 AM, 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@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.
> 
> 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.
> 
> Do we know if 054430e773c9a1e26f38e30156eff02dedfffc17 does in fact fix
> any of this or are we just hoping?
> 
> Brad
> 

Here's a link to the upstream bug which was closed by the patch that
we're reverting.

https://bugzilla.kernel.org/show_bug.cgi?id=52311

It's an interconnected mess of patches apparently, but the linked bug
does not occur before the 3.8 kernel.

Steve
Steve Conklin April 23, 2013, 3:09 p.m. UTC | #9 
On 04/23/2013 07:17 AM, Tim Gardner wrote:
> This patch only applies to Precise. Given the difficulty that this
> regression has caused, perhaps you should recheck your work and make
> sure you're sending the right patches for each release. I'm also a bit
> concerned that the frame buffer console patch mentioned by Luis is
> getting conflated into this bug.
> 
> rtg

I resent the patch for Quantal. They're functionally equivalent.

We're tracking multiple regressions right now, and we know that
reverting this single patch fixes one of them. There is another
regression that will hopefully be found soon through bisection,
and it may be one of the other i915 patches. But they appear to be
independent.

Steve
Brad Figg April 23, 2013, 6:31 p.m. UTC | #10 
On 04/22/2013 02:23 PM, 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) |
>
Steve Conklin April 23, 2013, 6:46 p.m. UTC | #11 
On 04/22/2013 04:23 PM, 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) |
>
Tim Gardner April 23, 2013, 7:31 p.m. UTC | #12 
On 04/23/2013 12:46 PM, Steve Conklin wrote:
> On 04/22/2013 04:23 PM, 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) |
>>
>
>

There is no BugLink in the Precise patch as applied to master-next.
Ben Hutchings June 8, 2013, 4:05 p.m. UTC | #13 
On Mon, 2013-04-22 at 16:23 -0500, Steve Conklin wrote:
> This reverts commit 30ae292ec68402c773ddc8c80f83f7cd84289a39.
> 
> This has been shown to cause GPU hangs

So this should be reverted in 3.2.y as well, right?  Or has there been
another change that makes it OK?

Ben.

> ---
>  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
> 
>
Luis Henriques June 11, 2013, 9:04 a.m. UTC | #14 
Ben Hutchings <ben@decadent.org.uk> writes:

> On Mon, 2013-04-22 at 16:23 -0500, Steve Conklin wrote:
>> This reverts commit 30ae292ec68402c773ddc8c80f83f7cd84289a39.
>> 
>> This has been shown to cause GPU hangs
>
> So this should be reverted in 3.2.y as well, right?  Or has there been
> another change that makes it OK?
>
> Ben.

We have reverted it in the Ubuntu Precise kernel as well (which is
based on the 3.2 kernel), as we had users reporting this regression
(http://bugs.launchpad.net/bugs/1140716).

So, I would say it is also applicable to 3.2.y, although I believe we
haven't actually tested a vanilla 3.2.y kernel.

Cheers,
Ben Hutchings June 25, 2013, 3:20 a.m. UTC | #15 
On Tue, 2013-06-11 at 10:04 +0100, Luis Henriques wrote:
> Ben Hutchings <ben@decadent.org.uk> writes:
> 
> > On Mon, 2013-04-22 at 16:23 -0500, Steve Conklin wrote:
> >> This reverts commit 30ae292ec68402c773ddc8c80f83f7cd84289a39.
> >> 
> >> This has been shown to cause GPU hangs
> >
> > So this should be reverted in 3.2.y as well, right?  Or has there been
> > another change that makes it OK?
> >
> > Ben.
> 
> We have reverted it in the Ubuntu Precise kernel as well (which is
> based on the 3.2 kernel), as we had users reporting this regression
> (http://bugs.launchpad.net/bugs/1140716).
> 
> So, I would say it is also applicable to 3.2.y, although I believe we
> haven't actually tested a vanilla 3.2.y kernel.

OK, I've queued up a revert.

Ben.
diff mbox

Patch

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) |