Re: [PATCH] Revert "drm/i915: fix infinite loop at gen6_update_ring_freq"

Paulo Zanoni
2014-04-28T18:14:31Z

2014-04-23 4:05 GMT-03:00 Daniel Vetter <daniel@ffwll.ch>:
> On Tue, Apr 22, 2014 at 06:25:12PM -0300, Paulo Zanoni wrote:
>> 2014-04-11 6:02 GMT-03:00 Daniel Vetter <daniel@ffwll.ch>:
>> > On Thu, Apr 10, 2014 at 10:52:26AM -0700, Ben Widawsky wrote:
>> >> On Thu, Apr 10, 2014 at 10:50:43AM -0700, Ben Widawsky wrote:
>> >> > On Thu, Apr 10, 2014 at 09:04:47AM +0200, Daniel Vetter wrote:
>> >> > > This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710.
>> >> > >
>> >> > > This patch duct-tapes over some issue in the current bdw rps patches
>> >> > > which must wait with enabling rc6/rps until the very first batch has
>> >> > > been submitted by userspace.
>> >> > >
>> >> > > But those patches aren't merged yet, and for upstream we need to have
>> >> > > an in-kernel emission of the very first batch. I shouldn't have
>> >> > > merged this patch so let's revert it again.
>> >> >
>> >> > I said this on the mailing last before you merged the patch.
>> >>
>> >> 20140402050338.GB13824@bwidawsk.net
>> >
>> > 20140402145813.GV7225@phenom.ffwll.local will explain things.
>>
>> There's now a regression report pointing to the revert:
>> https://bugs.freedesktop.org/show_bug.cgi?id=77565 .
>>
>> What is the proposed solution now? Just WARN and still avoid the
>> infinite loop? Or keep the infinite loop and leave the bug open?
>> Disable BDW runtime PM?
>
> I've thought that we can only hit this with the as-yet unmerged rc6
> patches on bdw, so I'm really confused why this blows up now?
>
> In any case I've thought Imre has stumbled over a similar issue on byt and
> he has a fix to prevent runtime pm until the delayed rps init has run.
> I've assigned the bug to him.
>
> Still confused why this suddenly blew up ...

Sorry for the delayed response.

The bug is very simple: since we did not enable RC6, by the time we
run gen6_update_ring_freq(), the RPS limits will all be zero. The loop
decrements a variable until it reaches a point where it is smaller
than the other. But since the other variable is zero, the loop won't
end since we can't be smaller than zero on the unsigned world, no
matter how much we decrement it.

This can probably be reproduced on non-BDW machines too, with RC6 disabled.

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

--
Paulo Zanoni